xiangfu0 commented on code in PR #10292:
URL: https://github.com/apache/pinot/pull/10292#discussion_r1110621495
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java:
##########
@@ -67,15 +74,24 @@ public class PinotConnection extends AbstractBaseConnection
{
}
_session = new org.apache.pinot.client.Connection(properties, brokers,
transport);
- _enableNullHandling =
Boolean.parseBoolean(properties.getProperty(QueryOptionKey.ENABLE_NULL_HANDLING));
+ for (String possibleQueryOption: POSSIBLE_QUERY_OPTIONS) {
Review Comment:
Maybe directly get a map of query option from `properties`, and you can use
it for any future supported query options as well.
For backward compatibility, you can set `enableNullHandling` if configured
as `properties.getProperty(QueryOptionKey.ENABLE_NULL_HANDLING)`
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java:
##########
@@ -67,15 +74,24 @@ public class PinotConnection extends AbstractBaseConnection
{
}
_session = new org.apache.pinot.client.Connection(properties, brokers,
transport);
- _enableNullHandling =
Boolean.parseBoolean(properties.getProperty(QueryOptionKey.ENABLE_NULL_HANDLING));
+ for (String possibleQueryOption: POSSIBLE_QUERY_OPTIONS) {
+ _queryOptions.put(possibleQueryOption,
Boolean.parseBoolean(properties.getProperty(possibleQueryOption)));
+ }
}
public org.apache.pinot.client.Connection getSession() {
return _session;
}
- public boolean isNullHandlingEnabled() {
- return _enableNullHandling;
+ protected String enableQueryOptions(String sql) {
+ StringBuilder optionsBuilder = new StringBuilder();
+ for (HashMap.Entry<String, Boolean> optionEntry: _queryOptions.entrySet())
{
+ if (optionEntry.getValue() && !sql.contains(optionEntry.getKey())) {
+
optionsBuilder.append(DriverUtils.createSetQueryOptionString(optionEntry.getKey()));
Review Comment:
Should append a kv pair.
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java:
##########
@@ -38,11 +40,16 @@
public class PinotConnection extends AbstractBaseConnection {
private static final Logger LOGGER =
LoggerFactory.getLogger(Connection.class);
+ protected static final String[] POSSIBLE_QUERY_OPTIONS = {
+ QueryOptionKey.ENABLE_NULL_HANDLING,
+ QueryOptionKey.USE_MULTISTAGE_ENGINE
+ };
private org.apache.pinot.client.Connection _session;
private boolean _closed;
private String _controllerURL;
private PinotControllerTransport _controllerTransport;
- private final boolean _enableNullHandling;
+ private final HashMap<String, Boolean> _queryOptions = new HashMap<String,
Boolean>();
Review Comment:
use `Map<String, String> _queryOptions` for future extensibility.
##########
pinot-clients/pinot-jdbc-client/src/main/java/org/apache/pinot/client/PinotConnection.java:
##########
@@ -67,15 +74,24 @@ public class PinotConnection extends AbstractBaseConnection
{
}
_session = new org.apache.pinot.client.Connection(properties, brokers,
transport);
- _enableNullHandling =
Boolean.parseBoolean(properties.getProperty(QueryOptionKey.ENABLE_NULL_HANDLING));
+ for (String possibleQueryOption: POSSIBLE_QUERY_OPTIONS) {
+ _queryOptions.put(possibleQueryOption,
Boolean.parseBoolean(properties.getProperty(possibleQueryOption)));
+ }
}
public org.apache.pinot.client.Connection getSession() {
return _session;
}
- public boolean isNullHandlingEnabled() {
- return _enableNullHandling;
+ protected String enableQueryOptions(String sql) {
Review Comment:
just return query options and make a static method in `DriverUtils` to take
both sql and query option map to reconstruct the query.
--
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]
