-
Notifications
You must be signed in to change notification settings - Fork 332
Support for creating unlogged tables (simplified) #699
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
base: master
Are you sure you want to change the base?
Conversation
These modifiers are keywords that can be inserted between the tokens "CREATE" and "TABLE". See https://www.postgresql.org/docs/current/sql-createtable.html. For example, in postgres we can use :modifiers to create unlogged tables: create table(:sessions, modifiers: UNLOGGED) The statement above now produces the following DDL: CREATE UNLOGGED TABLE sessions ...
greg-rychlewski
left a comment
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.
Thanks for the PR. It looks good. Besides the comment I left we should also add this to MySQL and SQL Server or raise if they don't support it.
| defp modifiers_expr(<<_, _::binary>> = value), do: [String.upcase(String.trim(value)), ?\s] | ||
| defp modifiers_expr(value), do: error!(nil, "PostgreSQL adapter expects :modifiers to be a non-empty string or nil, got #{inspect(value)}") |
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.
| defp modifiers_expr(<<_, _::binary>> = value), do: [String.upcase(String.trim(value)), ?\s] | |
| defp modifiers_expr(value), do: error!(nil, "PostgreSQL adapter expects :modifiers to be a non-empty string or nil, got #{inspect(value)}") | |
| defp modifiers_expr(modifiers) when is_binary(modifiers), do: [modifiers, ?\s] | |
| defp modifiers_expr(other), do: error!(nil, "PostgreSQL adapter expects :modifiers to be a string, got #{inspect(other)}") |
This is for consistency with similar things like options. The existing pattern is not to normalize and not to care if they send an empty string. Raising seems a bit harsh for empty string as well since it's still valid SQL and they might populate this from a configuration value that is empty in some environments.
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.
Makes sense, thanks for explaining
|
Thanks for the review @greg-rychlewski, I'll most likely have an update on it in the next weekend |
This PR introduces the
:modifiersfield toEcto.Migration.Tablestruct, which can be provided as an option toEcto.Migration.table/2.Modifiers¹ are known keywords that can be inserted between the tokens "CREATE" and "TABLE", for example the "UNLOGGED" keyword in PostgreSQL (see Create Table - PostgreSQL).
The statement above now produces the following DDL:
Also, the DDL logs have been updated to include the modifiers when creating a table (seemed like a nice touch):
$ mix ecto.migrate 17:11:49.826 [info] == Running 20251128173650 MyApp.Repo.Migrations.CreateWebhooksTable.change/0 forward 17:11:49.828 [info] create unlogged table webhooks 17:11:49.838 [info] == Migrated 20251128173650 in 0.0s¹ It was discussed here the use of the nomenclature parameters for this, but parameters aren't limited to show up between "CREATE" and "TABLE". For example, they can appear at the end of create table the statement. That's why I'm using the term modifiers.