(This if followup from #9570, where this came up because we changed a diesel model type from T to Option<T> without a corresponding change to the db schema, because the db schema already defined the column as nullable.)
We should audit for nullability discrepancies between the diesel schema and the db schema. A couple examples from main:
switch_port disagrees about the nullability of rack_id, switch_location, and port_name:
|
table! { |
|
switch_port (id) { |
|
id -> Uuid, |
|
rack_id -> Uuid, |
|
switch_location -> Text, |
|
port_name -> Text, |
|
port_settings_id -> Nullable<Uuid>, |
|
} |
|
} |
|
CREATE TABLE IF NOT EXISTS omicron.public.switch_port ( |
|
id UUID PRIMARY KEY, |
|
rack_id UUID, |
|
switch_location TEXT, |
|
port_name TEXT, |
|
port_settings_id UUID, |
|
|
|
CONSTRAINT switch_port_rack_locaction_name_unique UNIQUE ( |
|
rack_id, switch_location, port_name |
|
) |
|
); |
switch_port_settings_link_config disagrees about the nullability of mtu and speed. (At a glance it looks like they also disagree about port_settings_id and link_name, but those are implicitly NOT NULL because they compose the primary key. I think it'd be clearer if we tagged them with an explicit NOT NULL anyway, though.):
|
table! { |
|
switch_port_settings_link_config (port_settings_id, link_name) { |
|
port_settings_id -> Uuid, |
|
link_name -> Text, |
|
mtu -> Int4, |
|
fec -> Nullable<crate::enums::SwitchLinkFecEnum>, |
|
speed -> crate::enums::SwitchLinkSpeedEnum, |
|
autoneg -> Bool, |
|
lldp_link_config_id -> Nullable<Uuid>, |
|
tx_eq_config_id -> Nullable<Uuid>, |
|
} |
|
} |
|
CREATE TABLE IF NOT EXISTS omicron.public.switch_port_settings_link_config ( |
|
port_settings_id UUID, |
|
link_name TEXT, |
|
mtu INT4, |
|
fec omicron.public.switch_link_fec, |
|
speed omicron.public.switch_link_speed, |
|
autoneg BOOL NOT NULL DEFAULT false, |
|
lldp_link_config_id UUID, |
|
tx_eq_config_id UUID, |
|
|
|
PRIMARY KEY (port_settings_id, link_name) |
|
); |
Ideally, any of the columns which are not nullable on the Rust side could be made NOT NULL in a db migration, because we never would have inserted a NULL value. But this assumes (a) the types have always been not nullable on the Rust side and (b) there isn't an insertion path where the columns get default-value-NULL inserted.
(This if followup from #9570, where this came up because we changed a diesel model type from
TtoOption<T>without a corresponding change to the db schema, because the db schema already defined the column as nullable.)We should audit for nullability discrepancies between the diesel schema and the db schema. A couple examples from
main:switch_portdisagrees about the nullability ofrack_id,switch_location, andport_name:omicron/nexus/db-schema/src/schema.rs
Lines 114 to 122 in 4c9d4a0
omicron/schema/crdb/dbinit.sql
Lines 3516 to 3526 in 4c9d4a0
switch_port_settings_link_configdisagrees about the nullability ofmtuandspeed. (At a glance it looks like they also disagree aboutport_settings_idandlink_name, but those are implicitlyNOT NULLbecause they compose the primary key. I think it'd be clearer if we tagged them with an explicitNOT NULLanyway, though.):omicron/nexus/db-schema/src/schema.rs
Lines 161 to 172 in 4c9d4a0
omicron/schema/crdb/dbinit.sql
Lines 3597 to 3608 in 4c9d4a0
Ideally, any of the columns which are not nullable on the Rust side could be made
NOT NULLin a db migration, because we never would have inserted aNULLvalue. But this assumes (a) the types have always been not nullable on the Rust side and (b) there isn't an insertion path where the columns get default-value-NULL inserted.