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

Add the QUALIFY keyword #31065

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

ggevay
Copy link
Contributor

@ggevay ggevay commented Jan 16, 2025

This PR adds the QUALIFY keyword, which is a new reserved keyword. We should quickly add this now, before the first self-managed release is cut, because now we still can check that no user would get in conflict with the new reserved keyword by checking all cloud usage.

Discussed on Slack here and here.

I'm running a Snowflake query to check for occurrences right now, because of the reservedness of the keyword, see Slack discussions. (I checked it once 3 months ago, but checking again now.) Edit: Done. The new Snowflake query also didn't turn up any conflicts for either QUALIFY or WINDOW.

Motivation

Tips for reviewer

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@ggevay ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Jan 16, 2025
@ggevay ggevay requested a review from a team as a code owner January 16, 2025 15:33
@ggevay ggevay requested a review from jkosh44 January 16, 2025 15:33
@ggevay ggevay force-pushed the qualify-prototype branch from 7aa5d1f to f1c57a2 Compare January 16, 2025 15:37
@ggevay ggevay mentioned this pull request Jan 16, 2025
5 tasks
Copy link
Contributor

@jkosh44 jkosh44 left a comment

Choose a reason for hiding this comment

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

LGTM, but can you add some parser tests?

@ggevay
Copy link
Contributor Author

ggevay commented Jan 16, 2025

Thanks, will add parser tests!

@ggevay ggevay force-pushed the qualify-prototype branch 2 times, most recently from 43e5099 to ff6b73f Compare January 16, 2025 17:09
@ggevay ggevay force-pushed the qualify-prototype branch from ff6b73f to b2f7519 Compare January 16, 2025 19:40
@ggevay ggevay enabled auto-merge January 16, 2025 19:42
@ggevay ggevay merged commit db87a48 into MaterializeInc:main Jan 16, 2025
79 of 80 checks passed
def- added a commit to def-/materialize that referenced this pull request Jan 17, 2025
Seems to have happened accidentally in MaterializeInc#31065
@def- def- mentioned this pull request Jan 17, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ADAPTER Topics related to the ADAPTER layer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants