github-actions[bot] commented on code in PR #68407:
URL: https://github.com/apache/doris/pull/68407#discussion_r4079772906
##########
be/src/exprs/function/function_ip.h:
##########
@@ -792,72 +793,52 @@ class FunctionIPv4CIDRToRange : public IFunction {
DataTypePtr get_return_type_impl(const DataTypes& arguments) const
override {
DataTypePtr element = std::make_shared<DataTypeIPv4>();
- return std::make_shared<DataTypeStruct>(DataTypes {element, element},
- Strings {"min", "max"});
+ DataTypePtr result = std::make_shared<DataTypeStruct>(DataTypes
{element, element},
+ Strings {"min",
"max"});
+ if (arguments[0]->is_nullable() || arguments[1]->is_nullable()) {
+ return make_nullable(result);
+ }
+ return result;
}
+ bool use_default_implementation_for_nulls() const override { return false;
}
+
Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const
override {
- ColumnWithTypeAndName& ip_column = block.get_by_position(arguments[0]);
- ColumnWithTypeAndName& cidr_column =
block.get_by_position(arguments[1]);
-
- const auto& [ip_column_ptr, ip_col_const] =
unpack_if_const(ip_column.column);
- const auto& [cidr_column_ptr, cidr_col_const] =
unpack_if_const(cidr_column.column);
-
- const auto* col_ip_column = assert_cast<const
ColumnIPv4*>(ip_column_ptr.get());
- const auto* col_cidr_column = assert_cast<const
ColumnInt16*>(cidr_column_ptr.get());
+ const auto& ip_argument = block.get_by_position(arguments[0]);
+ const auto& cidr_argument = block.get_by_position(arguments[1]);
+ const auto ip = ColumnView<TYPE_IPV4>::create(ip_argument.column);
+ const auto cidr =
ColumnView<TYPE_SMALLINT>::create(cidr_argument.column);
- const typename ColumnIPv4::Container& vec_ip_input =
col_ip_column->get_data();
- const ColumnInt16::Container& vec_cidr_input =
col_cidr_column->get_data();
auto col_lower_range_output = ColumnIPv4::create(input_rows_count, 0);
auto col_upper_range_output = ColumnIPv4::create(input_rows_count, 0);
-
- ColumnIPv4::Container& vec_lower_range_output =
col_lower_range_output->get_data();
- ColumnIPv4::Container& vec_upper_range_output =
col_upper_range_output->get_data();
-
+ auto null_map = ColumnUInt8::create(input_rows_count, 0);
Review Comment:
**[P2] Keep the non-nullable path allocation-free**
When both arguments are declared non-nullable, `get_return_type_impl`
returns a non-nullable struct and neither `ColumnView` can report NULL, but
this still allocates and zero-fills `input_rows_count` bytes and the branch
below discards the map. The same dead O(rows) allocation was added at lines 883
and 1254 for the other two functions, so ordinary non-nullable queries now pay
it on every block. Please allocate/populate a null map only when the declared
result is nullable (and preserve a direct non-nullable fast path).
##########
be/src/exprs/function/function_ip.h:
##########
@@ -878,91 +859,113 @@ class FunctionIPv6CIDRToRange : public IFunction {
DataTypePtr get_return_type_impl(const DataTypes& arguments) const
override {
DataTypePtr element = std::make_shared<DataTypeIPv6>();
- return std::make_shared<DataTypeStruct>(DataTypes {element, element},
- Strings {"min", "max"});
+ DataTypePtr result = std::make_shared<DataTypeStruct>(DataTypes
{element, element},
+ Strings {"min",
"max"});
+ if (arguments[0]->is_nullable() || arguments[1]->is_nullable()) {
+ return make_nullable(result);
+ }
+ return result;
}
+ bool use_default_implementation_for_nulls() const override { return false;
}
+
Status execute_impl(FunctionContext* context, Block& block, const
ColumnNumbers& arguments,
uint32_t result, size_t input_rows_count) const
override {
const auto& addr_column_with_type_and_name =
block.get_by_position(arguments[0]);
const auto& cidr_column_with_type_and_name =
block.get_by_position(arguments[1]);
- const auto& [addr_column, add_col_const] =
- unpack_if_const(addr_column_with_type_and_name.column);
- const auto& [cidr_column, col_const] =
- unpack_if_const(cidr_column_with_type_and_name.column);
+ const auto cidr =
ColumnView<TYPE_SMALLINT>::create(cidr_column_with_type_and_name.column);
+ const auto addr_type =
addr_column_with_type_and_name.type->get_primitive_type();
- const auto* cidr_col = assert_cast<const
ColumnInt16*>(cidr_column.get());
- ColumnPtr col_res = nullptr;
-
- if (addr_column_with_type_and_name.type->get_primitive_type() ==
TYPE_IPV6) {
- const auto* ipv6_addr_column = assert_cast<const
ColumnIPv6*>(addr_column.get());
- col_res = execute_impl(*ipv6_addr_column, *cidr_col,
input_rows_count, add_col_const,
- col_const);
- } else if
(is_string_type(addr_column_with_type_and_name.type->get_primitive_type())) {
- ColumnPtr col_ipv6 =
-
convert_to_ipv6<IPConvertExceptionMode::Throw>(addr_column, nullptr);
- const auto* ipv6_addr_column = assert_cast<const
ColumnIPv6*>(col_ipv6.get());
- col_res = execute_impl(*ipv6_addr_column, *cidr_col,
input_rows_count, add_col_const,
- col_const);
+ auto col_res_lower_range = ColumnIPv6::create(input_rows_count, 0);
+ auto col_res_upper_range = ColumnIPv6::create(input_rows_count, 0);
+ auto& vec_res_lower_range = col_res_lower_range->get_data();
+ auto& vec_res_upper_range = col_res_upper_range->get_data();
+ auto null_map = ColumnUInt8::create(input_rows_count, 0);
+ auto& nulls = null_map->get_data();
+ if (addr_type == TYPE_IPV6) {
+ const auto addr =
ColumnView<TYPE_IPV6>::create(addr_column_with_type_and_name.column);
+ execute_impl(addr, cidr, input_rows_count, vec_res_lower_range,
vec_res_upper_range,
+ nulls);
+ } else if (is_string_type(addr_type)) {
+ const auto addr =
+
ColumnView<TYPE_STRING>::create(addr_column_with_type_and_name.column);
+ execute_impl(addr, cidr, input_rows_count, vec_res_lower_range,
vec_res_upper_range,
+ nulls);
} else {
return Status::RuntimeError(
"Illegal column {} of argument of function {}, Expected
IPv6 or String",
- addr_column->get_name(), get_name());
+ addr_column_with_type_and_name.column->get_name(),
get_name());
}
- block.replace_by_position(result, std::move(col_res));
+ ColumnPtr result_column = ColumnStruct::create(
+ Columns {std::move(col_res_lower_range),
std::move(col_res_upper_range)});
+ if (block.get_by_position(result).type->is_nullable()) {
+ result_column = ColumnNullable::create(std::move(result_column),
std::move(null_map));
+ }
+ block.replace_by_position(result, std::move(result_column));
return Status::OK();
}
- static ColumnPtr execute_impl(const ColumnIPv6& from_column, const
ColumnInt16& cidr_column,
- size_t input_rows_count, bool is_addr_const
= false,
- bool is_cidr_const = false) {
- auto col_res_lower_range = ColumnIPv6::create(input_rows_count, 0);
- auto col_res_upper_range = ColumnIPv6::create(input_rows_count, 0);
- auto& vec_res_lower_range = col_res_lower_range->get_data();
- auto& vec_res_upper_range = col_res_upper_range->get_data();
-
+private:
+ static void execute_impl(const ColumnView<TYPE_IPV6>& addr,
+ const ColumnView<TYPE_SMALLINT>& cidr, size_t
input_rows_count,
+ ColumnIPv6::Container& lower,
ColumnIPv6::Container& upper,
+ ColumnUInt8::Container& nulls) {
static constexpr UInt8 max_cidr_mask = IPV6_BINARY_LENGTH * 8;
+ for (size_t i = 0; i < input_rows_count; ++i) {
+ if (addr.is_null_at(i) || cidr.is_null_at(i)) {
+ nulls[i] = 1;
+ continue;
+ }
+ const auto prefix = cidr.value_at(i);
+ if (prefix < 0 || prefix > max_cidr_mask) {
+ throw Exception(ErrorCode::INVALID_ARGUMENT, "Illegal cidr
value '{}'",
+ std::to_string(prefix));
+ }
+ const auto address = addr.value_at(i);
+ apply_cidr_mask(reinterpret_cast<const char*>(&address),
+ reinterpret_cast<char*>(&lower[i]),
reinterpret_cast<char*>(&upper[i]),
+ cast_set<UInt8>(prefix));
+ }
+ }
- if (is_addr_const) {
- for (size_t i = 0; i < input_rows_count; ++i) {
- auto cidr = cidr_column.get_int(i);
- if (cidr < 0 || cidr > max_cidr_mask) {
- throw Exception(ErrorCode::INVALID_ARGUMENT, "Illegal cidr
value '{}'",
- std::to_string(cidr));
- }
- apply_cidr_mask(from_column.get_data_at(0).data,
-
reinterpret_cast<char*>(&vec_res_lower_range[i]),
-
reinterpret_cast<char*>(&vec_res_upper_range[i]),
- cast_set<UInt8>(cidr));
+ static void execute_impl(const ColumnView<TYPE_STRING>& addr,
+ const ColumnView<TYPE_SMALLINT>& cidr, size_t
input_rows_count,
+ ColumnIPv6::Container& lower,
ColumnIPv6::Container& upper,
+ ColumnUInt8::Container& nulls) {
+ static constexpr UInt8 max_cidr_mask = IPV6_BINARY_LENGTH * 8;
+ for (size_t i = 0; i < input_rows_count; ++i) {
+ if (addr.is_null_at(i) || cidr.is_null_at(i)) {
+ nulls[i] = 1;
+ continue;
}
- } else if (is_cidr_const) {
- auto cidr = cidr_column.get_int(0);
- if (cidr < 0 || cidr > max_cidr_mask) {
+ const auto prefix = cidr.value_at(i);
+ if (prefix < 0 || prefix > max_cidr_mask) {
throw Exception(ErrorCode::INVALID_ARGUMENT, "Illegal cidr
value '{}'",
- std::to_string(cidr));
+ std::to_string(prefix));
}
- for (size_t i = 0; i < input_rows_count; ++i) {
- apply_cidr_mask(from_column.get_data_at(i).data,
-
reinterpret_cast<char*>(&vec_res_lower_range[i]),
-
reinterpret_cast<char*>(&vec_res_upper_range[i]),
- cast_set<UInt8>(cidr));
+ const auto value = addr.value_at(i);
+ if (value.size == 0) {
+ throw Exception(ErrorCode::INVALID_ARGUMENT, "Invalid IPv6
value");
}
- } else {
- for (size_t i = 0; i < input_rows_count; ++i) {
- auto cidr = cidr_column.get_int(i);
- if (cidr < 0 || cidr > max_cidr_mask) {
- throw Exception(ErrorCode::INVALID_ARGUMENT, "Illegal cidr
value '{}'",
- std::to_string(cidr));
- }
- apply_cidr_mask(from_column.get_data_at(i).data,
-
reinterpret_cast<char*>(&vec_res_lower_range[i]),
-
reinterpret_cast<char*>(&vec_res_upper_range[i]),
- cast_set<UInt8>(cidr));
+ IPv6 address = 0;
+ Int64 parsed_ipv4 = 0;
+ bool parsed = false;
+ if (try_parse_ipv4(value.begin(), value.end(), parsed_ipv4)) {
Review Comment:
**[P2] Preserve the constant-string fast path**
When the address is a constant string but the CIDR varies (for example,
`ipv6_cidr_to_range('2001:db8::1', cidr_column)`), `addr.value_at(i)` correctly
reuses element 0, but this loop still runs `try_parse_ipv4`/`parse_ipv6_whole`
for every row. Before this change, `unpack_if_const` left a one-row nested
string, `convert_to_ipv6` parsed it once, and the mask loop reused the parsed
address. This turns one parse per block into O(rows) identical parses. Please
cache the first needed non-NULL constant parse after the row's NULL and
CIDR-range checks (or otherwise preserve the converted constant fast path).
--
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]
