Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/22319 )
Change subject: IMPALA-13657: Connect Calcite planner to Impala Frontend framework ...................................................................... Patch Set 18: (4 comments) http://gerrit.cloudera.org:8080/#/c/22319/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisResult.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisResult.java: http://gerrit.cloudera.org:8080/#/c/22319/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteAnalysisResult.java@50 PS18, Line 50: super(analysisDriver.getParsedStmt(), analysisDriver.getAnalyzer(), null); This should pass in "e" for the last argument. http://gerrit.cloudera.org:8080/#/c/22319/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java: http://gerrit.cloudera.org:8080/#/c/22319/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java@70 PS18, Line 70: if (!CalciteJniFrontend.optionSupportedInCalcite(queryCtx)) { : return new ParsedStatementImpl(queryCtx); : } I think this needs to fail. If we return a ParsedStatementImpl, that will get passed into Calcite's analysis code, which can't handle it. http://gerrit.cloudera.org:8080/#/c/22319/18/tests/query_test/test_calcite_planner.py File tests/query_test/test_calcite_planner.py: http://gerrit.cloudera.org:8080/#/c/22319/18/tests/query_test/test_calcite_planner.py@29 PS18, Line 29: class TestCalcitePlanner(ImpalaTestSuite): For this to run in precommit, we need the regular cluster startup to have the Calcite jar on the classpath. Otherwise, it will fail. I'll think about how this should work, but basically we'll need to figure it out before this merges. http://gerrit.cloudera.org:8080/#/c/22319/18/tests/query_test/test_calcite_planner.py@43 PS18, Line 43: @pytest.mark.execute_serially I think you can drop this. -- To view, visit http://gerrit.cloudera.org:8080/22319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3b30571beb797ede827ef4d794b8daefb130ccb1 Gerrit-Change-Number: 22319 Gerrit-PatchSet: 18 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@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: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Mon, 17 Mar 2025 22:23:19 +0000 Gerrit-HasComments: Yes
