Skip to content

[fix](function) Preserve trailing zero bytes in string distances #66236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Mryange wants to merge 1 commit into
base: master
Choose a base branch
from
+20 −25
Open

[fix](function) Preserve trailing zero bytes in string distances #66236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 15 additions & 18 deletions be/src/exprs/function/function_hamming_distance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,11 @@ class FunctionHammingDistance : public IFunction {

if (!has_nullable) {
if (left_const) {
RETURN_IF_ERROR(scalar_vector(left_str_col->get_data_at(0).trim_tail_padding_zero(),
*right_str_col, res_data));
RETURN_IF_ERROR(
scalar_vector(left_str_col->get_data_at(0), *right_str_col, res_data));

@github-actions github-actions Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Add a regression that reaches the patched BE paths

These removals are the entire behavior change, but the changed-file list contains no test/output file and the existing distance suite never constructs a trailing NUL. Literal-only cases are insufficient because Nereids StringArithmetic already counts U+0000, so they can pass on the base revision without executing these BE branches. Please add generated regression coverage that fails on the base and passes here, using table-backed binary values to exercise runtime vector/nullable and const-vector paths for Hamming, Levenshtein, and Damerau-Levenshtein (for example a\0 vs a and a\0 vs aX), plus a CHAR case proving storage padding stays invisible.

} else if (right_const) {
RETURN_IF_ERROR(vector_scalar(
*left_str_col, right_str_col->get_data_at(0).trim_tail_padding_zero(),
res_data));
RETURN_IF_ERROR(
vector_scalar(*left_str_col, right_str_col->get_data_at(0), res_data));
} else {
RETURN_IF_ERROR(vector_vector(*left_str_col, *right_str_col, res_data));
}
Expand All @@ -104,7 +103,7 @@ class FunctionHammingDistance : public IFunction {
return Status::OK();
}

const auto left = left_str_col->get_data_at(0).trim_tail_padding_zero();
const auto left = left_str_col->get_data_at(0);
RETURN_IF_ERROR(scalar_vector_nullable(left, *right_str_col, right_null_map, res_data,
null_map));
} else if (right_const) {
Expand All @@ -115,9 +114,8 @@ class FunctionHammingDistance : public IFunction {
return Status::OK();
}

RETURN_IF_ERROR(vector_scalar_nullable(
*left_str_col, right_str_col->get_data_at(0).trim_tail_padding_zero(),
left_null_map, res_data, null_map));
RETURN_IF_ERROR(vector_scalar_nullable(*left_str_col, right_str_col->get_data_at(0),
left_null_map, res_data, null_map));
} else {
for (size_t i = 0; i < input_rows_count; ++i) {
const bool left_is_null = left_null_map && (*left_null_map)[i];
Expand All @@ -128,9 +126,8 @@ class FunctionHammingDistance : public IFunction {
continue;
}

RETURN_IF_ERROR(hamming_distance(
left_str_col->get_data_at(i).trim_tail_padding_zero(),
right_str_col->get_data_at(i).trim_tail_padding_zero(), res_data[i], i));
RETURN_IF_ERROR(hamming_distance(left_str_col->get_data_at(i),
right_str_col->get_data_at(i), res_data[i], i));
}
}

Expand All @@ -149,8 +146,8 @@ class FunctionHammingDistance : public IFunction {
std::vector<size_t> left_offsets;
std::vector<size_t> right_offsets;
for (size_t i = 0; i < size; ++i) {
const auto left = lcol.get_data_at(i).trim_tail_padding_zero();
const auto right = rcol.get_data_at(i).trim_tail_padding_zero();
const auto left = lcol.get_data_at(i);
const auto right = rcol.get_data_at(i);
RETURN_IF_ERROR(hamming_distance_with_offsets(
left, left_offsets, false, simd::VStringFunctions::is_ascii(left), right,
right_offsets, false, simd::VStringFunctions::is_ascii(right), res[i], i));
Expand All @@ -167,7 +164,7 @@ class FunctionHammingDistance : public IFunction {
simd::VStringFunctions::get_utf8_char_offsets(rdata, right_offsets);
std::vector<size_t> left_offsets;
for (size_t i = 0; i < size; ++i) {
const auto left = lcol.get_data_at(i).trim_tail_padding_zero();
const auto left = lcol.get_data_at(i);
RETURN_IF_ERROR(hamming_distance_with_offsets(
left, left_offsets, false, simd::VStringFunctions::is_ascii(left), rdata,
right_offsets, true, right_ascii, res[i], i));
Expand All @@ -184,7 +181,7 @@ class FunctionHammingDistance : public IFunction {
simd::VStringFunctions::get_utf8_char_offsets(ldata, left_offsets);
std::vector<size_t> right_offsets;
for (size_t i = 0; i < size; ++i) {
const auto right = rcol.get_data_at(i).trim_tail_padding_zero();
const auto right = rcol.get_data_at(i);
RETURN_IF_ERROR(hamming_distance_with_offsets(
ldata, left_offsets, true, left_ascii, right, right_offsets, false,
simd::VStringFunctions::is_ascii(right), res[i], i));
Expand All @@ -208,7 +205,7 @@ class FunctionHammingDistance : public IFunction {
continue;
}

const auto left = lcol.get_data_at(i).trim_tail_padding_zero();
const auto left = lcol.get_data_at(i);
RETURN_IF_ERROR(hamming_distance_with_offsets(
left, left_offsets, false, simd::VStringFunctions::is_ascii(left), rdata,
right_offsets, true, right_ascii, res[i], i));
Expand All @@ -232,7 +229,7 @@ class FunctionHammingDistance : public IFunction {
continue;
}

const auto right = rcol.get_data_at(i).trim_tail_padding_zero();
const auto right = rcol.get_data_at(i);
RETURN_IF_ERROR(hamming_distance_with_offsets(
ldata, left_offsets, true, left_ascii, right, right_offsets, false,
simd::VStringFunctions::is_ascii(right), res[i], i));
Expand Down
12 changes: 5 additions & 7 deletions be/src/exprs/function/function_levenshtein.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ static StringRef string_ref_at(const ColumnString::Chars& data,
const ColumnString::Offsets& offsets, size_t i) {
DCHECK_LT(i, offsets.size());
const auto previous_offset = i == 0 ? 0 : offsets[i - 1];
return StringRef(data.data() + previous_offset, offsets[i] - previous_offset)
.trim_tail_padding_zero();
return StringRef(data.data() + previous_offset, offsets[i] - previous_offset);

@github-actions github-actions Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not evaluate retained payloads for NULL rows

This function family uses the default nullable wrapper with need_replace_null_data_to_default() == false, so mixed nullable blocks are evaluated on arbitrary nested values before the null map is reapplied. NULLIF(s, k) retains s under its NULL row; if one row has s = k = 4096 NUL bytes and another row is non-NULL, the base trims that nested row to empty, but this line keeps all 4096 bytes. damerau_levenshtein_distance then rejects (4096 + 2)^2 = 16,793,604 cells above the 16,777,216 cap and aborts the query even though that row's result must be NULL. Please replace or skip null-row nested data before running the distance implementation and add mixed null/non-null coverage.

}

static void get_utf8_char_offsets(const StringRef& ref, Utf8Offsets& offsets) {
Expand Down Expand Up @@ -390,15 +389,14 @@ struct StringDistanceImplBase {
ResultPaddedPODArray& res) {
const size_t size = offsets.size();
res.resize(size);
const auto constant_ref = constant.trim_tail_padding_zero();
const bool constant_ascii = simd::VStringFunctions::is_ascii(constant_ref);
const bool constant_ascii = simd::VStringFunctions::is_ascii(constant);
Utf8Offsets constant_offsets;
get_utf8_char_offsets(constant_ref, constant_offsets);
get_utf8_char_offsets(constant, constant_offsets);
Utf8Offsets value_offsets;
for (size_t i = 0; i < size; ++i) {
RETURN_IF_ERROR(distance_with_const_offsets(string_ref_at(data, offsets, i),
value_offsets, constant_ref,
constant_offsets, constant_ascii, res[i]));
value_offsets, constant, constant_offsets,
constant_ascii, res[i]));
}
return Status::OK();
}
Expand Down
Loading