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

Reduce shift-reduce conflicts #10500

Merged
merged 3 commits into from
Jun 14, 2022
Merged

Reduce shift-reduce conflicts #10500

merged 3 commits into from
Jun 14, 2022

Conversation

GuptaManan100
Copy link
Member

Description

This PR removes the shift-reduce conflicts introduced in #10347 by using the precedence symbol FUNCTION_CALL_NON_KEYWORD

Related Issue(s)

Checklist

  • "Backport me!" label has been added if this change should be backported
  • Tests were added or are not required Yes
  • Documentation was added or is not required Not required

Deployment Notes

@GuptaManan100 GuptaManan100 marked this pull request as ready for review June 14, 2022 07:58
@github-actions
Copy link
Contributor

Review Checklist

Hello reviewers! 👋 Please follow this checklist when reviewing this Pull Request.

General

  • Ensure that the Pull Request has the correct release notes label. release notes none should only be used for PRs that are so trivial that they need not be included.
  • If a new flag is being introduced, review whether it is really needed. The flag names should be clear and intuitive (as far as possible), and the flag's help should be descriptive.
  • If a workflow is added or modified, each items in Jobs should be named in order to mark it as required. If the workflow should be required, the GitHub Admin should be notified.

Bug fixes

  • There should be at least one unit or end-to-end test.
  • The Pull Request description should either include a link to an issue that describes the bug OR an actual description of the bug and how to reproduce, along with a description of the fix.

Non-trivial changes

  • There should be some code comments as to why things are implemented the way they are.

New/Existing features

  • Should be documented, either by modifying the existing documentation or creating new documentation.
  • New features should have a link to a feature request issue or an RFC that documents the use cases, corner cases and test cases.

Backward compatibility

  • Protobuf changes should be wire-compatible.
  • Changes to _vt tables and RPCs need to be backward compatible.
  • vtctl command output order should be stable and awk-able.

systay
systay approved these changes Jun 14, 2022
View reviewed changes
Copy link
Collaborator

@systay systay left a comment

Choose a reason for hiding this comment

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

Nice!

@GuptaManan100 GuptaManan100 merged commit 9f08615 into vitessio:main Jun 14, 2022
@GuptaManan100 GuptaManan100 deleted the reduce-shift-reduce-conflict branch June 14, 2022 13:28
@rsajwani
Copy link
Contributor

thanks :)

systay added a commit to planetscale/vitess that referenced this pull request Aug 19, 2022
@vmg @rsajwani
@systay @K-Kumar-01 @GuptaManan100 @frouioui @harshit-gangal @rohit-nayak-ps @dbussink
Cherry Pick all SQLParser changes (vitessio#904)
c07c82b 
* Refactor aggregation AST structs (vitessio#10347)

* SQLParser:Refactoring Add count struct

Signed-off-by: Rameez Sajwani <[email protected]>

* SQLParser:Refactoring Add countStar struct

Signed-off-by: Rameez Sajwani <[email protected]>

* SQLParser:Refactoring Add avg struct

Signed-off-by: Rameez Sajwani <[email protected]>

* SQLParser:Refactoring Add max struct

Signed-off-by: Rameez Sajwani <[email protected]>

* SQLParser:Refactoring Add min struct

Signed-off-by: Rameez Sajwani <[email protected]>

* SQLParser:Refactoring Add sum struct

Signed-off-by: Rameez Sajwani <[email protected]>

* SQLParser:Refactoring Fixing Parser Aggr Function

Signed-off-by: Rameez Sajwani <[email protected]>

* fix: the return type of count was wrong

Signed-off-by: Andres Taylor <[email protected]>

* Refacotring code

Signed-off-by: Rameez Sajwani <[email protected]>

* More refactoring and unit test cases fix

Signed-off-by: Rameez Sajwani <[email protected]>

* removing getarg from aggregate interface

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing bugs after merge

Signed-off-by: Rameez Sajwani <[email protected]>

* Optimizing code

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing planBuilder test cases`

Signed-off-by: Rameez Sajwani <[email protected]>

* Adding more aggregate functions

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing parser errors

Signed-off-by: Rameez Sajwani <[email protected]>

* fixing feedback , code review

Signed-off-by: Rameez Sajwani <[email protected]>

* Fix regression which making changes in previous commit

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing vdiff tests

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing replication test

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing vstreamer planbuilder test

Signed-off-by: Rameez Sajwani <[email protected]>

* remove redundant code

Signed-off-by: Rameez Sajwani <[email protected]>

Co-authored-by: Andres Taylor <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Add parsing support for performance schema functions (vitessio#10478)

* feat: add parsing support for performance schema functions

Signed-off-by: Kushal Kumar <[email protected]>

* fix: update sql.go by making parser

Signed-off-by: Kushal Kumar <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Reduce shift-reduce conflicts (vitessio#10500)

* feat: reduce shift-reduce conflicts by using the precedence symbol FUNCTION_CALL_NON_KEYWORD

Signed-off-by: Manan Gupta <[email protected]>

* feat: fix static check workflow to setup go in cases of parser changes too

Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Cleanup:  Remove 'Name' field from aggregate structure (vitessio#10507)

* Code clean up

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing unit test case failures

Signed-off-by: Rameez Sajwani <[email protected]>

* Fixing replicator planner unit tests

Signed-off-by: Rameez Sajwani <[email protected]>

* Adding unit test cases of canonical output

Signed-off-by: Rameez Sajwani <[email protected]>

* Fix unit test

Signed-off-by: Rameez Sajwani <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* User defined and sys variables (vitessio#10547)

* feat: add user defined variables and sys variables as separate structs and use user-defined variable in Execute statement

Signed-off-by: Manan Gupta <[email protected]>

* feat: remove parsing of at_id and at_at_id from places that shouldn't parse it

Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Andres Taylor <[email protected]>

* feat: use ci_identifier in more places

Signed-off-by: Andres Taylor <[email protected]>

* refactor: clean up the JSON AST structs a little

Signed-off-by: Andres Taylor <[email protected]>

* refactor: use remove unnesseccary scope

Signed-off-by: Andres Taylor <[email protected]>

* Created new Variable node to divide the work of ColName

Signed-off-by: Florent Poinsard <[email protected]>

* refactor fix sqlparser: SET ast and parsing

Signed-off-by: Andres Taylor <[email protected]>

* refactor: remove unused ast-structs

Signed-off-by: Andres Taylor <[email protected]>

* refactor: clean up how variables are parsed and formatted

Signed-off-by: Andres Taylor <[email protected]>

* fix: make the new AST work in the planbuilder

Signed-off-by: Andres Taylor <[email protected]>

* refactor: rename ColIdent and TableIdent

Signed-off-by: Andres Taylor <[email protected]>

* fix: set name should be parsed with SessionScope

Signed-off-by: Andres Taylor <[email protected]>

* chore: clean and document stuff

Signed-off-by: Andres Taylor <[email protected]>

* test: update test expectation

Signed-off-by: Andres Taylor <[email protected]>

* fix: default scope for transaction isolation should not be session

Signed-off-by: Andres Taylor <[email protected]>

* feat: handle local as a synonym for session for variable scopes

Signed-off-by: Andres Taylor <[email protected]>

* comments: clean up code comments

Signed-off-by: Andres Taylor <[email protected]>

Co-authored-by: Manan Gupta <[email protected]>
Co-authored-by: Florent Poinsard <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Revert "[Backport 14.0] enable schema tracking by default  (vitessio#10595)"

This reverts commit 8ebe1cc.

Signed-off-by: Vicent Marti <[email protected]>

* enable schema tracking by default (vitessio#10455)

* feat: enable schema tracking by default

Signed-off-by: Harshit Gangal <[email protected]>

* test: fix test setup

Signed-off-by: Harshit Gangal <[email protected]>

* test: fix vschema test setup

Signed-off-by: Harshit Gangal <[email protected]>

* test: turn off schema tracking on the tablet

Signed-off-by: Andres Taylor <[email protected]>

* test: fix test assertion

Signed-off-by: Andres Taylor <[email protected]>

* Change read query for checks that test to which keyspace a table is routed to. This uses the /queryz vttablet endpoint where the query gets expanded if schema tracking is enabled, hence failing an exact query check

Signed-off-by: Rohit Nayak <[email protected]>

* test: only use gen4 planner

Signed-off-by: Harshit Gangal <[email protected]>

* fix: column list population in insert with auto-inc column

Signed-off-by: Harshit Gangal <[email protected]>

* fix: change parser to keep empty column list as provided by user

Signed-off-by: Harshit Gangal <[email protected]>

* test: fix test expectation

Signed-off-by: Harshit Gangal <[email protected]>

Co-authored-by: Andres Taylor <[email protected]>
Co-authored-by: Rohit Nayak <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Add parsing support for GTID functions (vitessio#10579)

* feat: add parsing support for GTID functions

Signed-off-by: Kushal Kumar <[email protected]>

* test: add planner and end to end tests for a few gtid functions

Signed-off-by: Manan Gupta <[email protected]>

Co-authored-by: Manan Gupta <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Parameterize BIT types and fixes in HEX types (vitessio#10689)

* feat: add parsing for bitnums

Signed-off-by: Manan Gupta <[email protected]>

* test: add invalid cases to tests

Signed-off-by: Manan Gupta <[email protected]>

* feat: normalize bitnums to bit vals too

Signed-off-by: Manan Gupta <[email protected]>

* feat: add normalization for bit literals

Signed-off-by: Manan Gupta <[email protected]>

* feat: parameterize binary value to hex

Signed-off-by: Harshit Gangal <[email protected]>

* test: added e2e test in vtgate and vttablet

Signed-off-by: Harshit Gangal <[email protected]>

* fix: fix the type conversion from hexnum and hexval to binary

Signed-off-by: Harshit Gangal <[email protected]>

* test: refactor test

Signed-off-by: Harshit Gangal <[email protected]>

* fix: return varbinary and flaghex for hexval and hexnum in eval engine typeOf

Signed-off-by: Harshit Gangal <[email protected]>

* test: separate expectation for mysql and vitess result

Signed-off-by: Harshit Gangal <[email protected]>

* test: fix test expectation

Signed-off-by: Harshit Gangal <[email protected]>

* feat: added bitnum bind variable and test

Signed-off-by: Harshit Gangal <[email protected]>

* proto: vtadmin side update

Signed-off-by: Harshit Gangal <[email protected]>

* test: fixed test and added more bitnum bind var test

Signed-off-by: Harshit Gangal <[email protected]>

* added license header

Signed-off-by: Harshit Gangal <[email protected]>

Co-authored-by: Manan Gupta <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* Mark aggregate functions callable (vitessio#10805)

These should also implement the iCallable interface as they are also
callable as functions.

Signed-off-by: Dirkjan Bussink <[email protected]>
Signed-off-by: Vicent Marti <[email protected]>

* cherry-pick: fix bad merge

Signed-off-by: Vicent Marti <[email protected]>

Co-authored-by: rsajwani <[email protected]>
Co-authored-by: Andres Taylor <[email protected]>
Co-authored-by: Kushal Kumar <[email protected]>
Co-authored-by: Manan Gupta <[email protected]>
Co-authored-by: Manan Gupta <[email protected]>
Co-authored-by: Florent Poinsard <[email protected]>
Co-authored-by: Harshit Gangal <[email protected]>
Co-authored-by: Rohit Nayak <[email protected]>
Co-authored-by: Dirkjan Bussink <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@systay systay systay approved these changes

@harshit-gangal harshit-gangal harshit-gangal approved these changes

@rsajwani rsajwani Awaiting requested review from rsajwani

Assignees
No one assigned
Labels
Component: Query Serving Type: Internal Cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GuptaManan100 @rsajwani @systay @harshit-gangal