This is an automated email from the ASF dual-hosted git repository.
Mryange 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 2d053c83252 [fix](function) Avoid validating NULL payloads in scalar
functions (#68886)
2d053c83252 is described below
commit 2d053c8325215695e9e924d3e4d9841c7dbda5db
Author: Mryange <[email protected]>
AuthorDate: Mon Oct 12 18:25:47 2026 +0800
[fix](function) Avoid validating NULL payloads in scalar functions (#68886)
### What problem does this PR solve?
Issue Number: N/A
Related PR: #68332
Problem Summary:
Debug NULL-payload injection exposed query failures in `format_round`,
array construction, and mixed TIMESTAMP_NS/TIMESTAMPTZ comparisons. Root
cause: these functions consumed the arbitrary nested values of NULL rows
and performed range validation before NULL semantics were applied.
Enable the existing NULL-payload default replacement for `format_round`.
For `array_with_constant` and `array_repeat`, check the length null map
before reading the payload, preserving the existing empty-array result
for NULL lengths. For mixed temporal comparisons, compare CCTZ local
civil fields directly without rejecting NULL payloads or copying
nullable input columns; retain signed years across date boundaries and
nanosecond comparison precision.
This PR contains only the three function fixes, based on master. It does
not include the debug injection changes from #68332.
### Release note
Fix query errors caused by arbitrary NULL-row payloads in formatting,
array construction, and mixed temporal comparisons.
### Check List (For Author)
- Test
- [x] Regression test
- [ ] Unit Test
- [x] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
Before migration to master, the identical patch was validated on the
NULL-payload injection branch: ASAN BE build and changed-line clang-tidy
passed. Six suites passed: `test_format_round_decimal`,
`test_format_round_decimal256`, `fix_array_type_and_lambda_func`,
`test_timestamp_ns_mixed_datetime_expressions`,
`test_array_functions_array_with_const`, and `nereids_scalar_fn_Array`.
Supplemental SQL checked mixed temporal `nullif`, NULL-safe equality,
NULL inputs, and empty arrays for NULL lengths.
`test_pythonudf_module_advanced` was attempted but blocked by disabled
Python UDF support. No BE unit tests were run. After migration,
clang-format 16, header hygiene, and `git diff --check` passed; the
master-based branch was not rebuilt.
- Behavior changed:
- [ ] No.
- [x] Yes. NULL-row payloads no longer trigger these validation errors;
mixed temporal comparisons can compare local civil dates outside the
DATETIMEV2 range.
- 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
---
.../array/function_array_with_constant.cpp | 17 +++++--
be/src/exprs/function/function_string_format.h | 2 +
be/src/exprs/function/functions_comparison.h | 53 +++++++++++-----------
.../function/cast/cast_to_timestamptz_test.cpp | 15 ++++--
4 files changed, 53 insertions(+), 34 deletions(-)
diff --git a/be/src/exprs/function/array/function_array_with_constant.cpp
b/be/src/exprs/function/array/function_array_with_constant.cpp
index f7c689929aa..56e55acded6 100644
--- a/be/src/exprs/function/array/function_array_with_constant.cpp
+++ b/be/src/exprs/function/array/function_array_with_constant.cpp
@@ -73,11 +73,14 @@ public:
Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const
override {
- auto num =
block.get_by_position(arguments[FunctionType::param_num_idx])
- .column->convert_to_full_column_if_const();
- num = is_column_nullable(*num)
- ? assert_cast<const
ColumnNullable*>(num.get())->get_nested_column_ptr()
- : num;
+ const auto num_column =
block.get_by_position(arguments[FunctionType::param_num_idx])
+
.column->convert_to_full_column_if_const();
+ auto num = num_column;
+ const NullMap* num_null_map = nullptr;
+ if (const auto* nullable_num =
check_and_get_column<ColumnNullable>(*num)) {
+ num_null_map = &nullable_num->get_null_map_data();
+ num = nullable_num->get_nested_column_ptr();
+ }
auto value =
block.get_by_position(arguments[FunctionType::param_val_idx])
.column->convert_to_full_column_if_const();
auto offsets_col = ColumnOffset64::create();
@@ -88,6 +91,10 @@ public:
array_sizes.reserve(input_rows_count);
// The array size will never gt int max value.
for (int i = 0; i < input_rows_count; ++i) {
+ if (num_null_map && (*num_null_map)[i]) {
+ offsets.push_back(offset);
+ continue;
+ }
auto array_size = num->get_int(i);
if (UNLIKELY(array_size < 0) || UNLIKELY(array_size >
max_array_size_as_field)) {
return Status::InvalidArgument("Array size should in range(0,
{}) in function: {}",
diff --git a/be/src/exprs/function/function_string_format.h
b/be/src/exprs/function/function_string_format.h
index e3e58d5acdc..04d596df10e 100644
--- a/be/src/exprs/function/function_string_format.h
+++ b/be/src/exprs/function/function_string_format.h
@@ -511,6 +511,8 @@ public:
static FunctionPtr create() { return
std::make_shared<FunctionStringFormatRound>(); }
String get_name() const override { return name; }
+ bool need_replace_null_data_to_default() const override { return true; }
+
DataTypePtr get_return_type_impl(const DataTypes& arguments) const
override {
if (arguments.size() != 2) {
throw doris::Exception(ErrorCode::INVALID_ARGUMENT,
diff --git a/be/src/exprs/function/functions_comparison.h
b/be/src/exprs/function/functions_comparison.h
index 2d3e976a705..0f21c80c140 100644
--- a/be/src/exprs/function/functions_comparison.h
+++ b/be/src/exprs/function/functions_comparison.h
@@ -639,7 +639,8 @@ private:
template <typename TemporalValue>
static int compare_timestamp_ns_with_temporal(const TimeStampNsValue&
timestamp,
- const TemporalValue&
temporal) {
+ const TemporalValue&
temporal,
+ uint32_t
temporal_nanosecond) {
const auto timestamp_datetime = timestamp.to_datetime();
const auto compare_part = [](auto left, auto right) {
if (left < right) {
@@ -650,7 +651,8 @@ private:
}
return 0;
};
- int comparison = compare_part(timestamp_datetime.year(),
temporal.year());
+ int comparison =
compare_part(static_cast<int64_t>(timestamp_datetime.year()),
+ static_cast<int64_t>(temporal.year()));
if (comparison == 0) {
comparison = compare_part(timestamp_datetime.month(),
temporal.month());
}
@@ -667,7 +669,7 @@ private:
comparison = compare_part(timestamp_datetime.second(),
temporal.second());
}
if (comparison == 0) {
- comparison = compare_part(timestamp.nanosecond(),
temporal.microsecond() * 1000);
+ comparison = compare_part(timestamp.nanosecond(),
temporal_nanosecond);
}
return comparison;
}
@@ -676,7 +678,6 @@ private:
Status execute_timestamp_ns_temporal(FunctionContext* context, Block&
block, uint32_t result,
const ColumnPtr& left_column,
const ColumnPtr& right_column,
- const DataTypePtr& temporal_type,
size_t input_rows_count) const {
static_assert(TemporalPType == TYPE_DATE || TemporalPType ==
TYPE_DATEV2 ||
TemporalPType == TYPE_DATETIME || TemporalPType ==
TYPE_DATETIMEV2 ||
@@ -699,18 +700,19 @@ private:
reinterpret_cast<const
TemporalValue&>(temporals[temporal_is_const ? 0 : row]);
int comparison;
if constexpr (TemporalPType == TYPE_TIMESTAMPTZ) {
- DateV2Value<DateTimeV2ValueType> local_datetime;
- const auto scale = temporal_type->get_scale();
- if (!temporal.to_datetime(local_datetime,
context->state()->timezone_obj(), scale,
- scale)) [[unlikely]] {
- // Preserve the comparison error instead of re-entering
the failing formatter.
- return Status::InvalidArgument(
- "can not compare timestamptz {} with TIMESTAMP_NS
in timezone {}",
- temporal.utc_dt().to_string(scale),
context->state()->timezone());
- }
- comparison = compare_timestamp_ns_with_temporal(timestamp,
local_datetime);
+ const auto utc_datetime = temporal.utc_dt();
+ const cctz::civil_second utc_civil(utc_datetime.year(),
utc_datetime.month(),
+ utc_datetime.day(),
utc_datetime.hour(),
+ utc_datetime.minute(),
utc_datetime.second());
+ const auto utc_time = cctz::convert(utc_civil,
cctz::utc_time_zone());
+ const auto local_civil = cctz::convert(utc_time,
context->state()->timezone_obj());
+ // Keep the signed civil year across date boundaries. NULL
payloads are
+ // normalized without validation; the nullable framework masks
their results.
+ comparison = compare_timestamp_ns_with_temporal(timestamp,
local_civil,
+
utc_datetime.microsecond() * 1000);
} else {
- comparison = compare_timestamp_ns_with_temporal(timestamp,
temporal);
+ comparison = compare_timestamp_ns_with_temporal(timestamp,
temporal,
+
temporal.microsecond() * 1000);
}
if constexpr (!timestamp_ns_on_left) {
comparison = -comparison;
@@ -855,17 +857,16 @@ public:
const DataTypePtr& left_type = col_with_type_and_name_left.type;
const DataTypePtr& right_type = col_with_type_and_name_right.type;
-#define EXECUTE_TIMESTAMP_NS_TEMPORAL_COMPARISON(TYPE)
\
- if (left_type->get_primitive_type() == TYPE_TIMESTAMP_NS &&
\
- right_type->get_primitive_type() == TYPE) {
\
- return execute_timestamp_ns_temporal<TYPE, true>(context, block,
result, col_left_ptr, \
- col_right_ptr,
right_type, \
- input_rows_count);
\
- }
\
- if (left_type->get_primitive_type() == TYPE &&
\
- right_type->get_primitive_type() == TYPE_TIMESTAMP_NS) {
\
- return execute_timestamp_ns_temporal<TYPE, false>(
\
- context, block, result, col_left_ptr, col_right_ptr,
left_type, input_rows_count); \
+#define EXECUTE_TIMESTAMP_NS_TEMPORAL_COMPARISON(TYPE)
\
+ if (left_type->get_primitive_type() == TYPE_TIMESTAMP_NS &&
\
+ right_type->get_primitive_type() == TYPE) {
\
+ return execute_timestamp_ns_temporal<TYPE, true>(context, block,
result, col_left_ptr, \
+ col_right_ptr,
input_rows_count); \
+ }
\
+ if (left_type->get_primitive_type() == TYPE &&
\
+ right_type->get_primitive_type() == TYPE_TIMESTAMP_NS) {
\
+ return execute_timestamp_ns_temporal<TYPE, false>(context, block,
result, col_left_ptr, \
+ col_right_ptr,
input_rows_count); \
}
EXECUTE_TIMESTAMP_NS_TEMPORAL_COMPARISON(TYPE_DATE)
diff --git a/be/test/exprs/function/cast/cast_to_timestamptz_test.cpp
b/be/test/exprs/function/cast/cast_to_timestamptz_test.cpp
index ee925e95c53..a6174839c52 100644
--- a/be/test/exprs/function/cast/cast_to_timestamptz_test.cpp
+++ b/be/test/exprs/function/cast/cast_to_timestamptz_test.cpp
@@ -529,11 +529,20 @@ TEST_F(CastTimeStampTzTest,
timestamp_ns_local_year_overflow_returns_status) {
block.get_by_position(1).column = std::move(ns_column);
block.insert({nullptr, std::make_shared<DataTypeUInt8>(),
"comparison"});
FunctionComparison<EqualsOp, NameEquals> equals;
- // Error reporting must not try to display the unrepresentable
session-local year.
+ FunctionComparison<LessOp, NameLess> less;
+ // Comparisons use signed civil years even outside the cast's
representable range.
for (const ColumnNumbers& inputs : {ColumnNumbers {0, 1},
ColumnNumbers {1, 0}}) {
ASSERT_NO_THROW(status = equals.execute_impl(&context, block,
inputs, 2, 1));
- EXPECT_EQ(status.code(), ErrorCode::INVALID_ARGUMENT);
- EXPECT_NE(status.to_string().find("can not compare timestamptz"),
std::string::npos);
+ ASSERT_TRUE(status.ok()) << status;
+ EXPECT_EQ(assert_cast<const
ColumnUInt8&>(*block.get_by_position(2).column)
+ .get_element(0),
+ 0);
+
+ ASSERT_NO_THROW(status = less.execute_impl(&context, block,
inputs, 2, 1));
+ ASSERT_TRUE(status.ok()) << status;
+ EXPECT_EQ(assert_cast<const
ColumnUInt8&>(*block.get_by_position(2).column)
+ .get_element(0),
+ inputs[0] == 0 ? !upper : upper);
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
