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

executor, planner: fix some cases for natural_using_join #20977

Merged
merged 9 commits into from
Nov 12, 2020

Conversation

XuHuaiyu
Copy link
Contributor

@XuHuaiyu XuHuaiyu commented Nov 11, 2020

What problem does this PR solve?

Issue Number:
close #20477
close #20441
close #15844
close #20958
close #20476

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:
Support querying the coalesced columns in natural_using_join.

How it Works:

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Integration test

Side effects

/AN

Release note

  • Fix the bug that can not query the coalesced column when use using-join.

@XuHuaiyu XuHuaiyu added type/bugfix This PR fixes a bug. sig/planner SIG: Planner needs-cherry-pick-4.0 labels Nov 11, 2020
@XuHuaiyu XuHuaiyu requested review from a team as code owners November 11, 2020 03:41
@XuHuaiyu XuHuaiyu requested review from qw4990, SunRunAway, winoros and lzmhhh123 and removed request for a team and SunRunAway November 11, 2020 03:41
@XuHuaiyu XuHuaiyu changed the title executor, planner: fix some cases for using join executor, planner: fix some cases for natural_using_join Nov 11, 2020
@github-actions github-actions bot added the sig/execution SIG execution label Nov 11, 2020
@ichn-hu ichn-hu mentioned this pull request Nov 11, 2020
@lzmhhh123
Copy link
Contributor

/reward 900

@ti-challenge-bot
Copy link

Reward success.

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.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 11, 2020
field.SetText(name.ColName.O)
resultList = append(resultList, field)
list := unfoldWildStar(field, p.OutputNames(), p.Schema().Columns)
if isJoin && join.redundantSchema != nil && field.WildCard.Table.L != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Add comment for this part.

planner/core/logical_plan_builder.go Show resolved Hide resolved
@@ -978,6 +980,19 @@ func (b *PlanBuilder) buildProjectionField(ctx context.Context, p LogicalPlan, f
return newCol, name, nil
}

func findColFromNaturalUsingJoin(p LogicalPlan, col *expression.Column) (name *types.FieldName) {
Copy link
Member

Choose a reason for hiding this comment

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

Add comment for the function header.

if idx < 0 {
return -1, nil
switch x := p.(type) {
Copy link
Member

Choose a reason for hiding this comment

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

Add comment for this switch clause

// We record the full `rightPlan.Schema` as `redundantSchema` in order to
// record the redundant column in `rightPlan` and the output columns order
// of the `rightPlan`.
// For SQL like `select t1.*, t2.* from t1 left join t2 using(a)`, we can
Copy link
Member

Choose a reason for hiding this comment

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

three-way join will be a better example.

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

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 12, 2020
@XuHuaiyu
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 12, 2020
@XuHuaiyu
Copy link
Contributor Author

/run-all-tests

1 similar comment
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@XuHuaiyu merge failed.

@XuHuaiyu XuHuaiyu merged commit c854594 into pingcap:master Nov 12, 2020
@XuHuaiyu XuHuaiyu deleted the issue20477 branch November 12, 2020 10:04
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 12, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #21021

ti-srebot added a commit that referenced this pull request Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
4 participants