github-actions[bot] commented on code in PR #68133:
URL: https://github.com/apache/doris/pull/68133#discussion_r4035564829
##########
be/test/exprs/function/function_like_test.cpp:
##########
@@ -144,6 +144,84 @@ TEST(FunctionLikeTest, like) {
func_name, const_pattern_input_types, data_set));
}
+TEST(FunctionLikeTest, like_matches_whole_value) {
+ std::string func_name = "like";
+
+ DataSet data_set = {
+ // A trailing newline belongs to the value, so a pattern that is
anchored at the
+ // tail must not match across it.
+ {{std::string("acb"), std::string("a_b")}, uint8_t(1)},
+ {{std::string("acb\n"), std::string("a_b")}, uint8_t(0)},
+ {{std::string("acb\r\n"), std::string("a_b")}, uint8_t(0)},
+ {{std::string("acb\n\n"), std::string("a_b")}, uint8_t(0)},
+ {{std::string("acbx"), std::string("a_b")}, uint8_t(0)},
+ {{std::string("\nacb"), std::string("a_b")}, uint8_t(0)},
+ {{std::string("abc"), std::string("a%c")}, uint8_t(1)},
+ {{std::string("abc\n"), std::string("a%c")}, uint8_t(0)},
+ {{std::string("abc\n"), std::string("%b%c")}, uint8_t(0)},
+ // The newline is an ordinary character for '_' and '%'.
+ {{std::string("a\nb"), std::string("a_b")}, uint8_t(1)},
+ {{std::string("a\nb"), std::string("a%b")}, uint8_t(1)},
+ {{std::string("acb\n"), std::string("a_b_")}, uint8_t(1)},
+ {{std::string("acb\n"), std::string("a_b%")}, uint8_t(1)},
+ {{std::string("abc\n"), std::string("a_c%")}, uint8_t(1)},
+ // '_' stands for one character, not for one byte.
+ {{std::string("a中b"), std::string("a_b")}, uint8_t(1)},
+ {{std::string("a中b\n"), std::string("a_b")}, uint8_t(0)},
+ // An empty pattern only matches an empty value.
+ {{std::string(""), std::string("")}, uint8_t(1)},
+ {{std::string("\n"), std::string("")}, uint8_t(0)},
+ // The shortcut paths and the regex path must agree on the same
value.
+ {{std::string("acb\n"), std::string("acb")}, uint8_t(0)},
+ {{std::string("abc\n"), std::string("%c")}, uint8_t(0)},
+ {{std::string("abc\n"), std::string("a%")}, uint8_t(1)},
+ // A pattern open at the tail must not make the engines walk the
rest of the value:
+ // `.*` only matches valid UTF-8, so anchoring it with `\z` would
make RE2 reject a
+ // value whose tail is not valid UTF-8 while Hyperscan still
accepts it.
+ {{std::string("acb\xff", 4), std::string("a_b%")}, uint8_t(1)},
Review Comment:
[P2] Avoid asserting undefined Hyperscan UTF-8 behavior
`check_function_all_arg_comb` includes constant-pattern runs, and `a_b%`
bypasses the LIKE fast paths because of `_`. That makes
`construct_like_const_state` compile `^a.b` with `HS_FLAG_UTF8` and pass this
four-byte `acb\xff` buffer to `hs_scan`. [Hyperscan's
API](https://intel.github.io/hyperscan/dev-reference/api_constants.html#c.HS_FLAG_UTF8)
explicitly says that scanning invalid UTF-8 with this flag has undefined
results, so this expected `1` can vary across engine versions or architectures
and cannot prove Hyperscan/RE2 equivalence. Please route invalid input through
a matcher with defined byte semantics (or otherwise keep it out of the UTF-8
Hyperscan path) before asserting this result.
##########
be/src/exprs/function/like.cpp:
##########
@@ -809,6 +810,10 @@ void FunctionLike::convert_like_pattern(const
LikeSearchState* state, const std:
}
if (c == '%') {
+ if (i + 1 == pattern.size()) {
Review Comment:
[P2] Preserve wildcard parity with an explicit backslash escape
This leaves a supported trailing-wildcard form outside the fix. With a
one-byte backslash as the explicit escape, pattern bytes `[0x5c, 0x5c, 0x25]`
mean a literal backslash followed by wildcard `%`, but
`replace_pattern_by_escape` first collapses the pair to `[0x5c, 0x25]`.
Downstream fast-path recognition can then classify `%` as a literal, and the
regex fallback likewise consumes it as escaped, so the pattern no longer has an
open literal-backslash prefix. The normalization bug predates this branch, but
both constant and nonconstant three-argument LIKE paths remain incorrect.
Please preserve the original default-backslash representation (or otherwise
retain escape parity), and add coverage showing that a literal-backslash prefix
followed by `%` remains open.
##########
be/src/exprs/function/like.cpp:
##########
@@ -809,6 +810,10 @@ void FunctionLike::convert_like_pattern(const
LikeSearchState* state, const std:
}
if (c == '%') {
+ if (i + 1 == pattern.size()) {
+ // a trailing `%` matches anything, and so does appending
nothing
+ return;
+ }
re_pattern->append(".*");
Review Comment:
[P2] Handle invalid bytes for interior percent wildcards too
The new invalid-input case establishes that SQL `%` must be able to absorb
bytes that are not valid UTF-8, but this change makes only the terminal
wildcard byte-safe. For example, `a%b` still becomes `^a.*b\z`; the nonconstant
path runs that through UTF-8 RE2, where `.` matches a code point and cannot
traverse the `0xff` in bytes `[0x61, 0xff, 0x62]`, so it returns false. The
constant-pattern Hyperscan path is undefined for the same invalid input,
creating an additional path split. Although the interior case predates this
hunk, it leaves the byte-semantics contract introduced here incomplete. Please
give every `%` defined byte-safe semantics (or validate/route invalid input
consistently), and cover constant, nonconstant, pushdown, and NOT LIKE forms
with an invalid byte between required literals. [RE2 syntax
reference](https://github.com/google/re2/wiki/Syntax)
--
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]
