-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add support for extra SQL statements in table creation and dropping #7261
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
Conversation
d2cf6b9 to
ad915a5
Compare
ad915a5 to
667968f
Compare
| } | ||
| } | ||
|
|
||
| if ($createForeignKeys && $table->hasOption(Table::OPTION_EXTRA_CREATE_SQL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should be adding new options to the Table class. Options is an untyped associative array whose value types need to be validated at runtime. Using options has been already deprecated for some schema objects like indexes.
And as for the feature itself, Table is a value object that represents the table schema. The SQL used to create the table is defined by the platform, not Table. If we allow that, I can imagine all sorts of issues with schema introspection, migrations and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I fully get the concern but anyway: would you like to suggest me an alternative that'd address your requirements and that'd enable the target I'm trying to achieve here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, sorry. I don't know how exactly the schema management capabilities from DBAL are integrated into Symfony and what the use case is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Symfony, this is used to generate migration files that contain extra statements, in our case a trigger and a function on PgSQL, to leverage pg_notify. See symfony/symfony#62820 for how this PR would be used. That's quite simple and effective plumbing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s also a footgun similar to custom column DDL where people use it for various workarounds and then get surprised why their schema migrations don’t work as expected.
In this specific case, it doesn’t look like you need the DBAL to execute these platform-specific and application-specific statements. You can execute them in Symfony before or after the migration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow your lead here. Symfony isn't really involved here, this is all doctrine/* code, with a lot of hardcoded stuff. About the risk either: I don't see how preventing ppl from achieving what they need by closing everything makes a better platform.
You can execute them in Symfony before or after the migration
What we need to achieve is generating migration files with extra statements that people can then inspect/commit. How does hooking before/after the migration come into play in your mind?
|
I'll try another approach that doesn't need this. |
This provides a simple and effective way to restore a functionality that was dropped from DBAL v4 without replacement, affecting downstream projects, see #5784
The related issue on Symfony is symfony/symfony#54039 but more projects are desperately in need of this capability.
This PR adds support for injecting custom SQL statements during table creation and dropping operations through two new table options:
Table::OPTION_EXTRA_CREATE_SQL: An array of SQL statements that are executed after a table is createdTable::OPTION_EXTRA_DROP_SQL: An array of SQL statements that are executed before a table is dropped