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

Update grammar #33

Merged
merged 2 commits into from
Sep 5, 2022
Merged

Update grammar #33

merged 2 commits into from
Sep 5, 2022

Conversation

EnchoMishinevQC
Copy link
Contributor

Adding two constructs to the grammar.

  1. DROP FUNCTION
  2. FILLFACTOR as an index option

Most files are autogenerated. It makes most sense to review the changes only in tsql.g4

@ivergara
Copy link
Collaborator

ivergara commented Sep 1, 2022

@EnchoMishinevQC I'd suggest that next time you split this into two commits one having the change in the grammar and the other adding the generated files. Then it's faster and easier to just see the relevant commit with the changes of the grammar ;)

@EnchoMishinevQC
Copy link
Contributor Author

@EnchoMishinevQC I'd suggest that next time you split this into two commits one having the change in the grammar and the other adding the generated files. Then it's faster and easier to just see the relevant commit with the changes of the grammar ;)

Good thinking. Did it for this PR as well.

kklein
kklein approved these changes Sep 5, 2022
View reviewed changes
Copy link
Collaborator

@kklein kklein left a comment

Choose a reason for hiding this comment

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

FILLFACTOR as an index option

I don't see with what change this has been addressed - other than that lgtm!

@EnchoMishinevQC
Copy link
Contributor Author

FILLFACTOR as an index option

I don't see with what change this has been addressed - other than that lgtm!

Index options allow an arbitrary simple_id construct. The FILLFACTOR wasn't part of it - it's added in this PR.

@EnchoMishinevQC EnchoMishinevQC merged commit d27d4ee into main Sep 5, 2022
@EnchoMishinevQC EnchoMishinevQC deleted the update-grammar branch September 5, 2022 08:39
@svengiegerich svengiegerich mentioned this pull request Jan 18, 2023
Merged
@svengiegerich svengiegerich mentioned this pull request Mar 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kklein kklein kklein approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@EnchoMishinevQC @ivergara @kklein