birschick-bq commented on code in PR #2695:
URL: https://github.com/apache/arrow-adbc/pull/2695#discussion_r2038151779
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs:
##########
@@ -1242,12 +1242,7 @@ private static StructArray GetColumnSchema(TableInfo
tableInfo)
nullBitmapBuffer.Build());
}
- protected abstract void SetPrecisionScaleAndTypeName(
- short colType,
- string typeName,
- TableInfo? tableInfo,
- int columnSize,
- int decimalDigits);
+ public abstract void SetPrecisionScaleAndTypeName(short columnType,
string typeName, TableInfo? tableInfo, int columnSize, int decimalDigits);
Review Comment:
Could we use `protected internal` or `internal` here, instead of `public`?
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Statement.cs:
##########
@@ -426,12 +427,128 @@ private async Task<QueryResult>
GetQueryResult(TSparkDirectResults? directResult
int columnCount = HiveServer2Reader.GetColumnCount(rowSet);
int rowCount = HiveServer2Reader.GetRowCount(rowSet,
columnCount);
IReadOnlyList<IArrowArray> data =
HiveServer2Reader.GetArrowArrayData(rowSet, columnCount, schema,
Connection.DataTypeConversion);
+
+ // Enhance column schema results if this is a GetColumns query
+ if (SqlQuery?.ToLowerInvariant() == GetColumnsCommandName)
Review Comment:
I think you can refactor this method so that you don't need to test if the
current query is the `GetColumnsCommandName`. The `EnhanceGetColumnsResult`
should be called from within `GetColumnsAsync`.
I can propose a refactor that uses virtual implementations, if you want a
more concreate suggestion.
##########
csharp/src/Drivers/Apache/Spark/SparkConnection.cs:
##########
@@ -63,7 +63,7 @@ public override AdbcStatement CreateStatement()
protected internal override int PositionRequiredOffset => 1;
- protected override void SetPrecisionScaleAndTypeName(
+ public override void SetPrecisionScaleAndTypeName(
Review Comment:
Could we use `protected internal` or `internal` here, instead of `public`?
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs:
##########
@@ -1364,7 +1359,7 @@ private static IArrowType GetArrowType(int columnTypeId,
string typeName, bool i
}
}
- protected async Task<TRowSet> FetchResultsAsync(TOperationHandle
operationHandle, long batchSize = BatchSizeDefault, CancellationToken
cancellationToken = default)
+ public async Task<TRowSet> FetchResultsAsync(TOperationHandle
operationHandle, long batchSize = BatchSizeDefault, CancellationToken
cancellationToken = default)
Review Comment:
Could we use `protected internal` or `internal` here, instead of `public`?
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2HttpConnection.cs:
##########
@@ -224,7 +224,7 @@ protected override TOpenSessionReq CreateSessionRequest()
return req;
}
- protected override void SetPrecisionScaleAndTypeName(
+ public override void SetPrecisionScaleAndTypeName(
Review Comment:
Could we use `protected internal` or `internal` here, instead of `public`?
##########
csharp/src/Drivers/Apache/Impala/ImpalaConnection.cs:
##########
@@ -86,7 +86,7 @@ protected override Task<TRowSet>
GetRowSetAsync(TGetSchemasResp response, Cancel
protected internal override Task<TRowSet>
GetRowSetAsync(TGetPrimaryKeysResp response, CancellationToken
cancellationToken = default) =>
FetchResultsAsync(response.OperationHandle, cancellationToken:
cancellationToken);
- protected override void SetPrecisionScaleAndTypeName(
+ public override void SetPrecisionScaleAndTypeName(
Review Comment:
Could we use `protected internal` or `internal` here, instead of `public`?
--
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]
