Skip to content

feat(csharp/src/Drivers): Enable setting BatchSizeStopCondition, MaxMessageSize & MaxFrameSize#3684

Merged
CurtHagenlocher merged 4 commits into
apache:mainfrom
sudhiremmadi:clientSideBatching
Dec 2, 2025
Merged

feat(csharp/src/Drivers): Enable setting BatchSizeStopCondition, MaxMessageSize & MaxFrameSize#3684
CurtHagenlocher merged 4 commits into
apache:mainfrom
sudhiremmadi:clientSideBatching

Conversation

@sudhiremmadi

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot added this to the ADBC Libraries 21 milestone Nov 4, 2025
Comment thread csharp/src/Drivers/Apache/ApacheParameters.cs Outdated

@CurtHagenlocher CurtHagenlocher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! It's hard for me to be sure because of the layering in this code, but it looks like previously some of the connections had their readers initialized with enableBatchSizeStopConditionDefault = false but now all of them have a default value of true. Did I read that correctly? If so, is that change deliberate and could it be breaking?

Comment thread csharp/src/Drivers/Apache/ApacheParameters.cs
Comment thread csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs Outdated
Comment thread csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs Outdated
@sudhiremmadi

Copy link
Copy Markdown
Contributor Author

Thanks! It's hard for me to be sure because of the layering in this code, but it looks like previously some of the connections had their readers initialized with enableBatchSizeStopConditionDefault = false but now all of them have a default value of true. Did I read that correctly? If so, is that change deliberate and could it be breaking?

Changed the default value back to false, Thanks

@CurtHagenlocher

Copy link
Copy Markdown
Contributor

Thanks! It's hard for me to be sure because of the layering in this code, but it looks like previously some of the connections had their readers initialized with enableBatchSizeStopConditionDefault = false but now all of them have a default value of true. Did I read that correctly? If so, is that change deliberate and could it be breaking?

Changed the default value back to false, Thanks

I'm sorry; I wasn't very clear about this. Two of the connections had a value of false but the others seemed to have a value of true. Let me take another look and enumerate this more precisely as I've deleted my notes from yesterday :(.

@sudhiremmadi

Copy link
Copy Markdown
Contributor Author

Thanks! It's hard for me to be sure because of the layering in this code, but it looks like previously some of the connections had their readers initialized with enableBatchSizeStopConditionDefault = false but now all of them have a default value of true. Did I read that correctly? If so, is that change deliberate and could it be breaking?

Changed the default value back to false, Thanks

I'm sorry; I wasn't very clear about this. Two of the connections had a value of false but the others seemed to have a value of true. Let me take another look and enumerate this more precisely as I've deleted my notes from yesterday :(.

Any update on this? Default value is set to false for all 3 drivers

| `adbc.proxy_options.proxy_uid` | Username for proxy authentication. Only feature-complete in Spark driver. Required when proxy_auth is True | |
| `adbc.proxy_options.proxy_pwd` | Password for proxy authentication. Only feature-complete in Spark driver. Required when proxy_auth is True | |
| `adbc.telemetry.trace_parent` | The [trace parent](https://www.w3.org/TR/trace-context/#traceparent-header) identifier for an existing [trace context](https://www.w3.org/TR/trace-context/) \(span/activity\) in a tracing system. This option is most likely to be set using `Statement.SetOption` to set the trace parent for driver interaction with a specific `Statement`. However, it can also be set using `Driver.Open`, `Database.Connect` or `Connection.SetOption` to set the trace parent for all interactions with the driver on that specific `Connection`. | |
| `adbc.apache.statement.batch_size_stop_condition` | Flag to enable/disable stopping reading based on batch size condition | `False` |

@CurtHagenlocher CurtHagenlocher Dec 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please move this up to line ~44 so we keep these in roughly sorted order.

@CurtHagenlocher CurtHagenlocher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks! Can you please submit a followup PR where the option names in the README.md files are a little closer to sorted?

| `adbc.standard_options.tls.allow_hostname_mismatch` | If hostname mismatch is allowed for ssl. One of `True`, `False` | `False` |
| `adbc.standard_options.tls.trusted_certificate_path` | The full path of the tls/ssl certificate .pem file containing custom CA certificates for verifying the server when connecting over TLS | |
| `adbc.telemetry.trace_parent` | The [trace parent](https://www.w3.org/TR/trace-context/#traceparent-header) identifier for an existing [trace context](https://www.w3.org/TR/trace-context/) \(span/activity\) in a tracing system. This option is most likely to be set using `Statement.SetOption` to set the trace parent for driver interaction with a specific `Statement`. However, it can also be set using `Driver.Open`, `Database.Connect` or `Connection.SetOption` to set the trace parent for all interactions with the driver on that specific `Connection`. | |
| `adbc.apache.statement.batch_size_stop_condition` | Flag to enable/disable stopping reading based on batch size condition | `False` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similarly, please move up to line ~44

@CurtHagenlocher CurtHagenlocher merged commit 7cb8c0b into apache:main Dec 2, 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.

4 participants