Review: Approve db This will also need a follow-up hot patch (targeted to master, and applied live by IS) to create a unique partial index on `AccessArtifact(vulnerability)` and to validate the replaced `has_artifact` constraint. Compare `database/schema/archive/patch-2210-26-4.sql`.
Diff comments: > diff --git a/database/schema/patch-2210-50-0.sql > b/database/schema/patch-2210-50-0.sql > new file mode 100644 > index 0000000..0c23462 > --- /dev/null > +++ b/database/schema/patch-2210-50-0.sql > @@ -0,0 +1,115 @@ > +-- Copyright 2022 Canonical Ltd. This software is licensed under the > +-- GNU Affero General Public License version 3 (see the file LICENSE). > + > +SET client_min_messages=ERROR; > + > +ALTER TABLE Vulnerability > + ADD COLUMN access_policy integer, > + ADD COLUMN access_grants integer[]; > + > +CREATE TABLE VulnerabilitySubscription ( > + id serial PRIMARY KEY, > + person integer REFERENCES Person NOT NULL, > + vulnerability integer REFERENCES Vulnerability NOT NULL, > + date_created timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT > TIME ZONE 'UTC') NOT NULL, > + creator integer REFERENCES Person NOT NULL Please rename this to `subscribed_by`, to match most of the other `*Subscription` tables. > +); > + > +COMMENT ON TABLE VulnerabilitySubscription IS ''; Can you come up with a non-empty table comment here? > +COMMENT ON COLUMN VulnerabilitySubscription.person IS 'The person > subscribing to the vulnerability.'; > +COMMENT ON COLUMN VulnerabilitySubscription.vulnerability IS 'The > vulnerability being subscribed to.'; > +COMMENT ON COLUMN VulnerabilitySubscription.date_created IS 'The date when > the subscription was created.'; > +COMMENT ON COLUMN VulnerabilitySubscription.creator IS 'The person who > created the subscription.'; > + > +CREATE INDEX vulnerabilitysubscription__person__idx > + ON VulnerabilitySubscription (person); There should also be `CREATE UNIQUE INDEX vulnerabilitysubscription__person__vulnerability__key ON VulnerabilitySubscription (person, vulnerability)`, and since the index on just `(person)` would be a leftmost prefix of the columns in that unique index, you can drop this one. I'm not sure why `BugSubscription` doesn't have a unique index on `(person, bug)`. That might be an oversight. > + > +CREATE INDEX vulnerabilitysubscription__vulnerability__idx > + ON VulnerabilitySubscription (vulnerability); > + > +CREATE INDEX vulnerabilitysubscription__creator__idx > + ON VulnerabilitySubscription (creator); > + > +ALTER TABLE AccessArtifact > + ADD COLUMN vulnerability integer REFERENCES Vulnerability; > + > + > +ALTER TABLE AccessArtifact DROP CONSTRAINT has_artifact; > +ALTER TABLE AccessArtifact > + ADD CONSTRAINT has_artifact CHECK ( > + (null_count(ARRAY[bug, branch, gitrepository, snap, specification, > ocirecipe, vulnerability]) = 6)) NOT VALID; > + > + > +CREATE OR REPLACE FUNCTION vulnerability_denorm_access(vulnerability_id > integer) > + RETURNS void LANGUAGE plpgsql AS > +$$ > +DECLARE > + info_type integer; > +BEGIN > + SELECT > + -- information type: 1 = public > + COALESCE(Vulnerability.information_type, 1) > + INTO info_type > + FROM Vulnerability where id = vulnerability_id; > + > + UPDATE Vulnerability > + SET access_policy = policies[1], access_grants = grants > + FROM > + build_access_cache( > + (SELECT id FROM Vulnerability WHERE vulnerability = > vulnerability_id), > + info_type) > + AS (policies integer[], grants integer[]) > + WHERE id = vulnerability_id; > +END; > +$$; > + > +CREATE OR REPLACE FUNCTION accessartifact_denorm_to_artifacts(artifact_id > integer) > + RETURNS void > + LANGUAGE plpgsql > + AS $$ > +DECLARE > + artifact_row accessartifact%ROWTYPE; > +BEGIN > + SELECT * INTO artifact_row FROM accessartifact WHERE id = artifact_id; > + IF artifact_row.bug IS NOT NULL THEN > + PERFORM bug_flatten_access(artifact_row.bug); > + END IF; > + IF artifact_row.branch IS NOT NULL THEN > + PERFORM branch_denorm_access(artifact_row.branch); > + END IF; > + IF artifact_row.gitrepository IS NOT NULL THEN > + PERFORM gitrepository_denorm_access(artifact_row.gitrepository); > + END IF; > + IF artifact_row.snap IS NOT NULL THEN > + PERFORM snap_denorm_access(artifact_row.snap); > + END IF; > + IF artifact_row.specification IS NOT NULL THEN > + PERFORM specification_denorm_access(artifact_row.specification); > + END IF; > + IF artifact_row.ocirecipe IS NOT NULL THEN > + PERFORM ocirecipe_denorm_access(artifact_row.ocirecipe); > + END IF; > + IF artifact_row.vulnerability IS NOT NULL THEN > + PERFORM vulnerability_denorm_access(artifact_row.vulnerability); > + END IF; > + RETURN; > +END; > +$$; > + > +COMMENT ON FUNCTION accessartifact_denorm_to_artifacts(artifact_id integer) > IS > + 'Denormalize the policy access and artifact grants to bugs, branches, > git repositories, snaps, specifications, ocirecipe, and vulnerabilities.'; Maybe fix the oddly singular "ocirecipe" to "OCI recipes" while you're here. > + > +-- A trigger to handle vulnerability.information_type changes. > +CREATE OR REPLACE FUNCTION vulnerability_maintain_access_cache_trig() > RETURNS trigger > + LANGUAGE plpgsql as $$ > +BEGIN > + PERFORM vulnerability_denorm_access(NEW.id); > + RETURN NULL; > +END; > +$$; > + > +CREATE TRIGGER vulnerability_maintain_access_cache > + AFTER INSERT OR UPDATE OF information_type ON Vulnerability > + FOR EACH ROW EXECUTE PROCEDURE > vulnerability_maintain_access_cache_trig(); > + > +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 50, 0); Following the recent rebaseline, please rename this patch to `patch-2211-02-0.sql`, and change this to `INSERT INTO LaunchpadDatabaseRevision VALUES (2211, 02, 0);`. > diff --git a/database/schema/security.cfg b/database/schema/security.cfg > index 56eb404..e23e78a 100644 > --- a/database/schema/security.cfg > +++ b/database/schema/security.cfg > @@ -2466,6 +2466,7 @@ public.vote = SELECT, UPDATE > public.votecast = SELECT, UPDATE > public.vulnerability = SELECT, UPDATE > public.vulnerabilityactivity = SELECT, UPDATE > +public.vulnerabilitysubscription = SELECT, UPDATE, DELETE > public.webhook = SELECT, UPDATE > public.wikiname = SELECT, UPDATE > public.xref = SELECT, UPDATE I think you'll also need to add `EXECUTE` permission for the new `vulnerability_denorm_access` function to the `[public]` section near the top, add `SELECT, INSERT, UPDATE, DELETE` permission for `vulnerabilitysubscription` to the `[launchpad_main]` section, and add `SELECT, UPDATE, DELETE` permission for `vulnerabilitysubscription` to the `[sharing-jobs]` section. -- https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/426864 Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/launchpad:vulnerability-subscription-sql into launchpad:db-devel. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
