[fix](be) Reject incompatible aggregate state parameters

[HappenLee]

What problem does this PR solve?

Issue Number: N/A

Related PR: N/A

Problem Summary:

AggState types describe argument types but do not include constant parameter values. For example, topn_merge over a UNION ALL of topn_state('a', 1) and topn_state('a', 3) can combine incompatible states. TopN previously overwrote the destination N/capacity, making results depend on merge order. Other configurable aggregates silently adopted incompatible settings; legacy percentile arrays could index beyond the destination state when quantile counts differed.

Reject incompatible configured states with INVALID_ARGUMENT. Apply parameter compatibility checks to TopN variants, histogram, percentile variants, limited collect, group_concat, intersect_count, exponential moving average, sequence functions and both window_funnel implementations. Initialize approximate percentile digests with the source compression and retain serialized field layouts. V2 funnel serialization uses a backward-compatible sorted-field tag for configured eventless states.

The review follow-up establishes these contracts:

• FE requires a constant collect_set limit and constant window_funnel window/mode parameters, including ordinary, _state and _combine calls. Both funnel implementations and the default alias are covered.
• FE rejects NaN reservoir levels using the valid interval [0, 1]. EMA rejects NaN half-decays during serialization and result production.
• Initialized exact-percentile states retain their quantile configuration even when all samples are NaN. Initialized eventless sequence states retain their pattern and argument count. Incompatible configurations fail in either merge order, including serialized sequence-state merges; truly uninitialized states remain identities.
• Initialized eventless window_funnel_v2 states (including the default window_funnel alias) retain window/mode and reject incompatible configurations before empty-event fast paths. Fresh and reset states remain identities, including after serialization. Legacy states remain readable; the new configured-empty tag is also readable by legacy sorted-flag decoding.
• Negative-limit collect states and zero-half-decay EMA states remain intentionally non-contributing during serialized-state merging. The EMA exception is documented in source and docs/exponential-moving-average.md.

Scope exclusions: linear_histogram changes were removed; its existing constant/finite parameter issues will be handled separately. The multi-distinct group_concat wrapper bypasses nested merge validation and remains deferred to a follow-up. These review threads remain open.

Release note

Merging covered aggregate states with incompatible parameters now returns an error instead of producing incorrect results or risking an out-of-bounds access. collect_set limits and window_funnel window/mode parameters must be constant. NaN reservoir levels and newly produced EMA states/results with NaN half-decay are rejected. All-false window_funnel states retain their configuration and reject incompatible window/mode merges.

Check List (For Author)

• Test:
  • Regression test: test_agg_state_parameters, window_funnel, and window_funnel_v2 all passed against the local worktree cluster. The latest run passed all three suites with zero failures/skips. Coverage includes 48 new all-false mismatch combinations across V1/V2/default alias, both operand orders and merge/union, plus rejection of varying funnel parameters in ordinary/state/combine calls. The original binary fails the newly added all-false mismatch case because it raises no exception.
  • Unit Test: latest related BE ASAN run passed 58 tests across aggregate-state, V1/V2 funnel, serialization-compatibility and sequence suites. Coverage includes direct/serialized fresh adoption, reset serialization, compatible eventless states and legacy header framing. Earlier FE validation passed all 5 tests in WindowFunnelParameterTest and PercentileReservoirParameterTest.
• Local validation: The current BE ASAN build passed. The earlier FE build passed with zero FE Checkstyle violations; this follow-up changes no FE code. BE clang-format 16, header hygiene, and whitespace checks passed. clang-tidy was rerun: no diagnostics remain on changed lines. The full check still reports pre-existing diagnostics, including the unmatched NOLINTEND in be/src/core/types.h and existing V2 funnel source/test diagnostics. Local BE startup uses the OpenBLAS build-cache setting USE_OPENMP=FALSE; that setting is not part of this patch.
• Behavior changed:
  • Yes, as described above. Serialized state layouts are unchanged.
• Does this need documentation?
  • Yes. Added a local EMA semantics note; no separate documentation PR.

Check List (For Reviewer who merge this PR)

• Confirm the release note
• Confirm test cases
• Confirm document
• Add branch pick label

[hello-stephen]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[HappenLee]

/review

[HappenLee]

run buildall

[github-actions]

Request changes — capped/incomplete after the mandatory three review rounds. Ten accepted correctness issues are attached inline. The final round found the exact-percentile configured-empty case, so the search cap was reached with a new accepted issue; every candidate found so far is adjudicated, but the review status must remain capped/incomplete.

Critical checkpoint conclusions:

• Goal and proof: The intended rejection of incompatible AggState parameters is not complete. The new unit tests cover direct and deserialize-and-merge behavior, reset, compatible reuse, and ordinary mismatches; the regression suite covers both _merge and _union orders. They do not cover the accepted sentinel, non-finite, configured-empty, distinct-wrapper, or within-state _combine cases.
• Scope and clarity: The production diff is focused on state comparison/reset logic and tests. The local checks are generally small and placed before mutation, but the overall fix omits supported wrapper and state-construction paths identified inline.
• Concurrency: These are per-aggregate-place state transitions; no new shared mutable state, thread entry, lock, atomic, or lock-order issue is introduced. Distributed partial aggregation matters semantically, as noted inline, but not as a data race.
• Lifecycle: Create, add, reset, nullable wrapping, serialization/deserialization, empty-state adoption, and result finalization were traced. Most reset paths restore their sentinel correctly; several accepted findings arise because configured-but-empty and legal sentinel-valued states are classified inconsistently. No static-initialization or ownership cycle is involved.
• Configuration: No Doris configuration item is added or changed.
• Compatibility: Function symbols and serialized layouts/version gates are unchanged, so the patch is wire-format compatible. Enforcement behavior is naturally unavailable on older BEs during a rolling upgrade, but no incompatible state encoding was introduced.
• Parallel paths: Direct, vector, selected, nullable, serialized-column, spill, _state, _union, and _merge paths ultimately reach nested merge. The exceptions are the outer multi-distinct wrapper and raw _combine/within-state paths called out inline.
• Conditional checks: The new identity/equality conditions are not valid for accepted -1, zero, NaN, all-false, and all-NaN cases. Supported integer/string intersect-key equality was rechecked and the proposed floating-key issue was dismissed because that BE instantiation is unreachable.
• Test coverage and results: Test structure and expected-error oracles follow repository conventions, and the pure-text header-hygiene check passed. Per the review-runner instruction, no build or test suite was executed here. At submission time, live GitHub checks show CheckStyle and Clang Formatter passing; BE UT and performance report TeamCity build failed without diagnostics in this runner, and COMPILE remains pending.
• Observability: Existing function-specific exceptions are sufficient for these local validation failures; no new log or metric requirement was found.
• Transactions, persistence, and data writes: No transaction, EditLog, persisted metadata, storage visibility, MoW, or write-atomicity path is changed.
• FE/BE variables: No new variable or thrift field is introduced, so there is no propagation fan-out to update.
• Performance: The added comparisons are bounded by small parameter collections and occur before state mutation; no material CPU, memory, or allocation regression was found.
• Other issues: No additional user review focus was supplied. The TopN zero-expansion report was dismissed as pre-existing serialization behavior rather than a defect introduced by these comparisons.

Please address every inline issue and add the corresponding boundary/path regression coverage.

[github-actions]

[P1] Do not use a valid zero decay as the empty-state marker

A half decay of 0.0 is accepted and explicitly handled by this aggregate, and add() can populate such a state. This return drops that populated source; in the reverse mismatch direction, the following half_decay == 0.0 branch adopts the nonzero configuration instead of rejecting it. The nullable wrapper already tracks whether the aggregate received a non-null row, so use an explicit initialization/emptiness signal rather than a legal parameter value, and add zero/nonzero tests in both directions.

[HappenLee]

After discussing the intended contract, we will preserve the existing zero-half-decay behavior: zero returns 0 and serves as the empty/non-contributing state marker. A serialized zero-half-decay state is intentionally ignored by _merge/_union, even if it was built from non-null rows. Thus 0 versus a nonzero half-decay does not raise a mismatch; the compatibility check applies to contributing states with nonzero half-decays.

I have documented this exception in the function's source comment and a local documentation note, and withdrawn the proposed NaN-sentinel implementation. This is a clarification of the intended contract rather than a change to distinguish an initialized zero configuration. The separate NaN-equality concern remains open for discussion.

The documentation-only change passed formatting, header-hygiene, and whitespace checks. Runtime tests were not rerun. All changes remain local; no code has been pushed.

[github-actions]

[P2] Define equality for every accepted half-decay value

FE accepts any constant numeric half decay, including a NaN double. The first NaN-configured source initializes the destination; merging a second identically configured source reaches this branch and throws because NaN != NaN. Either reject non-finite values at both FE and BE construction boundaries or use equality semantics that are reflexive for every accepted configuration, with direct and serialized identical-NaN coverage.

[HappenLee]

Implemented locally: NaN half-decay is now unsupported. A shared check in ExponentialMovingAverageData::write() and get() throws INVALID_ARGUMENT with exponential_moving_average half decay must not be NaN. This rejects newly produced NaN-configured states before they can be serialized for subsequent merging, and also rejects direct finalization paths.

The check is deliberately at serialization/finalization boundaries rather than in per-row add(). Existing merge() compatibility checks and zero-half-decay semantics are unchanged. Only a NaN half-decay is rejected; this does not prohibit NaN input values/results or change infinity handling. No backward-compatibility handling for previously serialized NaN states is added, as discussed for this unreleased function.

All 8 AggregateStateParametersTest tests passed under ASAN, including checks that both output APIs reject NaN and that zero/one half-decays still work. Added SQL expected-error cases for the ordinary, _state, and _combine functions have not been run. Formatting and header-hygiene checks passed; clang-tidy encountered pre-existing diagnostics in core/types.h and unchanged aggregate code. No code has been pushed; the fixes will be submitted together.

[github-actions]

[P1] Cover the multi-distinct AggState wrapper

This validation runs only when the nested group_concat state is merged. multi_distinct_group_concat_state remains supported, but AggregateFunctionDistinct::merge() unions only its outer argument set and finalization later calls nested add(); it never invokes this method. States with ',' and ';' therefore merge silently and whichever tuple is iterated first selects the separator for all values. Either make that wrapper unsupported for AggState or preserve and validate its separator before unioning, with both operand orders in regression coverage.

[HappenLee]

Thanks for pointing out this gap. The distinct wrapper merges and serializes its outer argument set, then feeds the reconstructed arguments to the nested aggregate via add() during finalization. It does not call the nested group_concat merge(), so the separator check added here does not cover this path.

We will defer this issue to a separate follow-up PR. Preserving support requires the wrapper to retain and validate its configuration before combining argument sets; alternatively, restricting AggState support needs consistent handling across function combinators, DDL, and existing stored states. We do not want to fold that broader change into this PR.

No implementation change for this issue is included here. Leaving this thread unresolved to make the remaining gap explicit.

[github-actions]

[P1] Establish a stable, finite histogram configuration before merge

FE requires neither constant nor finite interval/offset values, while add() overwrites both for every row. A linear_histogram_combine(value, interval_column, offset_column) state can therefore contain buckets computed under several configurations but serialize only the last one; distributed partials then throw or pass here based on their last rows. NaN also passes the existing range predicates, and two identically configured NaN partials reject each other at this raw comparison. Enforce constant/finite parameters or validate them on every add, and cover _combine plus distributed varying-parameter cases.

[HappenLee]

The missing constant/finite parameter validation and the per-row parameter overwrite both predate this PR. We will address these existing linear_histogram parameter-validation issues together in a follow-up change. No additional fix for this issue will be included in this PR; leaving the thread unresolved to track the deferred work.

[github-actions]

[P2] Treat an all-NaN exact-percentile state as identity

add_single_range()/add_many_range() set inited_flag and the quantiles before _append(), but _append() discards every floating NaN. Thus percentile_state(NaN, 0.25) serializes as initialized with zero retained values. Merging it with a contributing 0.75 state now throws here in either order even though the all-NaN side cannot affect the result; the reservoir path already uses retained-sample emptiness for this case. Check values.empty() symmetrically before comparing/adopting levels and add both direct and serialized merge orders.

[HappenLee]

We do not consider this a bug under the intended parameter-compatibility contract. An uninitialized state has no quantile configuration and can be ignored. A state built from all-NaN input is initialized and retains its explicitly configured quantile, even though it has no retained samples. Merging a 0.25-configured state with a 0.75-configured state must therefore reject the mismatch in either operand order, including after serialization. Sample emptiness does not erase an established configuration. We will keep the current exact-percentile check; the reservoir implementation does not define the contract for this function.

[github-actions]

[P2] Reject or consistently compare non-finite reservoir levels

A binary prepared DOUBLE parameter can supply NaN as a DoubleLiteral; the FE check value < 0 || value > 1 admits it, and BE stores the level with the sample. After the first state initializes the destination, an identically configured second state throws here because NaN != NaN. Add finite-value validation at FE and BE boundaries, or define equality for every accepted level, and cover the prepared/direct plus serialized paths.

[HappenLee]

Implemented locally in the FE PercentileReservoir legality check: changed the rejection condition to !(value >= 0 && value <= 1) and added a comment explaining that the old out-of-range comparisons both evaluate to false for NaN. The new condition rejects NaN and infinities while preserving valid levels, including 0 and 1. StateCombinator and CombineCombinator delegate to this nested legality check as well.

This is a scoped FE validation change. No BE validation or handling of previously serialized NaN-configured states is added.

FE Checkstyle passed with zero violations, git diff --check passed, and manual Java predicate checks covered NaN, both infinities, out-of-range values, negative zero, endpoints and an interior value. FE unit tests, SQL regression tests and the binary prepared-statement path were not run. The change is saved in local commit 243c526 and has not been pushed to this PR.

[github-actions]

[P1] Treat eventless sequence states symmetrically

AggregateFunctionSequenceBase::add() initializes the pattern before data.add() discards an all-false event row. Such a state has init_flag == true but an empty events_list: as RHS it is ignored above, while as LHS this branch refuses to adopt a differently configured contributing RHS and the next branch throws. Merge success therefore depends on operand order. If the destination has no stored events, adopt the contributing source's pattern/parser state just as an empty RHS is ignored, and test both sequence_match and sequence_count orders.

[github-actions]

[P1] Validate funnel parameters while building each state

FE checks only the types of window and mode; both V1 and V2 add() overwrite them on every row while retaining events gathered under prior values. Because _combine delegates raw adds, one serialized state can already mix configurations, and ordinary distributed partials may throw or silently merge here according to their final row rather than the full input. Require these parameters to be constant or compare them against the first observed values on every add; apply the same invariant to V2 and cover _combine and distributed varying-parameter inputs.

[HappenLee]

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 a8b018b, together with the previously agreed review follow-ups in this PR update.

[HappenLee]

/review

[github-actions]

Requesting changes for one new correctness gap.

The inline comment covers a configured-eventless identity bug in WindowFunnelStateV2: non-null all-false input records and serializes window/mode, but merge() uses the empty event payload as if the state were truly fresh. Incompatible V2 states, including the default window_funnel alias, are consequently ignored or overwritten in both direct and serialized merge orders. This is distinct from the existing funnel thread about varying parameters within one state.

Critical checkpoint conclusions:

• Task goal and data correctness: compatibility validation is sound for the other reviewed aggregate families and configured contributing paths, but this V2 identity bypass leaves the PR's core configured-state guarantee incomplete.
• Tests and observed results: the added BE helper and regression suite cover direct/serialized failures, reverse order, reset/fresh adoption, aliases, and FE constant checks, but all funnel mismatch cases use a true event and miss configured all-false V2 states. Per the review prompt, I did not run builds or tests. At submission, live Clang Formatter and CheckStyle checks pass; the broader build/test workflows are skipped and the automated review checks remain pending. The PR records the author's local passing results.
• Scope and parallel paths: the change is focused. Ordinary/state/combine/merge/union, nullable, map/foreach, distinct, MV, prepared-expression, distributed, and default-alias paths were traced. The already-known multi_distinct_group_concat bypass remains in its existing thread and was not duplicated.
• Lifecycle, concurrency, and configuration: fresh, reset, all-null, and all-false states were distinguished. The suspected collect reset defect was disproved because aggregate reset destroys and reconstructs its data. No shared mutable/static state, runtime config refresh, or concurrency mechanism is introduced.
• Compatibility and failure behavior: serialized layouts are unchanged and same-configuration states remain readable across versions. Existing comparisons run before payload mutation and INVALID_ARGUMENT propagates through wrappers; V2's early identity branches alone bypass the check. Any fix that changes its state bytes needs backward-compatible/versioned handling.
• Persistence, data writes, observability, and performance: there is no storage/data-write or observability change. Added comparisons are bounded by existing parameter containers and have no material performance or allocation concern.
• Existing review context: existing inline threads were treated as hard duplicate fences. The current head still leaves the already-raised zero-half-decay EMA and multi-distinct group-concat concerns open; they were not reposted.
• User focus and completion: no additional user focus was supplied. The complete PR was reviewed, all candidates were accepted, dismissed, or duplicate-fenced, and the review converged in two rounds on head a8b018b9ef62be2b8f32becc097acf037ba08691.

[hello-stephen]

TPC-H: Total hot run time: 16625 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 58e47d9cc897a170041b5e48642b076e1e476bdb, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17641	3013	2973	2973
q2	2067	253	226	226
q3	10268	883	522	522
q4	4669	246	200	200
q5	7677	565	381	381
q6	134	116	94	94
q7	520	493	385	385
q8	9231	841	951	841
q9	3407	2384	2373	2373
q10	6516	843	727	727
q11	413	199	178	178
q12	618	249	199	199
q13	18127	1525	1145	1145
q14	154	146	138	138
q15	q16	428	398	371	371
q17	1384	898	872	872
q18	3073	2212	2210	2210
q19	1272	845	762	762
q20	375	286	196	196
q21	5685	1606	1847	1606
q22	334	267	226	226
Total cold run time: 93993 ms
Total hot run time: 16625 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3337	3299	3293	3293
q2	500	387	370	370
q3	2206	2379	2243	2243
q4	1167	1142	881	881
q5	2166	2087	2126	2087
q6	167	117	85	85
q7	1039	921	849	849
q8	1571	1362	1379	1362
q9	3089	3029	3029	3029
q10	1836	1789	1646	1646
q11	345	264	246	246
q12	443	421	335	335
q13	1473	1551	1139	1139
q14	169	175	163	163
q15	q16	392	392	357	357
q17	3580	3328	3322	3322
q18	4728	4336	4663	4336
q19	873	802	830	802
q20	1037	957	825	825
q21	3836	3101	3351	3101
q22	420	345	312	312
Total cold run time: 34374 ms
Total hot run time: 30783 ms

[hello-stephen]

TPC-DS: Total hot run time: 81454 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 58e47d9cc897a170041b5e48642b076e1e476bdb, data reload: false

query5	4249	415	343	343
query6	378	133	127	127
query7	4967	434	227	227
query8	291	118	125	118
query9	8692	2866	2882	2866
query10	391	209	184	184
query11	5391	1056	912	912
query12	122	70	68	68
query13	1202	416	321	321
query14	6115	2195	2067	2067
query14_1	1959	1953	1934	1934
query15	176	123	113	113
query16	903	366	375	366
query17	809	465	368	368
query18	2342	328	242	242
query19	168	141	108	108
query20	72	71	74	71
query21	202	100	86	86
query22	5434	5425	5310	5310
query23	6744	6286	6048	6048
query23_1	5975	5965	6190	5965
query24	7311	1072	746	746
query24_1	757	781	769	769
query25	436	295	252	252
query26	1232	233	138	138
query27	2784	411	246	246
query28	4708	1509	1505	1505
query29	945	444	350	350
query30	256	157	129	129
query31	813	399	327	327
query32	148	81	75	75
query33	464	218	179	179
query34	997	809	468	468
query35	402	400	348	348
query36	589	550	505	505
query37	123	81	69	69
query38	992	840	803	803
query39	479	485	478	478
query39_1	479	471	461	461
query40	204	92	79	79
query41	59	57	56	56
query42	74	72	72	72
query43	238	239	209	209
query44	1012	547	535	535
query45	117	115	106	106
query46	759	807	536	536
query47	747	743	756	743
query48	299	315	226	226
query49	532	240	180	180
query50	743	248	191	191
query51	8064	7887	7871	7871
query52	65	66	60	60
query53	191	195	143	143
query54	195	149	158	149
query55	71	61	57	57
query56	204	175	190	175
query57	691	676	717	676
query58	194	160	164	160
query59	1190	1214	1085	1085
query60	243	173	168	168
query61	101	108	120	108
query62	358	203	168	168
query63	170	142	137	137
query64	2743	670	573	573
query65	1725	1599	1582	1582
query66	1892	310	199	199
query67	10045	9673	9752	9673
query68	2986	1237	736	736
query69	348	229	192	192
query70	675	619	635	619
query71	243	180	163	163
query72	2282	1668	1473	1473
query73	644	640	346	346
query74	1997	1227	1132	1132
query75	1191	1081	943	943
query76	2389	718	523	523
query77	269	250	199	199
query78	3997	3597	3208	3208
query79	2741	842	603	603
query80	1573	318	260	260
query81	526	157	132	132
query82	737	132	96	96
query83	288	212	188	188
query84	294	110	84	84
query85	855	337	291	291
query86	473	174	161	161
query87	1013	958	898	898
query88	3323	2114	2070	2070
query89	296	195	174	174
query90	2201	134	124	124
query91	128	115	101	101
query92	97	61	71	61
query93	3319	1150	689	689
query94	646	258	228	228
query95	518	239	306	239
query96	772	576	266	266
query97	1054	1048	1035	1035
query98	168	134	130	130
query99	413	337	310	310
Total cold run time: 181295 ms
Total hot run time: 81454 ms

[hello-stephen]

ClickBench: Total hot run time: 14.74 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 58e47d9cc897a170041b5e48642b076e1e476bdb, data reload: false

query1	0.00	0.01	0.01
query2	0.08	0.04	0.04
query3	0.25	0.12	0.11
query4	1.60	0.10	0.09
query5	0.18	0.16	0.15
query6	1.26	0.69	0.68
query7	0.04	0.01	0.00
query8	0.05	0.03	0.03
query9	0.29	0.21	0.22
query10	0.36	0.34	0.35
query11	0.17	0.13	0.12
query12	0.15	0.12	0.12
query13	0.30	0.32	0.31
query14	0.43	0.44	0.44
query15	0.36	0.34	0.34
query16	0.22	0.23	0.23
query17	0.69	0.68	0.66
query18	0.18	0.18	0.17
query19	1.10	1.11	1.14
query20	0.02	0.01	0.01
query21	15.48	0.17	0.12
query22	5.08	0.04	0.04
query23	16.17	0.25	0.10
query24	3.03	0.30	0.27
query25	0.11	0.05	0.05
query26	0.81	0.18	0.14
query27	0.04	0.03	0.03
query28	3.60	0.55	0.27
query29	12.50	3.21	2.58
query30	0.29	0.13	0.14
query31	2.80	0.36	0.16
query32	3.54	0.33	0.23
query33	1.46	1.47	1.43
query34	15.40	2.24	1.79
query35	1.76	1.69	1.70
query36	0.46	0.30	0.29
query37	0.06	0.04	0.04
query38	0.05	0.04	0.03
query39	0.03	0.03	0.03
query40	0.12	0.08	0.08
query41	0.09	0.03	0.02
query42	0.04	0.02	0.03
query43	0.04	0.03	0.03
Total cold run time: 90.69 s
Total hot run time: 14.74 s

[HappenLee]

/review

[github-actions]

Requesting changes on exact head 21892a505bf8789c96f101531c7cfb6c9d55364c. I found four new issues: a finite-level reservoir state loses its configuration when every sample is NaN, and legacy reset states for Window Funnel V2, Histogram, and TopN acquire incompatible configured-state semantics on a new reader. The review converged after three complete normal/risk rounds; no user-provided focus narrowed the review.

Two unresolved blockers already have inline threads and are intentionally not duplicated: EMA zero half-decay identity and multi-distinct group-concat bypass.

Checkpoint conclusions:

• Goal and proof: The compatibility goal is only partially met. Added compatible/mismatch tests cover many ordinary cases, but they miss finite-level/all-NaN reservoir states and old-writer add-then-reset bytes consumed by a new reader.
• Scope: The 23-file change is broad but coherent around AggState parameter validation, FE legality, tests, and EMA documentation. No unrelated source change was identified.
• Concurrency and locking: No threads, shared synchronization, or lock protocols change. Aggregate state ownership remains engine-controlled; the defects are lifecycle/wire-compatibility issues, not data races.
• Lifecycle and static initialization: Reset, serialization, deserialization, merge, and analytic empty-frame lifecycles were traced. The three legacy-reset findings are lifecycle defects. No new static/global initialization or lifetime cycle appears.
• Configuration items: No Doris configuration item or dynamic configuration behavior changes.
• Compatibility and rolling upgrades: Nominal field layouts remain unchanged, and sequence/V2 framing is otherwise preserved, but unchanged Histogram/TopN headers and inferred V2 initialization reinterpret old reset identities; these are rolling-upgrade blockers.
• Parallel paths: Ordinary, _state, _combine, _merge, _union, analytic, nullable, batch/vector, alias, scalar/array/weighted, V1/V2, bitmap, and DISTINCT-related paths were checked. The known multi-distinct group-concat bypass remains covered by its existing thread; no additional bypass survived review.
• Special conditions and similar paths: Fresh, configured-empty, contributing, reset, sentinel-colliding, NaN/infinity, empty-array, zero-weight/capacity, and both merge orders were checked. The reservoir payload-empty test is the remaining distinct sentinel/configuration defect.
• Test coverage and correctness: BE/FE/regression additions exercise broad positive and negative behavior, but lack the four exact triggers in the inline findings. No reviewer build or test was run because the review prompt prohibits it. The author reports an ASAN build, 58 related unit tests, and three regression suites passing. Current CI has one CheckStyle failure caused by JUnit 4 imports in fe/fe-core/src/test/java/org/apache/doris/cloud/rpc/VersionHelperTest.java, outside this PR's authoritative changed-file list, so it is not a finding against this patch.
• Observability: Compatibility failures are deterministic exceptions with adequate parameter context where applicable; no new metrics or logging are required.
• Transactions, persistence, and data writes: No transaction protocol, catalog persistence, storage write, atomicity, or crash-recovery path changes. Persisted AggState bytes are in scope and are the source of the three mixed-version findings.
• FE/BE transmission: No new Thrift/session variable or cross-process configuration field is introduced. FE legality and BE function/state behavior were checked for naming/type consistency.
• Performance: New checks are bounded constant-time comparisons except existing-size bitmap key membership; no material performance regression was identified.
• Other: Validation generally occurs before payload mutation and wrapper exception propagation/null suppression are preserved, apart from the reservoir fast-path issue described inline. No security-sensitive behavior is in scope.

[github-actions]

[P2] Preserve finite configuration for all-NaN reservoir states

add() records input_level before ReservoirSampler::insert() drops NaN, and serialization still writes that finite level, so an all-NaN state is configured even though data.empty() is true. This return silently ignores an incompatible configured RHS; in the reverse order the empty-destination branch overwrites its level. That contradicts the configured-state contract already applied to exact percentile and eventless funnel states, and it affects direct plus serialized merge/union. Please track configuration separately from retained samples, compare levels before the payload-empty fast paths, and cover valid finite levels with all-NaN samples in both operand orders.

[github-actions]

[P1] Preserve identity semantics for legacy reset states

Before this change, V2 reset() cleared only events_list/sorted, so an old BE can serialize a reset identity while stale window/mode values remain in its 0/1 header. This is production-reachable with window_funnel_v2_union(stored_state) OVER (... ROWS ...): the analytic path resets before an empty frame and the non-null _union result serializes the nested state. This inference marks those old reset bytes initialized, so a new BE can spuriously reject a different valid configuration. Legacy tag-0/1 empty states had identity semantics under the old merge logic and are indistinguishable from old all-false states; preserve that legacy meaning (using tag 2 only for new configured-empty states), and add an old add-then-reset writer/new-reader test.

[github-actions]

[P1] Treat legacy reset histograms as identities

Before this PR, reset() cleared only ordered_map, so an old BE can serialize a reset state as a nonzero bucket count followed by zero elements. This is reachable through histogram_union(stored_state) OVER (... ROWS ...): the analytic path resets before an empty frame and serializes the non-null _union result. A new BE then treats the stale count as configuration here and throws against a different valid Histogram state in either operand order, although the reset state has no buckets. Since every configured non-null Histogram add inserts an entry, recognize the legacy nonzero-count/zero-element encoding as reset identity before comparing parameters, and add old add-then-reset writer/new-reader coverage.

[github-actions]

[P1] Define compatibility for legacy reset TopN states

Old reset() cleared only counter_map, so an old-BE analytic topn*_union empty frame can serialize a reset identity with stale nonzero N/capacity. This new check treats those bytes as configured and can reject a different valid state in either order. Payload emptiness is not enough to repair it: accepted inputs such as topn_array_state(1, 1, 0) establish N=1/capacity=0 but serialize zero retained elements too. Please add old add-then-reset writer/new-reader tests for all TopN variants and introduce a versioned/tagged initialization policy (or another explicit rolling-upgrade rule) that distinguishes reset identities from legitimately configured empty states.