Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/21109 )
Change subject: IMPALA-12872: Use Calcite for optimization - part 1: simple queries ...................................................................... Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/21109/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java: http://gerrit.cloudera.org:8080/#/c/21109/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteJniFrontend.java@56 PS15, Line 56: public class CalciteJniFrontend extends JniFrontend { Future: One path forward is that as the Calcite planner gains functionality, it eventually gets pulled into the regular frontend. In that world, Calcite planning would be controlled by a query option and wouldn't need to be its own subclass of JniFrontend. The normal JniFrontend would call out to the right planner (while trying to share lots of code). How does that line up with your own idea of what the future looks like? We have a lot of logic in various parts of the existing frontend that we want to reuse if can. For example, Frontend's getTExecRequest() has a lot of logic for workload-aware autoscaling and considering plans for different sizes of executor groups. The Calcite planner implements something similar to Frontend's doCreateExecRequest(). http://gerrit.cloudera.org:8080/#/c/21109/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java: http://gerrit.cloudera.org:8080/#/c/21109/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteMetadataHandler.java@93 PS15, Line 93: // load the relevant tables in the query from catalogd : this.stmtTableCache_ = stmtMetadataLoader.loadTables(tableVisitor.tableNames_); Future: When this supports views, are we thinking that StmtMetadataLoader will do that under the covers? It feels like StmtMetadataLoader only really needs the StatementBase for limited things. -- To view, visit http://gerrit.cloudera.org:8080/21109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I453fd75b7b705f4d7de1ed73c3e24cafad0b8c98 Gerrit-Change-Number: 21109 Gerrit-PatchSet: 15 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Tue, 26 Mar 2024 02:03:44 +0000 Gerrit-HasComments: Yes
