diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java index d8cfa2cf6..1e024d525 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java +++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java @@ -3387,12 +3387,12 @@ public class Analyzer { TypeCompatibility regularCompatibility = this.getRegularCompatibilityLevel(); TypeCompatibility permissiveCompatibility = this.getPermissiveCompatibilityLevel(); - TypeCompatibility compatibilityLevel = regularCompatibility; // Determine compatible types for exprs, position by position. List firstList = exprLists.get(0); List widestExprs = new ArrayList<>(firstList.size()); for (int i = 0; i < firstList.size(); ++i) { + TypeCompatibility compatibilityLevel = regularCompatibility; // Type compatible with the i-th exprs of all expr lists. // Initialize with type of i-th expr in first list. Type compatibleType = firstList.get(i).getType(); diff --git a/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java b/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java index 3b06e05cc..d05ef83e5 100644 --- a/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java @@ -18,6 +18,7 @@ package org.apache.impala.analysis; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TBoolLiteral; import org.apache.impala.thrift.TExprNode; @@ -89,11 +90,12 @@ public class BoolLiteral extends LiteralExpr { } @Override - protected Expr uncheckedCastTo(Type targetType) throws AnalysisException { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws AnalysisException { if (targetType.equals(this.type_)) { return this; } else { - return new CastExpr(targetType, this); + return new CastExpr(targetType, this, compatibility); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java index 7e31da5ec..909f3bedd 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CastExpr.java +++ b/fe/src/main/java/org/apache/impala/analysis/CastExpr.java @@ -25,6 +25,7 @@ import org.apache.impala.catalog.PrimitiveType; import org.apache.impala.catalog.ScalarFunction; import org.apache.impala.catalog.ScalarType; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TCastExpr; import org.apache.impala.thrift.TExpr; @@ -51,10 +52,14 @@ public class CastExpr extends Expr { // Stores the value of the FORMAT clause. private final String castFormat_; + // Stores the compatibility level with which the cast was defined. + private final TypeCompatibility compatibility_; + /** * C'tor for "pre-analyzed" implicit casts. */ - public CastExpr(Type targetType, Expr e, String format) { + public CastExpr( + Type targetType, Expr e, String format, TypeCompatibility compatibility) { super(); Preconditions.checkState(targetType.isValid()); Preconditions.checkNotNull(e); @@ -62,6 +67,7 @@ public class CastExpr extends Expr { targetTypeDef_ = null; isImplicit_ = true; castFormat_ = format; + compatibility_ = compatibility; // replace existing implicit casts if (e instanceof CastExpr) { CastExpr castExpr = (CastExpr) e; @@ -84,7 +90,11 @@ public class CastExpr extends Expr { } public CastExpr(Type targetType, Expr e) { - this(targetType, e, null); + this(targetType, e, null, TypeCompatibility.DEFAULT); + } + + public CastExpr(Type targetType, Expr e, TypeCompatibility compatibility) { + this(targetType, e, null, compatibility); } /** @@ -101,6 +111,7 @@ public class CastExpr extends Expr { targetTypeDef_ = targetTypeDef; children_.add(e); castFormat_ = format; + compatibility_ = TypeCompatibility.DEFAULT; } /** @@ -112,6 +123,7 @@ public class CastExpr extends Expr { isImplicit_ = other.isImplicit_; noOp_ = other.noOp_; castFormat_ = other.castFormat_; + compatibility_ = TypeCompatibility.DEFAULT; } private static String getFnName(Type targetType) { @@ -272,6 +284,8 @@ public class CastExpr extends Expr { public boolean isImplicit() { return isImplicit_; } + public TypeCompatibility getCompatibility() { return compatibility_; } + @Override protected void analyzeImpl(Analyzer analyzer) throws AnalysisException { Preconditions.checkState(!isImplicit_); @@ -317,7 +331,8 @@ public class CastExpr extends Expr { // for every type since it is redundant with STRING. Casts to go through 2 casts: // (1) cast to string, to stringify the value // (2) cast to CHAR, to truncate or pad with spaces - CastExpr tostring = new CastExpr(ScalarType.STRING, children_.get(0), castFormat_); + CastExpr tostring = + new CastExpr(ScalarType.STRING, children_.get(0), castFormat_, compatibility_); tostring.analyze(); children_.set(0, tostring); } diff --git a/fe/src/main/java/org/apache/impala/analysis/DateLiteral.java b/fe/src/main/java/org/apache/impala/analysis/DateLiteral.java index 4260954e3..9fc5f7cae 100644 --- a/fe/src/main/java/org/apache/impala/analysis/DateLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/DateLiteral.java @@ -17,13 +17,8 @@ package org.apache.impala.analysis; -import java.util.Arrays; -import java.time.format.DateTimeFormatter; -import java.time.format.DateTimeParseException; -import java.time.LocalDate; - -import com.google.common.base.Preconditions; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.InternalException; import org.apache.impala.service.FeSupport; @@ -111,11 +106,12 @@ public class DateLiteral extends LiteralExpr { } @Override - protected Expr uncheckedCastTo(Type targetType) throws AnalysisException { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws AnalysisException { if (targetType.equals(type_)) { return this; } else { - return new CastExpr(targetType, this); + return new CastExpr(targetType, this, compatibility); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/Expr.java b/fe/src/main/java/org/apache/impala/analysis/Expr.java index 0453e01c8..954430ad4 100644 --- a/fe/src/main/java/org/apache/impala/analysis/Expr.java +++ b/fe/src/main/java/org/apache/impala/analysis/Expr.java @@ -1110,7 +1110,14 @@ abstract public class Expr extends TreeNode implements ParseNode, Cloneabl if (smap == null) return result; result = result.substituteImpl(smap, analyzer); result.analyze(analyzer); - if (preserveRootType && !type_.equals(result.getType())) result = result.castTo(type_); + if (preserveRootType && !type_.equals(result.getType())) { + if (this instanceof CastExpr) { + CastExpr thisCastExpr = (CastExpr) this; + TypeCompatibility compatibility = thisCastExpr.getCompatibility(); + return result.castTo(type_, compatibility); + } + return result.castTo(type_); + } return result; } @@ -1570,7 +1577,7 @@ abstract public class Expr extends TreeNode implements ParseNode, Cloneabl "targetType=" + targetType + " type=" + type); } } - return uncheckedCastTo(targetType); + return uncheckedCastTo(targetType, compatibility); } public final Expr castTo(Type targetType) throws AnalysisException { @@ -1578,20 +1585,23 @@ abstract public class Expr extends TreeNode implements ParseNode, Cloneabl } /** - * Create an expression equivalent to 'this' but returning targetType; - * possibly by inserting an implicit cast, - * or by returning an altogether new expression - * or by returning 'this' with a modified return type'. - * @param targetType - * type to be cast to - * @return cast expression, or converted literal, - * should never return null - * @throws AnalysisException - * when an invalid cast is asked for, for example, - * failure to convert a string literal to a date literal + * Create an expression equivalent to 'this' but returning targetType; possibly by + * inserting an implicit cast, or by returning an altogether new expression or by + * returning 'this' with a modified return type'. + * + * @param targetType type to be cast to + * @param compatibility compatibility level used to calculate the cast + * @return cast expression, or converted literal, should never return null + * @throws AnalysisException when an invalid cast is asked for, for example, failure to + * convert a string literal to a date literal */ + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws AnalysisException { + return new CastExpr(targetType, this, compatibility); + } + protected Expr uncheckedCastTo(Type targetType) throws AnalysisException { - return new CastExpr(targetType, this); + return uncheckedCastTo(targetType, TypeCompatibility.DEFAULT); } /** diff --git a/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java b/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java index 9a2f1e3e4..1a493d52a 100644 --- a/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/NullLiteral.java @@ -18,6 +18,7 @@ package org.apache.impala.analysis; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.thrift.TExprNode; import org.apache.impala.thrift.TExprNodeType; @@ -44,7 +45,7 @@ public class NullLiteral extends LiteralExpr { public static NullLiteral create(Type type) { NullLiteral l = new NullLiteral(); l.analyzeNoThrow(null); - l.uncheckedCastTo(type); + l.uncheckedCastTo(type, TypeCompatibility.DEFAULT); return l; } @@ -65,7 +66,7 @@ public class NullLiteral extends LiteralExpr { public String getStringValue() { return "NULL"; } @Override - protected Expr uncheckedCastTo(Type targetType) { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) { Preconditions.checkState(targetType.isValid()); type_ = targetType; return this; diff --git a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java index e9b4b12c6..0dfb60d92 100644 --- a/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java @@ -23,6 +23,7 @@ import java.math.RoundingMode; import org.apache.impala.catalog.ScalarType; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.SqlCastException; import org.apache.impala.thrift.TDecimalLiteral; @@ -462,10 +463,11 @@ public class NumericLiteral extends LiteralExpr { * @throws SqlCastException */ @Override - protected Expr uncheckedCastTo(Type targetType) throws SqlCastException { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws SqlCastException { Preconditions.checkState(targetType.isNumericType() || targetType.isStringType()); if (targetType.isStringType()) { - return new CastExpr(targetType, this); + return new CastExpr(targetType, this, compatibility); } if (type_ == targetType) return this; try { @@ -479,7 +481,7 @@ public class NumericLiteral extends LiteralExpr { return new NumericLiteral(converted, targetType); } } catch (SqlCastException e) { - return new CastExpr(targetType, this); + return new CastExpr(targetType, this, compatibility); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java index dbd415322..65e74d098 100644 --- a/fe/src/main/java/org/apache/impala/analysis/SlotRef.java +++ b/fe/src/main/java/org/apache/impala/analysis/SlotRef.java @@ -28,6 +28,7 @@ import org.apache.impala.catalog.HdfsFileFormat; import org.apache.impala.catalog.StructType; import org.apache.impala.catalog.TableLoadingException; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.UnsupportedFeatureException; import org.apache.impala.thrift.TExprNode; @@ -460,12 +461,13 @@ public class SlotRef extends Expr { } @Override - protected Expr uncheckedCastTo(Type targetType) throws AnalysisException { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws AnalysisException { if (type_.isNull()) { // Hack to prevent null SlotRefs in the BE return NullLiteral.create(targetType); } else { - return super.uncheckedCastTo(targetType); + return super.uncheckedCastTo(targetType, compatibility); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java b/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java index eb1f331fe..70f6ad9e0 100644 --- a/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/StringLiteral.java @@ -23,6 +23,7 @@ import java.math.BigDecimal; import org.apache.impala.catalog.ScalarType; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.compat.MetastoreShim; import org.apache.impala.thrift.TExprNode; @@ -148,7 +149,8 @@ public class StringLiteral extends LiteralExpr { } @Override - protected Expr uncheckedCastTo(Type targetType) throws AnalysisException { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws AnalysisException { Preconditions.checkState(targetType.isNumericType() || targetType.isDateOrTimeType() || targetType.equals(this.type_) || targetType.isStringType()); if (targetType.equals(this.type_)) { @@ -161,7 +163,7 @@ public class StringLiteral extends LiteralExpr { // Let the BE do the cast // - it is in Boost format in case target type is TIMESTAMP // - CCTZ is used for conversion in case target type is DATE. - return new CastExpr(targetType, this); + return new CastExpr(targetType, this, compatibility); } return this; } diff --git a/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java b/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java index b6a13484e..7ac78d5f9 100644 --- a/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java +++ b/fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java @@ -21,6 +21,7 @@ import java.util.Arrays; import com.google.common.base.Preconditions; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.thrift.TExprNode; import org.apache.impala.thrift.TExprNodeType; @@ -84,11 +85,12 @@ public class TimestampLiteral extends LiteralExpr { } @Override - protected Expr uncheckedCastTo(Type targetType) throws AnalysisException { + protected Expr uncheckedCastTo(Type targetType, TypeCompatibility compatibility) + throws AnalysisException { if (targetType.equals(type_)) { return this; } else { - return new CastExpr(targetType, this); + return new CastExpr(targetType, this, compatibility); } } diff --git a/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java b/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java index 23f8d881f..45317f9bb 100644 --- a/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/NumericLiteralTest.java @@ -23,6 +23,7 @@ import java.math.BigDecimal; import org.apache.impala.catalog.ScalarType; import org.apache.impala.catalog.Type; +import org.apache.impala.catalog.TypeCompatibility; import org.apache.impala.common.AnalysisException; import org.apache.impala.common.InvalidValueException; import org.apache.impala.common.SqlCastException; @@ -435,23 +436,24 @@ public class NumericLiteralTest { { // Integral types NumericLiteral n = new NumericLiteral(BigDecimal.ZERO); - Expr result = n.uncheckedCastTo(Type.BIGINT); + Expr result = n.uncheckedCastTo(Type.BIGINT, TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(Type.BIGINT, n.getType()); - result = n.uncheckedCastTo(Type.TINYINT); + result = n.uncheckedCastTo(Type.TINYINT, TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(Type.TINYINT, n.getType()); - result = n.uncheckedCastTo(ScalarType.createDecimalType(5, 0)); + result = n.uncheckedCastTo( + ScalarType.createDecimalType(5, 0), TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(ScalarType.createDecimalType(5, 0), n.getType()); } { // Integral types, with overflow NumericLiteral n = new NumericLiteral(ABOVE_SMALLINT); - Expr result = n.uncheckedCastTo(Type.BIGINT); + Expr result = n.uncheckedCastTo(Type.BIGINT, TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(Type.BIGINT, n.getType()); - Expr result2 = n.uncheckedCastTo(Type.SMALLINT); + Expr result2 = n.uncheckedCastTo(Type.SMALLINT, TypeCompatibility.DEFAULT); assertTrue(result2 instanceof CastExpr); assertEquals(Type.SMALLINT, result2.getType()); } @@ -459,10 +461,11 @@ public class NumericLiteralTest { // Decimal types, with overflow // Note: not safe to reuse above value after exception NumericLiteral n = new NumericLiteral(ABOVE_SMALLINT); - Expr result = n.uncheckedCastTo(Type.BIGINT); + Expr result = n.uncheckedCastTo(Type.BIGINT, TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(Type.BIGINT, n.getType()); - Expr result2 = n.uncheckedCastTo(ScalarType.createDecimalType(2, 0)); + Expr result2 = n.uncheckedCastTo( + ScalarType.createDecimalType(2, 0), TypeCompatibility.DEFAULT); assertTrue(result2 instanceof CastExpr); assertEquals(ScalarType.createDecimalType(2, 0), result2.getType()); } @@ -470,13 +473,16 @@ public class NumericLiteralTest { // Decimal types NumericLiteral n = new NumericLiteral(new BigDecimal("123.45")); assertEquals(ScalarType.createDecimalType(5, 2), n.getType()); - Expr result = n.uncheckedCastTo(ScalarType.createDecimalType(6, 3)); + Expr result = n.uncheckedCastTo( + ScalarType.createDecimalType(6, 3), TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(ScalarType.createDecimalType(6, 3), n.getType()); - result = n.uncheckedCastTo(ScalarType.createDecimalType(5, 2)); + result = n.uncheckedCastTo( + ScalarType.createDecimalType(5, 2), TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(ScalarType.createDecimalType(5, 2), n.getType()); - result = n.uncheckedCastTo(ScalarType.createDecimalType(4, 1)); + result = n.uncheckedCastTo( + ScalarType.createDecimalType(4, 1), TypeCompatibility.DEFAULT); assertNotSame(n, result); assertEquals(ScalarType.createDecimalType(4, 1), result.getType()); assertEquals("123.5", ((NumericLiteral) result).toSql()); @@ -602,29 +608,30 @@ public class NumericLiteralTest { public void testCast() throws SqlCastException { NumericLiteral n = NumericLiteral.create(1000); assertEquals(Type.SMALLINT, n.getType()); - Expr result = n.uncheckedCastTo(Type.TINYINT); + Expr result = n.uncheckedCastTo(Type.TINYINT, TypeCompatibility.DEFAULT); assertTrue(result instanceof CastExpr); assertEquals(Type.TINYINT, result.getType()); - result = n.uncheckedCastTo(Type.INT); + result = n.uncheckedCastTo(Type.INT, TypeCompatibility.DEFAULT); assertSame(n, result); assertEquals(Type.INT, n.getType()); n = new NumericLiteral(new BigDecimal("123.45")); assertEquals(ScalarType.createDecimalType(5, 2), n.getType()); - assertSame(n, n.uncheckedCastTo(ScalarType.createDecimalType(6, 3))); + assertSame(n, + n.uncheckedCastTo(ScalarType.createDecimalType(6, 3), TypeCompatibility.DEFAULT)); assertEquals(ScalarType.createDecimalType(6, 3), n.getType()); n = new NumericLiteral(new BigDecimal("123.45")); Type newType = ScalarType.createDecimalType(4, 1); - result = n.uncheckedCastTo(newType); + result = n.uncheckedCastTo(newType, TypeCompatibility.DEFAULT); assertNotSame(result, n); assertTrue(result instanceof NumericLiteral); assertEquals(newType, result.getType()); NumericLiteral n2 = (NumericLiteral) result; assertEquals("123.5", n2.getValue().toString()); - Expr result2 = n2.uncheckedCastTo(Type.SMALLINT); + Expr result2 = n2.uncheckedCastTo(Type.SMALLINT, TypeCompatibility.DEFAULT); assertNotSame(result2, result); assertEquals(Type.SMALLINT, result2.getType()); assertEquals("124", ((NumericLiteral)result2).getValue().toString()); diff --git a/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test b/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test index d21eeacd5..0b258fc3c 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test +++ b/testdata/workloads/functional-query/queries/QueryTest/insert-unsafe.test @@ -213,4 +213,13 @@ AnalysisException: Unsafe implicit cast is prohibited for non-const expression: INSERT INTO unsafe_insert(double_col) select string_col from unsafe_insert; ---- CATCH AnalysisException: Unsafe implicit cast is prohibited for non-const expression: string_col - +==== +---- QUERY +# Mixing unsafe and regular compatibility on column level, unsafe union between 1 and "1" +# regular union between column 'string_col' and "100". +INSERT INTO unsafe_insert(int_col, string_col) select 1, string_col from unsafe_insert union select "1", "100"; +==== +---- QUERY +# Regression test for expression substitution on unsafe casts. +INSERT INTO unsafe_insert(int_col, string_col) select "1", "1" from unsafe_insert union select 1, "1"; +==== \ No newline at end of file