[schema][test] Add a data migration validation test #4783
Conversation
| #[derive(PartialEq, Clone, Debug)] | ||
| struct SqlEnum { | ||
| name: String, | ||
| variant: String, | ||
| } |
There was a problem hiding this comment.
I added some types and type conversions here so that I could easily read rows from the database in the "checks after migration" step.
This is necessary because we're using tokio_postgres instead of Diesel to access the DB here. More on that below.
| // TODO: This isn't exhaustive, feel free to add more. | ||
| // | ||
| // These should only be necessary for rows where the database schema changes also choose to | ||
| // populate data. | ||
| } | ||
|
|
||
| impl From<bool> for AnySqlType { |
There was a problem hiding this comment.
These From impls are so that I can pass anything that Into<AnySqlType> when checking the values of columns from rows pulled out of the DB.
| @@ -218,31 +269,53 @@ impl<'a> tokio_postgres::types::FromSql<'a> for AnySqlType { | |||
| if f32::accepts(ty) { | |||
| return Ok(AnySqlType::Float4(f32::from_sql(ty, raw)?)); | |||
| } | |||
| if serde_json::value::Value::accepts(ty) { | |||
There was a problem hiding this comment.
Urgh. So I feel like I'm basically implementing OID checking here, which I'd rather do via a match statement? But reading https://docs.rs/tokio-postgres/0.7.10/tokio_postgres/types/index.html , I'm struggling to find a way to match on the Type type, rather than have this chain of if-statements.
- Type::kind returns an enum of possible "flavors of type", but this isn't enough to infer the conversion to one of our "rust types" in the
AnySqlTypevariant. - Type::oid can return a
u32that exactly tells us the resulting oid. But... it's not a const method? So I can't actually access it from all the constant types declared in the "Associated Constants" of https://docs.rs/tokio-postgres/0.7.10/tokio_postgres/types/struct.Type.html#implementations.
Frustratingly, this info does exist in the postgres_types crate: https://docs.rs/postgres-types/0.2.6/src/postgres_types/type_gen.rs.html#207 but it isn't exposed publically.
| Kind::Enum(_) => { | ||
| Ok(AnySqlType::Enum(SqlEnum { | ||
| name: ty.name().to_string(), | ||
| variant: std::str::from_utf8(raw)?.to_string(), | ||
| })) | ||
| }, |
There was a problem hiding this comment.
This is the laziest possible de-serialization of a user-defined type possible.
"If it's a user-defined type, just throw it into some strings, and do no further checks".
The tests below can perform this validation if they want it -- we don't need compile-time checks on old versions of the DB.
| struct NamedSqlValue { | ||
| // It's a little redunant to include the column name alongside each value, | ||
| // but it results in a prettier diff. | ||
| struct ColumnValue { |
There was a problem hiding this comment.
Only tangentially related to this change, but IMO makes the usage a little more clear.
| ColumnValue::new("ip_pool_id", POOL1), | ||
| ColumnValue::new("resource_type", type_silo.clone()), | ||
| ColumnValue::new("resource_id", SILO1), | ||
| ColumnValue::new("is_default", true), |
There was a problem hiding this comment.
Note that by using weakly-typed access to SQL here, this will continue to work, even if the "new version" of the SQL table changes (Same above during the insertions -- those will be operating on the 23.0.0 version, even if we later upgrade to 30.0.0 and keep changing all these tables).
|
I have high confidence the failures here are due to #4779 , going to merge even though we're seeing limited timeouts. |
Adds a schema test with "before" / "after" hooks, and adds an example specifically for the "23.0.0" migration.
My intent is that this can be used for any other schema migrations that would like to execute arbitrary SQL checks
against the new schema too.
Fixes #4747