launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/database/schema/patch-2210-12-0.sql b/database/schema/patch-2210-12-0.sql
> new file mode 100644
> index 0000000..6e63371
> --- /dev/null
> +++ b/database/schema/patch-2210-12-0.sql
> @@ -0,0 +1,35 @@
> +-- Copyright 2020 Canonical Ltd.  This software is licensed under the
> +-- GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +SET client_min_messages=ERROR;
> +
> +
> +CREATE TABLE signingkey (
> +    id serial PRIMARY KEY,
> +    key_type integer NOT NULL,
> +    description text NULL,

Our convention is not to explicitly write NULL for nullable columns, though it doesn't hurt.

> +    fingerprint text NOT NULL,
> +    public_key bytea NOT NULL,
> +    date_created timestamp without time zone NOT NULL
> +        DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC')
> +);
> +
> +
> +CREATE TABLE archivesigningkey (
> +    id serial PRIMARY KEY,
> +    archive integer NOT NULL REFERENCES archive,
> +    distro_series integer NULL REFERENCES distroseries,

As also mentioned in my review of the code branch, I think this should probably be something like earliest_distro_series, to fend off possible confusion.

> +    signing_key integer NOT NULL REFERENCES signingkey,
> +    date_created timestamp without time zone NOT NULL
> +        DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC')
> +);
> +
> +
> +CREATE INDEX archivesigningkey__archive__idx
> +    ON archivesigningkey(archive);
> +
> +CREATE INDEX archivesigningkey__distro_series__idx
> +    ON archivesigningkey(distro_series);

I don't think archivesigningkey__distro_series__idx will ever be used; AFAICS it should be deleted.  You'll also need:

CREATE UNIQUE INDEX signingkey__key_type__fingerprint__key
    ON signingkey (key_type, fingerprint);

In principle there should probably also be a unique constraint on (SigningKey.key_type, ArchiveSigningKey.archive, ArchiveSigningKey.earliest_distro_series), but I'm not sure how to express that.  Hmm.

> +
> +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 12, 0);
> +


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/379218
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:db-patch-lp-signing-integration into launchpad:db-devel.