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

Alter table add constraint postgres #4116

Merged

Conversation

griffio
Copy link
Contributor

@griffio griffio commented Apr 27, 2023

Minimal support for Postgres alter table add constraint as learning how all this works.

  ALTER TABLE test
  ADD CONSTRAINT idx_external_event_id
  UNIQUE (external_event_id);

The changes are made here on sqldelight as the postgressql grammer was moved from sql-psi

Noticed that issue was raised in AlecKazakova/sql-psi#286

Have signed contributors agreement.


TODOs

Check constraints - there needs to be support for type expr functions like char_length

e.g

ALTER TABLE distributors ADD CONSTRAINT zipchk CHECK (char_length(zipcode) = 5);

This is copied from mysql fixtures to the postgressql
dialect.
Update parserImports to include ADD and FOREIGN override types.
This was necessary for all the fixture tests to pass including the inherited fixture
tests from sql-psi.

Copied similar grammar structure from mysql dialect.

Issues found:
This is minimal enough for unique key and primary key - as I am figuring it out as I go along
CHECK constraints need to be addressed as currently useful functions like `char_length` are missing.
Copy link
Collaborator

@AlecKazakova AlecKazakova left a comment

Choose a reason for hiding this comment

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

awesome! Thank you!

@AlecKazakova AlecKazakova merged commit d7a125a into cashapp:master Apr 27, 2023
@griffio griffio deleted the alter-table-add-constraint-postgres branch April 28, 2023 08:00
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.

None yet

2 participants