This is an automated email from the ASF dual-hosted git repository.
HappenLee 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 2a550292371 [fix](be) Preserve floating point values in CASE branch
selection (#67896)
2a550292371 is described below
commit 2a55029237141b5ed8e2177b262d493f9511089a
Author: HappenLee <[email protected]>
AuthorDate: Mon Sep 14 20:25:04 2026 +0800
[fix](be) Preserve floating point values in CASE branch selection (#67896)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary: A non-nullable FLOAT/DOUBLE CASE can return NaN for a
finite selected branch when an unselected branch contains Infinity or
NaN. For example, with ordinary CASE evaluation, selecting `1`, an
overflowing multiplication, and `2` over three rows returns `NaN,
Infinity, NaN` instead of `1, Infinity, 2`. The same result assembly
also loses the sign of selected negative zero.
The result assembly multiplies each branch value by a zero/one mask and
adds it to the result. IEEE-754 arithmetic makes `0 * Infinity` and `0 *
NaN` equal NaN. Use conditional stores to copy the selected floating
point value without arithmetic. DATE, DATETIME, DATEV2, DATETIMEV2,
TIMESTAMP_NS and TIMESTAMPTZ share this conditional-store loop,
replacing their previous ternary assignments. This form allows AVX2
masked loads/stores, while a ternary assignment can become a conditional
pointer load that inhibits vectorization.
Retain the existing default-value initialization and the arithmetic
accumulator for the other numeric types. Removing the redundant
initialization for the direct-store types is deferred.
Add bitwise unit tests, SQL regression tests and a benchmark that
directly calls the production result assembly function. The typed unit
tests cover all eight types with both uint8_t and uint16_t branch
indices.
### Release note
Fix incorrect FLOAT/DOUBLE CASE results caused by unselected non-finite
branch values, and preserve selected negative zero.
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [x] Manual test (details below)
- [ ] No need to test or manual test.
Validation of the date/time follow-up:
- All 48 `VCaseSelectionTest*` tests pass via `./run-be-ut.sh -j 48
--run --filter='VCaseSelectionTest*'` with ASAN. Coverage includes all
eight types, both index widths, constants, boundary values, vector tails
and 255/257 branches.
- clang-format 16, header hygiene, `git diff --check` and clang-tidy
pass. For clang-tidy, the header was explicitly mapped to the generated
selection-test compiler flags to avoid failed automatic header
inference.
- `./build.sh --be -j 48` with ASAN compiled and linked the BE
successfully. Full build/packaging did not pass: the later
`kuromoji_build_dict` process reported an ASAN double-free during
OpenBLAS/OpenMP initialization (`dlsym` / `_dlerror_run`), before
entering main. The separate standard ASAN unit-test build and execution
passed.
- Default-value initialization and the remaining arithmetic accumulator
are unchanged.
Earlier validation of the floating point fix (before the date/time
follow-up):
- All 12 original floating point unit tests fail on the original
implementation and pass with ASAN after the fix. These cases are
retained in the expanded typed suite. Coverage includes non-finite
values, signed zero and subnormal values.
- `test_case_float_nonfinite` and `test_short_circuit_evaluation` pass.
The original implementation fails the new SQL regression. Golden output
was generated and independently verified through short-circuit
evaluation using the regression runner. These SQL suites were not rerun
for the date/time follow-up.
- ASAN BE build, clang-format 16, header hygiene and clang-tidy checks
pass for the earlier revision.
- RELEASE benchmarks built with Clang 21.1.8 and `-O3 -msse4.2 -mavx2`
on Xeon Platinum 8457C cover 42 scenarios. With identical input, a fixed
CPU, five repetitions per run and before/after/after/before ordering,
median CPU time decreases by approximately 5%–39% (18.5% geometric mean
reduction). This measures floating point result assembly on this AVX2
machine, including result allocation; these figures do not measure the
date/time follow-up.
- Disassembly of all four floating point/index-width specializations in
the linked binaries confirms `vmaskmovps/pd`, replacing floating point
multiply/add instructions without fast-math.
- Behavior changed:
- [ ] No.
- [x] Yes. Return the selected floating point value without
contamination from other branches or loss of negative zero. Date/time
selection preserves the existing results.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
---
be/benchmark/benchmark_case_expr.hpp | 93 ++++++++++++++
be/benchmark/benchmark_main.cpp | 1 +
be/src/exprs/vcase_expr.h | 14 ++-
be/test/exprs/vcase_expr_test.cpp | 140 +++++++++++++++++++++
.../test_case_float_nonfinite.out | 81 ++++++++++++
.../test_case_float_nonfinite.groovy | 64 ++++++++++
6 files changed, 389 insertions(+), 4 deletions(-)
diff --git a/be/benchmark/benchmark_case_expr.hpp
b/be/benchmark/benchmark_case_expr.hpp
new file mode 100644
index 00000000000..7bf695267a9
--- /dev/null
+++ b/be/benchmark/benchmark_case_expr.hpp
@@ -0,0 +1,93 @@
+// 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.
+
+#pragma once
+
+#include <benchmark/benchmark.h>
+
+#include <random>
+
+#include "core/data_type/data_type_number.h"
+#include "exprs/vcase_expr.h"
+
+namespace doris {
+
+// Keep the production result assembly visible in disassembly as well as
timing it.
+template <typename IndexType, PrimitiveType PT>
+NO_INLINE ColumnPtr run_case_selection(const VCaseExpr& expr, const IndexType*
indices,
+ std::vector<ColumnPtr>& columns, size_t
rows) {
+ return expr._execute_update_result_impl<IndexType,
ColumnVector<PT>>(indices, columns, rows);
+}
+
+template <typename IndexType, PrimitiveType PT>
+void BM_CaseFloatSelection(benchmark::State& state) {
+ const size_t rows = state.range(0);
+ const size_t branches = state.range(1);
+ const auto distribution = state.range(2);
+ TExprNode node;
+ node.__set_node_type(TExprNodeType::CASE_EXPR);
+ node.__set_type(DataTypeNumber<PT>().to_thrift());
+ node.__set_is_nullable(false);
+ node.case_expr.__set_has_else_expr(true);
+ VCaseExpr expr(node);
+ std::mt19937 rng(20260912);
+ std::vector<IndexType> indices(rows);
+ for (size_t row = 0; row < rows; ++row) {
+ // Interleaved, random, or 99% ELSE. All inputs are finite for
before/after comparison.
+ indices[row] = static_cast<IndexType>(distribution == 0 ? row %
branches
+ : distribution == 1
+ ? rng() % branches
+ : (row % 100 == 0 ?
rng() % branches : 0));
+ }
+ std::vector<ColumnPtr> columns;
+ for (size_t branch = 0; branch < branches; ++branch) {
+ auto column = ColumnVector<PT>::create(rows);
+ for (size_t row = 0; row < rows; ++row) {
+ column->get_data()[row] =
+ static_cast<typename ColumnVector<PT>::value_type>((row +
branch + 1) * 0.125);
+ }
+ columns.push_back(std::move(column));
+ }
+ for (auto _ : state) {
+ auto result = run_case_selection<IndexType, PT>(expr, indices.data(),
columns, rows);
+ benchmark::DoNotOptimize(result);
+ }
+ state.SetItemsProcessed(state.iterations() * rows);
+}
+
+inline void case_float_arguments(benchmark::internal::Benchmark* benchmark) {
+ for (int64_t rows : {31, 4096, 65536}) {
+ for (int64_t branches : {3, 16}) {
+ for (int64_t distribution : {0, 1, 2}) {
+ benchmark->Args({rows, branches, distribution});
+ }
+ }
+ }
+}
+
+BENCHMARK_TEMPLATE(BM_CaseFloatSelection, uint8_t,
TYPE_FLOAT)->Apply(case_float_arguments);
+BENCHMARK_TEMPLATE(BM_CaseFloatSelection, uint8_t,
TYPE_DOUBLE)->Apply(case_float_arguments);
+BENCHMARK_TEMPLATE(BM_CaseFloatSelection, uint16_t, TYPE_FLOAT)
+ ->Args({4096, 257, 0})
+ ->Args({4096, 257, 1})
+ ->Args({4096, 257, 2});
+BENCHMARK_TEMPLATE(BM_CaseFloatSelection, uint16_t, TYPE_DOUBLE)
+ ->Args({4096, 257, 0})
+ ->Args({4096, 257, 1})
+ ->Args({4096, 257, 2});
+
+} // namespace doris
diff --git a/be/benchmark/benchmark_main.cpp b/be/benchmark/benchmark_main.cpp
index 8f1ffd7efc8..acbc591effd 100644
--- a/be/benchmark/benchmark_main.cpp
+++ b/be/benchmark/benchmark_main.cpp
@@ -26,6 +26,7 @@
#include "benchmark_arrow_validation.hpp"
#include "benchmark_binary_arithmetic.hpp"
#include "benchmark_bit_pack.hpp"
+#include "benchmark_case_expr.hpp"
#include "benchmark_column_array_view.hpp"
#include "benchmark_column_array_view_distance.hpp"
#include "benchmark_fastunion.hpp"
diff --git a/be/src/exprs/vcase_expr.h b/be/src/exprs/vcase_expr.h
index 5bb2fbdefa3..7c3f27fb7ac 100644
--- a/be/src/exprs/vcase_expr.h
+++ b/be/src/exprs/vcase_expr.h
@@ -242,15 +242,21 @@ private:
then_columns[i].get())
->get_data()
.data();
- if constexpr (std::is_same_v<ColumnType, ColumnDate> ||
+ if constexpr (std::is_same_v<ColumnType, ColumnFloat32> ||
+ std::is_same_v<ColumnType, ColumnFloat64> ||
+ std::is_same_v<ColumnType, ColumnDate> ||
std::is_same_v<ColumnType, ColumnDateTime> ||
std::is_same_v<ColumnType, ColumnDateV2> ||
std::is_same_v<ColumnType, ColumnDateTimeV2> ||
std::is_same_v<ColumnType, ColumnTimeStampNs> ||
std::is_same_v<ColumnType, ColumnTimeStampTz>) {
- for (int row_idx = 0; row_idx < rows_count; row_idx++) {
- result_raw_data[row_idx] = (then_idx[row_idx] == i) ?
column_raw_data[row_idx]
- :
result_raw_data[row_idx];
+ // Arithmetic masking propagates unselected NaN/Infinity and
loses signed zero.
+ // Conditional stores also let the compiler vectorize without
loading from a
+ // selected source/destination pointer, as a ternary
assignment can do.
+ for (size_t row_idx = 0; row_idx < rows_count; row_idx++) {
+ if (then_idx[row_idx] == i) {
+ result_raw_data[row_idx] = column_raw_data[row_idx];
+ }
}
} else {
for (int row_idx = 0; row_idx < rows_count; row_idx++) {
diff --git a/be/test/exprs/vcase_expr_test.cpp
b/be/test/exprs/vcase_expr_test.cpp
new file mode 100644
index 00000000000..0412c0156c7
--- /dev/null
+++ b/be/test/exprs/vcase_expr_test.cpp
@@ -0,0 +1,140 @@
+// 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 "exprs/vcase_expr.h"
+
+#include <gtest/gtest.h>
+
+#include <array>
+#include <bit>
+#include <limits>
+#include <type_traits>
+
+#include "core/data_type/data_type_date.h"
+#include "core/data_type/data_type_date_or_datetime_v2.h"
+#include "core/data_type/data_type_date_time.h"
+#include "core/data_type/data_type_number.h"
+#include "core/data_type/data_type_timestamp_ns.h"
+#include "core/data_type/data_type_timestamptz.h"
+#include "core/type_limit.h"
+
+namespace doris {
+
+template <typename Index, PrimitiveType PT>
+struct CaseSelectionTypes {
+ using IndexType = Index;
+ using ColumnType = ColumnVector<PT>;
+ using DataType = typename PrimitiveTypeTraits<PT>::DataType;
+};
+
+using CaseSelectionTestTypes = ::testing::Types<
+ CaseSelectionTypes<uint8_t, TYPE_FLOAT>, CaseSelectionTypes<uint8_t,
TYPE_DOUBLE>,
+ CaseSelectionTypes<uint16_t, TYPE_FLOAT>, CaseSelectionTypes<uint16_t,
TYPE_DOUBLE>,
+ CaseSelectionTypes<uint8_t, TYPE_DATE>, CaseSelectionTypes<uint8_t,
TYPE_DATETIME>,
+ CaseSelectionTypes<uint8_t, TYPE_DATEV2>, CaseSelectionTypes<uint8_t,
TYPE_DATETIMEV2>,
+ CaseSelectionTypes<uint8_t, TYPE_TIMESTAMP_NS>,
+ CaseSelectionTypes<uint8_t, TYPE_TIMESTAMPTZ>,
CaseSelectionTypes<uint16_t, TYPE_DATE>,
+ CaseSelectionTypes<uint16_t, TYPE_DATETIME>,
CaseSelectionTypes<uint16_t, TYPE_DATEV2>,
+ CaseSelectionTypes<uint16_t, TYPE_DATETIMEV2>,
+ CaseSelectionTypes<uint16_t, TYPE_TIMESTAMP_NS>,
+ CaseSelectionTypes<uint16_t, TYPE_TIMESTAMPTZ>>;
+
+template <typename T>
+class VCaseSelectionTest : public ::testing::Test {
+protected:
+ using Index = typename T::IndexType;
+ using Column = typename T::ColumnType;
+ using Value = typename Column::value_type;
+ using Bits = std::conditional_t<sizeof(Value) == 4, uint32_t, uint64_t>;
+
+ void check_selection(size_t rows, size_t branches, bool constant) {
+ SCOPED_TRACE(::testing::Message()
+ << "rows=" << rows << " branches=" << branches << "
constant=" << constant);
+ TExprNode node;
+ node.__set_node_type(TExprNodeType::CASE_EXPR);
+ node.__set_type(typename T::DataType().to_thrift());
+ node.__set_is_nullable(false);
+ node.case_expr.__set_has_else_expr(true);
+ VCaseExpr expr(node);
+ const auto values = [] {
+ if constexpr (std::is_floating_point_v<Value>) {
+ // Arithmetic masking corrupts unselected infinities/NaNs, and
adding to +0 loses -0.
+ return std::array<Value, 9> {Value(1.25),
+ Value(-2.5),
+ Value(0.0),
+ Value(-0.0),
+
std::numeric_limits<Value>::infinity(),
+
-std::numeric_limits<Value>::infinity(),
+
std::numeric_limits<Value>::quiet_NaN(),
+
std::numeric_limits<Value>::denorm_min(),
+
std::numeric_limits<Value>::max()};
+ } else {
+ return std::array<Value, 3> {type_limit<Value>::min(),
Column::default_value(),
+ type_limit<Value>::max()};
+ }
+ }();
+ std::vector<Index> indices(rows);
+ for (size_t row = 0; row < rows; ++row) {
+ indices[row] = row % branches;
+ }
+ std::vector<ColumnPtr> columns;
+ for (size_t branch = 0; branch < branches; ++branch) {
+ auto column = Column::create(constant ? 1 : rows);
+ for (size_t row = 0; row < column->size(); ++row) {
+ column->get_data()[row] = values[(row / branches + branch) %
values.size()];
+ }
+ if (constant) {
+ columns.push_back(ColumnConst::create(std::move(column),
rows));
+ } else {
+ columns.push_back(std::move(column));
+ }
+ }
+ auto result = expr.template _execute_update_result_impl<Index,
Column>(indices.data(),
+
columns, rows);
+ const auto& actual = assert_cast<const Column&>(*result).get_data();
+ ASSERT_EQ(actual.size(), rows);
+ for (size_t row = 0; row < rows; ++row) {
+ const auto expected =
+ values[((constant ? 0 : row / branches) + indices[row]) %
values.size()];
+ // Compare bits to include NaN payloads, signed zero and subnormal
values.
+ ASSERT_EQ(std::bit_cast<Bits>(actual[row]),
std::bit_cast<Bits>(expected)) << row;
+ }
+ }
+};
+
+TYPED_TEST_SUITE(VCaseSelectionTest, CaseSelectionTestTypes);
+
+TYPED_TEST(VCaseSelectionTest, ValuesAndVectorTails) {
+ for (size_t rows : {0, 1, 3, 7, 8, 15, 16, 31, 32, 33, 4095, 4096, 4099}) {
+ this->check_selection(rows, 9, false);
+ }
+}
+
+TYPED_TEST(VCaseSelectionTest, ConstantBranches) {
+ for (size_t rows : {1, 31, 4099}) {
+ this->check_selection(rows, 9, true);
+ }
+}
+
+TYPED_TEST(VCaseSelectionTest, MaximumAndWideBranchIndices) {
+ // 255 columns still use uint8_t; 257 columns exercise indices beyond the
uint8_t range.
+ const size_t branches = sizeof(typename TypeParam::IndexType) == 1 ? 255 :
257;
+ this->check_selection(4099, branches, false);
+ this->check_selection(4099, branches, true);
+}
+
+} // namespace doris
diff --git
a/regression-test/data/query_p0/sql_functions/conditional_functions/test_case_float_nonfinite.out
b/regression-test/data/query_p0/sql_functions/conditional_functions/test_case_float_nonfinite.out
new file mode 100644
index 00000000000..74a2f239b99
--- /dev/null
+++
b/regression-test/data/query_p0/sql_functions/conditional_functions/test_case_float_nonfinite.out
@@ -0,0 +1,81 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !double_false_then --
+0 1 1 1
+1 Infinity -Infinity NaN
+2 2 2 2
+
+-- !double_false_else --
+0 1 1 1
+1 2 2 2
+2 Infinity -Infinity NaN
+
+-- !double_false_nullable --
+0 1
+1 Infinity
+2 \N
+
+-- !double_false_batches --
+1 1
+2 4097
+Infinity 1
+
+-- !float_false_then --
+0 1.0 1.0 1.0
+1 Infinity -Infinity NaN
+2 2.0 2.0 2.0
+
+-- !float_false_else --
+0 1.0 1.0 1.0
+1 2.0 2.0 2.0
+2 Infinity -Infinity NaN
+
+-- !float_false_nullable --
+0 1.0
+1 Infinity
+2 \N
+
+-- !float_false_batches --
+1.0 1
+2.0 4097
+Infinity 1
+
+-- !double_true_then --
+0 1 1 1
+1 Infinity -Infinity NaN
+2 2 2 2
+
+-- !double_true_else --
+0 1 1 1
+1 2 2 2
+2 Infinity -Infinity NaN
+
+-- !double_true_nullable --
+0 1
+1 Infinity
+2 \N
+
+-- !double_true_batches --
+1 1
+2 4097
+Infinity 1
+
+-- !float_true_then --
+0 1.0 1.0 1.0
+1 Infinity -Infinity NaN
+2 2.0 2.0 2.0
+
+-- !float_true_else --
+0 1.0 1.0 1.0
+1 2.0 2.0 2.0
+2 Infinity -Infinity NaN
+
+-- !float_true_nullable --
+0 1.0
+1 Infinity
+2 \N
+
+-- !float_true_batches --
+1.0 1
+2.0 4097
+Infinity 1
+
diff --git
a/regression-test/suites/query_p0/sql_functions/conditional_functions/test_case_float_nonfinite.groovy
b/regression-test/suites/query_p0/sql_functions/conditional_functions/test_case_float_nonfinite.groovy
new file mode 100644
index 00000000000..4df2bac1124
--- /dev/null
+++
b/regression-test/suites/query_p0/sql_functions/conditional_functions/test_case_float_nonfinite.groovy
@@ -0,0 +1,64 @@
+// 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_case_float_nonfinite") {
+ for (def shortCircuit : [false, true]) {
+ sql "set short_circuit_evaluation = ${shortCircuit}"
+ for (def type : ["double", "float"]) {
+ // Every row overflows from finite operands. Keeping number in the
expression
+ // prevents constant folding and makes unselected branches contain
infinity too.
+ def infinity = "cast((cast(number as double) + cast(1e308 as
double)) * cast(1e308 as double) as ${type})"
+ def nan = "cast((${infinity}) - (${infinity}) as ${type})"
+ "qt_${type}_${shortCircuit}_then" """
+ select number,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then ${infinity} else cast(2 as
${type}) end,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then cast(-(${infinity}) as
${type}) else cast(2 as ${type}) end,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then ${nan} else cast(2 as
${type}) end
+ from numbers("number" = "3") order by number
+ """
+ "qt_${type}_${shortCircuit}_else" """
+ select number,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then cast(2 as ${type}) else
${infinity} end,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then cast(2 as ${type}) else
cast(-(${infinity}) as ${type}) end,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then cast(2 as ${type}) else
${nan} end
+ from numbers("number" = "3") order by number
+ """
+ "qt_${type}_${shortCircuit}_nullable" """
+ select number,
+ case when number = 0 then cast(1 as ${type})
+ when number = 1 then ${infinity} end
+ from numbers("number" = "3") order by number
+ """
+ // The overflowing branch is evaluated in the first batch. Finite
rows must
+ // have the same result there and in the tail batch where it is
never selected.
+ "qt_${type}_${shortCircuit}_batches" """
+ select result, count(*) from (
+ select case when number = 0 then cast(1 as ${type})
+ when number = 1 then ${infinity}
+ else cast(2 as ${type}) end as result
+ from numbers("number" = "4099")
+ ) t group by result order by result
+ """
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
