CurtHagenlocher commented on code in PR #1830:
URL: https://github.com/apache/arrow-adbc/pull/1830#discussion_r1594662544
##########
csharp/src/Drivers/Apache/Spark/SparkConnection.cs:
##########
@@ -134,23 +134,6 @@ public override AdbcStatement CreateStatement()
return new SparkStatement(this);
}
- public override void Dispose()
- {
- /*
- if (this.client != null)
- {
- TCloseSessionReq r6 = new TCloseSessionReq(this.sessionHandle);
- this.client.CloseSession(r6).Wait();
-
- this.transport.Close();
- this.client.Dispose();
-
- this.transport = null;
- this.client = null;
- }
- */
- }
-
public override IArrowArrayStream GetInfo(IReadOnlyList<AdbcInfoCode>
codes)
Review Comment:
We are going to need to close the session on the Spark side or the resources
will remain in use. Please file a followup bug for this.
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -48,21 +48,21 @@ protected void ExecuteStatement()
this.operationHandle = executeResponse.OperationHandle;
}
- protected void PollForResponse()
+ protected async Task PollForResponseAsync()
{
TGetOperationStatusResp? statusResponse = null;
do
{
- if (statusResponse != null) { Thread.Sleep(500); }
+ if (statusResponse != null) { await Task.Delay(500); }
Review Comment:
Not blocking, but it would be nice if this were configurable via a statement
option.
##########
csharp/src/Drivers/Apache/Spark/SparkStatement.cs:
##########
@@ -102,6 +102,10 @@ public override UpdateResult ExecuteUpdate()
return new UpdateResult(affectedRows ?? -1);
}
+ public override QueryResult ExecuteQuery() =>
ExecuteQueryAsync().AsTask().Result;
+
+ public override UpdateResult ExecuteUpdate() =>
ExecuteUpdateAsync().Result;
+
Review Comment:
Should these implementations (and the one in ImpalaStatement) move into the
base class?
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs:
##########
@@ -46,17 +46,18 @@ internal TCLIService.Client Client
get { return this.client ?? throw new
InvalidOperationException("connection not open"); }
}
- public void Open()
+ public async Task OpenAsync()
Review Comment:
As this isn't part of the public API, please make it protected or internal.
--
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]
