CurtHagenlocher commented on code in PR #2043:
URL: https://github.com/apache/arrow-adbc/pull/2043#discussion_r1695726282
##########
csharp/src/Drivers/BigQuery/BigQueryConnection.cs:
##########
@@ -997,21 +997,18 @@ public override AdbcStatement CreateStatement()
{
Dictionary<string, string> options = new Dictionary<string,
string>();
+ List<string> statementOptions = new List<string>()
Review Comment:
Consider using string array instead of string list. Also consider using
literal syntax.
##########
csharp/test/Drivers/BigQuery/BigQueryTestingUtils.cs:
##########
@@ -86,6 +86,11 @@ BigQueryTestConfiguration testConfiguration
parameters.Add(BigQueryParameters.LargeResultsDestinationTable,
testConfiguration.LargeResultsDestinationTable);
}
+ if(testConfiguration.TimeoutMinutes.HasValue)
Review Comment:
nit: insert space before paren
##########
csharp/src/Drivers/BigQuery/BigQueryStatement.cs:
##########
@@ -55,7 +55,21 @@ public override QueryResult ExecuteQuery()
{
QueryOptions? queryOptions = ValidateOptions();
BigQueryJob job = this.client.CreateQueryJob(SqlQuery, null,
queryOptions);
- BigQueryResults results = job.GetQueryResults();
+
+ GetQueryResultsOptions getQueryResultsOptions = new
GetQueryResultsOptions();
+
+ if (this.Options != null &&
this.Options.ContainsKey(BigQueryParameters.GetQueryResultsOptionsTimeoutMinutes))
Review Comment:
`this.Options.TryGetValue` ?
##########
csharp/src/Drivers/BigQuery/BigQueryConnection.cs:
##########
@@ -997,21 +997,18 @@ public override AdbcStatement CreateStatement()
{
Dictionary<string, string> options = new Dictionary<string,
string>();
+ List<string> statementOptions = new List<string>()
+ {
+ BigQueryParameters.AllowLargeResults,
+ BigQueryParameters.UseLegacySQL,
+ BigQueryParameters.LargeDecimalsAsString,
+ BigQueryParameters.LargeResultsDestinationTable,
+ BigQueryParameters.GetQueryResultsOptionsTimeoutMinutes
+ };
+
foreach (KeyValuePair<string, string> keyValuePair in
this.properties)
Review Comment:
Consider iterating on the list of supported keywords instead of the list of
supplied keywords, to get the advantage of dictionary lookup, e.g.
```
foreach (string key in [BigQueryParameters.AllowLargeResults,
BigQueryParameters.LargeDecimalsAsString, ... ])
{
if (this.properties.TryGetValue(key, out string value))
{
options[key] = value;
}
}
```
--
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]
