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

fixes 4879 postgresql class-cast error in alter table rename column during migrations #4880

Merged
merged 8 commits into from
Jan 31, 2024

Conversation

griffio
Copy link
Contributor

@griffio griffio commented Dec 4, 2023

fixes #4879

🥼 🦠 🔬 This PR fixes the error in postgresql dialect migrations when renaming a column by making AlterTableRenameColumnMixin implement SqlColumDef.

Rename in migration fails when the queries are not renamed to the new column, this is because the
compiler annotation message that is expected raises a class cast exception instead.

SqlColumnAlias needs to have a parent element that implements SqlColumnDef.
The renamed column must also copy the nullable state from the renamed column so
that the correct nullable type is maintained after renaming.

Caused by: java.lang.ClassCastException: class app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl 
cannot be cast to class com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin 

(app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl and com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin are in unnamed module of loader 'app')

Build test failure reports

  Caused by:
        java.lang.ClassCastException: class app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl cannot be cast to class com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin (app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl and com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin are in unnamed module of loader 'app')
            at com.alecstrong.sql.psi.core.psi.mixins.InsertStmtValuesMixin.annotate(InsertStmtValuesMixin.kt:58)
            at app.cash.sqldelight.core.lang.psi.InsertStmtValuesMixin.annotate(InsertStmtValuesMixin.kt:61)
            at com.alecstrong.sql.psi.core.SqlCoreEnvironment.annotateRecursively(SqlCoreEnvironment.kt:169)

🍔 🐘 Have tested snapshot against local project https://github.com/griffio/sqldelight-postgres-01

@griffio griffio changed the title Add failure test case for rename Add failure test case for rename in migration Dec 4, 2023
@griffio griffio marked this pull request as ready for review December 6, 2023 18:05
@griffio griffio changed the title Add failure test case for rename in migration fixes 4879 postgresql class-cast error in alter table rename column during migrations Dec 6, 2023
@AlecKazakova
Copy link
Collaborator

Unsure how to resolve this conflict will need your help with that

@griffio
Copy link
Contributor Author

griffio commented Jan 29, 2024

Unsure how to resolve this conflict will need your help with that

I will have a look this week as there are PRs with a failing test now

@griffio
Copy link
Contributor Author

griffio commented Jan 30, 2024

Needs to be re-run ♻️ - problem fetching deps from npm

Rename in migration files
Caused by: java.lang.ClassCastException: class app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl cannot be cast to class com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin (app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl and com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin are in unnamed module of loader 'app')

`at com.alecstrong.sql.psi.core.psi.mixins.InsertStmtValuesMixin.annotate(InsertStmtValuesMixin.kt:58)`
This fixes the class cast exception so that the correct compiler annotation error messages are displayed

SqlColumnAlias needs to have a parent element that implements SqlColumnDef.
The renamed column must also copy the nullable state from the renamed column so
that the correct nullable type is maintained after renaming
first migrated to alpha remains a Long not null type
Revert to singleOrNulll
copy the original column (nullable) and set the new NamedElement from the columnAlias
fixes for AlterTableAlterColumnMixin
open AlterTableRenameColumnMixin, ideally we should be using this PostgreSql implementation
This is for consistency with other PR changes
@AlecKazakova AlecKazakova merged commit d1c912f into cashapp:master Jan 31, 2024
7 checks passed
@griffio griffio deleted the fix-rename-column-migration branch January 31, 2024 13:27
hfhbd pushed a commit that referenced this pull request Apr 2, 2024
…uring migrations (#4880)

* Add failure test case for rename

Rename in migration files
Caused by: java.lang.ClassCastException: class app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl cannot be cast to class com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin (app.cash.sqldelight.dialects.postgresql.grammar.psi.impl.PostgreSqlAlterTableRenameColumnImpl and com.alecstrong.sql.psi.core.psi.mixins.ColumnDefMixin are in unnamed module of loader 'app')

`at com.alecstrong.sql.psi.core.psi.mixins.InsertStmtValuesMixin.annotate(InsertStmtValuesMixin.kt:58)`

* Update AlterTableRenameColumnMixin to inherit SqlColumnDefImpl

This fixes the class cast exception so that the correct compiler annotation error messages are displayed

SqlColumnAlias needs to have a parent element that implements SqlColumnDef.
The renamed column must also copy the nullable state from the renamed column so
that the correct nullable type is maintained after renaming

* Now add the migrated property name

first migrated to alpha remains a Long not null type

* Update AlterTableRenameColumnMixin

Revert to singleOrNulll
copy the original column (nullable) and set the new NamedElement from the columnAlias

* AlterTableAlterColumnMixin

fixes for AlterTableAlterColumnMixin

* AlterTableRenameColumnMixin  extends AlterTableRenameColumnMixin

open AlterTableRenameColumnMixin, ideally we should be using this PostgreSql implementation
This is for consistency with other PR changes

* update test

* Rebase changes
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.

Postgresql migrations using rename column cause compilation error e.g ALTER TABLE X RENAME COLUMN y TO Z
2 participants