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 postgresql alter column default support for insert statement #4912

Merged

Conversation

griffio
Copy link
Contributor

@griffio griffio commented Dec 14, 2023

🚧 🍔 🧷 Adding this to enable SET or DROP DEFAULT in ALTER COLUMN migrations.

This is implemented for use by app.cash.sqldelight.core.lang.psi.InsertStmtValuesMixin to enable/disable the annotation message Cannot populate default values for columns ..., they must be specified in insert statement.

  • ColumnDefMixin is now open to allow subclass to call inherited hasDefaultValue
  • AlterTableAlterColumnMixin inherits from ColumnDefMixin

@griffio griffio marked this pull request as ready for review December 17, 2023 18:34
@AlecKazakova AlecKazakova enabled auto-merge (squash) January 29, 2024 14:08
add open class as we want to call the inherited hasDefaultValue()
We want a column_not_null_clause rule to read  SET or DROP DEFAULT
hasDefaultValue implemented by checking for a DEFAULT has SET or DROP,
otherwise call inherited hasDefaultValue

InsertStmtValuesMixin in the compiler use the result of ColumnDefMixin.hasDefaultValue
@griffio
Copy link
Contributor Author

griffio commented Jan 29, 2024

🤕 After mergings - there is a test fixture failure - will look into this week

execute[alter-table-alter-column]
    1.s line 64:28 - No column found to alter with name c1_id
    1.s line 74:28 - No column found to alter with name c1_id

Rename to correct column name
auto-merge was automatically disabled January 30, 2024 13:41

Head branch was pushed to by a user without write access

@griffio griffio force-pushed the add-postgresql-alter-column-default branch from 363e1e4 to 190cfef Compare January 30, 2024 13:41
@AlecKazakova
Copy link
Collaborator

thanks for fixing 👍

@AlecKazakova AlecKazakova merged commit a0823b4 into cashapp:master Jan 30, 2024
7 checks passed
@griffio griffio deleted the add-postgresql-alter-column-default branch January 30, 2024 14:32
hfhbd pushed a commit that referenced this pull request Apr 2, 2024
* Allow ColumnDefMixin to be inherited

add open class as we want to call the inherited hasDefaultValue()

* Update PostgreSql.bnf

We want a column_not_null_clause rule to read  SET or DROP DEFAULT

* AlterTableAlterColumnMixin  to inherit ColumnDefMixin

hasDefaultValue implemented by checking for a DEFAULT has SET or DROP,
otherwise call inherited hasDefaultValue

InsertStmtValuesMixin in the compiler use the result of ColumnDefMixin.hasDefaultValue

* Fix test

Rename to correct column name
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