Code review comment for lp:~michael.nelson/launchpad/627957-differences-schema-update

(14:43:29) stub: noodles775: versien isn't spelled that way
(14:44:26) noodles775: hrm... "base version to derived versien." I'll update it.
14:45
(14:45:22) Abel Deuring [~<email address hidden>] entered the room.
(14:45:56) stub: noodles775: So source_version and parent_source_version are debian version strings? If so we might want a CHECK constraint to ensure that.
(14:46:40) stub: ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid_source_version CHECK (valid_debian_version(source_version));
(14:47:02) noodles775: stub: yes they are. Thanks.
(14:49:37) stub: noodles775: With the new UNIQUE constraint, you can drop the distroseriesdifference__derived_series__idx index.
(14:50:25) stub: noodles775: The name of the new UNIQUE constraint should be distroseriesdifference__derived_series__source_package_name__key
(14:51:16) noodles775: stub: Great - I didn't realised the two-column index makes the single-column foreign-key index redundant. And yep, I'll rename the constraint.
(14:51:16) stub: So a NULL diff means it hasn't been calculated yet I assume. What do NULL versions mean? Probaly worth clarifying this in comments.sql
(14:51:58) noodles775: stub: yes, different types of differences... (unique to the derived series, missing from the derived series etc.). Should there be a constraint to ensure that both are not null together?
(14:52:40) noodles775: stub: er, ignore that (I was thinking about the versions fields).
(14:52:46) stub: If that is what our code expects, then yes. ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid...
(14:52:47) stub: ok
(14:53:49) stub: noodles775: patch-2208-12-0.sql
(14:54:11) noodles775: *sigh*. I need a tea. So yes, you had asked about NULL versions, so we should have a constraint to ensure both versions are not null.
(14:54:15) noodles775: Thanks.
(14:55:27) stub: If only one or the other can be NULL: ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid_versions CHECK (source_version IS NULL != parent_source_version IS NULL)
(14:55:42) noodles775: Great, thanks.
(14:56:54) stub: If we just want to check at most one is NULL: ALTER TABLE DistroSeriesDifference ADD CONSTRAINT valid_versions CHECK (coalesce(source_version, parent_source_version) IS NOT NULL)
(14:58:39) stub: or just 'source_version IS NOT NULL OR parent_source_version IS NOT NULL' is better
(14:58:53) noodles775: Yep.
(14:58:55) stub: I'm not sure which of those two rules is appropriate
15:00
(15:00:22) stub: Can they both be NULL? Can they both be NOT NULL?
(15:00:48) noodles775: They cannot both be null, but they can both be NOT NULL.
(15:02:49) stub: ok... so CHECK (source_version IS NOT NULL OR parent_source_version IS NOT NULL)