This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-4.2 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 66f60c1da509806ca2fa989259df65f4a38141b0 Author: Dongyang Li <[email protected]> AuthorDate: Sat Oct 10 09:25:51 2026 +0800 branch-4.1: [fix](expr) Canonicalize logical OR results (#68798) ### What problem does this PR solve? Backport #68308 / #68401 to branch-4.1. The repair is present on branch-4.1.4, but branch-4.1 at c8c0ddca4d4d452bcb236f2499c5fed9a3c68780 still has the same three unnormalized OR paths as 4.1.5-rc01 (7fda449b7afb09c266ce2c4fc738a4d5e46fc458). A nullable Boolean can contain an arbitrary hidden byte. When the other operand makes OR non-null, preserving the bitwise result can expose a value other than 0 or 1. Release regression reproduced incorrect Boolean grouping on the original candidate in both LOCAL and S3 modes. This is an exact backport of fc344110d103312a6bfafb55e936b76f26177b8e, preserving its author and all five changed files: normalize FunctionOr and both compound OR paths, add three BE unit tests, and include the original regression suite and unchanged golden output. No unrelated NULL/CASE implementation changes are included. ### Release note Fix noncanonical logical OR results involving hidden NULL payloads. ### Check List (For Author) - Validation: patch applies cleanly to the stated branch-4.1 base; all five resulting files are byte-identical to the original fixed commit; git diff --check passes. - Runtime validation: this new backport has not run BE unit tests or live regression yet. Required validation is FunctionsLogicalTest.* and test_json_extract_bool_null_payload in LOCAL/S3, with both short-circuit settings, against a binary built from this PR's exact SHA. - The existing candidate failure remains a release blocker. This PR does not replace the candidate artifact or establish a passing release result. - Duplicate check: currently open branch-4.1 PR file lists and matching repair references were checked; no equivalent open backport was found. Co-authored-by: Mingyu Chen (Rayner) <[email protected]> --- be/src/exprs/function/functions_logical.h | 4 +- be/src/exprs/vcompound_pred.h | 7 +- be/test/exprs/function/functions_logical_test.cpp | 143 +++++++++++++++++++ .../test_json_extract_bool_null_payload.out | 41 ++++++ .../test_json_extract_bool_null_payload.groovy | 153 +++++++++++++++++++++ 5 files changed, 345 insertions(+), 3 deletions(-) diff --git a/be/src/exprs/function/functions_logical.h b/be/src/exprs/function/functions_logical.h index 9b684376ac4..783e661422e 100644 --- a/be/src/exprs/function/functions_logical.h +++ b/be/src/exprs/function/functions_logical.h @@ -84,7 +84,9 @@ struct AndImpl { struct OrImpl { using ResultType = UInt8; - static inline constexpr ResultType apply(UInt8 a, UInt8 b) { return a | b; } + // A NULL row may carry an arbitrary nested byte. Canonicalize logical OR to 0 or 1 so that + // such a byte cannot become visible when the other operand makes the result non-NULL. + static inline constexpr ResultType apply(UInt8 a, UInt8 b) { return (a | b) != 0; } static inline constexpr ResultType apply_null(UInt8 a, UInt8 l_null, UInt8 b, UInt8 r_null) { // (<> || true) is true, (false || NULL) is NULL return (l_null & r_null) | (r_null & (r_null ^ a)) | (l_null & (l_null ^ b)); diff --git a/be/src/exprs/vcompound_pred.h b/be/src/exprs/vcompound_pred.h index 77b84a5733e..9fe4386fd30 100644 --- a/be/src/exprs/vcompound_pred.h +++ b/be/src/exprs/vcompound_pred.h @@ -625,7 +625,8 @@ private: if constexpr (is_and) { lhs[i] &= rhs[i]; } else { - lhs[i] |= rhs[i]; + // Logical OR must produce a canonical Boolean instead of preserving input bits. + lhs[i] = (lhs[i] | rhs[i]) != 0; } } } @@ -648,7 +649,9 @@ private: res_data[i] = lhs_data[i] & rhs_data[i]; } else { res_null[i] = apply_or_null(lhs_data[i], lhs_null[i], rhs_data[i], rhs_null[i]); - res_data[i] = lhs_data[i] | rhs_data[i]; + // A NULL row may carry an arbitrary nested byte. If the result remains NULL the + // byte is ignored; otherwise normalization prevents it from becoming visible. + res_data[i] = (lhs_data[i] | rhs_data[i]) != 0; } } } diff --git a/be/test/exprs/function/functions_logical_test.cpp b/be/test/exprs/function/functions_logical_test.cpp new file mode 100644 index 00000000000..90d7cea69c4 --- /dev/null +++ b/be/test/exprs/function/functions_logical_test.cpp @@ -0,0 +1,143 @@ +// 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/function/functions_logical.h" + +#include <gen_cpp/Exprs_types.h> +#include <gtest/gtest.h> + +#include <string> +#include <utility> +#include <vector> + +#include "core/block/block.h" +#include "core/column/column_const.h" +#include "core/column/column_nullable.h" +#include "core/data_type/data_type_nullable.h" +#include "core/data_type/data_type_number.h" +#include "core/data_type/primitive_type.h" +#include "exprs/vcompound_pred.h" +#include "exprs/vexpr_context.h" +#include "testutil/column_helper.h" + +namespace doris { + +namespace { + +ColumnWithTypeAndName nullable_boolean_column(std::vector<UInt8> data, std::vector<UInt8> null_map, + std::string name) { + return {ColumnHelper::create_nullable_column<DataTypeUInt8>(data, null_map), + make_nullable(std::make_shared<DataTypeUInt8>()), std::move(name)}; +} + +ColumnPtr execute_or(ColumnWithTypeAndName left, ColumnWithTypeAndName right, size_t rows) { + auto result_type = make_nullable(std::make_shared<DataTypeUInt8>()); + Block block({std::move(left), std::move(right), {nullptr, result_type, "result"}}); + auto status = FunctionOr::create()->execute_impl(nullptr, block, {0, 1}, 2, rows); + EXPECT_TRUE(status.ok()) << status.to_string(); + return block.get_by_position(2).column; +} + +class ColumnExpr final : public VExpr { +public: + ColumnExpr(ColumnPtr column, DataTypePtr type) + : VExpr(std::move(type), false), _column(std::move(column)) {} + + bool is_constant() const override { return false; } + + const std::string& expr_name() const override { + static const std::string name = "ColumnExpr"; + return name; + } + + Status execute_column_impl(VExprContext*, const Block*, const Selector*, size_t, + ColumnPtr& result_column) const override { + result_column = _column; + return Status::OK(); + } + +private: + ColumnPtr _column; +}; + +ColumnPtr execute_compound_or(ColumnWithTypeAndName left, ColumnWithTypeAndName right, + size_t rows) { + TExprNode node; + node.__set_type(create_type_desc(TYPE_BOOLEAN)); + node.__set_node_type(TExprNodeType::COMPOUND_PRED); + node.__set_opcode(TExprOpcode::COMPOUND_OR); + node.__set_num_children(2); + node.__set_is_nullable(true); + + auto compound = VCompoundPred::create_shared(node); + compound->add_child(std::make_shared<ColumnExpr>(std::move(left.column), left.type)); + compound->add_child(std::make_shared<ColumnExpr>(std::move(right.column), right.type)); + VExprContext context(compound); + ColumnPtr result; + auto status = compound->execute_column(&context, nullptr, nullptr, rows, result); + EXPECT_TRUE(status.ok()) << status.to_string(); + return result; +} + +void expect_boolean(const ColumnNullable& result, size_t row, UInt8 value) { + EXPECT_FALSE(result.is_null_at(row)); + EXPECT_EQ(assert_cast<const ColumnUInt8&>(result.get_nested_column()).get_data()[row], value); +} + +} // namespace + +TEST(FunctionsLogicalTest, NullableOrIgnoresNullPayload) { + auto result = + execute_or(nullable_boolean_column({65, 0, 65, 1, 65}, {1, 0, 1, 0, 1}, "left"), + nullable_boolean_column({1, 0, 0, 65, 127}, {0, 0, 0, 1, 1}, "right"), 5); + + const auto& nullable_result = assert_cast<const ColumnNullable&>(*result); + expect_boolean(nullable_result, 0, 1); + expect_boolean(nullable_result, 1, 0); + EXPECT_TRUE(nullable_result.is_null_at(2)); + expect_boolean(nullable_result, 3, 1); + EXPECT_TRUE(nullable_result.is_null_at(4)); +} + +TEST(FunctionsLogicalTest, NullableOrWithTrueConstantProducesCanonicalTrue) { + constexpr size_t rows = 4; + auto left = nullable_boolean_column({3, 65, 0, 255}, {1, 1, 1, 1}, "left"); + ColumnWithTypeAndName right { + ColumnConst::create(ColumnHelper::create_column<DataTypeUInt8>({1}), rows), + std::make_shared<DataTypeUInt8>(), "right"}; + + auto result = execute_or(std::move(left), std::move(right), rows); + const auto& nullable_result = assert_cast<const ColumnNullable&>(*result); + for (size_t row = 0; row < rows; ++row) { + expect_boolean(nullable_result, row, 1); + } +} + +TEST(FunctionsLogicalTest, CompoundNullableOrIgnoresNullPayload) { + auto result = execute_compound_or( + nullable_boolean_column({65, 0, 65, 1, 65}, {1, 0, 1, 0, 1}, "left"), + nullable_boolean_column({1, 0, 0, 65, 127}, {0, 0, 0, 1, 1}, "right"), 5); + + const auto& nullable_result = assert_cast<const ColumnNullable&>(*result); + expect_boolean(nullable_result, 0, 1); + expect_boolean(nullable_result, 1, 0); + EXPECT_TRUE(nullable_result.is_null_at(2)); + expect_boolean(nullable_result, 3, 1); + EXPECT_TRUE(nullable_result.is_null_at(4)); +} + +} // namespace doris diff --git a/regression-test/data/query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload.out b/regression-test/data/query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload.out new file mode 100644 index 00000000000..5c37ac4894d --- /dev/null +++ b/regression-test/data/query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload.out @@ -0,0 +1,41 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !or_payload_no_short_circuit -- +\N 66000 +true 34000 + +-- !and_or_payload_no_short_circuit -- +\N 66000 +true 34000 + +-- !case_count_no_short_circuit -- +100000 + +-- !case_branches_no_short_circuit -- +then_a 66000 +then_b 34000 + +-- !or_payload_short_circuit -- +\N 66000 +true 34000 + +-- !and_or_payload_short_circuit -- +\N 66000 +true 34000 + +-- !case_count_short_circuit -- +100000 + +-- !case_branches_short_circuit -- +then_a 66000 +then_b 34000 + +-- !nullable_or_truth_table -- +false false false +false null \N +false true true +null false \N +null null \N +null true true +true false true +true null true +true true true diff --git a/regression-test/suites/query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload.groovy b/regression-test/suites/query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload.groovy new file mode 100644 index 00000000000..7550fe0d6b6 --- /dev/null +++ b/regression-test/suites/query_p0/sql_functions/json_functions/test_json_extract_bool_null_payload.groovy @@ -0,0 +1,153 @@ +// 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_json_extract_bool_null_payload") { + sql "DROP TABLE IF EXISTS test_json_extract_bool_null_payload" + sql """ + CREATE TABLE test_json_extract_bool_null_payload ( + id BIGINT, + j STRING, + a BIGINT, + b BIGINT + ) DUPLICATE KEY(id) + DISTRIBUTED BY HASH(id) BUCKETS 1 + PROPERTIES ("replication_num" = "1") + """ + sql """ + INSERT INTO test_json_extract_bool_null_payload + SELECT number, + CASE WHEN number % 100 = 0 + THEN '{"flag": true}' + ELSE CONCAT('{"n": ', number, '}') + END, + number + CASE number % 3 + WHEN 0 THEN 10 + WHEN 1 THEN -100 + ELSE -200000 + END, + number + FROM numbers("number" = "100000") + """ + + sql "SET enable_sql_cache = false" + sql "SET short_circuit_evaluation = false" + + order_qt_or_payload_no_short_circuit """ + SELECT v, count(*) FROM ( + SELECT JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag') OR (a > b) AS v + FROM test_json_extract_bool_null_payload + ) t + GROUP BY v + ORDER BY v + """ + + order_qt_and_or_payload_no_short_circuit """ + SELECT v, count(*) FROM ( + SELECT (JSON_PARSE_ERROR_TO_NULL(j) IS NOT NULL + AND JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag')) + OR (a > b) AS v + FROM test_json_extract_bool_null_payload + ) t + GROUP BY v + ORDER BY v + """ + + qt_case_count_no_short_circuit """ + SELECT count(*) + FROM test_json_extract_bool_null_payload + WHERE (CASE + WHEN JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag') OR a > b THEN b + WHEN a < b THEN a + END) IS NOT NULL + """ + + order_qt_case_branches_no_short_circuit """ + SELECT CASE + WHEN JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag') OR a > b + THEN 'then_b' + WHEN a < b THEN 'then_a' + ELSE 'else' + END AS branch, + count(*) + FROM test_json_extract_bool_null_payload + GROUP BY branch + ORDER BY branch + """ + + sql "SET short_circuit_evaluation = true" + + order_qt_or_payload_short_circuit """ + SELECT v, count(*) FROM ( + SELECT JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag') OR (a > b) AS v + FROM test_json_extract_bool_null_payload + ) t + GROUP BY v + ORDER BY v + """ + + order_qt_and_or_payload_short_circuit """ + SELECT v, count(*) FROM ( + SELECT (JSON_PARSE_ERROR_TO_NULL(j) IS NOT NULL + AND JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag')) + OR (a > b) AS v + FROM test_json_extract_bool_null_payload + ) t + GROUP BY v + ORDER BY v + """ + + qt_case_count_short_circuit """ + SELECT count(*) + FROM test_json_extract_bool_null_payload + WHERE (CASE + WHEN JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag') OR a > b THEN b + WHEN a < b THEN a + END) IS NOT NULL + """ + + order_qt_case_branches_short_circuit """ + SELECT CASE + WHEN JSON_EXTRACT_BOOL(JSON_PARSE_ERROR_TO_NULL(j), '\$.flag') OR a > b + THEN 'then_b' + WHEN a < b THEN 'then_a' + ELSE 'else' + END AS branch, + count(*) + FROM test_json_extract_bool_null_payload + GROUP BY branch + ORDER BY branch + """ + + order_qt_nullable_or_truth_table """ + SELECT lhs.label, rhs.label, lhs.v OR rhs.v AS result + FROM ( + SELECT 'false' AS label, CAST(false AS BOOLEAN) AS v + UNION ALL + SELECT 'null', CAST(NULL AS BOOLEAN) + UNION ALL + SELECT 'true', CAST(true AS BOOLEAN) + ) lhs + CROSS JOIN ( + SELECT 'false' AS label, CAST(false AS BOOLEAN) AS v + UNION ALL + SELECT 'null', CAST(NULL AS BOOLEAN) + UNION ALL + SELECT 'true', CAST(true AS BOOLEAN) + ) rhs + ORDER BY lhs.label, rhs.label + """ +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
