github-actions[bot] commented on code in PR #66236:
URL: https://github.com/apache/doris/pull/66236#discussion_r3674491139
##########
be/src/exprs/function/function_hamming_distance.cpp:
##########
@@ -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));
Review Comment:
[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.
##########
be/src/exprs/function/function_levenshtein.cpp:
##########
@@ -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);
Review Comment:
[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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
