Skip to content

Conversation

@morrySnow
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

Co-authored-by: Jerry Hu mrhhsg@gmail.com

  1. Filtering is done at the sending end rather than the receiving end
  2. Projection is done at the sending end rather than the receiving end
  3. Each sender can use different shuffle policies to send data

Further comments

If this is a relatively large or complex change, kick off the discussion at dev@doris.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions github-actions bot added area/nereids area/pipeline area/planner Issues or PRs related to the query planner labels Jul 1, 2023
@morrySnow
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

clang-tidy review says "All clean, LGTM! 👍"

@morrySnow morrySnow force-pushed the runtimefilter_multi_send branch from 02685fd to 7a6532e Compare July 3, 2023 07:25
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

@morrySnow
Copy link
Contributor Author

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

PR approved by anyone and no changes requested.

@morrySnow morrySnow force-pushed the runtimefilter_multi_send branch from 7a6532e to 6faa3a6 Compare July 3, 2023 10:22
@morrySnow morrySnow added the dev/2.0.0 2.0.0 release label Jul 3, 2023
@morrySnow
Copy link
Contributor Author

run buildall

@morrySnow morrySnow removed the dev/2.0.0 2.0.0 release label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

clang-tidy review says "All clean, LGTM! 👍"

@morrySnow morrySnow added the dev/2.0.0 2.0.0 release label Jul 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

clang-tidy review says "All clean, LGTM! 👍"

@morrySnow
Copy link
Contributor Author

run buildall

Copy link
Contributor

@HappenLee HappenLee left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jul 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

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

@morrySnow morrySnow merged commit 90dd871 into master Jul 4, 2023
@morrySnow morrySnow deleted the runtimefilter_multi_send branch July 4, 2023 08:51
@xiaokang xiaokang added dev/2.0.0-merged and removed dev/2.0.0 2.0.0 release labels Jul 4, 2023
xiaokang pushed a commit that referenced this pull request Jul 4, 2023
… shuffle (#21412)

Co-authored-by: Jerry Hu <mrhhsg@gmail.com>

1. Filtering is done at the sending end rather than the receiving end
2. Projection is done at the sending end rather than the receiving end
3. Each sender can use different shuffle policies to send data
morrySnow added a commit that referenced this pull request Dec 15, 2025
…nsumer (#58964)

Related PR: #21412

Problem Summary:

This pull request improves the handling of distribution properties
(specifically "must shuffle") for `PhysicalProject` and `PhysicalFilter`
nodes in the query planner, and adds comprehensive unit tests to ensure
correctness. The main logic ensures that when certain child nodes
require shuffling, the planner correctly adjusts the distribution
requirements, especially in the presence of `Project`, `Filter`, and
`Limit` nodes.

Key changes include:

**Distribution Property Handling Enhancements:**

* Added logic in `ChildrenPropertiesRegulator` to check if a child node
under a `PhysicalProject` or `PhysicalFilter` requires a "must shuffle"
distribution, and to adjust the children’s properties accordingly. This
is done via the new `mustShuffleUnderProjectOrFilter` method.
* Included `PhysicalLimit` in the set of nodes that can trigger a
shuffle requirement, by updating imports and logic.

**Testing Improvements:**

* Added a new test class `ChildrenPropertiesRegulatorTest.java` with
detailed unit tests for the handling of "must shuffle" properties under
`Project`, `Filter`, and `Limit` nodes. These tests use mocks to
simulate various plan trees and assert correct distribution
specification propagation.

**Regression Test Coverage:**

* Added a new regression test in `cte.groovy` to verify correct behavior
when multiple `Project` nodes are present on a CTE consumer, ensuring
the planner handles such cases as expected.

These changes collectively make the planner more robust in handling
complex plan trees with respect to distribution requirements, and ensure
correctness through thorough testing.
github-actions bot pushed a commit that referenced this pull request Dec 15, 2025
…nsumer (#58964)

Related PR: #21412

Problem Summary:

This pull request improves the handling of distribution properties
(specifically "must shuffle") for `PhysicalProject` and `PhysicalFilter`
nodes in the query planner, and adds comprehensive unit tests to ensure
correctness. The main logic ensures that when certain child nodes
require shuffling, the planner correctly adjusts the distribution
requirements, especially in the presence of `Project`, `Filter`, and
`Limit` nodes.

Key changes include:

**Distribution Property Handling Enhancements:**

* Added logic in `ChildrenPropertiesRegulator` to check if a child node
under a `PhysicalProject` or `PhysicalFilter` requires a "must shuffle"
distribution, and to adjust the children’s properties accordingly. This
is done via the new `mustShuffleUnderProjectOrFilter` method.
* Included `PhysicalLimit` in the set of nodes that can trigger a
shuffle requirement, by updating imports and logic.

**Testing Improvements:**

* Added a new test class `ChildrenPropertiesRegulatorTest.java` with
detailed unit tests for the handling of "must shuffle" properties under
`Project`, `Filter`, and `Limit` nodes. These tests use mocks to
simulate various plan trees and assert correct distribution
specification propagation.

**Regression Test Coverage:**

* Added a new regression test in `cte.groovy` to verify correct behavior
when multiple `Project` nodes are present on a CTE consumer, ensuring
the planner handles such cases as expected.

These changes collectively make the planner more robust in handling
complex plan trees with respect to distribution requirements, and ensure
correctness through thorough testing.
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. area/nereids area/pipeline area/planner Issues or PRs related to the query planner dev/2.0.0-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants