From b399a5e9728f62f6712c0811c080a8d20c251a1d Mon Sep 17 00:00:00 2001 From: xufei Date: Fri, 6 May 2022 11:21:39 +0800 Subject: [PATCH] fix ifnull bug Signed-off-by: xufei --- .../DAGExpressionAnalyzerHelper.cpp | 10 +++---- tests/fullstack-test/expr/ifnull.test | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 tests/fullstack-test/expr/ifnull.test diff --git a/dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp b/dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp index 5173636bb04..cabd88e0ba7 100644 --- a/dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp +++ b/dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp @@ -102,7 +102,7 @@ String DAGExpressionAnalyzerHelper::buildIfNullFunction( const ExpressionActionsPtr & actions) { // rewrite IFNULL function with multiIf - // ifNull(arg1, arg2) -> multiIf(isNull(arg1), arg2, arg1) + // ifNull(arg1, arg2) -> multiIf(isNull(arg1), arg2, assumeNotNull(arg1)) // todo if arg1 is not nullable, then just return arg1 is ok const String & func_name = "multiIf"; Names argument_names; @@ -112,13 +112,13 @@ String DAGExpressionAnalyzerHelper::buildIfNullFunction( } String condition_arg_name = analyzer->getActions(expr.children(0), actions, false); - String tmp_else_arg_name = analyzer->getActions(expr.children(1), actions, false); + String else_arg_name = analyzer->getActions(expr.children(1), actions, false); String is_null_result = analyzer->applyFunction("isNull", {condition_arg_name}, actions, getCollatorFromExpr(expr)); - String not_null_else_arg_name = analyzer->applyFunction("assumeNotNull", {tmp_else_arg_name}, actions, nullptr); + String not_null_condition_arg_name = analyzer->applyFunction("assumeNotNull", {condition_arg_name}, actions, nullptr); argument_names.push_back(std::move(is_null_result)); - argument_names.push_back(std::move(not_null_else_arg_name)); - argument_names.push_back(std::move(condition_arg_name)); + argument_names.push_back(std::move(else_arg_name)); + argument_names.push_back(std::move(not_null_condition_arg_name)); return analyzer->applyFunction(func_name, argument_names, actions, getCollatorFromExpr(expr)); } diff --git a/tests/fullstack-test/expr/ifnull.test b/tests/fullstack-test/expr/ifnull.test new file mode 100644 index 00000000000..490d519421c --- /dev/null +++ b/tests/fullstack-test/expr/ifnull.test @@ -0,0 +1,28 @@ +# Copyright 2022 PingCAP, Ltd. +# +# Licensed 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. + +mysql> drop table if exists test.t; +mysql> create table test.t(a int, b int); +mysql> insert into test.t values(1, null),(null, 1); +mysql> alter table test.t set tiflash replica 1; +func> wait_table test t +mysql> set @@tidb_isolation_read_engines='tiflash'; select * from test.t where not ifnull(a > b, null); +mysql> set @@tidb_isolation_read_engines='tiflash'; set @@tidb_enforce_mpp=1; select a, b, a>b, ifnull(a > b, null), not ifnull(a > b, null) from test.t; ++------+------+------+---------------------+-------------------------+ +| a | b | a>b | ifnull(a > b, null) | not ifnull(a > b, null) | ++------+------+------+---------------------+-------------------------+ +| 1 | NULL | NULL | NULL | NULL | +| NULL | 1 | NULL | NULL | NULL | ++------+------+------+---------------------+-------------------------+ +mysql> drop table if exists test.t;