This is an automated email from the ASF dual-hosted git repository.
mrhhsg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new c6f73b7a821 [fix](function) Do not mutate shared nested column when IF
normalizes a nullable condition (#67931)
c6f73b7a821 is described below
commit c6f73b7a82117b1d561545d5e8ba6b6688344df5
Author: Jerry Hu <[email protected]>
AuthorDate: Fri Sep 18 16:30:14 2026 +0800
[fix](function) Do not mutate shared nested column when IF normalizes a
nullable condition (#67931)
### What problem does this PR solve?
Issue Number: None
Problem Summary:
When the condition of `IF` is a `Nullable(Boolean)` column, both
`VectorizedIfExpr` and `FunctionIf` normalize it by treating NULL as
false.
They did this by writing `nested[i] &= !null_map[i]` directly into the
nested column of the nullable condition.
That nested column can be shared with other columns of the same block.
`NULLIF(b, p)` is implemented as `if(b = p, NULL, b)` and wraps `b`
itself
as the nested column of its `Nullable(Boolean)` result, so
`IF(NULLIF(b, p), f, b)` overwrote `b` in place: the else branch read
the
polluted values and returned a wrong result, and any other projection of
`b` in the same block was polluted as well. With
`short_circuit_evaluation=true` the expression goes through a different
path and returned the correct result, which hid the bug.
Build a fresh condition column from `nested & !null_map` instead of
mutating the shared nested column in place.
### Release note
None
### Check List (For Author)
- Test:
- Unit Test:
`VConditionExprIfTest.NullableCondition_NotPolluteSharedNestedColumn`,
`FunctionIfTest.NullableConditionNotPolluteSharedNestedColumn` (both
fail without the fix)
- Regression test: `test_if_nullable_condition`
- Behavior changed: No
- Does this need documentation: No
https://claude.ai/code/session_013PjVxV5VYPRduC2fubqCzD
---
be/src/exprs/function/if.cpp | 19 ++--
be/src/exprs/vcondition_expr.cpp | 20 ++--
be/test/exprs/function/function_if_test.cpp | 101 +++++++++++++++++++++
be/test/exprs/vcondition_expr_test.cpp | 74 ++++++++++++++-
.../test_if_nullable_condition.out | 17 ++++
.../test_if_nullable_condition.groovy | 58 ++++++++++++
6 files changed, 270 insertions(+), 19 deletions(-)
diff --git a/be/src/exprs/function/if.cpp b/be/src/exprs/function/if.cpp
index 0ff43b30d6b..aaea95f72c9 100644
--- a/be/src/exprs/function/if.cpp
+++ b/be/src/exprs/function/if.cpp
@@ -430,17 +430,22 @@ public:
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<const
ColumnUInt8&>(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 1207b92dbc1..4c85c887f09 100644
--- a/be/src/exprs/vcondition_expr.cpp
+++ b/be/src/exprs/vcondition_expr.cpp
@@ -19,8 +19,11 @@
#include <glog/logging.h>
+#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<ColumnNullable>(*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<const
ColumnUInt8&>(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 00000000000..83c2a6f1fca
--- /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 <gtest/gtest.h>
+
+#include <memory>
+#include <vector>
+
+#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<UInt8>& 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<DataTypeUInt8>();
+ 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<void>(func->close(fn_ctx, FunctionContext::THREAD_LOCAL));
+ static_cast<void>(func->close(fn_ctx, FunctionContext::FRAGMENT_LOCAL));
+
+ const auto& result_data =
+ assert_cast<const
ColumnUInt8&>(*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<const ColumnUInt8&>(*b_column).get_data();
+ EXPECT_EQ(b_data[0], 1);
+ EXPECT_EQ(b_data[1], 1);
+ const auto& cond_nested_data =
+ assert_cast<const ColumnUInt8&>(
+ assert_cast<const
ColumnNullable&>(*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 83b5ea26aa8..98912b6641a 100644
--- a/be/test/exprs/vcondition_expr_test.cpp
+++ b/be/test/exprs/vcondition_expr_test.cpp
@@ -25,8 +25,10 @@
#include <cmath>
#include <limits>
#include <memory>
+#include <string>
#include <vector>
+#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<double>& 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<UInt8>& 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<int64_t>::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<DataTypeUInt8>();
+ 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<MockChildVExpr>(
+ cond_column, std::make_shared<DataTypeNullable>(bool_type)));
+ if_expr->add_child(std::make_shared<MockChildVExpr>(f_column, bool_type));
+ if_expr->add_child(std::make_shared<MockChildVExpr>(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<const
ColumnUInt8&>(*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<const ColumnUInt8&>(*b_column).get_data();
+ EXPECT_EQ(b_data[0], 1);
+ EXPECT_EQ(b_data[1], 1);
+ const auto& cond_nested_data =
+ assert_cast<const ColumnUInt8&>(
+ assert_cast<const
ColumnNullable&>(*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 00000000000..816b5b2dd32
--- /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 00000000000..0fd5cb261f1
--- /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
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
