Skip to content

Conversation

@msrathore-db
Copy link
Contributor

Used connection param of batchSize for cloudFetch. Changed the default value of the param to 2 million instead of 50k.

PECO-2732

…the default value for BatchSize to 2 million.
@github-actions github-actions bot added this to the ADBC Libraries 21 milestone Oct 4, 2025
private bool enablePKFK;
private bool runAsyncInThrift;

public new long BatchSize { get; private set; } = DatabricksBatchSizeDefault;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching from web, this new keyword is hiding the base class impl, not truly overriding it. Which means if the object is referenced by base class type it will still call base class function.
Can we change the base to virtual so we can call this override?

/// </summary>
internal class DatabricksStatement : SparkStatement, IHiveServer2Statement
{
private const long DatabricksBatchSizeDefault = 2000000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment of explaining why this is set to 2 million.

@msrathore-db msrathore-db changed the title feat(csharp/src/Drivers/Databricks): Used connection param of batchSize for cloudFetch. feat(csharp/src/Drivers/Databricks): Used connection param of batchSize for cloudFetch Oct 4, 2025
@msrathore-db msrathore-db marked this pull request as draft October 4, 2025 16:58
@msrathore-db msrathore-db marked this pull request as ready for review October 7, 2025 13:26
@CurtHagenlocher CurtHagenlocher merged commit 605bf75 into apache:main Oct 7, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants