From 1f87ecf8e00261fbac5231a63987bcf9120be939 Mon Sep 17 00:00:00 2001 From: zclllhhjj Date: Tue, 13 Aug 2024 15:01:51 +0800 Subject: [PATCH] [Fix](function) fix coredump because short of check on randoms arguments (#39255) ## Proposed changes Issue Number: close #xxx before: crash or ```sql mysql [test]>select random(1,array_size(split_by_string(fcst_emp,','))) from test_random; +---------------------------------------------------------+ | random(1, array_size(split_by_string(`fcst_emp`, ','))) | +---------------------------------------------------------+ | 7471044383762196303 | +---------------------------------------------------------+ 1 row in set (0.05 sec) ``` now: both for nereids and legacy planner: ```sql mysql [test]>select random(1,array_size(split_by_string(fcst_emp,','))) from test_random; ERROR 1105 (HY000): errCode = 2, detailMessage = (10.16.10.8)[INVALID_ARGUMENT]The param of rand function must be literal ``` doc pr: https://github.com/apache/doris-website/pull/992 --- be/src/vec/functions/random.cpp | 11 +++++++--- .../expressions/functions/scalar/Random.java | 11 ++++++++-- .../nereids_p0/system/test_query_sys.groovy | 21 +++++++++++++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/be/src/vec/functions/random.cpp b/be/src/vec/functions/random.cpp index f073491e37220c..1b8dea935d19a1 100644 --- a/be/src/vec/functions/random.cpp +++ b/be/src/vec/functions/random.cpp @@ -69,15 +69,19 @@ class Random : public IFunction { if (context->get_num_args() == 1) { // This is a call to RandSeed, initialize the seed if (!context->is_col_constant(0)) { - return Status::InvalidArgument("Seed argument to rand() must be constant."); + return Status::InvalidArgument("The param of rand function must be literal"); } uint32_t seed = 0; if (!context->get_constant_col(0)->column_ptr->is_null_at(0)) { seed = (*context->get_constant_col(0)->column_ptr)[0].get(); } generator->seed(seed); - } else { - // 0 or 2 args + } else if (context->get_num_args() == 2) { + if (!context->is_col_constant(0) || !context->is_col_constant(1)) { + return Status::InvalidArgument("The param of rand function must be literal"); + } + generator->seed(std::random_device()()); + } else { // zero args generator->seed(std::random_device()()); } } @@ -108,6 +112,7 @@ class Random : public IFunction { context->get_function_state(FunctionContext::THREAD_LOCAL)); DCHECK(generator != nullptr); + // checked in open() Int64 min = assert_cast( assert_cast( block.get_by_position(arguments[0]).column.get()) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Random.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Random.java index a7f3a360a6a752..5045d85c919421 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Random.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Random.java @@ -65,10 +65,17 @@ public Random(Expression arg) { */ public Random(Expression lchild, Expression rchild) { super("random", lchild, rchild); + } + + @Override + public void checkLegalityBeforeTypeCoercion() { // align with original planner behavior, refer to: // org/apache/doris/analysis/Expr.getBuiltinFunction() - Preconditions.checkState(lchild instanceof Literal && rchild instanceof Literal, - "The param of rand function must be literal"); + for (Expression child : children()) { + if (!child.isLiteral()) { + throw new AnalysisException("The param of rand function must be literal "); + } + } } /** diff --git a/regression-test/suites/nereids_p0/system/test_query_sys.groovy b/regression-test/suites/nereids_p0/system/test_query_sys.groovy index 85d612b9c17b59..e0e68f909fd94f 100644 --- a/regression-test/suites/nereids_p0/system/test_query_sys.groovy +++ b/regression-test/suites/nereids_p0/system/test_query_sys.groovy @@ -48,4 +48,25 @@ suite("test_query_sys", "query,p0") { sql "set enable_nereids_planner=true" def v2 = sql "select version()" assertEquals(v1, v2) + + test { + sql "select random(random());" + exception "The param of rand function must be literal" + } + + sql "set enable_nereids_planner=false" + sql """ + CREATE TABLE IF NOT EXISTS `test_random` ( + fcst_emp varchar(128) NOT NULL + ) ENGINE=OLAP + DISTRIBUTED BY HASH(`fcst_emp`) + PROPERTIES( + "replication_num" = "1", + "compression" = "LZ4" ); + """ + sql """ insert into test_random values('123,1233,4123,3131'); """ + test { + sql "select random(1,array_size(split_by_string(fcst_emp,','))) from test_random;" + exception "The param of rand function must be literal" + } }