walterddr commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1326251851


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +302,52 @@ public static Map<String, String> 
getOptionsFromJson(JsonNode request, String op
   public static Map<String, String> getOptionsFromString(String optionStr) {
     return 
Splitter.on(';').omitEmptyStrings().trimResults().withKeyValueSeparator('=').split(optionStr);
   }
+
+  public static Set<String> getTableNames(SqlNode sqlNode) {

Review Comment:
   i dont think this is the right way to go. we've removed this b/c without a 
proper SqlToRelConverter with typefactory and catalog plugin we can't be sure 
which of the identifiers are table, virtual/tmp tables and which are actually 
something we need routing info with. 
   
   IMO, there's 2 way of utilizing the java client, it can either directly hit 
a broker known to be able to handle the query, or it can hit controller. 
   1. if hits broker, it either success of fail.
   2. if it hits controller, it should provide an API to select broker based on 
SQL as well as list of tables --> b/c the controller can run the explain query 
to figure out 
       - what are all the tables associated, 
       - if there's any tenant intersection a broker can handle the request; 
       - handle more complex logic that must be implemented again in 
pinot-java-client if any of the logics changed in v2
   
   I would suggest adding a BrokerSelector API 
   ```
   String selectBroker(String 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]

Reply via email to