zclllyybb commented on code in PR #56497:
URL: https://github.com/apache/doris/pull/56497#discussion_r2382570207
##########
be/src/vec/functions/function_string.h:
##########
@@ -2353,17 +2354,14 @@ class FunctionStringDigestOneArg : public IFunction {
int argument_size = arguments.size();
std::vector<ColumnPtr> argument_columns(argument_size);
-
- std::vector<const ColumnString::Offsets*> offsets_list(argument_size);
- std::vector<const ColumnString::Chars*> chars_list(argument_size);
+ std::vector<uint8_t> is_const(argument_size, 0);
for (int i = 0; i < argument_size; ++i) {
Review Comment:
Should `FunctionStringDigestOneArg` have exactly one arg? if so, this
for-loop and `unpack_if_const` is useless?
##########
be/src/vec/functions/function_hash.cpp:
##########
@@ -148,33 +149,26 @@ struct XxHashImpl {
to_column.insert_many_defaults(input_rows_count);
}
auto& col_to_data = to_column.get_data();
- if (const auto* col_from = check_and_get_column<ColumnString>(column))
{
- const typename ColumnString::Chars& data = col_from->get_chars();
- const typename ColumnString::Offsets& offsets =
col_from->get_offsets();
- size_t size = offsets.size();
- ColumnString::Offset current_offset = 0;
- for (size_t i = 0; i < size; ++i) {
+ if (column->is_column_string() || typeid(*column) ==
typeid(ColumnVarbinary)) {
Review Comment:
use check_and_get_column is better, why change?
##########
be/src/vec/functions/function_string.h:
##########
@@ -2426,8 +2420,8 @@ class FunctionStringDigestSHA1 : public IFunction {
SHA1Digest digest;
for (size_t i = 0; i < input_rows_count; ++i) {
- int size = offset[i] - offset[i - 1];
- digest.reset(&data[offset[i - 1]], size);
+ StringRef data_ref = str_col->get_data_at(i);
Review Comment:
include this and following changes, dont directly call a virtual function.
the `assert_cast` is critical for performance.
##########
be/src/vec/functions/function_string.h:
##########
@@ -2353,17 +2354,14 @@ class FunctionStringDigestOneArg : public IFunction {
int argument_size = arguments.size();
std::vector<ColumnPtr> argument_columns(argument_size);
-
- std::vector<const ColumnString::Offsets*> offsets_list(argument_size);
- std::vector<const ColumnString::Chars*> chars_list(argument_size);
+ std::vector<uint8_t> is_const(argument_size, 0);
for (int i = 0; i < argument_size; ++i) {
- argument_columns[i] =
-
block.get_by_position(arguments[i]).column->convert_to_full_column_if_const();
- if (const auto* col_str = assert_cast<const
ColumnString*>(argument_columns[i].get())) {
- offsets_list[i] = &col_str->get_offsets();
- chars_list[i] = &col_str->get_chars();
- } else {
+ std::tie(argument_columns[i], is_const[i]) =
+
unpack_if_const(block.get_by_position(arguments[i]).column);
+
+ if (!(argument_columns[i]->is_column_string() ||
+ dynamic_cast<const
ColumnVarbinary*>(argument_columns[i].get()))) {
Review Comment:
`dynamic_cast` should be `check_and_get_column`. but just use `assert_cast`
here is ok
##########
be/src/vec/functions/function_string.h:
##########
@@ -2378,15 +2376,13 @@ class FunctionStringDigestOneArg : public IFunction {
for (size_t i = 0; i < input_rows_count; ++i) {
using ObjectData = typename Impl::ObjectData;
ObjectData digest;
- for (size_t j = 0; j < offsets_list.size(); ++j) {
- const auto& current_offsets = *offsets_list[j];
- const auto& current_chars = *chars_list[j];
-
- int size = current_offsets[i] - current_offsets[i - 1];
- if (size < 1) {
+ for (size_t j = 0; j < argument_columns.size(); ++j) {
+ StringRef data_ref = is_const[j] ?
argument_columns[j]->get_data_at(0)
Review Comment:
as the comment above, if this function only accept one arg, then no need to
deal with constancy.
--
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]
