This is an automated email from the ASF dual-hosted git repository.
zclllyybb 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 38fb05eaaab [fix](be) Normalize constant nullable TopN predicate
results (#66442)
38fb05eaaab is described below
commit 38fb05eaaab72739b34c942503bb0000f0967dd5
Author: HappenLee <[email protected]>
AuthorDate: Thu Aug 6 21:35:07 2026 +0800
[fix](be) Normalize constant nullable TopN predicate results (#66442)
Problem Summary: Nullable TopN comparisons can return a `ColumnConst`
that wraps a `ColumnNullable` result, especially when an input batch is
entirely NULL. The previous normalization only handled a top-level
`ColumnNullable`, so the constant nullable result could reach filter
execution without converting SQL NULL into the NULLS FIRST or NULLS LAST
filter decision.
This change recursively normalizes constant wrappers, collapses nullable
Boolean results according to the TopN null ordering, preserves the
constant shape, and aligns the unit-test expression metadata with
production nullability. The new unit test covers both NULLS FIRST and
NULLS LAST through `execute_column` and `execute_filter`.
---
be/src/exprs/vtopn_pred.h | 37 +++++++++++++--------
be/test/format_v2/parquet/parquet_scan_test.cpp | 2 +-
be/test/runtime/runtime_predicate_test.cpp | 44 ++++++++++++++++++++++++-
3 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/be/src/exprs/vtopn_pred.h b/be/src/exprs/vtopn_pred.h
index c2fe7c2fd0d..af680955d05 100644
--- a/be/src/exprs/vtopn_pred.h
+++ b/be/src/exprs/vtopn_pred.h
@@ -121,19 +121,7 @@ public:
RETURN_IF_ERROR(_function->execute(nullptr, temp_block, arguments,
num_columns_without_result,
temp_block.rows()));
result_column =
std::move(temp_block.get_by_position(num_columns_without_result).column);
- if (auto mutable_result = IColumn::mutate(std::move(result_column));
- auto* nullable =
check_and_get_column<ColumnNullable>(*mutable_result)) {
- auto& values =
assert_cast<ColumnUInt8&>(*nullable->get_nested_column_ptr()).get_data();
- const auto& null_map = nullable->get_null_map_data();
- // Master validates execute_type() against the physical result
column. Collapse SQL
- // NULL to the filter decision here: NULLS FIRST keeps it, while
NULLS LAST rejects it.
- for (size_t row = 0; row < values.size(); ++row) {
- values[row] = null_map[row] ? _predicate->nulls_first() :
values[row];
- }
- result_column = nullable->get_nested_column_ptr();
- } else {
- result_column = std::move(mutable_result);
- }
+ result_column = _normalize_filter_result(std::move(result_column));
DCHECK_EQ(result_column->size(), count);
return Status::OK();
}
@@ -284,6 +272,29 @@ public:
}
private:
+ ColumnPtr _normalize_filter_result(ColumnPtr column) const {
+ const size_t rows = column->size();
+ if (const auto* constant = check_and_get_column<ColumnConst>(*column))
{
+ auto nested =
_normalize_filter_result(constant->get_data_column_ptr());
+ return ColumnConst::create(std::move(nested), rows);
+ }
+
+ auto mutable_column = IColumn::mutate(std::move(column));
+ if (auto* nullable =
check_and_get_column<ColumnNullable>(*mutable_column)) {
+ auto& values =
assert_cast<ColumnUInt8&>(*nullable->get_nested_column_ptr()).get_data();
+ auto& null_map = nullable->get_null_map_data();
+ // Collapse SQL NULL to the filter decision: NULLS FIRST keeps it,
while NULLS LAST
+ // rejects it. Preserve the nullable wrapper so strict block
execution still matches
+ // the expression's declared Nullable(Boolean) type.
+ for (size_t row = 0; row < values.size(); ++row) {
+ values[row] = null_map[row] ? _predicate->nulls_first() :
values[row];
+ }
+ nullable->fill_false_to_nullmap(rows);
+ return mutable_column;
+ }
+ return mutable_column;
+ }
+
int _source_node_id;
std::string _expr_name;
RuntimePredicate* _predicate = nullptr;
diff --git a/be/test/format_v2/parquet/parquet_scan_test.cpp
b/be/test/format_v2/parquet/parquet_scan_test.cpp
index cc95b51cebf..eaca8f772c8 100644
--- a/be/test/format_v2/parquet/parquet_scan_test.cpp
+++ b/be/test/format_v2/parquet/parquet_scan_test.cpp
@@ -931,7 +931,7 @@ PreparedTopNConjunct create_topn_conjunct(RuntimeState*
state, int column_id,
TExprNode topn_node;
topn_node.__set_type(std::make_shared<DataTypeUInt8>()->to_thrift());
- topn_node.__set_is_nullable(false);
+ topn_node.__set_is_nullable(data_type->is_nullable());
auto topn = VTopNPred::create_shared(topn_node, 10, nullptr);
topn->add_child(VSlotRef::create_shared(column_id, column_id, -1,
data_type, "topn_column"));
auto conjunct = VExprContext::create_shared(std::move(topn));
diff --git a/be/test/runtime/runtime_predicate_test.cpp
b/be/test/runtime/runtime_predicate_test.cpp
index c6298b91a5b..baf27e72ddc 100644
--- a/be/test/runtime/runtime_predicate_test.cpp
+++ b/be/test/runtime/runtime_predicate_test.cpp
@@ -76,7 +76,7 @@ VExprContextSPtr create_prepared_topn_expr(MockRuntimeState*
state, const DataTy
TExprNode node;
node.__set_type(create_type_desc(PrimitiveType::TYPE_BOOLEAN));
- node.__set_is_nullable(false);
+ node.__set_is_nullable(data_type->is_nullable());
auto expr = VTopNPred::create_shared(node, SOURCE_NODE_ID, nullptr);
expr->add_child(VSlotRef::create_shared(SLOT_ID, 0, -1, data_type,
"topn_column"));
auto context = VExprContext::create_shared(std::move(expr));
@@ -306,4 +306,46 @@ TEST(RuntimePredicateTest,
NullableNullsFirstTopNSupportsRawNullSemantics) {
context->close();
}
+TEST(RuntimePredicateTest,
TopNPredicateNormalizesConstNullableComparisonResult) {
+ constexpr size_t rows = 3;
+ const auto type = make_nullable(std::make_shared<DataTypeInt32>());
+
+ for (const bool nulls_first : {false, true}) {
+ SCOPED_TRACE(nulls_first ? "NULLS FIRST" : "NULLS LAST");
+ MockRuntimeState state;
+ auto context = create_prepared_topn_expr(&state, type,
Field::create_field<TYPE_INT>(3),
+ true, nulls_first);
+
+ Block block;
+ block.insert(
+ {ColumnNullable::create(ColumnInt32::create(rows, 0),
ColumnUInt8::create(rows, 1)),
+ type, "topn_column"});
+
+ ColumnPtr result_column;
+ auto status = context->root()->execute_column(context.get(), &block,
nullptr, block.rows(),
+ result_column);
+ ASSERT_TRUE(status.ok()) << status;
+ ASSERT_TRUE(is_column_const(*result_column));
+ EXPECT_FALSE(result_column->is_null_at(0));
+ EXPECT_EQ(result_column->get_bool(0), nulls_first);
+
+ int result_column_id = -1;
+ status = context->execute(&block, &result_column_id);
+ ASSERT_TRUE(status.ok()) << status;
+ const auto& strict_result = block.get_by_position(result_column_id);
+ ASSERT_TRUE(is_column_const(*strict_result.column));
+ EXPECT_NE(nullptr,
check_and_get_column_with_const<ColumnNullable>(*strict_result.column));
+ EXPECT_FALSE(strict_result.column->is_null_at(0));
+ EXPECT_EQ(strict_result.column->get_bool(0), nulls_first);
+
+ IColumn::Filter filter(rows, 1);
+ bool can_filter_all = false;
+ status = context->execute_filter(&block, filter.data(), rows, false,
&can_filter_all);
+ ASSERT_TRUE(status.ok()) << status;
+ EXPECT_EQ(filter, IColumn::Filter(rows, nulls_first));
+ EXPECT_EQ(can_filter_all, !nulls_first);
+ context->close();
+ }
+}
+
} // namespace doris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
