From 67378a2dc39375e365c5e7c1c190b9448d4161bb Mon Sep 17 00:00:00 2001 From: minghong Date: Fri, 13 Jan 2023 12:09:53 +0800 Subject: [PATCH] [fix](nereids) fix bug in SequenceFunction legality check (#15812) 1. fix bug in sequence_match function 2. do type promotion instead of explicit cast for - varcharLiteral -> stringLiteral - charLiteral->stringLiteral --- .../functions/agg/SequenceFunction.java | 16 ++--- .../apache/doris/nereids/types/DataType.java | 15 ++--- .../doris/nereids/util/TypeCoercionUtils.java | 2 +- .../test_nereids_function.out | 7 +++ .../test_nereids_function.groovy | 58 +++++++++++++++++++ 5 files changed, 83 insertions(+), 15 deletions(-) create mode 100644 regression-test/data/nereids_syntax_p0/test_nereids_function.out create mode 100644 regression-test/suites/nereids_syntax_p0/test_nereids_function.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/SequenceFunction.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/SequenceFunction.java index 26e8163ee6..673c91479a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/SequenceFunction.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/SequenceFunction.java @@ -29,13 +29,14 @@ public interface SequenceFunction extends FunctionTrait { default void checkLegalityBeforeTypeCoercion() { String functionName = getName(); Expression firstArg = getArgument(0); - if (firstArg instanceof StringLikeLiteral) { - throw new AnalysisException("The pattern params of " + functionName - + " function must be string literal: " + toSql()); + if (!(firstArg instanceof StringLikeLiteral)) { + throw new AnalysisException("The pattern param `" + firstArg.toSql() + "` of " + functionName + + " function must be string literal, but it is " + + firstArg.getClass().getSimpleName()); } - if (!getArgumentType(1).isDateType()) { + if (!getArgumentType(1).isDateLikeType()) { throw new AnalysisException("The timestamp params of " + functionName - + " function must be DATE or DATETIME"); + + " function must be DATE or DATETIME, but it is " + getArgumentType(1)); } String pattern = ((StringLikeLiteral) firstArg).getStringValue(); if (!FunctionCallExpr.parsePattern(pattern)) { @@ -44,8 +45,9 @@ public interface SequenceFunction extends FunctionTrait { for (int i = 2; i < arity(); i++) { if (!getArgumentType(i).isBooleanType()) { - throw new AnalysisException("The 3th and subsequent params of " - + functionName + " function must be boolean"); + throw new AnalysisException("The param `" + child(i).toSql() + "` of " + + functionName + " function must be boolean, but it is " + + getArgumentType(i).getClass().getSimpleName()); } } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java index 89d2b11e92..2547702e3e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java @@ -27,6 +27,7 @@ import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral; import org.apache.doris.nereids.trees.expressions.literal.IntegerLiteral; import org.apache.doris.nereids.trees.expressions.literal.Literal; import org.apache.doris.nereids.trees.expressions.literal.SmallIntLiteral; +import org.apache.doris.nereids.trees.expressions.literal.StringLiteral; import org.apache.doris.nereids.types.coercion.AbstractDataType; import org.apache.doris.nereids.types.coercion.CharacterType; import org.apache.doris.nereids.types.coercion.IntegralType; @@ -52,12 +53,14 @@ public abstract class DataType implements AbstractDataType { protected static final NereidsParser PARSER = new NereidsParser(); // use class and supplier here to avoid class load deadlock. - private static final Map, Supplier> PROMOTION_MAP - = ImmutableMap., Supplier>builder() + private static final Map, Supplier> PROMOTION_MAP + = ImmutableMap., Supplier>builder() .put(TinyIntType.class, () -> SmallIntType.INSTANCE) .put(SmallIntType.class, () -> IntegerType.INSTANCE) .put(IntegerType.class, () -> BigIntType.INSTANCE) .put(FloatType.class, () -> DoubleType.INSTANCE) + .put(VarcharType.class, () -> StringType.INSTANCE) + .put(CharType.class, () -> StringType.INSTANCE) .build(); @Developing("This map is just use to search which itemType of the ArrayType is implicit castable for temporary." @@ -86,11 +89,7 @@ public abstract class DataType implements AbstractDataType { /** * create a specific Literal for a given dataType */ - public static Literal promoteNumberLiteral(Object value, DataType dataType) { - if (! (value instanceof Number)) { - return null; - } - + public static Literal promoteLiteral(Object value, DataType dataType) { if (dataType.equals(SmallIntType.INSTANCE)) { return new SmallIntLiteral(((Number) value).shortValue()); } else if (dataType.equals(IntegerType.INSTANCE)) { @@ -99,6 +98,8 @@ public abstract class DataType implements AbstractDataType { return new BigIntLiteral(((Number) value).longValue()); } else if (dataType.equals(DoubleType.INSTANCE)) { return new DoubleLiteral(((Number) value).doubleValue()); + } else if (dataType.equals(StringType.INSTANCE)) { + return new StringLiteral((String) value); } return null; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java index 796d852fc6..0c4e70ee00 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java @@ -380,7 +380,7 @@ public class TypeCoercionUtils { type = promoted; } if (type.equals(dataType)) { - Literal promoted = DataType.promoteNumberLiteral(((Literal) input).getValue(), dataType); + Literal promoted = DataType.promoteLiteral(((Literal) input).getValue(), dataType); if (promoted != null) { return promoted; } diff --git a/regression-test/data/nereids_syntax_p0/test_nereids_function.out b/regression-test/data/nereids_syntax_p0/test_nereids_function.out new file mode 100644 index 0000000000..3864dda84e --- /dev/null +++ b/regression-test/data/nereids_syntax_p0/test_nereids_function.out @@ -0,0 +1,7 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !match -- +true + +-- !not_match -- +false + diff --git a/regression-test/suites/nereids_syntax_p0/test_nereids_function.groovy b/regression-test/suites/nereids_syntax_p0/test_nereids_function.groovy new file mode 100644 index 0000000000..4bfb3839dc --- /dev/null +++ b/regression-test/suites/nereids_syntax_p0/test_nereids_function.groovy @@ -0,0 +1,58 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_nereids_function") { + sql "SET enable_nereids_planner=true" + + sql """ + DROP TABLE IF EXISTS sequence_match_test1 + """ + + sql """ + CREATE TABLE sequence_match_test1( + `uid` int COMMENT 'user id', + `date` datetime COMMENT 'date time', + `number` int NULL COMMENT 'number' + ) + DUPLICATE KEY(uid) + DISTRIBUTED BY HASH(uid) BUCKETS 3 + PROPERTIES ( + "replication_num" = "1" + ); + """ + + sql """ + INSERT INTO sequence_match_test1(uid, date, number) values (1, '2022-11-02 10:41:00', 1), + (2, '2022-11-02 11:41:00', 7), + (3, '2022-11-02 16:15:01', 3), + (4, '2022-11-02 19:05:04', 4), + (5, '2022-11-02 21:24:12', 5); + """ + + sql "SET enable_fallback_to_original_planner=false" + + sql """sync""" + + qt_match """ + SELECT sequence_match('(?1)(?2)', date, number = 1, number = 7) FROM sequence_match_test1; + """ + + qt_not_match """ + SELECT sequence_match('(?1)(?t>3600)(?2)', date, number = 1, number = 7) FROM sequence_match_test1; + """ + +}