This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 36e04f9120b branch-4.1: [fix](serde) Stop strict-mode string casts
gluing NULL rows onto the next value #66952 (#67194)
36e04f9120b is described below
commit 36e04f9120ba2311595ce1d163639f0b9fdedf15
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Aug 28 01:27:37 2026 +0800
branch-4.1: [fix](serde) Stop strict-mode string casts gluing NULL rows
onto the next value #66952 (#67194)
Cherry-picked from #66952
Co-authored-by: Jason Woods <[email protected]>
---
.../data_type_serde/data_type_decimal_serde.cpp | 16 +--
.../data_type_serde/data_type_number_serde.cpp | 14 +--
be/test/core/column/column_nullable_test.cpp | 29 ++++++
...l_from_string_strict_batch_null_offset_test.cpp | 98 ++++++++++++++++++
...e_from_string_strict_batch_null_offset_test.cpp | 99 ++++++++++++++++++
.../cast/test_cast_string_to_number_with_nulls.out | 29 ++++++
.../test_cast_string_to_number_with_nulls.groovy | 112 +++++++++++++++++++++
7 files changed, 372 insertions(+), 25 deletions(-)
diff --git a/be/src/core/data_type_serde/data_type_decimal_serde.cpp
b/be/src/core/data_type_serde/data_type_decimal_serde.cpp
index 97db07aa57d..65ae2361d6e 100644
--- a/be/src/core/data_type_serde/data_type_decimal_serde.cpp
+++ b/be/src/core/data_type_serde/data_type_decimal_serde.cpp
@@ -571,12 +571,8 @@ Status
DataTypeDecimalSerDe<T>::from_string_strict_mode_batch(
const auto row = str.size();
column.resize(row);
- const ColumnString::Chars* chars = &str.get_chars();
- const IColumn::Offsets* offsets = &str.get_offsets();
-
auto& column_to = assert_cast<ColumnType&>(column);
auto& vec_to = column_to.get_data();
- size_t current_offset = 0;
auto arg_precision = static_cast<UInt32>(precision);
auto arg_scale = static_cast<UInt32>(scale);
CastParameters params;
@@ -585,16 +581,10 @@ Status
DataTypeDecimalSerDe<T>::from_string_strict_mode_batch(
if (null_map && null_map[i]) {
continue;
}
- size_t next_offset = (*offsets)[i];
- size_t string_size = next_offset - current_offset;
-
- if (!CastToDecimal::from_string(StringRef(&(*chars)[current_offset],
string_size),
- vec_to[i], arg_precision, arg_scale,
params)) {
- return Status::InvalidArgument(
- "parse number fail, string: '{}'",
- std::string((char*)&(*chars)[current_offset],
string_size));
+ const auto str_ref = str.get_data_at(i);
+ if (!CastToDecimal::from_string(str_ref, vec_to[i], arg_precision,
arg_scale, params)) {
+ return Status::InvalidArgument("parse number fail, string: '{}'",
str_ref.to_string());
}
- current_offset = next_offset;
}
return Status::OK();
}
diff --git a/be/src/core/data_type_serde/data_type_number_serde.cpp
b/be/src/core/data_type_serde/data_type_number_serde.cpp
index ba69f190e31..344fcb361e0 100644
--- a/be/src/core/data_type_serde/data_type_number_serde.cpp
+++ b/be/src/core/data_type_serde/data_type_number_serde.cpp
@@ -1537,10 +1537,6 @@ Status
DataTypeNumberSerDe<T>::from_string_strict_mode_batch(
const auto size = str.size();
column.resize(size);
- size_t current_offset = 0;
- const ColumnString::Chars* chars = &str.get_chars();
- const IColumn::Offsets* offsets = &str.get_offsets();
-
auto& column_to = assert_cast<ColumnType&>(column);
auto& vec_to = column_to.get_data();
CastParameters params;
@@ -1549,16 +1545,10 @@ Status
DataTypeNumberSerDe<T>::from_string_strict_mode_batch(
if (null_map && null_map[i]) {
continue;
}
- size_t next_offset = (*offsets)[i];
- size_t string_size = next_offset - current_offset;
-
- StringRef str_ref(&(*chars)[current_offset], string_size);
+ const auto str_ref = str.get_data_at(i);
if (!try_parse_impl<T, true>(vec_to[i], str_ref, params)) {
- return Status::InvalidArgument(
- "parse number fail, string: '{}'",
- std::string((char*)&(*chars)[current_offset],
string_size));
+ return Status::InvalidArgument("parse number fail, string: '{}'",
str_ref.to_string());
}
- current_offset = next_offset;
}
return Status::OK();
}
diff --git a/be/test/core/column/column_nullable_test.cpp
b/be/test/core/column/column_nullable_test.cpp
index 088e2071795..a1007b12f6c 100644
--- a/be/test/core/column/column_nullable_test.cpp
+++ b/be/test/core/column/column_nullable_test.cpp
@@ -232,6 +232,35 @@ TEST(ColumnNullableTest,
UpdateCrc32cBatchDoesNotMutateSharedNestedColumn) {
EXPECT_EQ(mutable_block->rows(), block.rows());
}
+TEST(ColumnNullableTest, ApplyNullMapAfterMutateLeavesSharedSourceUnchanged) {
+ auto nested_mut = ColumnString::create();
+ nested_mut->insert_data("-", 1);
+ nested_mut->insert_data("-", 1);
+ nested_mut->insert_data("2628", 4);
+ ColumnPtr nested = std::move(nested_mut);
+
+ auto null_map_mut = ColumnUInt8::create();
+ null_map_mut->insert_value(0);
+ null_map_mut->insert_value(0);
+ null_map_mut->insert_value(0);
+ ColumnPtr null_map = std::move(null_map_mut);
+
+ ColumnPtr src = ColumnNullable::create(nested, null_map);
+ ColumnPtr held = src; // second owner, as if() holds arg_else.column
alongside the block's copy
+
+ auto cond = ColumnUInt8::create();
+ cond->insert_value(1);
+ cond->insert_value(1);
+ cond->insert_value(0);
+
+ // mirrors if(cond, NULL, nullable_col): mutate() the shared column, then
OR in the null map
+ auto mutated = (*std::move(held)).mutate();
+ assert_cast<ColumnNullable&>(*mutated).apply_null_map(*cond);
+
+ EXPECT_FALSE(assert_cast<const ColumnNullable&>(*src).has_null());
+ EXPECT_TRUE(assert_cast<const ColumnNullable&>(*mutated).has_null());
+}
+
TEST(ColumnNullableTest,
UpdateCrc32cBatchHashesNullAsNestedDefaultForWideType) {
auto nested_mut = ColumnInt64::create();
nested_mut->insert_value(10);
diff --git
a/be/test/core/data_type_serde/data_type_serde_decimal_from_string_strict_batch_null_offset_test.cpp
b/be/test/core/data_type_serde/data_type_serde_decimal_from_string_strict_batch_null_offset_test.cpp
new file mode 100644
index 00000000000..9df45f78035
--- /dev/null
+++
b/be/test/core/data_type_serde/data_type_serde_decimal_from_string_strict_batch_null_offset_test.cpp
@@ -0,0 +1,98 @@
+// 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 <array>
+
+#include "core/assert_cast.h"
+#include "core/column/column_decimal.h"
+#include "core/column/column_string.h"
+#include "core/data_type/data_type_string.h"
+#include "core/data_type_serde/data_type_decimal_serde.h"
+#include "testutil/column_helper.h"
+
+namespace doris {
+
+// Same defect as DataTypeNumberSerDeFromStringStrictModeBatchTest, in the
sibling decimal serde:
+// a row's own bytes are chars[offsets[i-1]..offsets[i]), and an
externally-marked null row must
+// not desync the cursor used to read every later row's bytes. Scale 0 so
parsed values compare
+// directly against the raw stored integer.
+TEST(DataTypeDecimalSerDeFromStringStrictModeBatchTest,
SkipsNullRowsWithoutDesyncingOffsets) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"-", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 2> null_map {1, 0};
+
+ auto column_to = ColumnDecimal32::create(0, 0);
+ DataTypeDecimalSerDe<TYPE_DECIMAL32> serde(9, 0);
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(column_to->size(), 2);
+ EXPECT_EQ(column_to->get_data()[1].value, 2628);
+}
+
+TEST(DataTypeDecimalSerDeFromStringStrictModeBatchTest,
SkipsMultipleConsecutiveNullRows) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"-", "-", "-",
"-", "-", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 6> null_map {1, 1, 1, 1, 1, 0};
+
+ auto column_to = ColumnDecimal32::create(0, 0);
+ DataTypeDecimalSerDe<TYPE_DECIMAL32> serde(9, 0);
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(column_to->size(), 6);
+ EXPECT_EQ(column_to->get_data()[5].value, 2628);
+}
+
+// With no preceding null rows an invalid value is still rejected, and the
error message
+// quotes exactly that row's own bytes.
+TEST(DataTypeDecimalSerDeFromStringStrictModeBatchTest,
RejectsInvalidValueWithoutPrecedingNulls) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"-", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 2> null_map {0, 0};
+
+ auto column_to = ColumnDecimal32::create(0, 0);
+ DataTypeDecimalSerDe<TYPE_DECIMAL32> serde(9, 0);
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_FALSE(st.ok());
+ EXPECT_NE(st.to_string().find("parse number fail, string: '-'"),
std::string::npos) << st;
+}
+
+// A null row whose nested slice is empty, as insert_default() produces, is
skipped the same
+// way as one that still carries bytes.
+TEST(DataTypeDecimalSerDeFromStringStrictModeBatchTest,
SkipsNullRowWithEmptyNestedSlice) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 2> null_map {1, 0};
+
+ auto column_to = ColumnDecimal32::create(0, 0);
+ DataTypeDecimalSerDe<TYPE_DECIMAL32> serde(9, 0);
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(column_to->size(), 2);
+ EXPECT_EQ(column_to->get_data()[1].value, 2628);
+}
+
+} // namespace doris
diff --git
a/be/test/core/data_type_serde/data_type_serde_from_string_strict_batch_null_offset_test.cpp
b/be/test/core/data_type_serde/data_type_serde_from_string_strict_batch_null_offset_test.cpp
new file mode 100644
index 00000000000..4bd73b54641
--- /dev/null
+++
b/be/test/core/data_type_serde/data_type_serde_from_string_strict_batch_null_offset_test.cpp
@@ -0,0 +1,99 @@
+// 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 <array>
+
+#include "core/assert_cast.h"
+#include "core/column/column_string.h"
+#include "core/column/column_vector.h"
+#include "core/data_type/data_type_string.h"
+#include "core/data_type_serde/data_type_number_serde.h"
+#include "testutil/column_helper.h"
+
+namespace doris {
+
+// A row's own bytes are chars[offsets[i-1]..offsets[i]); a caller may mark a
row null via an
+// externally-supplied null_map without the underlying ColumnString slice
being empty (this is
+// exactly what if(cond, NULL, nullable_col) produces via
ColumnNullable::apply_null_map).
+// from_string_strict_mode_batch must read every row from its own offsets
regardless of the
+// preceding rows' null-map bits.
+TEST(DataTypeNumberSerDeFromStringStrictModeBatchTest,
SkipsNullRowsWithoutDesyncingOffsets) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"-", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 2> null_map {1, 0};
+
+ auto column_to = ColumnInt64::create();
+ DataTypeNumberSerDe<TYPE_BIGINT> serde;
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(column_to->size(), 2);
+ EXPECT_EQ(column_to->get_data()[1], 2628);
+}
+
+TEST(DataTypeNumberSerDeFromStringStrictModeBatchTest,
SkipsMultipleConsecutiveNullRows) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"-", "-", "-",
"-", "-", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 6> null_map {1, 1, 1, 1, 1, 0};
+
+ auto column_to = ColumnInt64::create();
+ DataTypeNumberSerDe<TYPE_BIGINT> serde;
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(column_to->size(), 6);
+ EXPECT_EQ(column_to->get_data()[5], 2628);
+}
+
+// With no preceding null rows an invalid value is still rejected, and the
error message
+// quotes exactly that row's own bytes.
+TEST(DataTypeNumberSerDeFromStringStrictModeBatchTest,
RejectsInvalidValueWithoutPrecedingNulls) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"-", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 2> null_map {0, 0};
+
+ auto column_to = ColumnInt64::create();
+ DataTypeNumberSerDe<TYPE_BIGINT> serde;
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_FALSE(st.ok());
+ EXPECT_NE(st.to_string().find("parse number fail, string: '-'"),
std::string::npos) << st;
+}
+
+// A null row whose nested slice is empty, as insert_default() produces, is
skipped the same
+// way as one that still carries bytes.
+TEST(DataTypeNumberSerDeFromStringStrictModeBatchTest,
SkipsNullRowWithEmptyNestedSlice) {
+ auto str_col = ColumnHelper::create_column<DataTypeString>({"", "2628"});
+ const auto& col_str = assert_cast<const ColumnString&>(*str_col);
+ constexpr std::array<NullMap::value_type, 2> null_map {1, 0};
+
+ auto column_to = ColumnInt64::create();
+ DataTypeNumberSerDe<TYPE_BIGINT> serde;
+ DataTypeSerDe::FormatOptions options;
+
+ Status st = serde.from_string_strict_mode_batch(col_str, *column_to,
options, null_map.data());
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(column_to->size(), 2);
+ EXPECT_EQ(column_to->get_data()[1], 2628);
+}
+
+} // namespace doris
diff --git
a/regression-test/data/function_p0/cast/test_cast_string_to_number_with_nulls.out
b/regression-test/data/function_p0/cast/test_cast_string_to_number_with_nulls.out
new file mode 100644
index 00000000000..c2955478343
--- /dev/null
+++
b/regression-test/data/function_p0/cast/test_cast_string_to_number_with_nulls.out
@@ -0,0 +1,29 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !bigint_null_row_then_value --
+1 \N
+2 2628
+
+-- !decimal_null_row_then_value --
+1 \N
+2 2628
+
+-- !bigint_null_run_then_value --
+1 \N
+2 \N
+3 \N
+4 \N
+5 \N
+6 2628
+
+-- !decimal_null_run_then_value --
+1 \N
+2 \N
+3 \N
+4 \N
+5 \N
+6 2628
+
+-- !insert_select_null_row_then_value --
+1 \N
+2 2628
+
diff --git
a/regression-test/suites/function_p0/cast/test_cast_string_to_number_with_nulls.groovy
b/regression-test/suites/function_p0/cast/test_cast_string_to_number_with_nulls.groovy
new file mode 100644
index 00000000000..1430f266cac
--- /dev/null
+++
b/regression-test/suites/function_p0/cast/test_cast_string_to_number_with_nulls.groovy
@@ -0,0 +1,112 @@
+// 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_cast_string_to_number_with_nulls") {
+ sql "drop table if exists test_cast_string_to_number_with_nulls_bigint;"
+ sql """
+ create table test_cast_string_to_number_with_nulls_bigint (
+ id int,
+ s varchar(16)
+ ) duplicate key(id)
+ distributed by hash(id) buckets 1
+ properties("replication_num" = "1");
+ """
+
+ sql "drop table if exists test_cast_string_to_number_with_nulls_decimal;"
+ sql """
+ create table test_cast_string_to_number_with_nulls_decimal (
+ id int,
+ s varchar(16)
+ ) duplicate key(id)
+ distributed by hash(id) buckets 1
+ properties("replication_num" = "1");
+ """
+
+ sql "set enable_strict_cast=true;"
+
+ // if(s = '-', NULL, s) marks rows null without clearing their bytes from
the nested string
+ // column, so a strict cast must still read every row from its own offsets.
+ sql "truncate table test_cast_string_to_number_with_nulls_bigint;"
+ sql "insert into test_cast_string_to_number_with_nulls_bigint values (1,
'-'), (2, '2628');"
+ qt_bigint_null_row_then_value """
+ select id, cast(if(s = '-', NULL, s) as bigint) as v
+ from test_cast_string_to_number_with_nulls_bigint order by id;
+ """
+
+ sql "truncate table test_cast_string_to_number_with_nulls_decimal;"
+ sql "insert into test_cast_string_to_number_with_nulls_decimal values (1,
'-'), (2, '2628');"
+ qt_decimal_null_row_then_value """
+ select id, cast(if(s = '-', NULL, s) as decimal(10,0)) as v
+ from test_cast_string_to_number_with_nulls_decimal order by id;
+ """
+
+ // A run of null rows must not accumulate onto the next value either.
+ sql "truncate table test_cast_string_to_number_with_nulls_bigint;"
+ sql """
+ insert into test_cast_string_to_number_with_nulls_bigint values
+ (1, '-'), (2, '-'), (3, '-'), (4, '-'), (5, '-'), (6, '2628');
+ """
+ qt_bigint_null_run_then_value """
+ select id, cast(if(s = '-', NULL, s) as bigint) as v
+ from test_cast_string_to_number_with_nulls_bigint order by id;
+ """
+
+ sql "truncate table test_cast_string_to_number_with_nulls_decimal;"
+ sql """
+ insert into test_cast_string_to_number_with_nulls_decimal values
+ (1, '-'), (2, '-'), (3, '-'), (4, '-'), (5, '-'), (6, '2628');
+ """
+ qt_decimal_null_run_then_value """
+ select id, cast(if(s = '-', NULL, s) as decimal(10,0)) as v
+ from test_cast_string_to_number_with_nulls_decimal order by id;
+ """
+
+ // A non-null row that is not a number is still rejected, quoting that
row's own bytes.
+ sql "truncate table test_cast_string_to_number_with_nulls_bigint;"
+ sql "insert into test_cast_string_to_number_with_nulls_bigint values (1,
'-'), (2, '2628');"
+ test {
+ sql """
+ select id, cast(s as bigint) as v
+ from test_cast_string_to_number_with_nulls_bigint order by id;
+ """
+ exception "parse number fail, string: '-'"
+ }
+
+ // INSERT ... SELECT takes the strict path from enable_insert_strict,
independently of the
+ // session-scoped enable_strict_cast used by the queries above.
+ sql "set enable_strict_cast=false;"
+ sql "truncate table test_cast_string_to_number_with_nulls_bigint;"
+ sql "insert into test_cast_string_to_number_with_nulls_bigint values (1,
'-'), (2, '2628');"
+
+ sql "drop table if exists test_cast_string_to_number_with_nulls_target;"
+ sql """
+ create table test_cast_string_to_number_with_nulls_target (
+ id int,
+ v bigint
+ ) duplicate key(id)
+ distributed by hash(id) buckets 1
+ properties("replication_num" = "1");
+ """
+ sql """
+ insert into test_cast_string_to_number_with_nulls_target
+ select id, cast(if(s = '-', NULL, s) as bigint)
+ from test_cast_string_to_number_with_nulls_bigint;
+ """
+ qt_insert_select_null_row_then_value """
+ select id, v from test_cast_string_to_number_with_nulls_target order
by id;
+ """
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
