feat(csharp/src/Drivers/Databricks): Added support for connection param of maxBytesPerFetchRequest #3474
Conversation
msrathore-db
changed the title
CurtHagenlocher
changed the title
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks! In addition to the two specific comments I left, there's also the need to mollify the linter by cleaning up some whitespace and the desire to keep readme.md up-to-date with added options. (And again, the latter could be done as a separate PR.)
|
If this is done, can you click "Ready for review"? |
| MaxBytes = 404857600 | ||
| }; | ||
|
|
||
| private const int DefaultMaxRowsPerFetchRequest = 1000; |
There was a problem hiding this comment.
Why we set this to 1000? That is too small.
The OSS JDBC driver set to 2million for max rows and 404857600 for max bytes.
There was a problem hiding this comment.
This was the default value being used earlier.
We'll hold this PR until we find the optimal default value based on perf analysis.
|
Change CloudFetch path as well? |
CurtHagenlocher
left a comment
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks! Changes look good. Let me know if you want to check-in as-is or if you want to follow up on Eric's comment and/or mine.
…e default value for maxBytesPerFetchRequest to 400MB. CloudFetch fetch request now uses maxBytesPerFetchRequest
|
Now that the changes are finalised, we can merge this PR. |
4 participants
Description
Added support for the connection param of maxBytesPerFetchRequest. The parameter is used for direct results as well.
PECO-2734