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

Changed parsing for unions #8821

Merged
merged 18 commits into from
Sep 21, 2021
Merged

Changed parsing for unions #8821

merged 18 commits into from
Sep 21, 2021

Conversation

ritwizsinha
Copy link
Contributor

@ritwizsinha ritwizsinha commented Sep 15, 2021

Description

This pull request, makes changes to the select and union grammar in Vitess to match what is implemented in the MYSQL grammar. Here are the changes that have been made

  • The parser no longer accepts queries with limit/lock/order by in select with unions without parenthesis for example
select 1 order by 1 union select 2 order by 2

is a parsing error and this type of query is only accepted if it is parenthesized

(select 1 order by 1) union (select 2 order by 2)
  • We no longer need to have parenthesis in any other select statement apart from the ones mentioned above when working with unions
  • We no longer use ParenSelect struct to put parenthesis around statements but we check if the statement needs parenthesis in the ast_format file, and put parenthesis when needed. All utility functions for ParenSelect have been removed
  • The Select struct now accepts an additional field of into clause which is checked and passed when present as specified in the grammar
  • The union is now stored recursively with first statement field specifying the left select statement and the right side of union being specified by the UnionSelects field
  • Introduced precedence symbols SUBQUERY_AS_EXPR and EMPTY_FROM_CLAUSE to avoid ambiguity in grammar while parsing subqueries and into selects respectively. The same trick has been used and explained in the MYSQL grammar
  • Updated the parser and planner tests to reflect the current output of parser.

Related Issue(s)

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

ritwizsinha and others added 6 commits September 15, 2021 14:04
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
@ritwizsinha ritwizsinha force-pushed the test-branch branch 2 times, most recently from 78ff634 to 1fc5d4e Compare September 19, 2021 14:45
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
ritwizsinha and others added 9 commits September 20, 2021 10:07
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: ritwizsinha <ritwizsinha0@gmail.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
@GuptaManan100 GuptaManan100 added Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Sep 20, 2021
Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>
Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

LGTM! 🤟

@systay systay merged commit db30b95 into vitessio:main Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants