github-actions[bot] commented on code in PR #68426:
URL: https://github.com/apache/doris/pull/68426#discussion_r4089038881
##########
be/src/exprs/aggregate/aggregate_function_map_v2.cpp:
##########
@@ -56,16 +59,30 @@ AggregateFunctionPtr
create_aggregate_function_map_agg_v2(const std::string& nam
case PrimitiveType::TYPE_TIMESTAMP_NS:
case PrimitiveType::TYPE_TIMEV2:
case PrimitiveType::TYPE_TIMESTAMPTZ:
- return create_agg_function_map_agg_v2(argument_types,
result_is_nullable, attr);
+ return
create_agg_function_map_agg_v2<use_exact_key_frame>(argument_types,
+
result_is_nullable, attr);
+ case PrimitiveType::TYPE_IPV4:
+ case PrimitiveType::TYPE_IPV6:
+ if constexpr (use_exact_key_frame) {
+ return
create_agg_function_map_agg_v2<use_exact_key_frame>(argument_types,
+
result_is_nullable, attr);
+ }
+ break;
default:
- LOG(WARNING) << fmt::format("unsupported input type {} for aggregate
function {}",
- argument_types[0]->get_name(), name);
- return nullptr;
+ break;
}
+ LOG(WARNING) << fmt::format("unsupported input type {} for aggregate
function {}",
+ argument_types[0]->get_name(), name);
+ return nullptr;
}
void register_aggregate_function_map_agg_v2(AggregateFunctionSimpleFactory&
factory) {
- factory.register_function_both("map_agg_v2",
create_aggregate_function_map_agg_v2);
+ factory.register_function_both("map_agg_v2",
create_aggregate_function_map_agg_v2<true>);
+ constexpr auto old_be_exec_version =
SUPPORT_MAP_AGG_V2_EXACT_FRAME_VERSION - 1;
+ factory.register_alternative_function("map_agg_v2",
create_aggregate_function_map_agg_v2<false>,
Review Comment:
[P1] Keep persisted map states compatible across this wire-only fork.
`register_alternative_function` also records `map_agg_v2` in
`_function_change_map`. Once the current version becomes 16,
`DataTypeAggState::check_function_compatibility` compares compatibility 0 for
the current function with 15 for every pre-16 tablet/segment, so
`ColumnReader::create_agg_state` and tablet aggregation reject existing
`agg_state<map_agg>` data. Those columns still persist the unchanged
`ColumnMap` representation (`get_serialized_type()` and the state-column
methods are Map-based); only the transient `write`/`read` envelope changed
here. Please decouple wire routing from the persisted-state incompatibility
fence, and cover reading/compacting a version-15 map state at version 16.
##########
be/src/exprs/aggregate/aggregate_function_map_v2.cpp:
##########
@@ -56,16 +59,30 @@ AggregateFunctionPtr
create_aggregate_function_map_agg_v2(const std::string& nam
case PrimitiveType::TYPE_TIMESTAMP_NS:
case PrimitiveType::TYPE_TIMEV2:
case PrimitiveType::TYPE_TIMESTAMPTZ:
- return create_agg_function_map_agg_v2(argument_types,
result_is_nullable, attr);
+ return
create_agg_function_map_agg_v2<use_exact_key_frame>(argument_types,
+
result_is_nullable, attr);
+ case PrimitiveType::TYPE_IPV4:
+ case PrimitiveType::TYPE_IPV6:
+ if constexpr (use_exact_key_frame) {
+ return
create_agg_function_map_agg_v2<use_exact_key_frame>(argument_types,
+
result_is_nullable, attr);
+ }
+ break;
default:
- LOG(WARNING) << fmt::format("unsupported input type {} for aggregate
function {}",
- argument_types[0]->get_name(), name);
- return nullptr;
+ break;
}
+ LOG(WARNING) << fmt::format("unsupported input type {} for aggregate
function {}",
+ argument_types[0]->get_name(), name);
+ return nullptr;
}
void register_aggregate_function_map_agg_v2(AggregateFunctionSimpleFactory&
factory) {
- factory.register_function_both("map_agg_v2",
create_aggregate_function_map_agg_v2);
+ factory.register_function_both("map_agg_v2",
create_aggregate_function_map_agg_v2<true>);
+ constexpr auto old_be_exec_version =
SUPPORT_MAP_AGG_V2_EXACT_FRAME_VERSION - 1;
+ factory.register_alternative_function("map_agg_v2",
create_aggregate_function_map_agg_v2<false>,
+ false, old_be_exec_version);
+ factory.register_alternative_function("map_agg_v2",
create_aggregate_function_map_agg_v2<false>,
Review Comment:
[P1] Route the foreach wrapper through the negotiated version too. Nereids
emits `map_agg_v2_foreachv2`, so factory compatibility lookup is performed on
that outer name, while this block registers only `map_agg_v2`; the foreach-v2
creator then builds its nested function with `get_newest_version()`. A query
negotiated at version 15 therefore uses the exact-frame/IP-capable
implementation on a new BE but the legacy implementation on an old BE. Above
the compression threshold, an old padded partial state fails the new reader's
exact-consumption check, and the reverse direction sends an unpadded frame to
the old decoder; IP arrays also succeed or fail by peer. Please propagate the
query version to the nested lookup or version the generated wrapper names, with
a mixed-version foreach test.
--
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]
