You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alexguo-db (via GitHub)" <gi...@apache.org> on 2025/04/09 19:52:13 UTC
[PR] feat(csharp/src/Drivers/Databricks): Make Cloud Fetch options configurable at the connector level [arrow-adbc]
alexguo-db opened a new pull request, #2691:
URL: https://github.com/apache/arrow-adbc/pull/2691
- Currently, cloud fetch enablement is only configurable at the statement level, and this is not exposed to clients outside of arrow-adbc repo
- Make these options configurable at the driver/connector level
Tested E2E with `{ "adbc.databricks.cloudfetch.enabled", "true" }`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Make Cloud Fetch options configurable at the connection level [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher merged PR #2691:
URL: https://github.com/apache/arrow-adbc/pull/2691
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Make Cloud Fetch options configurable at the connection level [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #2691:
URL: https://github.com/apache/arrow-adbc/pull/2691#discussion_r2047325192
##########
csharp/src/Drivers/Databricks/DatabricksConnection.cs:
##########
@@ -29,10 +29,50 @@ namespace Apache.Arrow.Adbc.Drivers.Databricks
{
internal class DatabricksConnection : SparkHttpConnection
{
+ // CloudFetch configuration
+ private const long DefaultMaxBytesPerFile = 20 * 1024 * 1024; // 20MB
+
+ private bool _useCloudFetch = true;
+ private bool _canDecompressLz4 = true;
+ private long _maxBytesPerFile = DefaultMaxBytesPerFile;
+
public DatabricksConnection(IReadOnlyDictionary<string, string> properties) : base(properties)
{
+ // Parse CloudFetch options from connection properties
+ if (Properties.TryGetValue(DatabricksParameters.UseCloudFetch, out string? useCloudFetchStr) &&
+ bool.TryParse(useCloudFetchStr, out bool useCloudFetchValue))
+ {
+ _useCloudFetch = useCloudFetchValue;
Review Comment:
It's unfortunate that we're essentially validating these in two places: both here and in `DatabricksStatement`.
We've also somehow arrived in a weird place where these values are both connection properties (that can only be set before the connection is opened) and statement properties that can be set any time after the statement was created. But that's a preexisting condition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Make Cloud Fetch options configurable at the connection level [arrow-adbc]
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #2691:
URL: https://github.com/apache/arrow-adbc/pull/2691#discussion_r2047276895
##########
csharp/src/Drivers/Databricks/DatabricksConnection.cs:
##########
@@ -29,10 +29,50 @@ namespace Apache.Arrow.Adbc.Drivers.Databricks
{
internal class DatabricksConnection : SparkHttpConnection
{
+ // CloudFetch configuration
+ private const long DefaultMaxBytesPerFile = 20 * 1024 * 1024; // 20MB
+
+ private bool _useCloudFetch = true;
+ private bool _canDecompressLz4 = true;
+ private long _maxBytesPerFile = DefaultMaxBytesPerFile;
+
public DatabricksConnection(IReadOnlyDictionary<string, string> properties) : base(properties)
{
+ // Parse CloudFetch options from connection properties
+ if (Properties.TryGetValue(DatabricksParameters.UseCloudFetch, out string? useCloudFetchStr) &&
+ bool.TryParse(useCloudFetchStr, out bool useCloudFetchValue))
+ {
+ _useCloudFetch = useCloudFetchValue;
Review Comment:
Consider validating the string property. Right now, if someone were to typo as e.g. `properties["CloudFetch"] = "flase";` the property value would be silently ignored. (Similarly, `properties["CloudFetc"] = "false";` would be ignored but I'm not sure how consistently drivers are producing errors for unsupported properties.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(csharp/src/Drivers/Databricks): Make Cloud Fetch options configurable at the connection level [arrow-adbc]
Posted by "eric-wang-1990 (via GitHub)" <gi...@apache.org>.
eric-wang-1990 commented on code in PR #2691:
URL: https://github.com/apache/arrow-adbc/pull/2691#discussion_r2036076798
##########
csharp/src/Drivers/Databricks/DatabricksConnection.cs:
##########
@@ -29,10 +29,48 @@ namespace Apache.Arrow.Adbc.Drivers.Databricks
{
internal class DatabricksConnection : SparkHttpConnection
{
+ // CloudFetch configuration
+ private bool _useCloudFetch = true;
+ private bool _canDecompressLz4 = true;
+ private long _maxBytesPerFile = 20 * 1024 * 1024; // 20MB default
+
public DatabricksConnection(IReadOnlyDictionary<string, string> properties) : base(properties)
{
+ // Parse CloudFetch options from connection properties
+ if (properties.TryGetValue(DatabricksParameters.UseCloudFetch, out string? useCloudFetchStr) &&
Review Comment:
Misc: Looking at other examples https://github.com/apache/arrow-adbc/blob/9ba6bdb8bd42e35237dadfae758419846e6c442c/csharp/src/Drivers/Apache/Hive2/HiveServer2HttpConnection.cs#L64 seem you can use the base class method Properties.TryGetValue. Note it is the Properties function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org