This is an automated email from the ASF dual-hosted git repository. wzhou pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 4c00cbff7ee82f9a100746a97b07ce22b3fed5ae Author: Steve Carlin <[email protected]> AuthorDate: Mon Jun 17 17:14:38 2024 -0700 IMPALA-13136: Refactor AnalyzedFunctionCallExpr (for Calcite) The analyze method is now called after the Expr is constructed. This code is more in line with the existing way that Impala constructs the Expr object. Change-Id: Ideb662d9c7536659cb558bf62baec29c82217aa2 Reviewed-on: http://gerrit.cloudera.org:8080/21525 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Joe McDonnell <[email protected]> --- .../functions/AnalyzedFunctionCallExpr.java | 37 ++++++---------------- .../calcite/functions/AnalyzedNullLiteral.java | 11 +++---- .../impala/calcite/functions/RexCallConverter.java | 14 ++++---- .../calcite/functions/RexLiteralConverter.java | 28 ++++++---------- .../impala/calcite/rel/util/CreateExprVisitor.java | 22 +++++-------- 5 files changed, 38 insertions(+), 74 deletions(-) diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java index f13be70f2..f40d31e91 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java @@ -31,14 +31,12 @@ import org.apache.impala.common.ImpalaException; import java.util.List; /** - * A FunctionCallExpr that is always in an analyzed state + * A FunctionCallExpr specialized for Calcite. * - * The analysis for Calcite expressions can be done in the constructor - * rather than issuing a separate call to "analyze" after the object - * is constructed. - * - * For this class, we also want to override the "analyzeImpl" call since - * the "fn_" member is passed in rather than deduced at analysis time. + * The analysis for Calcite expressions is done through Calcite and + * does not need the analysis provided through the Impala expression. + * The analyzeImpl is overridden for FunctionCallExpr and only does + * the minimal analysis needed. * */ public class AnalyzedFunctionCallExpr extends FunctionCallExpr { @@ -49,40 +47,27 @@ public class AnalyzedFunctionCallExpr extends FunctionCallExpr { // variable so it can be properly set in analyzeImpl() private final Function savedFunction_; - private final Analyzer analyzer_; - // c'tor that takes a list of Exprs that eventually get converted to FunctionParams public AnalyzedFunctionCallExpr(Function fn, List<Expr> params, - RexCall rexCall, Type retType, Analyzer analyzer) throws ImpalaException { + RexCall rexCall, Type retType) { super(fn.getFunctionName(), params); this.savedFunction_ = fn; this.type_ = retType; - this.analyze(analyzer); - this.analyzer_ = analyzer; } // c'tor which does not depend on Calcite's RexCall but is used when Impala's // FunctionParams are created or there is some modifications to it public AnalyzedFunctionCallExpr(Function fn, FunctionParams funcParams, - Type retType, Analyzer analyzer) throws ImpalaException { + Type retType) { super(fn.getFunctionName(), funcParams); this.savedFunction_ = fn; this.type_ = retType; - this.analyze(analyzer); - this.analyzer_ = analyzer; } public AnalyzedFunctionCallExpr(AnalyzedFunctionCallExpr other) { super(other); this.savedFunction_ = other.savedFunction_; this.type_ = other.type_; - this.analyzer_ = other.analyzer_; - try { - this.analyze(this.analyzer_); - } catch (ImpalaException e) { - //TODO: IMPALA-13097: Don't throw runtime exception - throw new RuntimeException(e); - } } @Override @@ -106,12 +91,8 @@ public class AnalyzedFunctionCallExpr extends FunctionCallExpr { @Override public FunctionCallExpr cloneWithNewParams(FunctionParams params) { - try { - return new AnalyzedFunctionCallExpr(this.getFn(), params, - this.type_, analyzer_); - } catch (ImpalaException e) { - throw new IllegalStateException(e); - } + return new AnalyzedFunctionCallExpr(this.getFn(), params, + this.type_); } } diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedNullLiteral.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedNullLiteral.java index 3a3b74199..ea73a142b 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedNullLiteral.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedNullLiteral.java @@ -25,22 +25,21 @@ import org.apache.impala.common.AnalysisException; import org.apache.impala.common.ImpalaException; /** - * A NulLLiteral that is always in analyzed state + * A NulLLiteral specialized for Calcite + * + * The analysisImpl is overriden for Calcite so that the NullLiteral + * can be cast to the type that was determined by Calcite. */ public class AnalyzedNullLiteral extends NullLiteral { - private final Analyzer analyzer_; private final Type savedType_; - public AnalyzedNullLiteral(Analyzer analyzer, Type type) throws ImpalaException { - this.analyzer_ = analyzer; + public AnalyzedNullLiteral(Type type) { savedType_ = type; - this.analyze(analyzer); } public AnalyzedNullLiteral(AnalyzedNullLiteral other) { super(other); - this.analyzer_ = other.analyzer_; this.savedType_ = other.savedType_; } diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java index 8e3231848..0a91fe514 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java @@ -17,12 +17,12 @@ package org.apache.impala.calcite.functions; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexCall; import org.apache.calcite.rex.RexNode; -import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.Expr; import org.apache.impala.calcite.type.ImpalaTypeConverter; import org.apache.impala.catalog.Function; @@ -32,6 +32,7 @@ import org.apache.impala.common.ImpalaException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import java.util.List; /** @@ -44,8 +45,7 @@ public class RexCallConverter { /* * Returns the Impala Expr object for RexCallConverter. */ - public static Expr getExpr(RexCall rexCall, List<Expr> params, - RexBuilder rexBuilder, Analyzer analyzer) throws ImpalaException { + public static Expr getExpr(RexCall rexCall, List<Expr> params, RexBuilder rexBuilder) { String funcName = rexCall.getOperator().getName().toLowerCase(); @@ -54,17 +54,15 @@ public class RexCallConverter { Type impalaRetType = ImpalaTypeConverter.createImpalaType(fn.getReturnType(), rexCall.getType().getPrecision(), rexCall.getType().getScale()); - return new AnalyzedFunctionCallExpr(fn, params, rexCall, impalaRetType, analyzer); + return new AnalyzedFunctionCallExpr(fn, params, rexCall, impalaRetType); } - private static Function getFunction(RexCall call) throws ImpalaException { + private static Function getFunction(RexCall call) { List<RelDataType> argTypes = Lists.transform(call.getOperands(), RexNode::getType); String name = call.getOperator().getName(); Function fn = FunctionResolver.getFunction(name, call.getKind(), argTypes); - if (fn == null) { - throw new AnalysisException("Could not find function \"" + name + "\" in Impala " + Preconditions.checkNotNull(fn, "Could not find function \"" + name + "\" in Impala " + "with args " + argTypes + " and return type " + call.getType()); - } return fn; } } diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java index 5e5d706c3..960e5a21e 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java @@ -24,7 +24,6 @@ import org.apache.calcite.rex.RexLiteral; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.util.DateString; import org.apache.calcite.util.TimestampString; -import org.apache.impala.analysis.Analyzer; import org.apache.impala.analysis.Expr; import org.apache.impala.analysis.BoolLiteral; import org.apache.impala.analysis.DateLiteral; @@ -55,26 +54,23 @@ public class RexLiteralConverter { /* * Returns Expr object for ImpalaRexLiteral */ - public static Expr getExpr(RexLiteral rexLiteral, Analyzer analyzer) - throws ImpalaException { + public static Expr getExpr(RexLiteral rexLiteral) { if (SqlTypeName.INTERVAL_TYPES.contains(rexLiteral.getTypeName())) { - return new NumericLiteral( + return NumericLiteral.create( new BigDecimal(rexLiteral.getValueAs(Long.class)), Type.BIGINT); } switch (rexLiteral.getTypeName()) { case NULL: Type type = ImpalaTypeConverter.createImpalaType(rexLiteral.getType()); - return new AnalyzedNullLiteral(null, type); + return new AnalyzedNullLiteral(type); case BOOLEAN: - Expr boolExpr = new BoolLiteral(rexLiteral.getValueAs(Boolean.class)); - boolExpr.analyze(null); + Expr boolExpr = new BoolLiteral((Boolean) rexLiteral.getValueAs(Boolean.class)); return boolExpr; case BIGINT: case DECIMAL: case DOUBLE: - Expr numericExpr = new NumericLiteral(rexLiteral.getValueAs(BigDecimal.class), + Expr numericExpr = NumericLiteral.create(rexLiteral.getValueAs(BigDecimal.class), ImpalaTypeConverter.createImpalaType(rexLiteral.getType())); - numericExpr.analyze(null); return numericExpr; case CHAR: case VARCHAR: @@ -87,19 +83,18 @@ public class RexLiteralConverter { } Expr charExpr = new StringLiteral(rexLiteral.getValueAs(String.class), charType, false); - charExpr.analyze(null); return charExpr; case DATE: DateString dateStringClass = rexLiteral.getValueAs(DateString.class); String dateString = (dateStringClass == null) ? null : dateStringClass.toString(); Expr dateExpr = new DateLiteral(rexLiteral.getValueAs(Integer.class), dateString); - dateExpr.analyze(null); return dateExpr; case TIMESTAMP: - return createCastTimestampExpr(rexLiteral, analyzer); + return createCastTimestampExpr(rexLiteral); default: - throw new AnalysisException("Unsupported RexLiteral: " + Preconditions.checkState(false, "Unsupported RexLiteral: " + rexLiteral.getTypeName()); + return null; } } @@ -110,9 +105,7 @@ public class RexLiteralConverter { * If constant folding was not allowed, it means we did not have access to the backend * and thus need to do a cast in order to support conversion to a Timestamp. */ - private static Expr createCastTimestampExpr(RexLiteral rexLiteral, - Analyzer analyzer) - throws ImpalaException { + private static Expr createCastTimestampExpr(RexLiteral rexLiteral) { List<RelDataType> typeNames = ImmutableList.of(ImpalaTypeConverter.getRelDataType(Type.STRING)); @@ -120,7 +113,6 @@ public class RexLiteralConverter { List<Expr> argList = Lists.newArrayList(new StringLiteral(timestamp, Type.STRING, false)); Function castFunc = FunctionResolver.getFunction("casttotimestamp", typeNames); - return new AnalyzedFunctionCallExpr(castFunc, argList, null, Type.TIMESTAMP, - analyzer); + return new AnalyzedFunctionCallExpr(castFunc, argList, null, Type.TIMESTAMP); } } diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java index aec0295cd..8d0ee7e9e 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/CreateExprVisitor.java @@ -70,24 +70,16 @@ public class CreateExprVisitor extends RexVisitorImpl<Expr> { @Override public Expr visitCall(RexCall rexCall) { - try { - List<Expr> params = Lists.newArrayList(); - for (RexNode operand : rexCall.getOperands()) { - params.add(operand.accept(this)); - } - return RexCallConverter.getExpr(rexCall, params, rexBuilder_, analyzer_); - } catch (ImpalaException e) { - throw new RuntimeException(e); + List<Expr> params = Lists.newArrayList(); + for (RexNode operand : rexCall.getOperands()) { + params.add(operand.accept(this)); } + return RexCallConverter.getExpr(rexCall, params, rexBuilder_); } @Override public Expr visitLiteral(RexLiteral rexLiteral) { - try { - return RexLiteralConverter.getExpr(rexLiteral, analyzer_); - } catch (ImpalaException e) { - throw new RuntimeException(e); - } + return RexLiteralConverter.getExpr(rexLiteral); } @Override @@ -142,7 +134,9 @@ public class CreateExprVisitor extends RexVisitorImpl<Expr> { public static Expr getExpr(CreateExprVisitor visitor, RexNode operand) throws ImpalaException { try { - return operand.accept(visitor); + Expr expr = operand.accept(visitor); + expr.analyze(visitor.analyzer_); + return expr; } catch (Exception e) { throw new AnalysisException(e); }
