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

Error on dangling NO in CREATE SEQUENCE options #1104

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Error on dangling NO in CREATE SEQUENCE options #1104

merged 2 commits into from
Jan 24, 2024

Conversation

m-rph
Copy link
Contributor

@m-rph m-rph commented Jan 20, 2024

Fixes a bug where a NO meant for NO CACHE in create options would be consumed without being followed by CACHE.

Changelog: Fixes: #1103

Fixes a bug where a `NO` meant for `NO CACHE` in create options
would be consumed without being followed by `CACHE`.

Fixes: `#1103`
@tobyhede
Copy link
Contributor

LGTM

@alamb alamb changed the title Fixes 1103 - disallow dangling NO in create sequence options Error on dangling NO in create sequence options Jan 23, 2024
@alamb alamb changed the title Error on dangling NO in create sequence options Error on dangling NO in CREATE SEQUENCE options Jan 23, 2024
@coveralls
Copy link

coveralls commented Jan 23, 2024

Pull Request Test Coverage Report for Build 7632957069

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 87.596%

Totals Coverage Status
Change from base Build 7575556197: -0.3%
Covered Lines: 19194
Relevant Lines: 21912

💛 - Coveralls

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 @partiallytyped -- this looks great.

It looks like CI is failing due to needing to run cargo fmt -- any chance you can run that and we can get this PR in?

@m-rph
Copy link
Contributor Author

m-rph commented Jan 23, 2024

Sure, give me a moment.
Want me to rebase and squash?

@alamb
Copy link
Contributor

alamb commented Jan 24, 2024

Want me to rebase and squash?

No need -- I will do this as part of the merge process

@alamb alamb merged commit 498708c into apache:main Jan 24, 2024
10 checks passed
@alamb
Copy link
Contributor

alamb commented Jan 24, 2024

Thanks again @partiallytyped

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.

Create sequence allows dangling NO
4 participants