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

parser: disallow subquery without table alias #19102

Merged
merged 9 commits into from
Oct 9, 2020

Conversation

tangenta
Copy link
Contributor

@tangenta tangenta commented Aug 10, 2020

What problem does this PR solve?

Issue Number: close pingcap/parser#963

Parser PR: pingcap/parser#968

What is changed and how it works?

Proposal: xxx

What's Changed:
Add table alias validation in preprocess.go.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Side effects
N/A

Release note

  • SQL statements that omit table aliases will be parsed as the error 1248("Every derived table must have its own alias") instead of a syntax error.

Copy link
Contributor

@crazycs520 crazycs520 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 Aug 10, 2020
zimulala
zimulala previously approved these changes Aug 13, 2020
Copy link
Contributor

@zimulala zimulala 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 removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 13, 2020
ti-srebot
ti-srebot previously approved these changes Aug 13, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 13, 2020
@github-actions github-actions bot added the sig/execution SIG execution label Aug 20, 2020
@zz-jason
Copy link
Member

@tangenta please resolve conflicts and fix CI.

@ti-srebot
Copy link
Contributor

@tangenta, please update your pull request.

1 similar comment
@ti-srebot
Copy link
Contributor

@tangenta, please update your pull request.

@kennytm
Copy link
Contributor

kennytm commented Sep 25, 2020

(maybe @crazycs520 @zimulala should re-review since the SQL_MODE='oracle' behavior is a new feature)

crazycs520
crazycs520 previously approved these changes Sep 29, 2020
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

lgtm

@tangenta tangenta dismissed stale reviews from crazycs520 and ti-srebot via f7991a1 September 29, 2020 08:40
@tangenta
Copy link
Contributor Author

/merge

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

/run-all-tests

@tangenta
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@kennytm
Copy link
Contributor

kennytm commented Sep 30, 2020

@tangenta /merge won't work if you can't find a reviewer with write permission to submit the Green check mark. Approvals from reviewers without write access won't satisfy GitHub.

@ti-srebot
Copy link
Contributor

@tangenta, please update your pull request.

@tangenta
Copy link
Contributor Author

tangenta commented Oct 9, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@tangenta merge failed.

@kennytm
Copy link
Contributor

kennytm commented Oct 9, 2020

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 789581b into pingcap:master Oct 9, 2020
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Oct 9, 2020
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #20367

@kennytm
Copy link
Contributor

kennytm commented Oct 9, 2020

@tangenta please edit the PR description to include SQL_MODE='oracle' into the release note.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/executor component/parser sig/execution SIG execution sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

omit table alias in subselect clause reports syntax error
7 participants