Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/22306 )
Change subject: IMPALA-13653: Create hooks for Calcite planner in Frontend ...................................................................... Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/22306/11/fe/src/main/java/org/apache/impala/service/CompilerFactory.java File fe/src/main/java/org/apache/impala/service/CompilerFactory.java: http://gerrit.cloudera.org:8080/#/c/22306/11/fe/src/main/java/org/apache/impala/service/CompilerFactory.java@54 PS11, Line 54: /** : * Return a fallback compiler factory given a planner context allowing the : * implementation to choose if it should fallback based on a query option or : * the type of exception thrown by the failed compiler factory. : */ : public CompilerFactory getFallbackFactory(TQueryOptions options, Exception e); > I kinda disagree with this one, but I'm not gonna argue about it. Thought I would do something like this (which might fit better in the Calcite add-on patch rather than the original patch): private TExecRequest getTExecRequestWithFallback( PlanCtx planCtx, EventSequence timeline) throws ImpalaException { TExecRequest request = null; if (condition_to_try_calcite) { try { // getCalciteCompilerFactory() has a return type of CompilerFactory request = getTExecRequest(getCalciteCompilerFactory(), planCtx, timeline); addPlannerToProfile(CALCITE); } catch (Exception e) { // In the testing mode, this would run the original parser and see if this is a // QueryStmt. In production, this just returns true. // This could pass in the exception too, but I don't think it needs it right now. if (!shouldFallbackToRegularPlanner(planCtx)) { throw e; } LOG.info("Calcite planner failed: " + e); timeline.markEvent("Failing over from Calcite planner"); } } if (request == null) { addPlannerToProfile(PLANNER); result = getTExecRequest(getRegularCompilerFactory(), planCtx, timeline); } return result; } -- To view, visit http://gerrit.cloudera.org:8080/22306 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I978aa48b160984ee5d36244cd915940f838d141d Gerrit-Change-Number: 22306 Gerrit-PatchSet: 11 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: Tue, 18 Mar 2025 18:01:18 +0000 Gerrit-HasComments: Yes