Skip to content

Support multiple PARTITION statements in ALTER TABLE ADD statement #1011

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

Merged

Conversation

bitemyapp
Copy link
Contributor

@bitemyapp bitemyapp commented Oct 19, 2023

Revival of #657
Closes #657

Similar story to #1010

I compared the patches and nothing seemed to have changed significantly with the rebase. Please let me know what I'd need to do to make this merge-able. This patch includes #1010 so you could merge this one and close the other or merge the other first and then I can rebase this patch again.

@alamb
Copy link
Contributor

alamb commented Oct 20, 2023

Thanks @bitemyapp -- This is on my list -- can you please resolve the CI failures and I'll take a look

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution @bitemyapp -- I took the liberty of merge this branch up from main, fixing the compile error, and simplifying the logic.


#[test]
fn parse_describe() {
let describe = r#"DESCRIBE namespace.`table`"#;
Copy link
Contributor

Choose a reason for hiding this comment

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

this I think was part of #1010, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes sorry, this MR was based on top of 1010's because I was scrupulous/worried about accidentally having them trip over each other. I think that was a mistake and I should've kept the branches and patches independent of each other. Looks like it worked out anyway, thank you!

@alamb alamb changed the title Hive multi partition declarations Support multiple PARTITION statements in ALTER TABLE ADD statement Oct 24, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6626049286

  • 19 of 21 (90.48%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 87.383%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/ast/ddl.rs 4 5 80.0%
src/parser/mod.rs 10 11 90.91%
Totals Coverage Status
Change from base Build 6625649878: 0.0%
Covered Lines: 16996
Relevant Lines: 19450

💛 - Coveralls

@alamb alamb merged commit 004a8dc into apache:main Oct 24, 2023
serprex pushed a commit to serprex/sqlparser-rs that referenced this pull request Nov 6, 2023
apache#1011)

Co-authored-by: Chris A <chrisa@indeed.com>
Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants