github-actions[bot] commented on code in PR #68311:
URL: https://github.com/apache/doris/pull/68311#discussion_r4060253233
##########
be/src/exprs/function/function_string_misc.cpp:
##########
@@ -826,6 +826,12 @@ class FunctionNgramSearch : public IFunction {
}
auto pattern = assert_cast<const
ColumnString*>(argument_columns[1].get())->get_data_at(0);
auto gram_num = assert_cast<const
ColumnInt32*>(argument_columns[2].get())->get_element(0);
+ // Constant expressions unsupported by FE are evaluated on BE. Check
the value here
+ // since common-subexpression extraction can replace the argument with
a slot reference.
+ if (gram_num <= 0) {
Review Comment:
[P2] Validate the gram even when execution is skipped
This is the only value check, but `execute_impl` is not guaranteed to run.
`crc32('abc') % 0` remains a constant integral tree in FE and evaluates to NULL
on BE, where default NULL propagation returns before this line. Also, `select
ngram_search(cast(number as string), 'abc', crc32('abc') % 3) from
numbers("number"="0")` has a row-dependent root, so the empty projection skips
the function and the known-zero gram is never rejected. Literal NULL/zero grams
are rejected during analysis regardless of these shapes, so newly admitted
BE-only constants change the contract. Please validate semantic constants on a
path that survives CSE and zero-row execution, while retaining a pre-NULL batch
check for materialized nonempty slots, and cover both cases in both fold modes.
--
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]
