You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2023/08/12 22:52:57 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #11330: Fixing pinot client resolve table
xiangfu0 opened a new pull request, #11330:
URL: https://github.com/apache/pinot/pull/11330
Extract table name from SqlNode.
--
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
[GitHub] [pinot] abhioncbr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1292575928
##########
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:
Just a question, We got rid of a similar function before in this [PR](https://github.com/apache/pinot/pull/10873/files#diff-5bdf53db08d67e2823f300f3db3af05dc704f4ef5319299cc3233f9b55aba059L150). Are we trying to implement it again?
--
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
[GitHub] [pinot] walterddr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
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. and if we dont cache these info in client we wont be able to make proper decisions, it is simple not enough info to make that decision locally on the client
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);
```
and implement a query cache to basically cached the table list resolved from controller if the same query hits again. otherwise ask the controller for a list of proper brokers that can handle 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: 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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1294995756
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,151 @@ 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);
}
+
+ /**
+ * Returns all the table names from a given {@link SqlNode}.
+ * <pre>
+ * 1. FROM Clause (FromNode): The main location where the table name is specified.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name;
+ * }
+ * <pre>
+ * 2. JOIN Clauses (JoinNode): Table names will be part of INNER JOIN, LEFT JOIN, RIGHT JOIN, FULL JOIN, etc.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 JOIN table_name2 ON table_name1.column_name = table_name2.column_name;
+ * }
+ * <pre>
+ * 3. SubQueries in FROM Clause (SubQueryNode): Subqueries in the FROM clause might contain additional table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM (SELECT * FROM table_name) WHERE column_name = value;
+ * }
+ * <pre>
+ * 4. WITH Clause (WithNode): Common Table Expressions (CTEs) may contain table names.
+ * </pre>
+ * {@code
+ * WITH table_name1 AS (SELECT * FROM table_name2) SELECT * FROM table_name1;
+ * }
+ * <pre>
+ * 5. LATERAL or APPLY Operators (LateralNode, ApplyNode): These operators allow you to reference columns of
+ * preceding tables in FROM clause sub-queries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1, LATERAL (SELECT * FROM table_name2) AS table_name2;
+ * }
+ * <pre>
+ * 6. UNION, INTERSECT, EXCEPT Clauses (SetOperationNode): These set operations between multiple SELECT statements
+ * can also contain table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 UNION SELECT * FROM table_name2;
+ * }
+ * <pre>
+ * 7. WHERE Clause (WhereNode): WHERE clause can contain table names in subqueries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name WHERE column_name IN (SELECT * FROM table_name2);
+ * }
+ * @param sqlNode Sql Query Node
+ * @return Set of table names
+ */
+ public static Set<String> getTableNames(SqlNode sqlNode) {
+ Set<String> tableNames = new HashSet<>();
+ Set<String> tableNameAlias = new HashSet<>();
+ extractTableNames(sqlNode, tableNames, tableNameAlias);
+ tableNames.removeAll(tableNameAlias);
+ return tableNames;
+ }
+
+ public static void extractTableNames(SqlNode sqlNode, Set<String> tableNames, Set<String> tableNameAlias) {
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ } else {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ }
+ extractTableNames(((SqlSelect) sqlNode).getWhere(), tableNames, tableNameAlias);
+ } else if (sqlNode instanceof SqlJoin) {
+ // Handle SqlJoin query
+ SqlNode left = ((SqlJoin) sqlNode).getLeft();
+ SqlNode right = ((SqlJoin) sqlNode).getRight();
+ if (left instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) left)));
+ } else {
+ extractTableNames(left, tableNames, tableNameAlias);
+ }
+ if (right instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) right)));
+ } else {
+ extractTableNames(right, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlOrderBy) {
+ // Handle SqlOrderBy query
+ // tableNames.addAll(getTableNames(((SqlOrderBy) sqlNode).query));
+ for (SqlNode node : ((SqlOrderBy) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlBasicCall) {
+ // Handle SqlBasicCall query
+ if (((SqlBasicCall) sqlNode).getOperator() instanceof SqlAsOperator) {
+ SqlNode firstOperand = ((SqlBasicCall) sqlNode).getOperandList().get(0);
+ if (firstOperand instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) firstOperand));
+ } else {
+ extractTableNames(firstOperand, tableNames, tableNameAlias);
+ }
+ SqlNode secondOperand = ((SqlBasicCall) sqlNode).getOperandList().get(1);
+ if (secondOperand instanceof SqlIdentifier) {
+ tableNameAlias.add(getTableName((SqlIdentifier) secondOperand));
+ }
+ } else {
+ for (SqlNode node : ((SqlBasicCall) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ }
+ } else if (sqlNode instanceof SqlWith) {
+ // Handle SqlWith query
+ SqlWith sqlWith = (SqlWith) sqlNode;
+ List<SqlNode> withList = sqlWith.withList;
+ // Table names from body, it may contains table alias from WITH clause
+ extractTableNames(sqlWith.body, tableNames, tableNameAlias);
+ // Table alias from WITH clause, should be removed from the final results
+ withList.forEach(
+ sqlWithItem -> tableNames.remove(getTableName(((SqlWithItem) sqlWithItem).name)));
+ // Table names from WITH clause
+ withList.forEach(
+ sqlWithItem ->
+ extractTableNames(((SqlWithItem) sqlWithItem).getOperandList().get(2), tableNames, tableNameAlias));
+ } else if (sqlNode instanceof SqlSetOption) {
+ // Handle SqlSetOption query
+ for (SqlNode node : ((SqlSetOption) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlExplain) {
+ // Handle SqlExplain query
+ extractTableNames(((SqlExplain) sqlNode).getExplicandum(), tableNames, tableNameAlias);
+ }
+ tableNames.removeAll(tableNameAlias);
Review Comment:
Do we need to remove alias here, or gather all of them and remove all of them in the end?
--
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
[GitHub] [pinot] walterddr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
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. and if we dont cache these info in client we wont be able to make proper decisions, it is simple not enough info to make that decision locally on the client
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
[GitHub] [pinot] walterddr commented on pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #11330:
URL: https://github.com/apache/pinot/pull/11330#issuecomment-1720273025
Discussed with @Jackie-Jiang we should probably deprecate the BrokerSelect API
```
String selectBroker(String... tableName)
```
this API only make sense during v1 query where it is guarantee to only hint 1 table; and it is not a clear API for multi-stage engine, instead we should add
```
String selectBroker(String sql);
```
and encapsulate the current logic to parse table when SQL is a v1 query (for legacy compatibility) and use controller endpoint to select broker instead of a local broker cache in ControllerBaseBrokerSelector.
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1293117181
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,90 @@ 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) {
+ Set<String> tableNames = new HashSet<>();
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ tableNames.addAll(getTableNames(((SqlBasicCall) fromNode).getOperandList().get(0)));
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ tableNames.addAll(getTableNames(((SqlSelect) sqlNode).getWhere()));
Review Comment:
If I read correctly, the example query I posted will not check the `WHERE` clause because there is no recursion happening after line 314
--
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
[GitHub] [pinot] xiangfu0 commented on pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #11330:
URL: https://github.com/apache/pinot/pull/11330#issuecomment-1726696240
Close this as suggested by @walterddr to delegate the job to controller.
--
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
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1292603843
##########
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:
Yes, this logic is used in `pinot-java-client`, which depends on `pinot-common` not `pinot-query-planner`.
So the table name extraction logic there only works for v1 query and some simple v2 queries.
There are 3 possible options to solve this:
1. Let `pinot-java-client` depends `pinot-query-planner`;
2. Move some query compilation logics from `pinot-query-planner` to `pinot-common`, for `SqlNode` -> `RelNode`, but it requires the full initialization of Calcite PlannerContext with FrameworkConfig, Prepare.CatalogReader, RelDataTypeFactory, HepProgram.
3. Create a separated getTableName logic to `pinot-common`
--
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
[GitHub] [pinot] tibrewalpratik17 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1322951203
##########
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:
@xiangfu0 i think the major cause of removing this was maintaining the overhead of maintaining this function as we kept on finding edge cases where we were not extracting table names out. Though I see we are pushing to a random broker now so should be good. cc @walterddr what are your thoughts on bringing this method back?
--
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
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1295669041
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,151 @@ 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);
}
+
+ /**
+ * Returns all the table names from a given {@link SqlNode}.
+ * <pre>
+ * 1. FROM Clause (FromNode): The main location where the table name is specified.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name;
+ * }
+ * <pre>
+ * 2. JOIN Clauses (JoinNode): Table names will be part of INNER JOIN, LEFT JOIN, RIGHT JOIN, FULL JOIN, etc.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 JOIN table_name2 ON table_name1.column_name = table_name2.column_name;
+ * }
+ * <pre>
+ * 3. SubQueries in FROM Clause (SubQueryNode): Subqueries in the FROM clause might contain additional table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM (SELECT * FROM table_name) WHERE column_name = value;
+ * }
+ * <pre>
+ * 4. WITH Clause (WithNode): Common Table Expressions (CTEs) may contain table names.
+ * </pre>
+ * {@code
+ * WITH table_name1 AS (SELECT * FROM table_name2) SELECT * FROM table_name1;
+ * }
+ * <pre>
+ * 5. LATERAL or APPLY Operators (LateralNode, ApplyNode): These operators allow you to reference columns of
+ * preceding tables in FROM clause sub-queries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1, LATERAL (SELECT * FROM table_name2) AS table_name2;
+ * }
+ * <pre>
+ * 6. UNION, INTERSECT, EXCEPT Clauses (SetOperationNode): These set operations between multiple SELECT statements
+ * can also contain table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 UNION SELECT * FROM table_name2;
+ * }
+ * <pre>
+ * 7. WHERE Clause (WhereNode): WHERE clause can contain table names in subqueries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name WHERE column_name IN (SELECT * FROM table_name2);
+ * }
+ * @param sqlNode Sql Query Node
+ * @return Set of table names
+ */
+ public static Set<String> getTableNames(SqlNode sqlNode) {
+ Set<String> tableNames = new HashSet<>();
+ Set<String> tableNameAlias = new HashSet<>();
+ extractTableNames(sqlNode, tableNames, tableNameAlias);
+ tableNames.removeAll(tableNameAlias);
+ return tableNames;
+ }
+
+ public static void extractTableNames(SqlNode sqlNode, Set<String> tableNames, Set<String> tableNameAlias) {
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ } else {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ }
+ extractTableNames(((SqlSelect) sqlNode).getWhere(), tableNames, tableNameAlias);
+ } else if (sqlNode instanceof SqlJoin) {
+ // Handle SqlJoin query
+ SqlNode left = ((SqlJoin) sqlNode).getLeft();
+ SqlNode right = ((SqlJoin) sqlNode).getRight();
+ if (left instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) left)));
+ } else {
+ extractTableNames(left, tableNames, tableNameAlias);
+ }
+ if (right instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) right)));
+ } else {
+ extractTableNames(right, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlOrderBy) {
+ // Handle SqlOrderBy query
+ // tableNames.addAll(getTableNames(((SqlOrderBy) sqlNode).query));
Review Comment:
done
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,151 @@ 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);
}
+
+ /**
+ * Returns all the table names from a given {@link SqlNode}.
+ * <pre>
+ * 1. FROM Clause (FromNode): The main location where the table name is specified.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name;
+ * }
+ * <pre>
+ * 2. JOIN Clauses (JoinNode): Table names will be part of INNER JOIN, LEFT JOIN, RIGHT JOIN, FULL JOIN, etc.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 JOIN table_name2 ON table_name1.column_name = table_name2.column_name;
+ * }
+ * <pre>
+ * 3. SubQueries in FROM Clause (SubQueryNode): Subqueries in the FROM clause might contain additional table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM (SELECT * FROM table_name) WHERE column_name = value;
+ * }
+ * <pre>
+ * 4. WITH Clause (WithNode): Common Table Expressions (CTEs) may contain table names.
+ * </pre>
+ * {@code
+ * WITH table_name1 AS (SELECT * FROM table_name2) SELECT * FROM table_name1;
+ * }
+ * <pre>
+ * 5. LATERAL or APPLY Operators (LateralNode, ApplyNode): These operators allow you to reference columns of
+ * preceding tables in FROM clause sub-queries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1, LATERAL (SELECT * FROM table_name2) AS table_name2;
+ * }
+ * <pre>
+ * 6. UNION, INTERSECT, EXCEPT Clauses (SetOperationNode): These set operations between multiple SELECT statements
+ * can also contain table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 UNION SELECT * FROM table_name2;
+ * }
+ * <pre>
+ * 7. WHERE Clause (WhereNode): WHERE clause can contain table names in subqueries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name WHERE column_name IN (SELECT * FROM table_name2);
+ * }
+ * @param sqlNode Sql Query Node
+ * @return Set of table names
+ */
+ public static Set<String> getTableNames(SqlNode sqlNode) {
+ Set<String> tableNames = new HashSet<>();
+ Set<String> tableNameAlias = new HashSet<>();
+ extractTableNames(sqlNode, tableNames, tableNameAlias);
+ tableNames.removeAll(tableNameAlias);
+ return tableNames;
+ }
+
+ public static void extractTableNames(SqlNode sqlNode, Set<String> tableNames, Set<String> tableNameAlias) {
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ } else {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ }
+ extractTableNames(((SqlSelect) sqlNode).getWhere(), tableNames, tableNameAlias);
+ } else if (sqlNode instanceof SqlJoin) {
+ // Handle SqlJoin query
+ SqlNode left = ((SqlJoin) sqlNode).getLeft();
+ SqlNode right = ((SqlJoin) sqlNode).getRight();
+ if (left instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) left)));
+ } else {
+ extractTableNames(left, tableNames, tableNameAlias);
+ }
+ if (right instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) right)));
+ } else {
+ extractTableNames(right, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlOrderBy) {
+ // Handle SqlOrderBy query
+ // tableNames.addAll(getTableNames(((SqlOrderBy) sqlNode).query));
+ for (SqlNode node : ((SqlOrderBy) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlBasicCall) {
+ // Handle SqlBasicCall query
+ if (((SqlBasicCall) sqlNode).getOperator() instanceof SqlAsOperator) {
+ SqlNode firstOperand = ((SqlBasicCall) sqlNode).getOperandList().get(0);
+ if (firstOperand instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) firstOperand));
+ } else {
+ extractTableNames(firstOperand, tableNames, tableNameAlias);
+ }
+ SqlNode secondOperand = ((SqlBasicCall) sqlNode).getOperandList().get(1);
+ if (secondOperand instanceof SqlIdentifier) {
+ tableNameAlias.add(getTableName((SqlIdentifier) secondOperand));
+ }
+ } else {
+ for (SqlNode node : ((SqlBasicCall) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ }
+ } else if (sqlNode instanceof SqlWith) {
+ // Handle SqlWith query
+ SqlWith sqlWith = (SqlWith) sqlNode;
+ List<SqlNode> withList = sqlWith.withList;
+ // Table names from body, it may contains table alias from WITH clause
+ extractTableNames(sqlWith.body, tableNames, tableNameAlias);
+ // Table alias from WITH clause, should be removed from the final results
+ withList.forEach(
+ sqlWithItem -> tableNames.remove(getTableName(((SqlWithItem) sqlWithItem).name)));
+ // Table names from WITH clause
+ withList.forEach(
+ sqlWithItem ->
+ extractTableNames(((SqlWithItem) sqlWithItem).getOperandList().get(2), tableNames, tableNameAlias));
+ } else if (sqlNode instanceof SqlSetOption) {
+ // Handle SqlSetOption query
+ for (SqlNode node : ((SqlSetOption) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlExplain) {
+ // Handle SqlExplain query
+ extractTableNames(((SqlExplain) sqlNode).getExplicandum(), tableNames, tableNameAlias);
+ }
+ tableNames.removeAll(tableNameAlias);
Review Comment:
done
--
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
[GitHub] [pinot] codecov-commenter commented on pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11330:
URL: https://github.com/apache/pinot/pull/11330#issuecomment-1676144611
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11330?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11330](https://app.codecov.io/gh/apache/pinot/pull/11330?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (0dcd022) into [master](https://app.codecov.io/gh/apache/pinot/commit/51da2f5f67607ecff4903356d5ef0da47ab5ac06?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (51da2f5) will **decrease** coverage by `53.59%`.
> The diff coverage is `5.26%`.
```diff
@@ Coverage Diff @@
## master #11330 +/- ##
=============================================
- Coverage 68.18% 14.59% -53.59%
+ Complexity 6534 494 -6040
=============================================
Files 2239 2165 -74
Lines 120475 117332 -3143
Branches 18230 17788 -442
=============================================
- Hits 82141 17121 -65020
- Misses 32496 98796 +66300
+ Partials 5838 1415 -4423
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| java-11 | `14.59% <5.26%> (-53.55%)` | :arrow_down: |
| java-17 | `?` | |
| java-20 | `?` | |
| temurin | `14.59% <5.26%> (-53.59%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `14.59% <5.26%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files Changed](https://app.codecov.io/gh/apache/pinot/pull/11330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...pache/pinot/common/utils/request/RequestUtils.java](https://app.codecov.io/gh/apache/pinot/pull/11330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvcmVxdWVzdC9SZXF1ZXN0VXRpbHMuamF2YQ==) | `0.00% <0.00%> (-83.61%)` | :arrow_down: |
| [...t/controller/api/resources/PinotQueryResource.java](https://app.codecov.io/gh/apache/pinot/pull/11330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90UXVlcnlSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (-40.31%)` | :arrow_down: |
| [.../main/java/org/apache/pinot/client/Connection.java](https://app.codecov.io/gh/apache/pinot/pull/11330?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `69.04% <100.00%> (-1.41%)` | :arrow_down: |
... and [1708 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11330/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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
[GitHub] [pinot] abhioncbr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "abhioncbr (via GitHub)" <gi...@apache.org>.
abhioncbr commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1292815738
##########
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 think, we removed this function because it failed to get table names for some complex queries.
I am good with this method.
--
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
[GitHub] [pinot] walterddr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
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
[GitHub] [pinot] xiangfu0 commented on pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #11330:
URL: https://github.com/apache/pinot/pull/11330#issuecomment-1680390966
> precedence
This is actually valid, it means the scope of a query is limited, and there are some alias can be carried over but some are not.
Also need to handle the case of WITH and LATERAL which is more complex scenario.
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1294997737
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,151 @@ 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);
}
+
+ /**
+ * Returns all the table names from a given {@link SqlNode}.
+ * <pre>
+ * 1. FROM Clause (FromNode): The main location where the table name is specified.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name;
+ * }
+ * <pre>
+ * 2. JOIN Clauses (JoinNode): Table names will be part of INNER JOIN, LEFT JOIN, RIGHT JOIN, FULL JOIN, etc.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 JOIN table_name2 ON table_name1.column_name = table_name2.column_name;
+ * }
+ * <pre>
+ * 3. SubQueries in FROM Clause (SubQueryNode): Subqueries in the FROM clause might contain additional table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM (SELECT * FROM table_name) WHERE column_name = value;
+ * }
+ * <pre>
+ * 4. WITH Clause (WithNode): Common Table Expressions (CTEs) may contain table names.
+ * </pre>
+ * {@code
+ * WITH table_name1 AS (SELECT * FROM table_name2) SELECT * FROM table_name1;
+ * }
+ * <pre>
+ * 5. LATERAL or APPLY Operators (LateralNode, ApplyNode): These operators allow you to reference columns of
+ * preceding tables in FROM clause sub-queries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1, LATERAL (SELECT * FROM table_name2) AS table_name2;
+ * }
+ * <pre>
+ * 6. UNION, INTERSECT, EXCEPT Clauses (SetOperationNode): These set operations between multiple SELECT statements
+ * can also contain table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 UNION SELECT * FROM table_name2;
+ * }
+ * <pre>
+ * 7. WHERE Clause (WhereNode): WHERE clause can contain table names in subqueries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name WHERE column_name IN (SELECT * FROM table_name2);
+ * }
+ * @param sqlNode Sql Query Node
+ * @return Set of table names
+ */
+ public static Set<String> getTableNames(SqlNode sqlNode) {
+ Set<String> tableNames = new HashSet<>();
+ Set<String> tableNameAlias = new HashSet<>();
+ extractTableNames(sqlNode, tableNames, tableNameAlias);
+ tableNames.removeAll(tableNameAlias);
+ return tableNames;
+ }
+
+ public static void extractTableNames(SqlNode sqlNode, Set<String> tableNames, Set<String> tableNameAlias) {
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ } else {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ }
+ extractTableNames(((SqlSelect) sqlNode).getWhere(), tableNames, tableNameAlias);
+ } else if (sqlNode instanceof SqlJoin) {
+ // Handle SqlJoin query
+ SqlNode left = ((SqlJoin) sqlNode).getLeft();
+ SqlNode right = ((SqlJoin) sqlNode).getRight();
+ if (left instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) left)));
+ } else {
+ extractTableNames(left, tableNames, tableNameAlias);
+ }
+ if (right instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) right)));
+ } else {
+ extractTableNames(right, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlOrderBy) {
+ // Handle SqlOrderBy query
+ // tableNames.addAll(getTableNames(((SqlOrderBy) sqlNode).query));
Review Comment:
(minor) Remove?
--
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
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1292603843
##########
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:
Yes, this logic is used in `pinot-java-client`, which depends on `pinot-common` not `pinot-query-planner`.
So the table name extraction logic there only works for v1 query and some simple v2 queries.
There are 3 possible options to solve this:
1. Let `pinot-java-client` depends `pinot-query-planner`, use v2 `QueryEnvironment` from `pinot-query-planner` to compile `SqlNode` -> `RelNode`, then extract table names. But it requires the full initialization of Calcite PlannerContext with FrameworkConfig, Prepare.CatalogReader, RelDataTypeFactory, HepProgram. And the compilation time might take 10+ms, which is 10-100 times more than extracting from SqlNode.
2. Create a separated getTableName logic to `pinot-common`, hence is this PR.
##########
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:
Yes, this logic is used in `pinot-java-client`, which depends on `pinot-common` not `pinot-query-planner`.
So the table name extraction logic there only works for v1 query and some simple v2 queries.
There are 3 possible options to solve this:
1. Let `pinot-java-client` depends `pinot-query-planner`, use v2 `QueryEnvironment` from `pinot-query-planner` to compile `SqlNode` -> `RelNode`, then extract table names. But it requires the full initialization of Calcite PlannerContext with FrameworkConfig, Prepare.CatalogReader, RelDataTypeFactory, HepProgram. And the compilation time might take 10+ms, which is 10-100 times more than extracting from SqlNode.
2. Create a separated getTableName logic to `pinot-common`, hence this PR.
##########
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:
Yes, this logic is used in `pinot-java-client`, which depends on `pinot-common` not `pinot-query-planner`.
So the table name extraction logic there only works for v1 query and some simple v2 queries.
There are 2 possible options to solve this:
1. Let `pinot-java-client` depends `pinot-query-planner`, use v2 `QueryEnvironment` from `pinot-query-planner` to compile `SqlNode` -> `RelNode`, then extract table names. But it requires the full initialization of Calcite PlannerContext with FrameworkConfig, Prepare.CatalogReader, RelDataTypeFactory, HepProgram. And the compilation time might take 10+ms, which is 10-100 times more than extracting from SqlNode.
2. Create a separated getTableName logic to `pinot-common`, hence this PR.
--
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
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1293090335
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,90 @@ 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) {
+ Set<String> tableNames = new HashSet<>();
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ tableNames.addAll(getTableNames(((SqlBasicCall) fromNode).getOperandList().get(0)));
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ tableNames.addAll(getTableNames(((SqlSelect) sqlNode).getWhere()));
Review Comment:
Need to check recursively.
See more examples in tests:
https://github.com/apache/pinot/pull/11330/files#diff-a4bccc2639d28b1ed4eeb105200ce6da89b1f53876909863134a7084c1aa2b8cR95
and
https://github.com/apache/pinot/pull/11330/files#diff-a4bccc2639d28b1ed4eeb105200ce6da89b1f53876909863134a7084c1aa2b8cR147
--
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
[GitHub] [pinot] xiangfu0 commented on pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #11330:
URL: https://github.com/apache/pinot/pull/11330#issuecomment-1676914456
> Can we list down all the places where table name is valid as javadoc and in the PR description to ensure we won't miss any corner case?
added.
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1293064632
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,90 @@ 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) {
+ Set<String> tableNames = new HashSet<>();
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ tableNames.addAll(getTableNames(((SqlBasicCall) fromNode).getOperandList().get(0)));
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ tableNames.addAll(getTableNames(((SqlSelect) sqlNode).getWhere()));
Review Comment:
Can we skip `WHERE` clause for other if branches?
E.g. `SELECT * FROM myTable AS T1 WHERE col1 = (SELECT col2 FROM T2)`
--
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
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1293090335
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,90 @@ 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) {
+ Set<String> tableNames = new HashSet<>();
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ tableNames.addAll(getTableNames(((SqlBasicCall) fromNode).getOperandList().get(0)));
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ tableNames.addAll(getTableNames(((SqlSelect) sqlNode).getWhere()));
Review Comment:
Need to check recursively. This Where SqlNode is a SqlBasicCall, with SqlSelect inside.
See more examples in tests:
https://github.com/apache/pinot/pull/11330/files#diff-a4bccc2639d28b1ed4eeb105200ce6da89b1f53876909863134a7084c1aa2b8cR95
and
https://github.com/apache/pinot/pull/11330/files#diff-a4bccc2639d28b1ed4eeb105200ce6da89b1f53876909863134a7084c1aa2b8cR147
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1294995248
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,151 @@ 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);
}
+
+ /**
+ * Returns all the table names from a given {@link SqlNode}.
+ * <pre>
+ * 1. FROM Clause (FromNode): The main location where the table name is specified.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name;
+ * }
+ * <pre>
+ * 2. JOIN Clauses (JoinNode): Table names will be part of INNER JOIN, LEFT JOIN, RIGHT JOIN, FULL JOIN, etc.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 JOIN table_name2 ON table_name1.column_name = table_name2.column_name;
+ * }
+ * <pre>
+ * 3. SubQueries in FROM Clause (SubQueryNode): Subqueries in the FROM clause might contain additional table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM (SELECT * FROM table_name) WHERE column_name = value;
+ * }
+ * <pre>
+ * 4. WITH Clause (WithNode): Common Table Expressions (CTEs) may contain table names.
+ * </pre>
+ * {@code
+ * WITH table_name1 AS (SELECT * FROM table_name2) SELECT * FROM table_name1;
+ * }
+ * <pre>
+ * 5. LATERAL or APPLY Operators (LateralNode, ApplyNode): These operators allow you to reference columns of
+ * preceding tables in FROM clause sub-queries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1, LATERAL (SELECT * FROM table_name2) AS table_name2;
+ * }
+ * <pre>
+ * 6. UNION, INTERSECT, EXCEPT Clauses (SetOperationNode): These set operations between multiple SELECT statements
+ * can also contain table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 UNION SELECT * FROM table_name2;
+ * }
+ * <pre>
+ * 7. WHERE Clause (WhereNode): WHERE clause can contain table names in subqueries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name WHERE column_name IN (SELECT * FROM table_name2);
+ * }
+ * @param sqlNode Sql Query Node
+ * @return Set of table names
+ */
+ public static Set<String> getTableNames(SqlNode sqlNode) {
+ Set<String> tableNames = new HashSet<>();
+ Set<String> tableNameAlias = new HashSet<>();
+ extractTableNames(sqlNode, tableNames, tableNameAlias);
+ tableNames.removeAll(tableNameAlias);
+ return tableNames;
+ }
+
+ public static void extractTableNames(SqlNode sqlNode, Set<String> tableNames, Set<String> tableNameAlias) {
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ } else {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ }
+ extractTableNames(((SqlSelect) sqlNode).getWhere(), tableNames, tableNameAlias);
+ } else if (sqlNode instanceof SqlJoin) {
+ // Handle SqlJoin query
+ SqlNode left = ((SqlJoin) sqlNode).getLeft();
+ SqlNode right = ((SqlJoin) sqlNode).getRight();
+ if (left instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) left)));
+ } else {
+ extractTableNames(left, tableNames, tableNameAlias);
+ }
+ if (right instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) right)));
+ } else {
+ extractTableNames(right, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlOrderBy) {
+ // Handle SqlOrderBy query
+ // tableNames.addAll(getTableNames(((SqlOrderBy) sqlNode).query));
+ for (SqlNode node : ((SqlOrderBy) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlBasicCall) {
+ // Handle SqlBasicCall query
+ if (((SqlBasicCall) sqlNode).getOperator() instanceof SqlAsOperator) {
+ SqlNode firstOperand = ((SqlBasicCall) sqlNode).getOperandList().get(0);
+ if (firstOperand instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) firstOperand));
+ } else {
+ extractTableNames(firstOperand, tableNames, tableNameAlias);
+ }
+ SqlNode secondOperand = ((SqlBasicCall) sqlNode).getOperandList().get(1);
+ if (secondOperand instanceof SqlIdentifier) {
+ tableNameAlias.add(getTableName((SqlIdentifier) secondOperand));
+ }
+ } else {
+ for (SqlNode node : ((SqlBasicCall) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ }
+ } else if (sqlNode instanceof SqlWith) {
+ // Handle SqlWith query
+ SqlWith sqlWith = (SqlWith) sqlNode;
+ List<SqlNode> withList = sqlWith.withList;
+ // Table names from body, it may contains table alias from WITH clause
+ extractTableNames(sqlWith.body, tableNames, tableNameAlias);
+ // Table alias from WITH clause, should be removed from the final results
+ withList.forEach(
+ sqlWithItem -> tableNames.remove(getTableName(((SqlWithItem) sqlWithItem).name)));
Review Comment:
Should we add them into `tableNameAlias`?
--
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
[GitHub] [pinot] xiangfu0 commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1295667250
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -290,4 +303,151 @@ 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);
}
+
+ /**
+ * Returns all the table names from a given {@link SqlNode}.
+ * <pre>
+ * 1. FROM Clause (FromNode): The main location where the table name is specified.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name;
+ * }
+ * <pre>
+ * 2. JOIN Clauses (JoinNode): Table names will be part of INNER JOIN, LEFT JOIN, RIGHT JOIN, FULL JOIN, etc.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 JOIN table_name2 ON table_name1.column_name = table_name2.column_name;
+ * }
+ * <pre>
+ * 3. SubQueries in FROM Clause (SubQueryNode): Subqueries in the FROM clause might contain additional table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM (SELECT * FROM table_name) WHERE column_name = value;
+ * }
+ * <pre>
+ * 4. WITH Clause (WithNode): Common Table Expressions (CTEs) may contain table names.
+ * </pre>
+ * {@code
+ * WITH table_name1 AS (SELECT * FROM table_name2) SELECT * FROM table_name1;
+ * }
+ * <pre>
+ * 5. LATERAL or APPLY Operators (LateralNode, ApplyNode): These operators allow you to reference columns of
+ * preceding tables in FROM clause sub-queries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1, LATERAL (SELECT * FROM table_name2) AS table_name2;
+ * }
+ * <pre>
+ * 6. UNION, INTERSECT, EXCEPT Clauses (SetOperationNode): These set operations between multiple SELECT statements
+ * can also contain table names.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name1 UNION SELECT * FROM table_name2;
+ * }
+ * <pre>
+ * 7. WHERE Clause (WhereNode): WHERE clause can contain table names in subqueries.
+ * </pre>
+ * {@code
+ * SELECT * FROM table_name WHERE column_name IN (SELECT * FROM table_name2);
+ * }
+ * @param sqlNode Sql Query Node
+ * @return Set of table names
+ */
+ public static Set<String> getTableNames(SqlNode sqlNode) {
+ Set<String> tableNames = new HashSet<>();
+ Set<String> tableNameAlias = new HashSet<>();
+ extractTableNames(sqlNode, tableNames, tableNameAlias);
+ tableNames.removeAll(tableNameAlias);
+ return tableNames;
+ }
+
+ public static void extractTableNames(SqlNode sqlNode, Set<String> tableNames, Set<String> tableNameAlias) {
+ if (sqlNode instanceof SqlSelect) {
+ // Handle SqlSelect query
+ SqlNode fromNode = ((SqlSelect) sqlNode).getFrom();
+ if ((fromNode instanceof SqlBasicCall)
+ && (((SqlBasicCall) fromNode).getOperator() instanceof SqlAsOperator)) {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ } else if (fromNode instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) fromNode));
+ } else {
+ extractTableNames(fromNode, tableNames, tableNameAlias);
+ }
+ extractTableNames(((SqlSelect) sqlNode).getWhere(), tableNames, tableNameAlias);
+ } else if (sqlNode instanceof SqlJoin) {
+ // Handle SqlJoin query
+ SqlNode left = ((SqlJoin) sqlNode).getLeft();
+ SqlNode right = ((SqlJoin) sqlNode).getRight();
+ if (left instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) left)));
+ } else {
+ extractTableNames(left, tableNames, tableNameAlias);
+ }
+ if (right instanceof SqlIdentifier) {
+ tableNames.add(getTableName(((SqlIdentifier) right)));
+ } else {
+ extractTableNames(right, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlOrderBy) {
+ // Handle SqlOrderBy query
+ // tableNames.addAll(getTableNames(((SqlOrderBy) sqlNode).query));
+ for (SqlNode node : ((SqlOrderBy) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ } else if (sqlNode instanceof SqlBasicCall) {
+ // Handle SqlBasicCall query
+ if (((SqlBasicCall) sqlNode).getOperator() instanceof SqlAsOperator) {
+ SqlNode firstOperand = ((SqlBasicCall) sqlNode).getOperandList().get(0);
+ if (firstOperand instanceof SqlIdentifier) {
+ tableNames.add(getTableName((SqlIdentifier) firstOperand));
+ } else {
+ extractTableNames(firstOperand, tableNames, tableNameAlias);
+ }
+ SqlNode secondOperand = ((SqlBasicCall) sqlNode).getOperandList().get(1);
+ if (secondOperand instanceof SqlIdentifier) {
+ tableNameAlias.add(getTableName((SqlIdentifier) secondOperand));
+ }
+ } else {
+ for (SqlNode node : ((SqlBasicCall) sqlNode).getOperandList()) {
+ extractTableNames(node, tableNames, tableNameAlias);
+ }
+ }
+ } else if (sqlNode instanceof SqlWith) {
+ // Handle SqlWith query
+ SqlWith sqlWith = (SqlWith) sqlNode;
+ List<SqlNode> withList = sqlWith.withList;
+ // Table names from body, it may contains table alias from WITH clause
+ extractTableNames(sqlWith.body, tableNames, tableNameAlias);
+ // Table alias from WITH clause, should be removed from the final results
+ withList.forEach(
+ sqlWithItem -> tableNames.remove(getTableName(((SqlWithItem) sqlWithItem).name)));
Review Comment:
done.
--
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
[GitHub] [pinot] walterddr commented on a diff in pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #11330:
URL: https://github.com/apache/pinot/pull/11330#discussion_r1326255041
##########
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:
@Jackie-Jiang @xiangfu0 ^
--
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
[GitHub] [pinot] xiangfu0 closed pull request #11330: [multistage] Fixing pinot client resolve table name
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #11330: [multistage] Fixing pinot client resolve table name
URL: https://github.com/apache/pinot/pull/11330
--
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