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

Initial implementation of partitions (Create statement) #8691

Merged
merged 16 commits into from
Nov 17, 2021

Conversation

Thirumalai-Shaktivel
Copy link
Contributor

@Thirumalai-Shaktivel Thirumalai-Shaktivel commented Aug 26, 2021

Description

This PR adds grammar and AST structs and format funcs for partition operation subpartitions, partition/subpartition options.
Keywords related to partition statements are added. Sufficient partition cases were added to test the grammar change. Further partition grammar will be added in later PRs.

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

Signed-off-by: Thirumalai-Shaktivel <[email protected]>
GuptaManan100 and others added 6 commits August 26, 2021 18:45
Signed-off-by: Manan Gupta <[email protected]>
Signed-off-by: Thirumalai-Shaktivel <[email protected]>
Signed-off-by: Thirumalai-Shaktivel <[email protected]>
Signed-off-by: Thirumalai-Shaktivel <[email protected]>
Signed-off-by: Thirumalai-Shaktivel <[email protected]>
@Thirumalai-Shaktivel
Copy link
Contributor Author

Hi @GuptaManan100, this PR is ready

@Thirumalai-Shaktivel Thirumalai-Shaktivel changed the title Initial partition implementation Initial implementation of partitions (Create statement) Aug 27, 2021
@Thirumalai-Shaktivel Thirumalai-Shaktivel marked this pull request as ready for review August 27, 2021 07:22
@GuptaManan100 GuptaManan100 self-assigned this Aug 27, 2021
@GuptaManan100 GuptaManan100 added Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Aug 27, 2021
@Thirumalai-Shaktivel
Copy link
Contributor Author

Thirumalai-Shaktivel commented Aug 28, 2021

Anything else I need to fix?? If not, then this is good to go in... (I will send another PR for partition_definition and subpartition_definition)

go/vt/sqlparser/sql.y Outdated Show resolved Hide resolved
go/vt/sqlparser/ast.go Outdated Show resolved Hide resolved
go/vt/sqlparser/sql.y Outdated Show resolved Hide resolved
go/vt/sqlparser/sql.y Outdated Show resolved Hide resolved
@GuptaManan100
Copy link
Member

Apart from these requested changes, everything looks good to me! Great work

@Thirumalai-Shaktivel
Copy link
Contributor Author

Hi @GuptaManan100, thanks for the review. I made all the changes as you suggested...

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.

This looks good to me! 😺

@GuptaManan100
Copy link
Member

The unit tests seem to be failing, could you take a look

@Thirumalai-Shaktivel
Copy link
Contributor Author

It seems all the tests are passing in my local system, I don't know where the issue is!!

@harshit-gangal
Copy link
Member

It seems all the tests are passing in my local system, I don't know where the issue is!!

can you rebase from main and push it

@Thirumalai-Shaktivel
Copy link
Contributor Author

Looks good to me!

Signed-off-by: ritwizsinha <[email protected]>
Copy link
Contributor

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

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

LGTM all things parsing! Nice work!

@GuptaManan100 GuptaManan100 merged commit 30cde0e into vitessio:main Nov 17, 2021
@Thirumalai-Shaktivel
Copy link
Contributor Author

Thanks for reviewing and merging the code!!

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.

5 participants