chore(electric): Define __validate_table_column_types() at startup to use the up-to-date list of supported types #431
Conversation
VAX-1033 Future-proof the validation of column types by creating the SQL function at Electric startup time
See VAX-1016 for the larger context. The goal of this issue is to unblock the path for adding more types on the server without modifying existing Extension migrations or duplicating the SQL function definition in new migrations. |
alco
changed the title
alco
force-pushed
the
alco/vax-1033-define-sql-type-checking-function-at-startup
branch
from
7638a06 to
329f40c
Compare
| @@ -267,6 +278,8 @@ defmodule Electric.Postgres.Extension do | |||
| create_migration_table(txconn) | |||
|
|
|||
| with_migration_lock(txconn, fn -> | |||
| define_functions(txconn) | |||
There was a problem hiding this comment.
I think it's better to move the function definitions after the schema migrations. This would allow for any run-once preliminary setup to happen before defining the functions.
There was a problem hiding this comment.
That would mean taking the definition of the electrify function out of an internal migration and moving it to runtime as well. I wanted to avoid such drastic changes in this PR and only limit it to the type-checking function.
I can add a note explaining that this call needs to be moved elsewhere as part of VAX-1016.
alco
force-pushed
the
alco/vax-1033-define-sql-type-checking-function-at-startup
branch
2 times, most recently
from
c9a6082 to
d18bc15
Compare
icehaunter
force-pushed
the
alco/vax-1033-define-sql-type-checking-function-at-startup
branch
from
d18bc15 to
c4d6369
Compare
| |> Enum.join(",") | ||
| %> | ||
|
|
||
| CREATE OR REPLACE FUNCTION electric.__validate_table_column_types(table_name text) |
There was a problem hiding this comment.
I'm going to merge this PR in front of launch so that we have a better upgrade story, but electric here should probably be taken from @schema in Extension module, as in all other places.
There was a problem hiding this comment.
This code used to have <%= @schema %> when it was part of the internal migration because schema is passed as an argument to the migration's up() function. But I also noticed that the migration that adds conflict resolution triggers has electric hard-coded, and so I made it hard-coded here too before I knew the context in which the higher-level define_functions() function would be called in.
icehaunter
deleted the
alco/vax-1033-define-sql-type-checking-function-at-startup
branch
Closes VAX-1033.