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.

Sign up for GitHub

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

Jump to bottom

planner: don't push down null sensitive join conditions #19620

Merged
merged 13 commits into from
Nov 12, 2020
Merged

planner: don't push down null sensitive join conditions #19620

merged 13 commits into from
Nov 12, 2020

Conversation

eurekaka
Copy link
Contributor

@eurekaka eurekaka commented Aug 31, 2020 •
edited by SunRunAway
Loading

What problem does this PR solve?

Issue Number:

Problem Summary:

  • Wong result for some LeftOuterSemiJoin / AntiSemiJoin / AntiLeftOuterSemiJoin queries.

What is changed and how it works?

What's Changed:

  • Correctly maintain NotNull flag for aggregation result columns;
  • Mark column arguments of RowFunc as InOperand;
  • Don't push down join conditions containing InOperand columns;

How it Works:

  • Those filters are kept as OtherConditions of join, and can be correctly evaluated then.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

N/A

Release note

  • Fix wrong results for some semi join queries.

@eurekaka eurekaka added type/bugfix This PR fixes a bug. sig/planner SIG: Planner labels Aug 31, 2020
@eurekaka eurekaka requested review from a team as code owners August 31, 2020 09:56
@eurekaka eurekaka requested review from XuHuaiyu and removed request for a team and XuHuaiyu August 31, 2020 09:56
@github-actions github-actions bot added the sig/execution SIG execution label Aug 31, 2020
@eurekaka eurekaka force-pushed the row_func_anti_join branch 2 times, most recently from 72ea983 to 4b359a7 Compare August 31, 2020 11:14
@eurekaka eurekaka force-pushed the row_func_anti_join branch 3 times, most recently from e2cd4f9 to c360768 Compare September 1, 2020 11:10
@time-and-fate time-and-fate mentioned this pull request Sep 3, 2020
Closed
@eurekaka eurekaka mentioned this pull request Oct 16, 2020
Closed
lzmhhh123
lzmhhh123 reviewed Oct 19, 2020
View reviewed changes
Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

Could the PR fix #13389 ?

@ti-srebot
Copy link
Contributor

@eurekaka merge failed.

@lzmhhh123 lzmhhh123 dismissed stale reviews from ti-srebot and winoros via 174be8e November 12, 2020 06:44
@lzmhhh123
Copy link
Contributor

/run-all-tests

@lzmhhh123
Copy link
Contributor

/run-all-tests

@lzmhhh123
Copy link
Contributor

/run-common-test tidb-test=pr/1101
/run-integration-common-test tidb-test=pr/1101

@lzmhhh123
Copy link
Contributor

/run-integration-common-test tidb-test=pr/1101
/run-sqllogic-test

winoros
winoros reviewed Nov 12, 2020
View reviewed changes
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@lzmhhh123
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@lzmhhh123
Copy link
Contributor

/merge

@ti-srebot
Copy link
Contributor

@eurekaka merge failed.

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 3ef3e54 into pingcap:master Nov 12, 2020
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 12, 2020
@eurekaka @ti-srebot
cherry pick pingcap#19620 to release-3.0
b4e4c8a 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Nov 12, 2020
Closed
@ti-srebot
Copy link
Contributor

cherry pick to release-3.0 in PR #21018

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 12, 2020
@eurekaka @ti-srebot
cherry pick pingcap#19620 to release-4.0
c18d469 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Nov 12, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #21019

@eurekaka eurekaka deleted the row_func_anti_join branch November 19, 2020 09:34
eurekaka added a commit that referenced this pull request Nov 20, 2020
@ti-srebot @eurekaka
cherry pick #19620 to release-4.0 (#21019)
cc61a9f 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: Kenan Yao <cauchy1992@gmail.com>
@rebelice rebelice mentioned this pull request Nov 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winoros winoros winoros left review comments

@lzmhhh123 lzmhhh123 lzmhhh123 approved these changes

@ti-srebot ti-srebot ti-srebot left review comments

@qw4990 qw4990 Awaiting requested review from qw4990

Assignees
No one assigned
Labels
component/expression sig/execution SIG execution sig/planner SIG: Planner status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Milestone
No milestone
6 participants
@eurekaka @lzmhhh123 @winoros @ti-srebot @lonng @zz-jason