Skip to content

[fix](local shuffle) Keep local exchanges before pre-aggregations over key-breaking children - #70296

Merged
morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t04c-agg-local-child-breaks
Nov 5, 2026
Merged

morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t04c-agg-local-child-breaks

Conversation

@morningman

Copy link
Copy Markdown
Contributor

Background

On master the FE plans the local exchanges that redistribute rows among the pipeline tasks of one BE (enable_local_shuffle_planner, on by default). Each plan node follows the rule of the BE operator it becomes, required_data_distribution(), which BE applies itself when the FE planner is off. enable_local_exchange_before_agg (default true) controls the exchange before a LOCAL (first-phase) aggregation. Since #63766, BE skips it with the switch off only while the child keeps the grouping-key distribution; a child breaks it when it is serial or asks for a non-hash local exchange itself, such as the probe side of a nested loop join or a table function.

What problem does this PR solve?

Issue Number: None
Related PR: #63366 (introduced the FE rule without the child check), #63766 (the BE rule), #70177 (the same fix for streaming aggregation; this PR reuses its childBreaksLocalKeyDistribution()), #62438 (added the switch)
PR Type: Bugfix

Problem Summary:

With enable_local_exchange_before_agg=false, the FE planner dropped the local exchange below a LOCAL AggSink and below a streaming distinct pre-aggregation whatever their child was, while BE keeps it after a child that breaks the key distribution. Queries planned by the FE therefore got fewer local exchanges than BE planning gives, for example no hash exchange between a nested loop join and the pre-aggregation above it. This PR adds BE's child check to both cases. Results do not change; most of the diff is tests.

Details

  • Change map: production code +15 / -7 in AggregationNode (two conditions and their comments) and a PlanNode javadoc; tests +345 / -34: cases in LocalShuffleNodeCoverageTest and LocalExchangePlannerTest, and the new regression suite nereids_p0/local_shuffle/test_local_exchange_before_agg_breaking_child with its generated .out (+169).
  • Frequency: Always, when enable_local_exchange_before_agg=false (not the default; P0 randomizes it) and a LOCAL aggregation sits on a child that breaks the key distribution. Only plans and speed change: the FE always requires hash input at FIRST_MERGE and at the non-streaming DISTINCT dedup (a deliberate divergence from BE), so results stay right.
  • Root cause: AggregationNode.enforceAndDeriveLocalExchange() mirrors the BE operators, but its DistinctStreamingAgg and AggSink LOCAL branches fall back to the base class on !enableLeBeforeAgg alone, without BE's child_breaks_local_key_distribution(). [refactor](local shuffle) Move local exchange planning from BE to FE #63366 wrote them after [fix](be) Preserve agg hash shuffle after non-hash exchange #63766 had merged, from the rule before it.
  • Why existing tests missed it: Unit tests: the two "LE disabled" cases in LocalShuffleNodeCoverageTest use a child that keeps the key distribution, no case reached the DistinctStreamingAgg branch, and LocalExchangePlannerTest had no plan with the switch off. Regression: query_p0/join/test_agg_after_nested_loop_join_local_exchange ([fix](be) Preserve agg hash shuffle after non-hash exchange #63766's suite) and nereids_p0/local_shuffle/test_enable_local_exchange_before_agg compare only results, which were right, and no .out shows local exchanges.
  • Fix: Both branches fall back to the base class only when childBreaksLocalKeyDistribution() is false. After a key-breaking child, the AggSink LOCAL phase goes on to its hash requirement, and the streaming distinct pre-aggregation to enable_distinct_streaming_agg_force_passthrough (PASSTHROUGH, else the base class), as on BE; it never asks for hash.
  • Other paths and branches: Streaming aggregation got the check in [fix](fe) Honor enable_local_exchange_before_streaming_agg in the FE local shuffle planner #70177; BE needs no change. One known difference from BE planning remains, with the switch on or off: the FE lets an ADAPTIVE_PASSTHROUGH or PASSTHROUGH child output satisfy a PASSTHROUGH requirement, where BE Pipeline::need_to_local_exchange() always adds one (last row of the table in Testing). branch-4.1 and branch-4.2 have no FE local shuffle planner and are not affected.

Testing

  • LocalShuffleNodeCoverageTest and LocalExchangePlannerTest, two new cases each: all four fail without the fix (for example, a serial child got PASSTHROUGH instead of LOCAL_EXECUTION_HASH_SHUFFLE) and pass with it. The 12 fe-core test classes that plan local exchanges or print distributed plans pass (168 tests). Run locally.
  • A planner probe over 288 plans (18 query shapes × pooling on and off × agg_phase 0 and 2 × disable_streaming_preaggregations on and off × the switch): only 32 with the switch off change. 30 of them now equal the plan with the switch on; the other 2 keep the distribution of a UNION ALL, as the switch asks.
  • Local exchanges in the query profiles on a local 1 FE + 1 BE cluster (master 4530f761fa1; doris-fe.jar from 7c4b4462dcc with [fix](fe) Honor enable_local_exchange_before_streaming_agg in the FE local shuffle planner #70177, and for "after" with only AggregationNode.class replaced), switch off, parallel_pipeline_task_num=4, agg_phase=2. The AggSink rows set disable_streaming_preaggregations=true; the distinct rows are a GROUP BY without aggregate functions:
Query BE planner FE planner before FE planner after
LOCAL AggSink over a nested loop join ADAPTIVE_PASSTHROUGH, LOCAL_HASH_SHUFFLE ADAPTIVE_PASSTHROUGH ADAPTIVE_PASSTHROUGH, LOCAL_HASH_SHUFFLE
LOCAL AggSink over explode PASSTHROUGH, LOCAL_HASH_SHUFFLE PASSTHROUGH PASSTHROUGH, LOCAL_HASH_SHUFFLE
LOCAL AggSink over a one-tablet scan (serial) PASSTHROUGH, LOCAL_HASH_SHUFFLE PASSTHROUGH PASSTHROUGH, LOCAL_HASH_SHUFFLE
LOCAL AggSink over UNION ALL none none none
distinct pre-aggregation over explode PASSTHROUGH ×2 PASSTHROUGH PASSTHROUGH ×2
distinct pre-aggregation over a nested loop join ADAPTIVE_PASSTHROUGH, PASSTHROUGH ADAPTIVE_PASSTHROUGH ADAPTIVE_PASSTHROUGH
  • The new regression suite fails on the FE without the fix at its first plan check (nlj: [VNESTED]) and passes with it; its 16 result blocks (4 queries × both planners × the switch on and off) match results computed independently from the inserted rows. nereids_p0/local_shuffle (11 suites), query_p0/join/test_agg_after_nested_loop_join_local_exchange and nereids_syntax_p0/agg_4_phase pass on the same cluster with the fix.

Notes for Reviewers

Release note

None

Check List (For Author)

  • Behavior changed: No
  • Does this need documentation: No
  • Pre-submission status: Ready

AI Disclosure

  • Tool: Claude Code (Claude Opus 5.5)
  • Generated: the fix, the unit and regression tests (the .out generated by the framework), and this description
  • Human review: not stated
  • Prompts: The author asked to continue moving branch-4.2's changes back to master, where this follow-up of [fix](fe) Honor enable_local_exchange_before_streaming_agg in the FE local shuffle planner #70177 was recorded together with the outdated PlanNode javadoc, and chose to verify it on a local cluster before opening the PR.

…r key-breaking children

### Background

On master the FE plans the local exchanges that redistribute rows among the pipeline tasks of one BE (`enable_local_shuffle_planner`, on by default). Each plan node follows the rule of the BE operator it becomes, `required_data_distribution()`, which BE applies itself when the FE planner is off. `enable_local_exchange_before_agg` (default true) controls the exchange before a LOCAL (first-phase) aggregation. Since apache#63766, BE skips it with the switch off only while the child keeps the grouping-key distribution; a child breaks it when it is serial or asks for a non-hash local exchange itself, such as the probe side of a nested loop join or a table function.

### What problem does this PR solve?

Issue Number: None
Related PR: apache#63366 (introduced the FE rule without the child check), apache#63766 (the BE rule), apache#70177 (the same fix for streaming aggregation; this PR reuses its `childBreaksLocalKeyDistribution()`), apache#62438 (added the switch)
PR Type: Bugfix

Problem Summary:

With `enable_local_exchange_before_agg=false`, the FE planner dropped the local exchange below a LOCAL `AggSink` and below a streaming distinct pre-aggregation whatever their child was, while BE keeps it after a child that breaks the key distribution. Queries planned by the FE therefore got fewer local exchanges than BE planning gives, for example no hash exchange between a nested loop join and the pre-aggregation above it. This PR adds BE's child check to both cases. Results do not change; most of the diff is tests.

### Details

- **Change map:** production code +15 / -7 in `AggregationNode` (two conditions and their comments) and a `PlanNode` javadoc; tests +345 / -34: cases in `LocalShuffleNodeCoverageTest` and `LocalExchangePlannerTest`, and the new regression suite `nereids_p0/local_shuffle/test_local_exchange_before_agg_breaking_child` with its generated `.out` (+169).
- **Frequency:** Always, when `enable_local_exchange_before_agg=false` (not the default; P0 randomizes it) and a LOCAL aggregation sits on a child that breaks the key distribution. Only plans and speed change: the FE always requires hash input at FIRST_MERGE and at the non-streaming DISTINCT dedup (a deliberate divergence from BE), so results stay right.
- **Root cause:** `AggregationNode.enforceAndDeriveLocalExchange()` mirrors the BE operators, but its DistinctStreamingAgg and AggSink LOCAL branches fall back to the base class on `!enableLeBeforeAgg` alone, without BE's `child_breaks_local_key_distribution()`. apache#63366 wrote them after apache#63766 had merged, from the rule before it.
- **Why existing tests missed it:** Unit tests: the two "LE disabled" cases in `LocalShuffleNodeCoverageTest` use a child that keeps the key distribution, no case reached the DistinctStreamingAgg branch, and `LocalExchangePlannerTest` had no plan with the switch off. Regression: `query_p0/join/test_agg_after_nested_loop_join_local_exchange` (apache#63766's suite) and `nereids_p0/local_shuffle/test_enable_local_exchange_before_agg` compare only results, which were right, and no `.out` shows local exchanges.
- **Fix:** Both branches fall back to the base class only when `childBreaksLocalKeyDistribution()` is false. After a key-breaking child, the AggSink LOCAL phase goes on to its hash requirement, and the streaming distinct pre-aggregation to `enable_distinct_streaming_agg_force_passthrough` (PASSTHROUGH, else the base class), as on BE; it never asks for hash.
- **Other paths and branches:** Streaming aggregation got the check in apache#70177; BE needs no change. One known difference from BE planning remains, with the switch on or off: the FE lets an ADAPTIVE_PASSTHROUGH or PASSTHROUGH child output satisfy a PASSTHROUGH requirement, where BE `Pipeline::need_to_local_exchange()` always adds one (last row of the table in Testing). branch-4.1 and branch-4.2 have no FE local shuffle planner and are not affected.

### Testing

- `LocalShuffleNodeCoverageTest` and `LocalExchangePlannerTest`, two new cases each: all four fail without the fix (for example, a serial child got `PASSTHROUGH` instead of `LOCAL_EXECUTION_HASH_SHUFFLE`) and pass with it. The 12 fe-core test classes that plan local exchanges or print distributed plans pass (168 tests). Run locally.
- A planner probe over 288 plans (18 query shapes × pooling on and off × `agg_phase` 0 and 2 × `disable_streaming_preaggregations` on and off × the switch): only 32 with the switch off change. 30 of them now equal the plan with the switch on; the other 2 keep the distribution of a `UNION ALL`, as the switch asks.
- Local exchanges in the query profiles on a local 1 FE + 1 BE cluster (master `4530f761fa1`; `doris-fe.jar` from `7c4b4462dcc` with apache#70177, and for "after" with only `AggregationNode.class` replaced), switch off, `parallel_pipeline_task_num=4`, `agg_phase=2`. The `AggSink` rows set `disable_streaming_preaggregations=true`; the distinct rows are a `GROUP BY` without aggregate functions:

| Query | BE planner | FE planner before | FE planner after |
|---|---|---|---|
| LOCAL `AggSink` over a nested loop join | ADAPTIVE_PASSTHROUGH, LOCAL_HASH_SHUFFLE | ADAPTIVE_PASSTHROUGH | ADAPTIVE_PASSTHROUGH, LOCAL_HASH_SHUFFLE |
| LOCAL `AggSink` over `explode` | PASSTHROUGH, LOCAL_HASH_SHUFFLE | PASSTHROUGH | PASSTHROUGH, LOCAL_HASH_SHUFFLE |
| LOCAL `AggSink` over a one-tablet scan (serial) | PASSTHROUGH, LOCAL_HASH_SHUFFLE | PASSTHROUGH | PASSTHROUGH, LOCAL_HASH_SHUFFLE |
| LOCAL `AggSink` over `UNION ALL` | none | none | none |
| distinct pre-aggregation over `explode` | PASSTHROUGH ×2 | PASSTHROUGH | PASSTHROUGH ×2 |
| distinct pre-aggregation over a nested loop join | ADAPTIVE_PASSTHROUGH, PASSTHROUGH | ADAPTIVE_PASSTHROUGH | ADAPTIVE_PASSTHROUGH |

- The new regression suite fails on the FE without the fix at its first plan check (`nlj: [VNESTED]`) and passes with it; its 16 result blocks (4 queries × both planners × the switch on and off) match results computed independently from the inserted rows. `nereids_p0/local_shuffle` (11 suites), `query_p0/join/test_agg_after_nested_loop_join_local_exchange` and `nereids_syntax_p0/agg_4_phase` pass on the same cluster with the fix.

### Notes for Reviewers

- `baseClassRequire()` stays in both branches although only a non-serial child reaches them now, since a serial child always breaks the key distribution. It keeps the branches shaped like BE's call to the base class, as apache#70177 does for streaming aggregation.
- In `LocalShuffleNodeCoverageTest`, apache#70177's five key-breaking children moved into `childrenThatBreakKeyDistribution()` and its aggregate builder into `buildLocalPhaseAgg()`, so the new cases reuse them; apache#70177's case is otherwise unchanged.
- The `PlanNode#requiresShuffleForCorrectness` javadoc still named `enable_local_exchange_before_agg` for streaming aggregation, which reads `enable_local_exchange_before_streaming_agg` since apache#70177.

### Release note

None

### Check List (For Author)

- Behavior changed: No
- Does this need documentation: No
- Pre-submission status: Ready

### AI Disclosure

- Tool: Claude Code (Claude Opus 5.5)
- Generated: the fix, the unit and regression tests (the `.out` generated by the framework), and this description
- Human review: not stated
- Prompts: The author asked to continue moving branch-4.2's changes back to master, where this follow-up of apache#70177 was recorded together with the outdated `PlanNode` javadoc, and chose to verify it on a local cluster before opening the PR.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@hello-stephen

Copy link
Copy Markdown
Contributor

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?

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@morningman

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all six changed files at 60bcdca1e807049183f085298a9ff071e13a3cc2, including the upstream aggregate translation and downstream BE operator/local-exchange construction. No verified new actionable findings. No existing inline findings require carrying forward. No additional user focus was supplied.

Critical checkpoint conclusions:

  1. Goal and proof: the two new guards restore the BE child-distribution exception for disabled-switch LOCAL AggSink and streaming-distinct preaggregation. Added node, planner, and regression assertions target the changed behavior; runtime success was not independently verified.
  2. Scope and reuse: two conditions reuse the existing child predicate; the accompanying javadoc and test helper extraction are focused.
  3. Concurrency and locks: this is a synchronous per-query planning pass before plan serialization; no new thread, shared mutable state, lock, blocking I/O, or lock-order change.
  4. Lifecycle and initialization: no new resource ownership, cleanup obligation, or static-initialization dependency.
  5. Configuration: existing per-session switches are reused; no configuration is added or changed in mutability/propagation.
  6. Compatibility: no function-signature, storage-format, or wire-schema change; existing local-exchange types and serialization are used.
  7. Thrift/protobuf IDs: no gensrc definitions change.
  8. Parallel paths: regular StreamingAgg already uses the predicate; both remaining affected FE branches are updated and compared with BE. Existing FE acceptance of PASSTHROUGH/ADAPTIVE_PASSTHROUGH remains a documented preexisting difference, so this patch does not make every FE/BE plan shape identical.
  9. Conditions and special cases: child serial flags, NLJ join-kind exclusions, forced broadcast passthrough, table functions, and AssertNumRows match the corresponding BE rules. Finalize, FIRST_MERGE, non-streaming dedup, and empty-effective-key paths retain their requirements. Hash-key selection, explicit hash flavors, serial fanout, and correctness-ancestor propagation were traced without finding a new defect.
  10. Test coverage: unit cases cover five key-breaking child categories and both distinct force-passthrough values; planner cases cover NLJ, table function, and serial scan. Regression cases check changed shapes plus key-preserving UNION and compare four query shapes across both planners and both switch values. Existing negative/exclusion cases remain; sorted output, drop-before table lifecycle, and session resets were inspected.
  11. Changed results: all 16 golden blocks and their matrix labels were independently recomputed from the inserted rows and checked against the framework's lexical ordering; they match. The author reports framework generation, but generation provenance was not independently verified.
  12. Observability: existing EXPLAIN DISTRIBUTED PLAN exchange types and BE profiles expose the affected plan decisions; no additional instrumentation is needed for this scoped change.
  13. Transactions and persistence: no EditLog, replay, commit, visible-version, or delete-bitmap behavior changes.
  14. Writes and atomicity: production changes affect query planning only; no data-write or crash-recovery path changes.
  15. FE/BE variable passing: no new transmitted variable or sending point.
  16. Performance: the added planner checks and local redistributions implement the existing BE choice after key-breaking children; retained hash output can satisfy the hash requirement. No new execution hot loop or substantiated avoidable exchange was identified; performance was not measured.
  17. Other issues: production, test, and additional risk reviewers completed their scopes and final missed-case rechecks without a substantiated new finding. No unresolved static candidate or uncovered changed file remains.

Static review only: no builds, Doris tests, cluster runs, benchmarks, source edits, or submodule initialization were performed. Author-reported execution results were inspected as claims, not reproduced. Convergence is limited to this static review scope.

Review converged after 1 round(s).

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28618 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 60bcdca1e807049183f085298a9ff071e13a3cc2, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17583	5091	5063	5063
q2	2151	303	239	239
q3	10282	1374	807	807
q4	4679	459	337	337
q5	7434	792	527	527
q6	178	181	154	154
q7	741	790	596	596
q8	9254	1333	1074	1074
q9	5666	4187	4151	4151
q10	6818	1285	1016	1016
q11	429	242	222	222
q12	632	398	272	272
q13	18137	3619	3069	3069
q14	269	268	253	253
q15	q16	727	727	642	642
q17	1191	754	579	579
q18	7018	6227	6100	6100
q19	1247	966	597	597
q20	398	333	229	229
q21	5907	2447	2418	2418
q22	387	321	273	273
Total cold run time: 101128 ms
Total hot run time: 28618 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5987	5905	6087	5905
q2	710	608	590	590
q3	5576	5732	5362	5362
q4	2015	2158	1546	1546
q5	5817	5784	5697	5697
q6	242	192	165	165
q7	2243	2031	1784	1784
q8	2943	2620	2621	2620
q9	8018	7653	7521	7521
q10	3651	3551	3131	3131
q11	573	404	380	380
q12	677	654	491	491
q13	3253	3622	3053	3053
q14	276	288	268	268
q15	q16	649	683	616	616
q17	7668	7030	6879	6879
q18	12917	12174	12959	12174
q19	886	765	784	765
q20	2158	2141	1936	1936
q21	5608	4587	4641	4587
q22	493	445	420	420
Total cold run time: 72360 ms
Total hot run time: 65890 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 153522 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 60bcdca1e807049183f085298a9ff071e13a3cc2, data reload: false

query5	4309	585	443	443
query6	428	205	187	187
query7	4789	445	242	242
query8	325	176	155	155
query9	8803	3178	3198	3178
query10	471	299	247	247
query11	6055	3861	3548	3548
query12	143	91	87	87
query13	1253	428	322	322
query14	6154	3941	3638	3638
query14_1	3373	3390	3382	3382
query15	211	198	176	176
query16	932	498	429	429
query17	857	650	557	557
query18	2411	432	318	318
query19	196	170	144	144
query20	84	86	82	82
query21	209	127	108	108
query22	12967	12916	12802	12802
query23	13517	12872	12461	12461
query23_1	12527	12666	12617	12617
query24	7402	818	478	478
query24_1	495	487	488	487
query25	538	420	374	374
query26	1258	255	143	143
query27	2767	446	278	278
query28	4626	2045	2031	2031
query29	1595	614	479	479
query30	285	217	185	185
query31	996	926	791	791
query32	135	89	91	89
query33	510	275	231	231
query34	891	843	499	499
query35	724	749	634	634
query36	817	835	742	742
query37	124	93	78	78
query38	1747	1736	1663	1663
query39	782	782	745	745
query39_1	737	728	738	728
query40	213	109	93	93
query41	67	63	64	63
query42	84	81	81	81
query43	342	355	308	308
query44	1301	696	699	696
query45	180	176	169	169
query46	840	957	565	565
query47	2871	3041	2574	2574
query48	291	299	231	231
query49	588	394	283	283
query50	665	286	217	217
query51	10829	10402	10359	10359
query52	81	78	68	68
query53	190	206	148	148
query54	244	187	177	177
query55	73	65	63	63
query56	243	217	197	197
query57	1660	1605	1586	1586
query58	286	259	255	255
query59	2174	2208	2003	2003
query60	300	276	227	227
query61	145	146	142	142
query62	378	329	279	279
query63	186	155	156	155
query64	2723	919	725	725
query65	3320	3275	3253	3253
query66	1768	417	290	290
query67	22780	22692	22508	22508
query68	3192	994	611	611
query69	396	290	246	246
query70	835	791	779	779
query71	310	244	214	214
query72	2818	2610	2217	2217
query73	534	539	292	292
query74	4635	4508	4351	4351
query75	2346	2290	1911	1911
query76	2403	996	579	579
query77	339	364	266	266
query78	9808	9771	9315	9315
query79	1044	863	542	542
query80	525	418	363	363
query81	534	309	266	266
query82	581	141	107	107
query83	309	207	188	188
query84	307	126	114	114
query85	896	518	368	368
query86	316	238	226	226
query87	1987	1962	1805	1805
query88	3804	2810	2793	2793
query89	335	285	240	240
query90	1868	168	163	163
query91	164	151	129	129
query92	101	88	92	88
query93	1014	1010	578	578
query94	444	312	256	256
query95	563	385	299	299
query96	684	487	243	243
query97	2433	2430	2346	2346
query98	165	167	155	155
query99	770	758	653	653
Total cold run time: 237450 ms
Total hot run time: 153522 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.41 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 60bcdca1e807049183f085298a9ff071e13a3cc2, data reload: false

query1	0.01	0.00	0.01
query2	0.09	0.04	0.05
query3	0.26	0.09	0.09
query4	1.61	0.08	0.10
query5	0.28	0.25	0.24
query6	1.17	0.67	0.69
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.40	0.36	0.35
query10	0.60	0.58	0.58
query11	0.20	0.14	0.13
query12	0.18	0.15	0.15
query13	0.49	0.48	0.49
query14	1.06	1.01	1.01
query15	0.65	0.61	0.60
query16	0.35	0.36	0.35
query17	1.15	1.13	1.17
query18	0.19	0.21	0.19
query19	2.10	1.96	1.97
query20	0.02	0.02	0.01
query21	15.49	0.19	0.13
query22	4.94	0.06	0.05
query23	16.28	0.33	0.12
query24	2.83	0.62	0.46
query25	0.10	0.06	0.04
query26	1.36	0.20	0.16
query27	0.05	0.05	0.05
query28	3.39	0.81	0.37
query29	12.49	4.18	3.24
query30	0.28	0.13	0.11
query31	2.77	0.58	0.33
query32	3.21	0.60	0.50
query33	3.18	3.25	3.24
query34	15.38	3.98	3.37
query35	3.34	3.34	3.33
query36	0.55	0.46	0.44
query37	0.10	0.07	0.07
query38	0.04	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.14	0.16
query41	0.08	0.03	0.03
query42	0.03	0.02	0.02
query43	0.04	0.04	0.03
Total cold run time: 97.05 s
Total hot run time: 24.41 s

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 75.00% (3/4) 🎉
Increment coverage report
Complete coverage report

@924060929 924060929 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch restores the child-distribution exception when local exchanges before preaggregation are disabled: LOCAL AggSink retains its hash requirement after a distribution-breaking child, while streaming distinct preaggregation follows its passthrough setting.

Independent static review of all six changed files at 60bcdca found no actionable P0/P1/P2 regression. I traced aggregate translation, exchange enforcement, hash-key selection, serial boundaries, serialization, and the necessary BE consumers. Existing correctness requirements remain intact, and the tests cover changed shapes and key-preserving cases. APPROVE within the FE review scope and necessary FE/BE boundaries.

No builds or tests were run. The supplied current-head CI snapshot reports successful P0, FE UT, and compilation checks, but failed FE coverage. Merge requirements were not verified.

Automated FE-focused static review (gpt-6.1-sol, high). No build or tests were run.

@morningman
morningman merged commit 52362a4 into apache:master Nov 5, 2026
37 of 38 checks passed
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. backport/4.2-master reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants