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

Reply via email to