github-actions[bot] commented on code in PR #69223:
URL: https://github.com/apache/doris/pull/69223#discussion_r4309800747
##########
be/src/exprs/function/function_string_misc.cpp:
##########
@@ -970,9 +970,15 @@ class FunctionTranslate : public IFunction {
const auto* col_to = assert_cast<const
ColumnString*>(argument_columns[2].get());
bool is_ascii = col_source->is_ascii() && col_from->is_ascii() &&
col_to->is_ascii();
+ bool from_to_ascii = col_from->is_ascii() && col_to->is_ascii();
Review Comment:
[P2] Restrict the extra ASCII scan to constant mapping columns
`from_to_ascii` is computed for every call, but its result is used only when
both mapping arguments are constant. For `translate('你', 'a',
large_ascii_column)`, the old `is_ascii` check stops at the Unicode source and
the UTF-8 map reads only the first code point of each replacement string; this
new line instead scans the entire replacement column before still taking
`impl_vectors_utf8<false>`. Large unused replacement suffixes therefore add a
full column scan per batch, regressing a path this optimization does not
accelerate. Move this check inside the branch for two constant mapping
arguments.
##########
be/test/exprs/function/function_string_test.cpp:
##########
@@ -2891,6 +2889,41 @@ TEST(function_string_test, function_parse_url_test) {
}
}
+TEST(function_string_test, function_parse_url_null_invalid_part_test) {
+ const InputTypeSet two_args = {PrimitiveType::TYPE_VARCHAR,
PrimitiveType::TYPE_VARCHAR};
+ const DataSet null_urls = {{{Null(), std::string("INVALID")}, Null()},
+ {{Null(), std::string("HOST")}, Null()}};
+ check_function_all_arg_comb<DataTypeString, true>("parse_url", two_args,
null_urls);
+
+ const InputTypeSet three_args = {PrimitiveType::TYPE_VARCHAR,
PrimitiveType::TYPE_VARCHAR,
+ PrimitiveType::TYPE_VARCHAR};
+ const DataSet null_inputs = {
+ {{Null(), std::string("INVALID"), std::string("k")}, Null()},
+ {{std::string("http://h/p?k=v";), std::string("INVALID"), Null()},
Null()}};
+ check_function_all_arg_comb<DataTypeString, true>("parse_url", three_args,
null_inputs);
+}
+
+TEST(function_string_test, function_translate_mixed_const_map_test) {
+ const InputTypeSet input_types = {PrimitiveType::TYPE_VARCHAR,
+ Consted {PrimitiveType::TYPE_VARCHAR},
+ Consted {PrimitiveType::TYPE_VARCHAR}};
+ const DataSet replace_rows = {
+ {{std::string("aba"), std::string("ab"), std::string("XY")},
std::string("XYX")},
+ {{std::string("a你b好a"), std::string("ab"), std::string("XY")},
+ std::string("X你Y好X")},
+ {{std::string(""), std::string("ab"), std::string("XY")},
std::string("")},
+ {{Null(), std::string("ab"), std::string("XY")}, Null()}};
+ static_cast<void>(check_function<DataTypeString, true>("translate",
input_types, replace_rows));
Review Comment:
[P2] Build singleton nested columns for the constant map arguments
`check_function` inserts every dataset row into each argument column before
wrapping `Consted` arguments in `ColumnConst` (`function_test_util.h:352-359`).
With these four rows, constructing the `from` argument therefore throws
`Incorrect size of nested column in constructor of ColumnConst: 4, must be 1`
before `translate` executes; the five-row deletion case has the same problem.
Construct the mapping columns from one nested value each and broadcast them to
the source batch size so this test can run and exercise the mixed-column paths.
--
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]
