Skip to content

[fix](fe) Honor enable_local_exchange_before_streaming_agg in the FE local shuffle planner - #70177

Merged
morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t04a-streaming-agg-local-exchange
Nov 5, 2026
Merged

morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t04a-streaming-agg-local-exchange

Conversation

@morningman

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #66216, #66222, #63766

Problem Summary:

In short. This brings the FE local shuffle planner in line with enable_local_exchange_before_streaming_agg, which branch-4.1 / 4.2 #66216 and master #66222 added on the BE side only. With the FE planner, which master uses by default, a streaming pre-aggregation still asked for a local hash exchange on its grouping keys whatever the variable said. Master plans therefore had a LOCAL_EXECUTION_HASH_SHUFFLE local exchange that the BE planner, and branch-4.x, do not add, for example right above a UNION ALL or an external table scan. The FE now follows the same rule as BE StreamingAggOperatorX.

Background. #66222 added enable_local_exchange_before_streaming_agg (default false) so that a streaming pre-aggregation keeps the distribution it inherits. It replaced enable_local_exchange_before_agg for streaming aggregation. In StreamingAggOperatorX::required_data_distribution(), the operator asks for a hash local exchange on its partition expressions only when the variable is on, or when OperatorBase::child_breaks_local_key_distribution() (#63766) says the child spreads rows without regard to any key: the child is serial, or the child's own required distribution is a non-hash local exchange (PASSTHROUGH, ADAPTIVE_PASSTHROUGH, ...).

The problem, and what it cost. On master the FE plans local exchanges (enable_local_shuffle_planner, default true) and BE does not. AggregationNode.enforceAndDeriveLocalExchange() mirrors the BE operators, but its streaming aggregation branch still had the rule from before #66222. It never read the new variable. It skipped the hash requirement only when enable_local_exchange_before_agg was off, and then without the child check. With the default settings it required a hash distribution for every streaming pre-aggregation with grouping keys, so:

  • Below a streaming pre-aggregation whose child keeps the key distribution but does not report a hash distribution, the FE inserted a LOCAL_EXECUTION_HASH_SHUFFLE that BE planning does not. Examples are a UNION ALL and an external table scan that is not serial. Every row was hashed and moved once more before the pre-aggregation.
  • enable_local_exchange_before_streaming_agg=true had no effect with the FE planner.
  • With enable_local_exchange_before_agg=false, the FE dropped the hash requirement even after a child that breaks the key distribution, where BE keeps it.

None of this changes query results: a streaming pre-aggregation only affects performance, and the operators that need hash-distributed input for correctness ask for it themselves.

How this PR fixes it. The streaming aggregation branch of AggregationNode.enforceAndDeriveLocalExchange() now follows BE: after the existing hash join passthrough rule, it keeps the base class requirement (no exchange, or PASSTHROUGH over a serial child) unless enable_local_exchange_before_streaming_agg is on or the child breaks the key distribution. enable_local_exchange_before_agg no longer affects streaming aggregation. The new childBreaksLocalKeyDistribution() mirrors child_breaks_local_key_distribution() with each BE operator's required_data_distribution():

Child BE operator's required distribution Breaks the key distribution
serial on BE (pooling scan, serial exchange, ...) — yes
nested loop join ADAPTIVE_PASSTHROUGH, NOOP for null-aware anti, right outer / semi / anti and full outer joins yes, except those joins
broadcast hash join PASSTHROUGH when forced (enable_broadcast_join_force_passthrough) or when the probe child is serial in those two cases
table function, assert num rows PASSTHROUGH yes
anything else (scan, exchange, union, partitioned join, ...) NOOP or a hash distribution no

SelectNode and RepeatNode take the serial flag of their child, so the serial check already covers the base class rule (PASSTHROUGH over a serial child) for them.

Results. Local exchanges in the plans of three queries on a one-BE cluster, parallel_pipeline_task_num=4, non-pooling scans, taken from the query profiles:

Query BE planner FE planner before FE planner after
GROUP BY over UNION ALL of two tables none LOCAL_HASH_SHUFFLE none
GROUP BY over a nested loop join ADAPTIVE_PASSTHROUGH, LOCAL_HASH_SHUFFLE same same
GROUP BY over an Iceberg table with 6 data files none LOCAL_HASH_SHUFFLE none

A planner probe over 72 plans (8 query shapes × pooling / non-pooling scans × agg_phase 0 / 2 × the variable off / on) found only the UNION ALL plans changed with the variable off. A scan of an OLAP table, a nested loop join, explode, broadcast and shuffle joins, a LIMIT subquery and GROUPING SETS gave the same plans as before.

Release note

With the FE local shuffle planner (the default), a streaming pre-aggregation no longer adds a local hash exchange on its grouping keys above a child that keeps the key distribution, such as a UNION ALL or an external table scan. This matches the BE planner and branch-4.x. Set enable_local_exchange_before_streaming_agg=true to add the exchange.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

    New tests: 4 in LocalShuffleNodeCoverageTest (3 fail without the fix; the fourth pins the child check), 2 in LocalExchangePlannerTest (1 fails without the fix), and testSetOperationUnderAggHasHashShuffle now turns the variable on. The 9 FE test classes that plan local exchanges or print distributed plans pass (141 tests). Each of 6 mutations of the new code fails only the tests aimed at it: dropping the serial check, the nested loop join case, the hash join case, the table function / assert num rows case, or the nested loop join's exception for the serial joins, or ignoring the variable. The new regression nereids_p0/local_shuffle/test_local_exchange_before_streaming_agg passes on a local one-BE cluster, and its first plan check fails on the FE without this fix. The profile comparison in Results was taken on the same cluster.

  • Behavior changed:

    • No.
    • Yes.

    With the FE local shuffle planner, a streaming pre-aggregation follows enable_local_exchange_before_streaming_agg as described above, and enable_local_exchange_before_agg no longer affects it. Plans change, results do not.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

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

…local shuffle planner

### What problem does this PR solve?

Issue Number: None

Related PR: apache#66216, apache#66222, apache#63766

Problem Summary: apache#66222 (branch-4.1 / 4.2: apache#66216) added
enable_local_exchange_before_streaming_agg (default false) on the BE side:
StreamingAggOperatorX::required_data_distribution() keeps the distribution
a streaming pre-aggregation inherits, and asks for a local hash exchange
on its partition expressions only when the variable is on or when
OperatorBase::child_breaks_local_key_distribution() (apache#63766) says the
child is serial or itself requires a non-hash local exchange. The variable
replaced enable_local_exchange_before_agg for streaming aggregation.

On master the FE plans local exchanges by default
(enable_local_shuffle_planner) and BE does not. The streaming aggregation
branch of AggregationNode.enforceAndDeriveLocalExchange() still had the
rule from before apache#66222: it never read the new variable and, with
enable_local_exchange_before_agg at its default, required a hash
distribution for every streaming pre-aggregation with grouping keys. So
the default master plan hashed the input of a streaming pre-aggregation
again above a child that keeps the key distribution, such as a UNION ALL
or an external table scan that is not serial, where the BE planner and
branch-4.x add no local exchange. Setting the variable had no effect, and
enable_local_exchange_before_agg=false dropped the hash exchange even
after a child that breaks the key distribution. Results were not
affected: the operators that need hash-distributed input for correctness
ask for it themselves.

The FE now follows BE: after the hash join passthrough rule, a streaming
pre-aggregation keeps the base class requirement unless the variable is
on or the child breaks the key distribution, and
enable_local_exchange_before_agg no longer applies to it. The new
childBreaksLocalKeyDistribution() mirrors the BE check with each
operator's required distribution: a child that is serial on BE, a nested
loop join (ADAPTIVE_PASSTHROUGH, except null-aware anti, right outer /
semi / anti and full outer joins), a broadcast hash join that is forced
to passthrough or has a serial probe child, a table function and assert
num rows (PASSTHROUGH).

On a one-BE cluster with non-pooling scans, the profiles of GROUP BY over
UNION ALL and over an Iceberg table with 6 data files had a
LOCAL_HASH_SHUFFLE with the FE planner and none with the BE planner; they
now have none with both. GROUP BY over a nested loop join keeps
ADAPTIVE_PASSTHROUGH and LOCAL_HASH_SHUFFLE with both planners.

### Release note

With the FE local shuffle planner (the default), a streaming
pre-aggregation no longer adds a local hash exchange on its grouping keys
above a child that keeps the key distribution, such as a UNION ALL or an
external table scan, as with the BE planner and branch-4.x. Set
enable_local_exchange_before_streaming_agg=true to add it.

### Check List (For Author)

- Test: Unit Test, Regression test, Manual test
    - LocalShuffleNodeCoverageTest: 4 new tests (3 fail without the fix).
      LocalExchangePlannerTest: 2 new tests (1 fails without the fix);
      testSetOperationUnderAggHasHashShuffle now turns the variable on.
      141 tests in the 9 related FE test classes pass. Each of 6
      mutations of the new check fails only its target tests.
    - New regression nereids_p0/local_shuffle/
      test_local_exchange_before_streaming_agg passes on a local one-BE
      cluster; its default-plan check fails without the fix.
    - Manual: the profile comparison above.
- Behavior changed: Yes. With the FE planner, streaming pre-aggregation
  follows enable_local_exchange_before_streaming_agg, and
  enable_local_exchange_before_agg no longer affects it. Plans change,
  results do not.
- Does this need documentation: No

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.

Static review of PR #70177 at 9231589e704d2a4de53576c7c052f98195b69714: no substantiated new findings. No existing inline findings require retention. Review converged in one round, with two complete-scope reviewers and a separate risk-focused reviewer returning NO_NEW_VALUABLE_FINDINGS.

The FE streaming-preaggregation branch now follows StreamingAggOperatorX: it keeps inherited distribution with enable_local_exchange_before_streaming_agg=false, retains HASH after serial/non-hash-exchange children, and preserves the existing hash-join passthrough override. Traced flow: physical LOCAL/INPUT_TO_BUFFER aggregate -> distribute translation -> per-BE instance assignment -> FE exchange enforcement -> existing Thrift plan -> BE operators. Final/merge aggregation continues to enforce its own correctness requirements.

Critical checkpoint conclusions:

  1. Goal and test linkage: the change addresses the FE/BE setting mismatch. Unit cases cover both switch values and positive/negative child classifications; real-planner tests pin exact exchange placement; regression cases compare FE/BE planners across both values.
  2. Scope and clarity: one production class and four directly related test/output files; no unrelated production change.
  3. Concurrency and locks: new checks run during query-local planning, introduce no threads/shared mutable state, and add no lock, IO or journal wait. No new lock-order concern.
  4. Lifecycle and initialization: existing query-owned plan nodes and session context are reused; no new resource owner, retained cache or static initializer.
  5. Configuration: no setting added. The existing streaming switch is read for each plan; its existing Thrift forwarding is present. The general aggregation switch retains its separate non-streaming/distinct roles.
  6. Compatibility: no ABI, persistent format, node type or wire-layout change; emitted exchanges already exist.
  7. Thrift/protobuf IDs: no definition changes; branch field-ID comparison is inapplicable.
  8. Parallel paths: non-streaming final/merge and distinct-streaming branches are unchanged. Relevant serial/nonserial, broadcast/partitioned join, null-aware and right/full join exceptions, wrappers, scans, exchanges and spill delegates were checked.
  9. Special conditions: NL-join raw serial checks classify precisely the BE exception enums; runtime serial checks handle actual task mode. Nested-streaming and recursive-CTE concerns were dismissed using translator exchange boundaries and single-instance assignment evidence.
  10. Test coverage: added positive/negative unit and real-plan cases complement the result matrix. Shared test context resets the newly toggled flag; regression hints control relevant fuzzy variables and explain parsing matches actual formatting.
  11. Expected results: independently recalculated all 12 new result blocks; counts/sums and lexical order_qt ordering match. Static inspection cannot establish file-generation provenance.
  12. Observability: existing distributed explain includes streaming markers and exchange types, sufficient for this planner-only change.
  13. Transactions and persistence: no EditLog, replay, visible-version or commit/failover behavior changes.
  14. Data writes and atomicity: no production write path, rowset/delete-bitmap or crash-recovery change.
  15. FE/BE variables: no new transmitted field; the existing streaming flag is already serialized consistently.
  16. Performance and memory: intended redundant hash exchange is removed; retained serial redistribution and downstream correctness checks were traced. No new per-row work, large allocation or native memory-accounting change. Performance was not measured.
  17. Other invariants and issues: no newly swallowed error or reachable wrong-result/task-count path was substantiated. Committed-data visibility and storage invariants are outside this planner delta; aggregation correctness remains guarded by downstream requirements.

Coverage: all five changed files and relevant upstream/downstream mechanisms reviewed. No additional user focus was supplied, and no unresolved static candidate or coverage gap remains. This was static inspection only: no builds, Doris tests, cluster execution or benchmarks were run; author-reported runtime results were not independently reproduced.

Review converged after 1 round(s).

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17588	5210	5074	5074
q2	2162	306	259	259
q3	10224	1402	831	831
q4	4688	465	342	342
q5	7445	814	531	531
q6	184	183	153	153
q7	746	804	606	606
q8	9271	1348	1149	1149
q9	5638	4262	4209	4209
q10	6821	1298	1031	1031
q11	433	251	234	234
q12	647	406	278	278
q13	18095	3763	3040	3040
q14	280	272	258	258
q15	q16	730	730	661	661
q17	1219	754	582	582
q18	7081	6193	6104	6104
q19	1247	970	605	605
q20	403	347	224	224
q21	5485	2400	2465	2400
q22	372	328	274	274
Total cold run time: 100759 ms
Total hot run time: 28845 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5995	5897	6080	5897
q2	764	628	576	576
q3	5569	5792	5335	5335
q4	2028	2130	1530	1530
q5	5752	5855	5739	5739
q6	240	184	139	139
q7	2269	2036	1854	1854
q8	2983	2651	2675	2651
q9	7965	7666	7507	7507
q10	3637	3592	3201	3201
q11	572	410	384	384
q12	665	676	502	502
q13	3353	3666	3005	3005
q14	283	287	276	276
q15	q16	670	675	662	662
q17	7813	7084	6859	6859
q18	12907	12149	12863	12149
q19	965	829	845	829
q20	2137	2142	1909	1909
q21	5618	4594	4773	4594
q22	519	466	413	413
Total cold run time: 72704 ms
Total hot run time: 66011 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 153926 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 9231589e704d2a4de53576c7c052f98195b69714, data reload: false

query5	4316	611	437	437
query6	436	214	194	194
query7	4829	445	225	225
query8	326	187	152	152
query9	8773	3199	3211	3199
query10	491	306	260	260
query11	5837	3862	3596	3596
query12	146	95	90	90
query13	1241	456	334	334
query14	6108	3940	3638	3638
query14_1	3474	3450	3420	3420
query15	211	200	179	179
query16	939	437	456	437
query17	858	657	527	527
query18	2400	410	303	303
query19	192	158	134	134
query20	84	84	81	81
query21	214	121	103	103
query22	13128	13013	12771	12771
query23	13539	12866	12441	12441
query23_1	12560	12519	12453	12453
query24	7339	812	473	473
query24_1	468	468	480	468
query25	513	391	342	342
query26	1277	256	135	135
query27	2760	435	285	285
query28	4657	2040	2025	2025
query29	1568	574	431	431
query30	302	218	177	177
query31	1004	930	800	800
query32	160	90	88	88
query33	515	298	257	257
query34	956	858	486	486
query35	731	750	637	637
query36	814	825	744	744
query37	144	96	86	86
query38	1845	1756	1673	1673
query39	791	801	743	743
query39_1	748	721	736	721
query40	221	117	97	97
query41	72	70	68	68
query42	90	83	83	83
query43	338	358	314	314
query44	1367	718	706	706
query45	185	195	169	169
query46	821	967	564	564
query47	2967	2813	2786	2786
query48	306	321	231	231
query49	587	398	294	294
query50	692	281	219	219
query51	10686	10611	10717	10611
query52	77	85	76	76
query53	193	216	154	154
query54	235	219	189	189
query55	67	67	64	64
query56	254	220	221	220
query57	1652	1661	1547	1547
query58	298	271	266	266
query59	2172	2187	1994	1994
query60	297	249	242	242
query61	175	163	167	163
query62	393	338	291	291
query63	191	159	163	159
query64	2883	1066	827	827
query65	3346	3274	3267	3267
query66	1776	428	302	302
query67	22589	22665	22512	22512
query68	3140	1022	634	634
query69	369	284	245	245
query70	873	775	765	765
query71	301	229	217	217
query72	2764	2572	2289	2289
query73	514	518	292	292
query74	4635	4527	4344	4344
query75	2358	2312	1925	1925
query76	2389	1011	593	593
query77	357	375	277	277
query78	9949	9709	9185	9185
query79	1001	910	540	540
query80	1197	413	354	354
query81	584	324	272	272
query82	913	132	105	105
query83	296	189	186	186
query84	314	128	109	109
query85	853	426	364	364
query86	398	236	221	221
query87	1979	1926	1795	1795
query88	3838	2748	2757	2748
query89	335	280	243	243
query90	1886	179	166	166
query91	160	151	126	126
query92	91	89	92	89
query93	1041	996	579	579
query94	641	309	271	271
query95	590	335	363	335
query96	673	496	243	243
query97	2434	2415	2331	2331
query98	160	153	150	150
query99	790	758	643	643
Total cold run time: 239218 ms
Total hot run time: 153926 ms

@hello-stephen

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

query1	0.01	0.00	0.00
query2	0.10	0.05	0.05
query3	0.27	0.09	0.09
query4	1.62	0.12	0.12
query5	0.24	0.25	0.25
query6	1.17	0.67	0.67
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.41	0.35	0.35
query10	0.58	0.63	0.62
query11	0.21	0.15	0.14
query12	0.18	0.15	0.15
query13	0.48	0.49	0.48
query14	1.04	1.01	1.00
query15	0.63	0.61	0.61
query16	0.35	0.35	0.36
query17	1.16	1.13	1.15
query18	0.20	0.20	0.20
query19	2.06	2.01	1.96
query20	0.02	0.01	0.02
query21	15.49	0.20	0.14
query22	4.99	0.06	0.05
query23	16.27	0.34	0.12
query24	3.12	0.56	0.37
query25	0.10	0.08	0.06
query26	1.01	0.22	0.15
query27	0.06	0.03	0.06
query28	2.71	0.66	0.36
query29	12.49	4.21	3.23
query30	0.31	0.15	0.11
query31	2.77	0.60	0.32
query32	3.23	0.61	0.51
query33	3.21	3.16	3.14
query34	15.35	3.97	3.36
query35	3.33	3.31	3.34
query36	0.57	0.44	0.43
query37	0.10	0.07	0.07
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.18	0.15	0.14
query41	0.09	0.04	0.03
query42	0.04	0.03	0.04
query43	0.04	0.04	0.03
Total cold run time: 96.38 s
Total hot run time: 24.29 s

@morningman
morningman merged commit d5f624f into apache:master Nov 5, 2026
37 of 38 checks passed
@morningman
morningman requested a review from 924060929 November 5, 2026 07:19
morningman added a commit that referenced this pull request Nov 5, 2026
…r key-breaking children (#70296)

### 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()`. #63366 wrote them
after #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` (#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
#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 #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 #70177 does for streaming aggregation.
- In `LocalShuffleNodeCoverageTest`, #70177's five key-breaking children
moved into `childrenThatBreakKeyDistribution()` and its aggregate
builder into `buildLocalPhaseAgg()`, so the new cases reuse them;
#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 #70177.
morningman added a commit that referenced this pull request Nov 6, 2026
…#70197)

### What problem does this PR solve?

Issue Number: None

Related PR: #69273, #70177

Problem Summary:

**In short.** Found while bringing branch-4.2 #66216 to the FE local
shuffle planner (#70177): four session variables that decide the local
exchanges of a plan are not forwarded to the master. #69273 made the
planner and executor variables travel with a statement that a follower
or an observer forwards to the master,
`enable_local_exchange_before_agg` among them, but not
`enable_local_exchange_before_streaming_agg`,
`enable_streaming_agg_hash_join_force_passthrough`,
`enable_distinct_streaming_agg_force_passthrough` and
`force_to_local_shuffle`. A session-level `SET` of one of them is
dropped on that path, and the master plans the statement with its own
global value. This PR forwards the four as well.

**Background.** A statement that has to run on the master FE (`INSERT
INTO ... SELECT`, `CREATE TABLE ... AS SELECT`, ...) and arrives at a
follower or an observer is forwarded: `FEOpExecutor` sends
`SessionVariable#getForwardVariables()`, and `ConnectProcessor` on the
master applies them with `setForwardedSessionVariables()` to a session
that starts from the master's global variables. Only variables whose
`@VarAttr` sets `needForward` (or one of the `affectQueryResult*`
attributes) are sent.

**The problem, and what it cost.** These four do not set it:

| Variable | Default | What it decides |
|---|---|---|
| `enable_local_exchange_before_streaming_agg` (#66222) | false |
whether a streaming pre-aggregation asks for a local hash exchange on
its grouping keys: BE `StreamingAggOperatorX`, and the FE planner with
#70177 |
| `enable_streaming_agg_hash_join_force_passthrough` | true | whether a
streaming pre-aggregation right above a hash join asks for a passthrough
local exchange: the FE planner (`AggregationNode`) and BE
`StreamingAggOperatorX` |
| `enable_distinct_streaming_agg_force_passthrough` | true | the same
for a distinct streaming pre-aggregation: the FE planner and BE
`DistinctStreamingAggOperatorX` |
| `force_to_local_shuffle` | false | makes every scan serial
(`ScanNode#isSerialNode`), so the FE planner pools the scan and adds a
local shuffle above it |

So a session that sets one of them gets that plan only for the
statements its own FE runs. A forwarded statement, such as an `INSERT
INTO ... SELECT` or a CTAS issued through a follower or an observer, is
planned with the master's global value, and `EXPLAIN` on the follower
shows a plan that the master does not run. Query results stay the same,
since these variables only decide how rows move between the pipeline
tasks of a BE; what differs is the plan, and with it the speed of the
statement.

**How this PR fixes it.** Set `needForward = true` on the four, as
#69273 does for the others, and add them to the list in
`SessionVariablesTest#testForwardPlanAndExecutionKnobs`.

**Results.**

| Statement forwarded from a follower after `SET <variable> =
<non-default value>` | Before | After |
|---|---|---|
| Value the master plans it with | the master's global value | the
session's value |

branch-4.1 and branch-4.2 have the same gap, and #69273 is not on them
either.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants