CurtHagenlocher commented on code in PR #3684:
URL: https://github.com/apache/arrow-adbc/pull/3684#discussion_r2491397181
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs:
##########
@@ -1717,5 +1719,23 @@ private static void ThrowErrorResponse(TStatus status,
AdbcStatusCode adbcStatus
throw new HiveServer2Exception(status.ErrorMessage, adbcStatusCode)
.SetSqlState(status.SqlState)
.SetNativeError(status.ErrorCode);
+
+ protected TConfiguration GetTconfiguration()
+ {
+
Properties.TryGetValue(ThriftTransportSizeConstants.MaxMessageSize, out string?
maxMessageSize);
Review Comment:
nit: can these be reordered slightly so that the code pertaining to a single
property stays together? e.g.
```
var thriftConfig = new TConfiguration();
Properties.TryGetValue(ThriftTransportSizeConstants.MaxMessageSize, out string?
maxMessageSize);
if (int.TryParse(maxMessageSize, out int maxMessageSizeValue) &&
maxMessageSizeValue > 0)
{
thriftConfig.MaxMessageSize = maxMessageSizeValue;
}
```
##########
csharp/src/Drivers/Apache/ApacheParameters.cs:
##########
@@ -24,6 +24,7 @@ public class ApacheParameters
{
public const string PollTimeMilliseconds =
"adbc.apache.statement.polltime_ms";
public const string BatchSize = "adbc.apache.statement.batch_size";
+ public const string BatchSizeStopCondition =
"adbc.apache.statement.batch_size_stop_condition";
Review Comment:
Does this new parameter apply to all the Thrift-using drivers? Can you add
it to the `README.md` files for whichever ones it applies to? There are
separate READMEs for Hive2, Impala, Spark and Databricks.
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -352,6 +358,8 @@ protected async Task<IResponse>
ExecuteStatementAsync(CancellationToken cancella
public virtual long BatchSize { get; protected set; } =
HiveServer2Connection.BatchSizeDefault;
+ public virtual bool EnableBatchSizeStopCondition { get; protected set;
} = HiveServer2Connection.EnableBatchSizeStopConditionDefault;
Review Comment:
Nothing currently overrides this. Should it still be virtual?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
