diff --git a/be/src/exprs/function/if.cpp b/be/src/exprs/function/if.cpp index 0ff43b30d6b9df..aaea95f72c902d 100644 --- a/be/src/exprs/function/if.cpp +++ b/be/src/exprs/function/if.cpp @@ -430,17 +430,22 @@ class FunctionIf : public IFunction { DCHECK(remove_nullable(arg_cond.type)->get_primitive_type() == PrimitiveType::TYPE_BOOLEAN); - // update nested column by null map + // Treat NULL as false. The nested column may be shared with other columns of the + // block (e.g. NULLIF wraps its first argument as the nested column), so build a + // new condition column instead of mutating the nested column in place. + const auto rows = nullable->size(); const auto* __restrict null_map = nullable->get_null_map_data().data(); - auto* __restrict nested_bool_data = - ((ColumnUInt8&)(nullable->get_nested_column())).get_data().data(); - auto rows = nullable->size(); + const auto* __restrict nested_bool_data = + assert_cast(nullable->get_nested_column()) + .get_data() + .data(); + auto cond_column = ColumnUInt8::create(rows); + auto* __restrict cond_data = cond_column->get_data().data(); for (size_t i = 0; i < rows; i++) { - nested_bool_data[i] &= !null_map[i]; + cond_data[i] = nested_bool_data[i] & !null_map[i]; } auto column_size = block.columns(); - block.insert({nullable->get_nested_column_ptr(), remove_nullable(arg_cond.type), - arg_cond.name}); + block.insert({std::move(cond_column), remove_nullable(arg_cond.type), arg_cond.name}); handled = true; return _execute_impl_internal(context, block, {column_size, arguments[1], arguments[2]}, diff --git a/be/src/exprs/vcondition_expr.cpp b/be/src/exprs/vcondition_expr.cpp index 1207b92dbc1118..4c85c887f0919e 100644 --- a/be/src/exprs/vcondition_expr.cpp +++ b/be/src/exprs/vcondition_expr.cpp @@ -19,8 +19,11 @@ #include +#include "core/assert_cast.h" #include "core/column/column.h" #include "core/column/column_const.h" +#include "core/column/column_nullable.h" +#include "core/column/column_vector.h" #include "exprs/function_context.h" #include "util/simd/bits.h" @@ -377,17 +380,20 @@ Status VectorizedIfExpr::execute_for_null_condition(Block& block, const ColumnNu if (const auto* nullable = check_and_get_column(*arg_cond.column)) { DCHECK(remove_nullable(arg_cond.type)->get_primitive_type() == PrimitiveType::TYPE_BOOLEAN); - // update nested column by null map + // Treat NULL as false. The nested column may be shared with other columns of the + // block (e.g. NULLIF wraps its first argument as the nested column), so build a new + // condition column instead of mutating the nested column in place. + const auto rows = nullable->size(); const auto* __restrict null_map = nullable->get_null_map_data().data(); - auto* __restrict nested_bool_data = - ((ColumnUInt8&)(nullable->get_nested_column())).get_data().data(); - auto rows = nullable->size(); + const auto* __restrict nested_bool_data = + assert_cast(nullable->get_nested_column()).get_data().data(); + auto cond_column = ColumnUInt8::create(rows); + auto* __restrict cond_data = cond_column->get_data().data(); for (size_t i = 0; i < rows; i++) { - nested_bool_data[i] &= !null_map[i]; + cond_data[i] = nested_bool_data[i] & !null_map[i]; } auto column_size = block.columns(); - block.insert( - {nullable->get_nested_column_ptr(), remove_nullable(arg_cond.type), arg_cond.name}); + block.insert({std::move(cond_column), remove_nullable(arg_cond.type), arg_cond.name}); handled = true; return _execute_impl_internal(block, {column_size, arguments[1], arguments[2]}, result, diff --git a/be/test/exprs/function/function_if_test.cpp b/be/test/exprs/function/function_if_test.cpp new file mode 100644 index 00000000000000..83c2a6f1fcaeb7 --- /dev/null +++ b/be/test/exprs/function/function_if_test.cpp @@ -0,0 +1,101 @@ +// 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. + +#include + +#include +#include + +#include "common/status.h" +#include "core/assert_cast.h" +#include "core/block/block.h" +#include "core/column/column.h" +#include "core/column/column_nullable.h" +#include "core/column/column_vector.h" +#include "core/data_type/data_type_nullable.h" +#include "core/data_type/data_type_number.h" +#include "core/types.h" +#include "exprs/function/function.h" +#include "exprs/function/simple_function_factory.h" +#include "exprs/function_context.h" +#include "testutil/function_utils.h" + +namespace doris { + +static ColumnPtr make_bool_column(const std::vector& values) { + auto column = ColumnUInt8::create(); + for (auto v : values) { + column->insert_value(v); + } + return column; +} + +// Same shape as `IF(NULLIF(b, p), f, b)`: NULLIF is if(b = p, NULL, b) and wraps b's +// column itself as the nested column of its Nullable(Boolean) result. The outer IF treats +// a NULL condition as false; that normalization must not be written into the shared +// nested column, otherwise both the else branch and every other user of b see the +// polluted values. +// Input: +// b (non-nullable bool): [1, 1] +// cond = Nullable(nested = b's column, null_map = [0, 1]), logically [true, NULL] +// f (non-nullable bool): [0, 0] +// Expected IF(cond, f, b): [0, 1]; b (and cond's nested column) must stay [1, 1]. +TEST(FunctionIfTest, NullableConditionNotPolluteSharedNestedColumn) { + auto bool_type = std::make_shared(); + auto nullable_bool_type = make_nullable(bool_type); + + ColumnPtr b_column = make_bool_column({1, 1}); + ColumnPtr f_column = make_bool_column({0, 0}); + ColumnPtr cond_column = ColumnNullable::create(b_column, make_bool_column({0, 1})); + + Block block({{cond_column, nullable_bool_type, "cond"}, + {f_column, bool_type, "f"}, + {b_column, bool_type, "b"}, + {nullptr, bool_type, "result"}}); + + auto func = SimpleFunctionFactory::instance().get_function( + "if", {block.get_by_position(0), block.get_by_position(1), block.get_by_position(2)}, + bool_type); + ASSERT_TRUE(func != nullptr); + + FunctionUtils fn_utils(bool_type, {nullable_bool_type, bool_type, bool_type}, false); + auto* fn_ctx = fn_utils.get_fn_ctx(); + ASSERT_TRUE(func->open(fn_ctx, FunctionContext::FRAGMENT_LOCAL).ok()); + ASSERT_TRUE(func->open(fn_ctx, FunctionContext::THREAD_LOCAL).ok()); + auto st = func->execute(fn_ctx, block, {0, 1, 2}, 3, 2); + ASSERT_TRUE(st.ok()) << st.to_string(); + static_cast(func->close(fn_ctx, FunctionContext::THREAD_LOCAL)); + static_cast(func->close(fn_ctx, FunctionContext::FRAGMENT_LOCAL)); + + const auto& result_data = + assert_cast(*block.get_by_position(3).column).get_data(); + ASSERT_EQ(result_data.size(), 2); + EXPECT_EQ(result_data[0], 0); + EXPECT_EQ(result_data[1], 1); + + const auto& b_data = assert_cast(*b_column).get_data(); + EXPECT_EQ(b_data[0], 1); + EXPECT_EQ(b_data[1], 1); + const auto& cond_nested_data = + assert_cast( + assert_cast(*cond_column).get_nested_column()) + .get_data(); + EXPECT_EQ(cond_nested_data[0], 1); + EXPECT_EQ(cond_nested_data[1], 1); +} + +} // namespace doris diff --git a/be/test/exprs/vcondition_expr_test.cpp b/be/test/exprs/vcondition_expr_test.cpp index 83b5ea26aa8fd9..98912b6641a03b 100644 --- a/be/test/exprs/vcondition_expr_test.cpp +++ b/be/test/exprs/vcondition_expr_test.cpp @@ -25,8 +25,10 @@ #include #include #include +#include #include +#include "core/assert_cast.h" #include "core/column/column_nullable.h" #include "core/column/column_vector.h" #include "core/data_type/data_type_date_or_datetime_v2.h" @@ -38,9 +40,10 @@ namespace doris { -// Build a minimal TExprNode as the input of VectorizedCoalesceExpr. +// Build a minimal TExprNode as the input of a VConditionExpr. // Only fields required by the VExpr base ctor (so that create_data_type works) are set. -static TExprNode make_coalesce_node(TPrimitiveType::type ptype, bool is_nullable, int scale = -1) { +static TExprNode make_function_node(const std::string& fn_name, TPrimitiveType::type ptype, + bool is_nullable, int scale = -1) { TExprNode node; node.node_type = TExprNodeType::FUNCTION_CALL; node.num_children = 0; @@ -59,14 +62,18 @@ static TExprNode make_coalesce_node(TPrimitiveType::type ptype, bool is_nullable node.__set_type(type_desc); TFunction fn; - TFunctionName fn_name; - fn_name.function_name = "coalesce"; - fn.name = fn_name; + TFunctionName function_name; + function_name.function_name = fn_name; + fn.name = function_name; node.__set_fn(fn); return node; } +static TExprNode make_coalesce_node(TPrimitiveType::type ptype, bool is_nullable, int scale = -1) { + return make_function_node("coalesce", ptype, is_nullable, scale); +} + // Mock child expression: returns the pre-injected ColumnPtr / DataTypePtr to the parent expr. // Behavior is modeled after MockVExprForTryCast in try_cast_expr_test.cpp. class MockChildVExpr : public VExpr { @@ -119,6 +126,15 @@ static ColumnPtr make_float64_column(const std::vector& values) { return col; } +// Helper: build a non-nullable Boolean column from a list of 0/1 values. +static ColumnPtr make_bool_column(const std::vector& values) { + auto col = ColumnUInt8::create(); + for (auto v : values) { + col->insert_value(v); + } + return col; +} + // Helper: extract the Float64 value at `row` from the result column, // handling both nullable and non-nullable cases. static double get_float64_value(const ColumnPtr& column, size_t row, bool* is_null = nullptr) { @@ -409,4 +425,52 @@ TEST_F(VConditionExprCoalesceTest, TimeStampNs) { EXPECT_EQ(values[3].epoch_nanos(), std::numeric_limits::min()); } +class VConditionExprIfTest : public ::testing::Test {}; + +// Same shape as `IF(NULLIF(b, p), f, b)`: NULLIF wraps b's column itself as the nested +// column of its Nullable(Boolean) result. IF treats a NULL condition as false; that +// normalization must not be written into the shared nested column, otherwise the else +// branch (and every other user of b in the block) reads polluted values. +// Input: +// b (non-nullable bool): [1, 1] +// cond = Nullable(nested = b's column, null_map = [0, 1]), logically [true, NULL] +// f (non-nullable bool): [0, 0] +// Expected IF(cond, f, b): [0, 1]; b (and cond's nested column) must stay [1, 1]. +TEST_F(VConditionExprIfTest, NullableCondition_NotPolluteSharedNestedColumn) { + auto if_node = make_function_node("if", TPrimitiveType::BOOLEAN, /*is_nullable=*/false); + auto if_expr = VectorizedIfExpr::create_shared(if_node); + auto bool_type = std::make_shared(); + if_expr->data_type() = bool_type; + + ColumnPtr b_column = make_bool_column({1, 1}); + ColumnPtr f_column = make_bool_column({0, 0}); + ColumnPtr cond_column = ColumnNullable::create(b_column, make_bool_column({0, 1})); + + if_expr->add_child(std::make_shared( + cond_column, std::make_shared(bool_type))); + if_expr->add_child(std::make_shared(f_column, bool_type)); + if_expr->add_child(std::make_shared(b_column, bool_type)); + + VExprContext context(if_expr); + ColumnPtr result; + auto st = if_expr->execute_column_impl(&context, /*block=*/nullptr, /*selector=*/nullptr, + /*count=*/2, result); + ASSERT_TRUE(st.ok()) << st.to_string(); + ASSERT_TRUE(result.get() != nullptr); + const auto& result_data = assert_cast(*result).get_data(); + ASSERT_EQ(result_data.size(), 2); + EXPECT_EQ(result_data[0], 0); + EXPECT_EQ(result_data[1], 1); + + const auto& b_data = assert_cast(*b_column).get_data(); + EXPECT_EQ(b_data[0], 1); + EXPECT_EQ(b_data[1], 1); + const auto& cond_nested_data = + assert_cast( + assert_cast(*cond_column).get_nested_column()) + .get_data(); + EXPECT_EQ(cond_nested_data[0], 1); + EXPECT_EQ(cond_nested_data[1], 1); +} + } // namespace doris diff --git a/regression-test/data/query_p0/sql_functions/conditional_functions/test_if_nullable_condition.out b/regression-test/data/query_p0/sql_functions/conditional_functions/test_if_nullable_condition.out new file mode 100644 index 00000000000000..816b5b2dd32f3f --- /dev/null +++ b/regression-test/data/query_p0/sql_functions/conditional_functions/test_if_nullable_condition.out @@ -0,0 +1,17 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !if_nullif -- +1 false +2 true + +-- !if_nullif_projection -- +1 true false false true false +2 true true false \N true + +-- !if_nullif_short_circuit -- +1 false +2 true + +-- !if_nullif_projection_short_circuit -- +1 true false false true false +2 true true false \N true + diff --git a/regression-test/suites/query_p0/sql_functions/conditional_functions/test_if_nullable_condition.groovy b/regression-test/suites/query_p0/sql_functions/conditional_functions/test_if_nullable_condition.groovy new file mode 100644 index 00000000000000..0fd5cb261f137e --- /dev/null +++ b/regression-test/suites/query_p0/sql_functions/conditional_functions/test_if_nullable_condition.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_if_nullable_condition") { + sql "drop table if exists test_if_nullable_condition" + sql """ + create table test_if_nullable_condition ( + id int, + b boolean not null, + p boolean not null, + f boolean not null + ) duplicate key(id) + distributed by hash(id) buckets 1 + properties ("replication_num" = "1") + """ + sql """ + insert into test_if_nullable_condition values + (1, true, false, false), + (2, true, true, false) + """ + + // nullif(b, p) is [true, NULL] and reuses b as the nested column of its result. + // IF treats the NULL condition as false; that normalization must not be written into + // the column shared with the else branch and with the other projected columns. + sql "set short_circuit_evaluation = false" + qt_if_nullif """ + select id, if(nullif(b, p), f, b) as r + from test_if_nullable_condition order by id + """ + qt_if_nullif_projection """ + select id, b, p, f, nullif(b, p) as cond, if(nullif(b, p), f, b) as r + from test_if_nullable_condition order by id + """ + + sql "set short_circuit_evaluation = true" + qt_if_nullif_short_circuit """ + select id, if(nullif(b, p), f, b) as r + from test_if_nullable_condition order by id + """ + qt_if_nullif_projection_short_circuit """ + select id, b, p, f, nullif(b, p) as cond, if(nullif(b, p), f, b) as r + from test_if_nullable_condition order by id + """ +}