Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix](planner)fix conjunct planned on exchange node #18042

Merged
merged 4 commits into from
Mar 27, 2023

Conversation

sohardforaname
Copy link
Contributor

@sohardforaname sohardforaname commented Mar 23, 2023

Proposed changes

Issue Number: close #xxx

Problem summary

sql like:
select k5, k6, SUM(k3) AS k3
from (
select
k5,
date_format(k6, '%Y-%m-%d') as k6,
count(distinct k3) as k3
from t
group by k5, k6
) AS temp where 1=1
group by k5, k6;

will throw exception since conjuncts planned on exchange node, because exchange node cannot handle conjuncts, now we skip exchange node when planning conjuncts, which fixes the bug.
notice: the bug occurs iff the conjunct is always true like 1=1 above.

Checklist(Required)

  • Does it affect the original behavior
  • Has unit tests been added
  • Has document been added or modified
  • Does it need to update dependencies
  • Is this PR support rollback (If NO, please explain WHY)

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/planner Issues or PRs related to the query planner kind/test labels Mar 23, 2023
@sohardforaname
Copy link
Contributor Author

run buildall

@sohardforaname
Copy link
Contributor Author

run p0

@morningman morningman added usercase Important user case type label dev/1.2.3 labels Mar 24, 2023
@englefly
Copy link
Contributor

add some explanation in problem summary

  1. the root cause is exchange node is not able to filter data
  2. remove the 1=1 in view, it is not the conjunct mentioned in this pr
  3. change the 1=1 to a more general equal.

@sohardforaname
Copy link
Contributor Author

add some explanation in problem summary

  1. the root cause is exchange node is not able to filter data
  2. remove the 1=1 in view, it is not the conjunct mentioned in this pr
  3. change the 1=1 to a more general equal.

done

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

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

@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@morrySnow morrySnow merged commit 894f38a into apache:master Mar 27, 2023
morningman pushed a commit that referenced this pull request Mar 28, 2023
sql like: 
select k5, k6, SUM(k3) AS k3 
from ( 
    select
        k5,
        date_format(k6, '%Y-%m-%d') as k6,
        count(distinct k3) as k3 
    from t 
    group by k5, k6
) AS temp where 1=1
group by k5, k6;

will throw exception since conjuncts planned on exchange node, because exchange node cannot handle conjuncts, now we skip exchange node when planning conjuncts, which fixes the bug. 
notice: the bug occurs iff the conjunct is always true like 1=1 above.
gnehil pushed a commit to gnehil/doris that referenced this pull request Apr 21, 2023
sql like: 
select k5, k6, SUM(k3) AS k3 
from ( 
    select
        k5,
        date_format(k6, '%Y-%m-%d') as k6,
        count(distinct k3) as k3 
    from t 
    group by k5, k6
) AS temp where 1=1
group by k5, k6;

will throw exception since conjuncts planned on exchange node, because exchange node cannot handle conjuncts, now we skip exchange node when planning conjuncts, which fixes the bug. 
notice: the bug occurs iff the conjunct is always true like 1=1 above.
mongo360 pushed a commit to mongo360/doris that referenced this pull request Jul 12, 2023
sql like: 
select k5, k6, SUM(k3) AS k3 
from ( 
    select
        k5,
        date_format(k6, '%Y-%m-%d') as k6,
        count(distinct k3) as k3 
    from t 
    group by k5, k6
) AS temp where 1=1
group by k5, k6;

will throw exception since conjuncts planned on exchange node, because exchange node cannot handle conjuncts, now we skip exchange node when planning conjuncts, which fixes the bug. 
notice: the bug occurs iff the conjunct is always true like 1=1 above.
SWJTU-ZhangLei pushed a commit to SWJTU-ZhangLei/incubator-doris that referenced this pull request Jul 25, 2023
commit f5d6201
Author: morrySnow <101034200+morrySnow@users.noreply.github.com>
Date:   Wed Mar 22 10:53:15 2023 +0800

    [fix](planner) should always execute projection plan (apache#17885)

    1. should always execute projection plan, whatever the statement it is.
    2. should always execute projection plan, since we only have vectorized engine now

commit ad61c84
Author: mch_ucchi <41606806+sohardforaname@users.noreply.github.com>
Date:   Mon Mar 27 17:50:52 2023 +0800

    [fix](planner) fix conjunct planned on exchange node (apache#18042)

    sql like:
    select k5, k6, SUM(k3) AS k3
    from (
        select
            k5,
            date_format(k6, '%Y-%m-%d') as k6,
            count(distinct k3) as k3
        from t
        group by k5, k6
    ) AS temp where 1=1
    group by k5, k6;

    will throw exception since conjuncts planned on exchange node, because exchange node cannot handle conjuncts, now we skip exchange node when planning conjuncts, which fixes the bug.
    notice: the bug occurs iff the conjunct is always true like 1=1 above.
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/planner Issues or PRs related to the query planner dev/1.2.4-merged kind/test reviewed usercase Important user case type label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants