You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "walterddr (via GitHub)" <gi...@apache.org> on 2023/09/14 16:45:36 UTC

[GitHub] [pinot] walterddr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name

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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org