fx19880617 commented on issue #6298: URL: https://github.com/apache/incubator-pinot/issues/6298#issuecomment-759335011
> Thanks for the clarification! > > Yes, parsing `SHOW <OBJECT>` should be straight forward implementation. > Here is the list of SHOW SQL Statements in the scope (Please add if I have missed any): > > 1. SHOW **TABLES** > 2. SHOW **COLUMNS** FROM <table_name> > > **Question:** > Looking at `PinotQuery.java` and `BrokerResponse.java` it's implemented mainly for `SELECT` SQL statement adding `DESCRIBE` and `SHOW` would have to make quite a few changes to handle this. Instead, what do you think about the below implementation: > > 1. Entry point - `PinotClientRequest.processSqlQueryPost` - No change > 2. In `BaseBrokerRequestHandler.handleRequest`, call is made to `CalciteSqlComplier.compileToPinotQuery` based on query format which in this case would be `CALCITE_SQL_COMPILER` for SQL. Instead enhance `PinotQueryParserFactory` in switch to parse the SQL type (a) SHOW SQL, (b) DESCRIBE SQL, and (c) all others. Will add subclass to implement interface `AbstractCompiler.compileToBrokerRequest` method for case (a) and (b) which can be enhanced later to handle other cases within the scope like `LIKE | WHERE`. > 3. In the query executor - implement overloading loading method to handle (a) and (b) and add Switch based on `PinotQueryParserFactory` to all overloading methods. > 4. In the handler, implement call to Controller REST API and return `BrokerResponse`. > > Sorry for not having concise, but let me know your thoughts! Thanks! Hi @amarnathkarthik , thanks for sharing the thoughts! I think it's overall good! Since we are moving to SQL and plan to deprecate PQL, we can actually move this logic completely to CalciteParser. I also looked at `CalciteSqlParser`, for `SHOW TABLES`, it throws `org.apache.calcite.sql.parser.SqlParseException: Non-query expression encountered in illegal context`. For query like `DESCRIBE TABLE myTable`, it generates a `SqlDescribeTable` node. Then one way to solve it is to extend `SqlCall` to `SqlShowTables` class and update `CalciteSqlParser` parse the query to it. Then we can check sqlNode instance type for it: We have SqlOrderBy and SqlSelect for now, then we can add SqlShowTables/SqlDescribeTable nodes. Example here in Flink: ``` https://github.com/apache/flink/tree/master/flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/dql https://github.com/apache/flink/blob/master/flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/dql/SqlShowTables.java https://github.com/apache/flink/blob/master/flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl#L401 ``` ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
