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 4897 sqlite alter table rename column #4899

Merged

Conversation

griffio
Copy link
Contributor

@griffio griffio commented Dec 8, 2023

fixes #4897

🚧 🦺 fixes for sqlite_3_25 -> sqlite_3_35

  • AlterTableColumnAliasMixin parse node as a SqlColumnName
  • AlterTableRenameColumnMixin extends SqlColumnDefImpl
  • sqlite.bnf replace {column_alias} with alter_table_column_alias implements SqlColumnName

🛸 It's possible to import the fixed alter_table_rename_column into sqlite_3_35 from sqlite_3_25
👽 as sqlite_3_35 adds some more rules to alter_table_rules the alter_table_rename_column needs to be re-added.
🐄 It wasn't possible to re-implement the new changes to the rule alter_table_rename_table insqlite_3_35 sqlite.bnf so the below is much cleaner.

 "static app.cash.sqldelight.dialects.sqlite_3_25.grammar.SqliteParserUtil.alterTableRenameColumnExt"
 "static app.cash.sqldelight.dialects.sqlite_3_25.grammar.SqliteParser.alter_table_rename_column_real"
...

alter_table_rules ::= (
  {alter_table_add_column}
  | {alter_table_rename_table}
  | alter_table_rename_column_inherited
  | alter_table_drop_column

....

private alter_table_rename_column_inherited ::= <<alterTableRenameColumnExt <<alter_table_rename_column_real>>>>

Maybe some other tests?

@griffio griffio force-pushed the fix-sqlite-alter-table-rename-column branch from 24b7519 to 0c108a5 Compare December 8, 2023 12:57
@griffio griffio marked this pull request as ready for review December 8, 2023 19:59
AlterTableColumnAliasMixin
AlterTableRenameColumnMixin

implement as SqlColumnDef
Add alter-table-rename-column-sqlite

Needs as sqlite doesn't use JDBCPreparedStatement so test cannot be shared with `alter-table-rename-column`
alter_table_rename_column is imported into sqlite_3_35 as the alter_table_rules are
overridden and need to include the alter_table_rename_column rules as well.

This appears to be cleaner than trying to reimplement alter_table_rename_column
Use singleOrNull as potentially if the columnName is renamed already the predicate may return null

Therefore the annotation should be displayed as a compiler message "No column found to modify with name <columnName>"
@griffio griffio force-pushed the fix-sqlite-alter-table-rename-column branch from 096b2a0 to 96fecb7 Compare January 4, 2024 13:54
@AlecKazakova AlecKazakova merged commit 33d6a61 into cashapp:master Jan 29, 2024
7 checks passed
@griffio griffio deleted the fix-sqlite-alter-table-rename-column branch January 29, 2024 15:36
hfhbd pushed a commit that referenced this pull request Apr 2, 2024
* fixes for Sqlite alter column

AlterTableColumnAliasMixin
AlterTableRenameColumnMixin

implement as SqlColumnDef

* Add MigrationQueryTest

Add alter-table-rename-column-sqlite

Needs as sqlite doesn't use JDBCPreparedStatement so test cannot be shared with `alter-table-rename-column`

* Add inherited alter_table_rename_column rule

alter_table_rename_column is imported into sqlite_3_35 as the alter_table_rules are
overridden and need to include the alter_table_rename_column rules as well.

This appears to be cleaner than trying to reimplement alter_table_rename_column

* Update AlterTableRenameColumnMixin

Use singleOrNull as potentially if the columnName is renamed already the predicate may return null

Therefore the annotation should be displayed as a compiler message "No column found to modify with name <columnName>"
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.

Sqlite 3.25 ALTER TABLE ... RENAME COLUMN ... TO ... statement compilation error in migrations
2 participants