Jackie-Jiang commented on code in PR #15281:
URL: https://github.com/apache/pinot/pull/15281#discussion_r2011049029
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java:
##########
@@ -70,10 +66,10 @@ public class PinotConnection extends AbstractBaseConnection
{
}
_session = new org.apache.pinot.client.Connection(properties, brokers,
transport);
- for (String possibleQueryOption: POSSIBLE_QUERY_OPTIONS) {
- Object property = properties.getProperty(possibleQueryOption);
- if (property != null) {
- _queryOptions.put(possibleQueryOption, parseOptionValue(property));
+ for (Map.Entry<Object, Object> property : properties.entrySet()) {
+ String configKey = QueryOptionsUtils.getConfigKey(property.getKey());
+ if (property.getKey().equals(configKey)) {
Review Comment:
This check would prevent case insensitivity
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java:
##########
@@ -438,4 +438,11 @@ private static IllegalArgumentException
longParseException(String optionName, @N
return new IllegalArgumentException(
String.format("%s must be a number between %d and 2^63-1, got: %s",
optionName, minValue, optionValue));
}
+
+ public static String getConfigKey(Object property) {
Review Comment:
Suggest moving this next to `resolveCaseInsensitiveOptions()` and rename it
to `resolveCaseInsensitiveKey()`. We should annotate this method as `Nullable`,
and `resolveCaseInsensitiveOptions()` can call this method to resolve each
individual option
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
