github-actions[bot] commented on code in PR #67121:
URL: https://github.com/apache/doris/pull/67121#discussion_r3852083400
##########
be/src/exprs/function/function_string_misc.cpp:
##########
@@ -141,9 +142,11 @@ class FunctionAutoPartitionName : public IFunction {
auto& res_offset = res->get_offsets();
res_offset.resize(input_rows_count);
- const char* partition_type = chars_list[0]->raw_data();
+ std::string partition_type(chars_list[0]->raw_data(),
(*offsets_list[0])[0]);
+ std::transform(partition_type.begin(), partition_type.end(),
partition_type.begin(),
+ [](unsigned char c) { return
static_cast<char>(std::tolower(c)); });
// partition type is list|range
- if (std::strncmp(partition_type, "list", 4) == 0) {
+ if (partition_type == "list") {
Review Comment:
[P1] Keep FE and BE keyword normalization locale-independent. FE still
validates both literals with parameterless Java `String.toLowerCase()`, while
this byte-wise C-locale transform only canonicalizes ASCII. With a
Turkish-default FE, `LİST`.toLowerCase() is `list`, so
`auto_partition_name('LİST', 'x')` is accepted as a two-argument list call; the
raw literal is transmitted unchanged, becomes `lİst` here, misses this branch,
and the range path indexes the missing third argument. `MİNUTE` has the same
FE-accepted/BE-unmatched split. Please use `Locale.ROOT` in FE validation or
transmit the validated canonical token, and add Turkish-default-locale coverage.
##########
be/test/exprs/function/function_string_test.cpp:
##########
@@ -81,6 +81,28 @@ DataSet make_md5_varbinary_dataset(const
std::vector<std::string>& inputs) {
} // namespace
+TEST(function_string_test, function_auto_partition_name_case_insensitive_test)
{
+ const InputTypeSet list_input_types = {PrimitiveType::TYPE_VARCHAR,
+ PrimitiveType::TYPE_VARCHAR};
+ const DataSet list_data_set = {
+ {{"LIST", "edc_server2"}, "pedc5fserver211"},
+ {{"LiSt", "edc_server2"}, "pedc5fserver211"},
+ };
+ ASSERT_TRUE(check_function<DataTypeString, true>("auto_partition_name",
list_input_types,
+ list_data_set)
+ .ok());
+
+ const InputTypeSet range_input_types = {
+ PrimitiveType::TYPE_VARCHAR, PrimitiveType::TYPE_VARCHAR,
PrimitiveType::TYPE_VARCHAR};
+ const DataSet range_data_set = {
+ {{"RANGE", "MONTH", "2022-12-12 19:20:30"}, "p20221201000000"},
+ {{"rAnGe", "dAy", "2022-12-12 19:20:30"}, "p20221212000000"},
+ };
+ ASSERT_TRUE(check_function<DataTypeString, true>("auto_partition_name",
range_input_types,
Review Comment:
[P1] Make these checks use the function's real result and literal shapes.
Both calls request `Nullable(String)` through `<DataTypeString, true>`, but
`auto_partition_name` declares non-nullable `String`, so the function builder
rejects them before execution. With that corrected, the plain input descriptors
still create two-row non-const columns while the implementation reads dispatch
and range granularity only from row 0. Both range rows therefore execute
`MONTH`, making the second actual result `p20221201000000` rather than
`p20221212000000`; `LiSt`/`rAnGe`/`dAy` in row 2 never independently control
the paths they claim to test. Please use non-nullable
`check_function<DataTypeString>` calls and split each casing into a separate
one-row invocation, marking production-literal arguments constant.
--
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]
