sql-psi icon indicating copy to clipboard operation
sql-psi copied to clipboard

add create_type_stmt and alter_type_stmt

Open griffio opened this issue 1 year ago • 4 comments

Explicitly add fake extension points for dialects to implement

This is added to sql-psi core as needs to be referenced in sql-delight to use in Database schema generation

e.g create_type_stmt psi elements must be ordered before create_table_stmt elements

closes #533

griffio avatar Jul 18 '23 14:07 griffio

i think you can accomplish this without fake rules by overriding the stmt type to include your new rules in a downstream dialect

AlecKazakova avatar Jul 18 '23 15:07 AlecKazakova

i think you can accomplish this without fake rules by overriding the stmt type to include your new rules in a downstream dialect

I had tried it before without the Fake assignment and the PSI elements are not generated in core (That was why I assumed the fake declaration was needed for this purpose). i.e. these are not generated without FAKE

com/alecstrong/sql/psi/core/psi/SqlAlterTypeStmt.java
com/alecstrong/sql/psi/core/psi/SqlCreateTypeStmt.java

The reason for adding into core is these need to be referenced in Database Generation - rather than having Postgres types. I also didn't want to have to redeclare all the stmt elements in a dialect just to add a couple more.

griffio avatar Jul 18 '23 15:07 griffio

But you can simple use extension_stmt, how is this different?

hfhbd avatar Aug 17 '23 08:08 hfhbd

@hfhbd

Yes - the override extension_stmt method is how I am proceeding with other tickets 👍

However, CREATE TYPE stmts must be executed before CREATE TABLE stmt that use the new types, the SqlDelight compiler outputs CREATE TABLE stmts first ( 🌳 TreeUtil?) in the generated database source.

The above change was tested with my branch https://github.com/cashapp/sqldelight/compare/master...griffio:sqldelight:add-postgres-enum-type

Where I had to add ordering for createtype in TreeUtil https://github.com/cashapp/sqldelight/compare/master...griffio:sqldelight:add-postgres-enum-type#diff-f77d27d75a872743e1e5330b54cbc046fbdebc09bb4a95c496b40ca9979b7281

Maybe this generated ordering is not an issue for migrations? - so am happy to go with extension_stmt

🕊️ I believe that the core grammar should have these (CREATE/ALTER/DROP) place-holder (abstract) statements that can be overridden by Dialects rather than using extension_stmt all the time. See https://github.com/AlecKazakova/sql-psi/issues/533

Thanks 🍔

griffio avatar Aug 17 '23 10:08 griffio