HappenLee commented on code in PR #67805:
URL: https://github.com/apache/doris/pull/67805#discussion_r3982099713
##########
be/src/exprs/aggregate/aggregate_function_window_funnel.h:
##########
@@ -295,6 +295,15 @@ struct WindowFunnelState {
if (other.events_list.empty()) {
return;
}
+
+ if (events_list.empty()) {
+ window = other.window;
+ window_funnel_mode = other.window_funnel_mode;
+ } else if (UNLIKELY(window != other.window ||
Review Comment:
Fixed by requiring both window and mode to be constant during FE legality
checking in WindowFunnel (V1) and WindowFunnelV2. The ordinary functions and
their _state/_combine forms delegate to these checks, so varying configuration
columns are rejected before any state is built. Timestamp and event arguments
can still be columns. Existing BE merge-time comparisons remain necessary for
states built separately with different constant configurations.
Added FE unit coverage for both implementations and all three forms, plus 18
expected-error SQL cases covering window_funnel, window_funnel_v1 and
window_funnel_v2 with varying window or mode. All 5 FE parameter tests
(including reservoir NaN coverage) and all 3 regression suites
(test_agg_state_parameters, window_funnel, window_funnel_v2) passed. The BE
ASAN/FE build and FE Checkstyle also passed.
This fix is included in commit a8b018b9ef6, together with the previously
agreed review follow-ups in this PR update.
--
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]
