mrhhsg opened a new pull request, #66104:
URL: https://github.com/apache/doris/pull/66104
### What problem does this PR solve?
Issue Number: None
Related PR: #62438, #63529, #63766
Problem Summary:
This safely backports #62438 to `branch-4.1`, adding the
`enable_local_exchange_before_agg` session variable so users can skip
unnecessary local exchanges before eligible non-finalizing aggregation.
A literal cherry-pick of #62438 is unsafe: when a serial or another non-hash
local exchange precedes aggregation, disabling the exchange can split equal
group or DISTINCT keys across local pipeline tasks and produce incorrect
results. This backport therefore also includes the final correctness guard from
#63766 and the serial-exchange regression coverage from #63529:
- Skip the local exchange only when the child preserves local key
distribution.
- Require a hash exchange after serial and other non-hash local exchanges.
- Handle a non-hash `LocalExchangeSource` when the downstream target
requires hash distribution.
- Cover the session-variable Thrift propagation, distribution decisions,
serial DISTINCT aggregation, and nested-loop-join DISTINCT aggregation.
`branch-4.1` already contains the compatible
`enable_local_exchange_before_agg` Thrift field, so this PR keeps the existing
field id and adds no duplicate IDL field.
### Release note
Add the `enable_local_exchange_before_agg` session variable. Disabling it
can reduce local exchanges before eligible non-finalizing aggregation while
preserving required hash exchanges for correctness.
### Check List (For Author)
- Test
- [ ] Regression test (worktree-isolated local build and focused
regression run in progress)
- [x] Unit Test
- `./run-be-ut.sh --run
--filter='StreamingAggOperatorTest.require_hash_shuffle_after_non_hash_local_exchange:DistinctStreamingAggOperatorTest.require_hash_shuffle_after_non_hash_local_exchange:AggOperatorRequiredDistributionTest.*:AggOperatorGroupByLimitOptTestWithGroupBy.*:AggOperatorTestWithGroupBy.test_2_phase:AggOperatorTestWithGroupBy.other_case_3:AggOperatorTestWithOutGroupBy.*'
-j 64` (21 tests passed)
- `./run-fe-ut.sh --run org.apache.doris.qe.VariableMgrTest` (12
tests passed)
- [ ] 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
- Behavior changed:
- [ ] No.
- [x] Yes. Add a session variable that controls eligible local exchanges
before aggregation while retaining correctness-required hash exchanges.
- Does this need documentation?
- [x] No. This is a branch backport of an existing session variable.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
