From bf808e9aa68c3f1761dfdcec8c9ffae8231ec0d5 Mon Sep 17 00:00:00 2001 From: jakevin Date: Thu, 28 Sep 2023 12:11:50 +0800 Subject: [PATCH] [fix](Nereids): tolerate DateLike overflow in SQL CAST/CONVERT (#24943) - explicit type cast, we need tolerate overflow and convert it to be NULL - implicit type cast, throw exception --- .../nereids/parser/LogicalPlanBuilder.java | 4 +- .../rules/FoldConstantRuleOnFE.java | 7 +- .../doris/nereids/trees/expressions/Cast.java | 19 +++- .../functions/BuiltinFunctionBuilder.java | 2 +- .../functions/scalar/ConvertTz.java | 16 ++- .../expressions/literal/DateTimeLiteral.java | 26 +++-- .../doris/nereids/types/DateTimeV2Type.java | 3 - .../SimplifyComparisonPredicateSqlTest.java | 98 +++++++++++++++++-- 8 files changed, 147 insertions(+), 28 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java index cc32a41216..24fb223f1f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java @@ -1425,7 +1425,7 @@ public class LogicalPlanBuilder extends DorisParserBaseVisitor { public Expression visitCast(DorisParser.CastContext ctx) { return ParserUtils.withOrigin(ctx, () -> { DataType dataType = ((DataType) typedVisit(ctx.dataType())).conversion(); - Expression cast = new Cast(getExpression(ctx.expression()), dataType); + Expression cast = new Cast(getExpression(ctx.expression()), dataType, true); return processCast(cast, dataType); }); } @@ -1470,7 +1470,7 @@ public class LogicalPlanBuilder extends DorisParserBaseVisitor { public Expression visitConvertType(DorisParser.ConvertTypeContext ctx) { return ParserUtils.withOrigin(ctx, () -> { DataType dataType = ((DataType) typedVisit(ctx.type)).conversion(); - Expression cast = new Cast(getExpression(ctx.argument), dataType); + Expression cast = new Cast(getExpression(ctx.argument), dataType, true); return processCast(cast, dataType); }); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnFE.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnFE.java index e1778fe521..00230b1894 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnFE.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rules/FoldConstantRuleOnFE.java @@ -352,7 +352,12 @@ public class FoldConstantRuleOnFE extends AbstractExpressionRewriteRule { try { return ((DateLikeType) dataType).fromString(((StringLikeLiteral) child).getStringValue()); } catch (AnalysisException t) { - return new NullLiteral(dataType); + if (cast.isExplicitType()) { + return new NullLiteral(dataType); + } else { + // If cast is from type coercion, we don't use NULL literal and will throw exception. + throw t; + } } } try { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Cast.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Cast.java index c450e7f7a2..e50acf7d01 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Cast.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/Cast.java @@ -33,16 +33,31 @@ import java.util.Objects; */ public class Cast extends Expression implements UnaryExpression { + // CAST can be from SQL Query or Type Coercion. + private final boolean isExplicitType; + private final DataType targetType; + public Cast(Expression child, DataType targetType, boolean isExplicitType) { + super(ImmutableList.of(child)); + this.targetType = Objects.requireNonNull(targetType, "targetType can not be null"); + this.isExplicitType = isExplicitType; + } + public Cast(Expression child, DataType targetType) { super(ImmutableList.of(child)); this.targetType = Objects.requireNonNull(targetType, "targetType can not be null"); + this.isExplicitType = false; } - private Cast(List child, DataType targetType) { + private Cast(List child, DataType targetType, boolean isExplicitType) { super(child); this.targetType = Objects.requireNonNull(targetType, "targetType can not be null"); + this.isExplicitType = isExplicitType; + } + + public boolean isExplicitType() { + return isExplicitType; } @Override @@ -72,7 +87,7 @@ public class Cast extends Expression implements UnaryExpression { @Override public Cast withChildren(List children) { Preconditions.checkArgument(children.size() == 1); - return new Cast(children, getDataType()); + return new Cast(children, targetType, isExplicitType); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/BuiltinFunctionBuilder.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/BuiltinFunctionBuilder.java index 23bc62e5f0..74c4a918cf 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/BuiltinFunctionBuilder.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/BuiltinFunctionBuilder.java @@ -83,7 +83,7 @@ public class BuiltinFunctionBuilder extends FunctionBuilder { if (isVariableLength) { return builderMethod.newInstance(toVariableLengthArguments(arguments)); } else { - return builderMethod.newInstance(arguments.stream().toArray(Object[]::new)); + return builderMethod.newInstance(arguments.toArray()); } } catch (Throwable t) { String argString = arguments.stream() diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConvertTz.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConvertTz.java index 17a0ccfb33..75270fa022 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConvertTz.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConvertTz.java @@ -18,9 +18,12 @@ package org.apache.doris.nereids.trees.expressions.functions.scalar; import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.Cast; import org.apache.doris.nereids.trees.expressions.Expression; import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; +import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; +import org.apache.doris.nereids.trees.expressions.literal.StringLikeLiteral; import org.apache.doris.nereids.trees.expressions.shape.TernaryExpression; import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; import org.apache.doris.nereids.types.DateTimeType; @@ -49,7 +52,18 @@ public class ConvertTz extends ScalarFunction * constructor with 3 arguments. */ public ConvertTz(Expression arg0, Expression arg1, Expression arg2) { - super("convert_tz", arg0, arg1, arg2); + super("convert_tz", castDateTime(arg0), arg1, arg2); + } + + private static Expression castDateTime(Expression arg0) { + // https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_convert-tz + // convert_tz() should be explicit cast, so we create a explicit cast here + try { + return arg0 instanceof StringLikeLiteral ? new Cast(arg0, DateTimeV2Type.forTypeFromString( + ((StringLikeLiteral) arg0).getStringValue()), true) : arg0; + } catch (Exception e) { + return new NullLiteral(DateTimeV2Type.SYSTEM_DEFAULT); + } } /** diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DateTimeLiteral.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DateTimeLiteral.java index aabe6f1e19..2d5ea77072 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DateTimeLiteral.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DateTimeLiteral.java @@ -101,17 +101,27 @@ public class DateTimeLiteral extends DateLiteral { * determine scale by datetime string */ public static int determineScale(String s) { - TemporalAccessor dateTime = parse(s); - int microSecond = DateUtils.getOrDefault(dateTime, ChronoField.MICRO_OF_SECOND); - - if (microSecond == 0) { + if (!s.contains("-") && !s.contains(":")) { return 0; } - - int scale = 6; - while (microSecond % 10 == 0) { + s = normalize(s); + if (s.length() <= 19 || s.charAt(19) != '.') { + return 0; + } + // from index 19 find the index of first char which is not digit + int scale = 0; + for (int i = 20; i < s.length(); i++) { + if (!Character.isDigit(s.charAt(i))) { + break; + } + scale++; + } + // trim the tailing zero + for (int i = 19 + scale; i >= 19; i--) { + if (s.charAt(i) != '0') { + break; + } scale--; - microSecond /= 10; } return scale; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DateTimeV2Type.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DateTimeV2Type.java index 38f85cbcc4..0aadd376df 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DateTimeV2Type.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DateTimeV2Type.java @@ -83,9 +83,6 @@ public class DateTimeV2Type extends DateLikeType { * may be we need to check for validity? */ public static DateTimeV2Type forTypeFromString(String s) { - if (!s.contains(".")) { - return DateTimeV2Type.SYSTEM_DEFAULT; - } int scale = DateTimeLiteral.determineScale(s); return DateTimeV2Type.of(scale); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateSqlTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateSqlTest.java index 86f06cacbe..4201da388b 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateSqlTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/rules/SimplifyComparisonPredicateSqlTest.java @@ -17,10 +17,14 @@ package org.apache.doris.nereids.rules.expression.rules; +import org.apache.doris.nereids.exceptions.AnalysisException; +import org.apache.doris.nereids.trees.expressions.literal.NullLiteral; +import org.apache.doris.nereids.types.DateTimeV2Type; import org.apache.doris.nereids.util.MemoPatternMatchSupported; import org.apache.doris.nereids.util.PlanChecker; import org.apache.doris.utframe.TestWithFeService; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; class SimplifyComparisonPredicateSqlTest extends TestWithFeService implements MemoPatternMatchSupported { @@ -55,8 +59,7 @@ class SimplifyComparisonPredicateSqlTest extends TestWithFeService implements Me logicalFilter() .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(a < 2023-06-16 00:00:00)"))) .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(b < 111.12)"))) - ) - .printlnTree(); + ); PlanChecker.from(connectContext) .analyze("select * from log_items_test where a <= '2023-06-15 23:59:59.999' and b <= 111.111;") @@ -65,16 +68,14 @@ class SimplifyComparisonPredicateSqlTest extends TestWithFeService implements Me logicalFilter() .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(a <= 2023-06-16 00:00:00)"))) .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(b <= 111.11)"))) - ) - .printlnTree(); + ); PlanChecker.from(connectContext) .analyze("select * from log_items_test where a = '2023-06-15 23:59:59.999' and b = 111.111;") .rewrite() .matches( logicalEmptyRelation() - ) - .printlnTree(); + ); PlanChecker.from(connectContext) .analyze("select * from log_items_test where a > '2023-06-15 23:59:59.999' and b > 111.111;") @@ -83,8 +84,7 @@ class SimplifyComparisonPredicateSqlTest extends TestWithFeService implements Me logicalFilter() .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(a > 2023-06-16 00:00:00)"))) .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(b > 111.11)"))) - ) - .printlnTree(); + ); PlanChecker.from(connectContext) .analyze("select * from log_items_test where a >= '2023-06-15 23:59:59.999' and b >= 111.111;") @@ -93,7 +93,85 @@ class SimplifyComparisonPredicateSqlTest extends TestWithFeService implements Me logicalFilter() .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(a >= 2023-06-16 00:00:00)"))) .when(f -> f.getConjuncts().stream().anyMatch(e -> e.toSql().equals("(b >= 111.12)"))) - ) - .printlnTree(); + ); + } + + @Test + void dateLikeOverflow() { + PlanChecker.from(connectContext) + .analyze("select CAST('2021-01-32 00:00:00' AS DATETIME(6))") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0).equals(new NullLiteral(DateTimeV2Type.of(6)))) + ) + ); + + PlanChecker.from(connectContext) + .analyze("select CONVERT('2021-01-32 00:00:00', DATETIME(6))") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0).equals(new NullLiteral(DateTimeV2Type.of(6)))) + ) + ); + + PlanChecker.from(connectContext) + .analyze("select CONVERT('2021-01-30 00:00:00.0000001', DATETIME(6))") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0).equals(new NullLiteral(DateTimeV2Type.of(6)))) + ) + ); + + PlanChecker.from(connectContext) + .analyze("select CONVERT_TZ('2021-01-32 00:00:00', '+08:00', 'America/London') = '2021-01-30'") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0) instanceof NullLiteral) + ) + ); + + PlanChecker.from(connectContext) + .analyze("select CONVERT_TZ('2021-01-32 00:00:00', '+08:00', 'America/London')") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0) instanceof NullLiteral) + ) + ); + + PlanChecker.from(connectContext) + .analyze("select CONVERT_TZ('2021-01-32 00:00:00.0000001', '+08:00', 'America/London')") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0) instanceof NullLiteral) + ) + ); + + PlanChecker.from(connectContext) + .analyze("select CONVERT_TZ('2021-01-32 00:00:00.001', '+08:00', 'America/London') = '2021-01-30'") + .rewrite() + .matches( + logicalResultSink( + logicalOneRowRelation().when(p -> p.getProjects().get(0).child(0) instanceof NullLiteral) + ) + ); + + Assertions.assertThrows(AnalysisException.class, () -> PlanChecker.from(connectContext) + .analyze("select CAST('2021-01-32 00:00:00' AS DATETIME(6)) = '2021-01-32 00:00:00'") + .rewrite() + ); + Assertions.assertThrows(AnalysisException.class, () -> PlanChecker.from(connectContext) + .analyze("select CAST('2021-01-32 00:00:00' AS DATETIME(6)) = '2021-01-32 23:00:00'") + .rewrite() + ); + Assertions.assertThrows(AnalysisException.class, () -> PlanChecker.from(connectContext) + .analyze("select CAST('2021-01-32 00:00:00' AS DATETIME(6)) = '1000'") + .rewrite() + ); } }