CurtHagenlocher commented on code in PR #3256:
URL: https://github.com/apache/arrow-adbc/pull/3256#discussion_r2270972855
##########
csharp/src/Drivers/Apache/Hive2/IHiveServer2Statement.cs:
##########
@@ -40,7 +34,9 @@ internal interface IHiveServer2Statement : ITracingStatement
/// Checks if direct results are available.
/// </summary>
/// <returns>True if direct results are available and contain result
data, false otherwise.</returns>
- bool HasDirectResults { get; }
+ bool HasDirectResults(IResponse response);
+
+ bool TryGetDirectResults(IResponse response, out TSparkDirectResults?
directResults);
Review Comment:
Given that all the other methods in the interface have doc comments, could
you add them here too?
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -384,20 +375,7 @@ private void UpdateBatchSizeIfValid(string key, string
value) => BatchSize = !st
public override void Dispose()
{
- this.TraceActivity(activity =>
- {
- if (Response != null && Response.OperationHandle != null &&
Response.DirectResults?.CloseOperation?.Status?.StatusCode !=
TStatusCode.SUCCESS_STATUS)
- {
- CancellationToken cancellationToken =
ApacheUtility.GetCancellationToken(QueryTimeoutSeconds,
ApacheUtility.TimeUnit.Seconds);
-
activity?.AddTag(SemanticConventions.Db.Operation.OperationId,
Response.OperationHandle!.OperationId.Guid, "N");
- TCloseOperationReq request = new
TCloseOperationReq(Response.OperationHandle!);
- TCloseOperationResp resp =
Connection.Client.CloseOperation(request, cancellationToken).Result;
- HiveServer2Connection.HandleThriftResponse(resp.Status,
activity);
- }
- Response = null;
-
- base.Dispose();
- });
+ base.Dispose();
Review Comment:
Can we just remove this method?
##########
csharp/src/Drivers/Databricks/Reader/DatabricksOperationStatusPoller.cs:
##########
@@ -69,7 +73,7 @@ private async Task PollOperationStatus(CancellationToken
cancellationToken)
{
while (!cancellationToken.IsCancellationRequested)
{
- var operationHandle = _statement.Response!.OperationHandle;
+ TOperationHandle? operationHandle = _response?.OperationHandle;
Review Comment:
`_response` isn't marked as nullable so the `?` shouldn't be needed here.
--
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]
