CurtHagenlocher commented on code in PR #3022:
URL: https://github.com/apache/arrow-adbc/pull/3022#discussion_r2190477444
##########
csharp/src/Drivers/BigQuery/BigQueryParameters.cs:
##########
@@ -23,29 +25,48 @@ namespace Apache.Arrow.Adbc.Drivers.BigQuery
internal class BigQueryParameters
{
public const string AccessToken = "adbc.bigquery.access_token";
+ public const string AllowLargeResults =
"adbc.bigquery.allow_large_results";
public const string AudienceUri = "adbc.bigquery.audience_uri";
- public const string ProjectId = "adbc.bigquery.project_id";
+ public const string AuthenticationType = "adbc.bigquery.auth_type";
public const string BillingProjectId =
"adbc.bigquery.billing_project_id";
public const string ClientId = "adbc.bigquery.client_id";
public const string ClientSecret = "adbc.bigquery.client_secret";
- public const string RefreshToken = "adbc.bigquery.refresh_token";
- public const string AuthenticationType = "adbc.bigquery.auth_type";
+ public const string ClientTimeout = "adbc.bigquery.client.timeout";
+ public const string EvaluationKind =
"adbc.bigquery.multiple_statement.evaluation_kind";
+ public const string GetQueryResultsOptionsTimeout =
"adbc.bigquery.get_query_results_options.timeout";
+ public const string IncludeConstraintsWithGetObjects =
"adbc.bigquery.include_constraints_getobjects";
+ public const string IncludePublicProjectId =
"adbc.bigquery.include_public_project_id";
public const string JsonCredential =
"adbc.bigquery.auth_json_credential";
- public const string AllowLargeResults =
"adbc.bigquery.allow_large_results";
- public const string LargeResultsDestinationTable =
"adbc.bigquery.large_results_destination_table";
- public const string UseLegacySQL = "adbc.bigquery.use_legacy_sql";
public const string LargeDecimalsAsString =
"adbc.bigquery.large_decimals_as_string";
- public const string Scopes = "adbc.bigquery.scopes";
- public const string IncludeConstraintsWithGetObjects =
"adbc.bigquery.include_constraints_getobjects";
- public const string ClientTimeout = "adbc.bigquery.client.timeout";
+ public const string LargeResultsDataset =
"adbc.bigquery.large_results_dataset";
+ public const string LargeResultsDestinationTable =
"adbc.bigquery.large_results_destination_table";
+ public const string MaxFetchConcurrency =
"adbc.bigquery.max_fetch_concurrency";
public const string MaximumRetryAttempts =
"adbc.bigquery.maximum_retries";
+ public const string ProjectId = "adbc.bigquery.project_id";
+ public const string RefreshToken = "adbc.bigquery.refresh_token";
public const string RetryDelayMs = "adbc.bigquery.retry_delay_ms";
- public const string GetQueryResultsOptionsTimeout =
"adbc.bigquery.get_query_results_options.timeout";
- public const string MaxFetchConcurrency =
"adbc.bigquery.max_fetch_concurrency";
- public const string IncludePublicProjectId =
"adbc.bigquery.include_public_project_id";
- public const string StatementType =
"adbc.bigquery.multiple_statement.statement_type";
+ public const string Scopes = "adbc.bigquery.scopes";
public const string StatementIndex =
"adbc.bigquery.multiple_statement.statement_index";
- public const string EvaluationKind =
"adbc.bigquery.multiple_statement.evaluation_kind";
+ public const string StatementType =
"adbc.bigquery.multiple_statement.statement_type";
+ public const string UseLegacySQL = "adbc.bigquery.use_legacy_sql";
+
+ // these values are safe to log any time
+ private static HashSet<string> safeToLog = new HashSet<string>()
+ {
+ AllowLargeResults, AuthenticationType, BillingProjectId, ClientId,
ClientTimeout,
+ EvaluationKind, GetQueryResultsOptionsTimeout,
IncludeConstraintsWithGetObjects,
+ IncludePublicProjectId, LargeDecimalsAsString,
LargeResultsDataset, LargeResultsDestinationTable,
+ MaxFetchConcurrency, MaximumRetryAttempts, ProjectId,
RetryDelayMs, StatementIndex,
+ StatementType, UseLegacySQL
+ };
+
+ public static bool IsSafeToLog(string name)
+ {
+ if (safeToLog.Contains(name.ToLower()))
Review Comment:
I think it's generally better to declare the `HashSet` as taking a
case-insensitive comparer than to do `ToLower` as the latter may cause an
allocation. (In this case it probably won't because the options are generally
defined as lower-case; this is more about the pattern.)
Otherwise, this should probably be `ToLowerInvariant()` as I don't think
we'd want a culture-specific conversion.
##########
csharp/src/Drivers/BigQuery/BigQueryParameters.cs:
##########
@@ -23,29 +25,48 @@ namespace Apache.Arrow.Adbc.Drivers.BigQuery
internal class BigQueryParameters
{
public const string AccessToken = "adbc.bigquery.access_token";
+ public const string AllowLargeResults =
"adbc.bigquery.allow_large_results";
public const string AudienceUri = "adbc.bigquery.audience_uri";
- public const string ProjectId = "adbc.bigquery.project_id";
+ public const string AuthenticationType = "adbc.bigquery.auth_type";
public const string BillingProjectId =
"adbc.bigquery.billing_project_id";
public const string ClientId = "adbc.bigquery.client_id";
public const string ClientSecret = "adbc.bigquery.client_secret";
- public const string RefreshToken = "adbc.bigquery.refresh_token";
- public const string AuthenticationType = "adbc.bigquery.auth_type";
+ public const string ClientTimeout = "adbc.bigquery.client.timeout";
+ public const string EvaluationKind =
"adbc.bigquery.multiple_statement.evaluation_kind";
+ public const string GetQueryResultsOptionsTimeout =
"adbc.bigquery.get_query_results_options.timeout";
+ public const string IncludeConstraintsWithGetObjects =
"adbc.bigquery.include_constraints_getobjects";
+ public const string IncludePublicProjectId =
"adbc.bigquery.include_public_project_id";
public const string JsonCredential =
"adbc.bigquery.auth_json_credential";
- public const string AllowLargeResults =
"adbc.bigquery.allow_large_results";
- public const string LargeResultsDestinationTable =
"adbc.bigquery.large_results_destination_table";
- public const string UseLegacySQL = "adbc.bigquery.use_legacy_sql";
public const string LargeDecimalsAsString =
"adbc.bigquery.large_decimals_as_string";
- public const string Scopes = "adbc.bigquery.scopes";
- public const string IncludeConstraintsWithGetObjects =
"adbc.bigquery.include_constraints_getobjects";
- public const string ClientTimeout = "adbc.bigquery.client.timeout";
+ public const string LargeResultsDataset =
"adbc.bigquery.large_results_dataset";
+ public const string LargeResultsDestinationTable =
"adbc.bigquery.large_results_destination_table";
+ public const string MaxFetchConcurrency =
"adbc.bigquery.max_fetch_concurrency";
public const string MaximumRetryAttempts =
"adbc.bigquery.maximum_retries";
+ public const string ProjectId = "adbc.bigquery.project_id";
+ public const string RefreshToken = "adbc.bigquery.refresh_token";
public const string RetryDelayMs = "adbc.bigquery.retry_delay_ms";
- public const string GetQueryResultsOptionsTimeout =
"adbc.bigquery.get_query_results_options.timeout";
- public const string MaxFetchConcurrency =
"adbc.bigquery.max_fetch_concurrency";
- public const string IncludePublicProjectId =
"adbc.bigquery.include_public_project_id";
- public const string StatementType =
"adbc.bigquery.multiple_statement.statement_type";
+ public const string Scopes = "adbc.bigquery.scopes";
public const string StatementIndex =
"adbc.bigquery.multiple_statement.statement_index";
- public const string EvaluationKind =
"adbc.bigquery.multiple_statement.evaluation_kind";
+ public const string StatementType =
"adbc.bigquery.multiple_statement.statement_type";
+ public const string UseLegacySQL = "adbc.bigquery.use_legacy_sql";
+
+ // these values are safe to log any time
+ private static HashSet<string> safeToLog = new HashSet<string>()
+ {
+ AllowLargeResults, AuthenticationType, BillingProjectId, ClientId,
ClientTimeout,
+ EvaluationKind, GetQueryResultsOptionsTimeout,
IncludeConstraintsWithGetObjects,
+ IncludePublicProjectId, LargeDecimalsAsString,
LargeResultsDataset, LargeResultsDestinationTable,
+ MaxFetchConcurrency, MaximumRetryAttempts, ProjectId,
RetryDelayMs, StatementIndex,
+ StatementType, UseLegacySQL
+ };
+
+ public static bool IsSafeToLog(string name)
+ {
+ if (safeToLog.Contains(name.ToLower()))
Review Comment:
(I just noticed that there are at least three other `ToLower()` in this
project that maybe should be `ToLowerInvariant()` -- though in the case of
`BigQueryConnection.DescToField` I'm 1) not sure why we're changing the case of
the column name and 2) we're calling `ToLower` twice on the same value instead
of just once and storing it in a local.)
--
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]
