launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve db

Thanks, just a couple comments.

Diff comments:

> diff --git a/database/schema/patch-2210-31-0.sql b/database/schema/patch-2210-31-0.sql
> new file mode 100644
> index 0000000..3a15c86
> --- /dev/null
> +++ b/database/schema/patch-2210-31-0.sql
> @@ -0,0 +1,47 @@
> +-- Copyright 2021 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 Message
> +    ADD COLUMN date_deleted timestamp without time zone,
> +    ADD COLUMN date_last_edited timestamp without time zone;
> +
> +CREATE TABLE MessageRevision (
> +    id serial PRIMARY KEY,
> +    message integer NOT NULL REFERENCES Message,
> +    subject text,
> +    revision integer NOT NULL,

I somewhat disagree with Colin here, and would default to using something like (message, date_created, id) as the ordering, but revision doesn't complicate things by much.

But can you put it before subject, so the key columns are first?

> +    date_created timestamp without time zone NOT NULL,
> +    date_deleted timestamp without time zone
> +) WITH (fillfactor='100');
> +
> +CREATE UNIQUE INDEX messagerevision__message__revision__key
> +    ON MessageRevision(message, revision);
> +
> +COMMENT ON TABLE MessageRevision IS 'Old versions of an edited Message';
> +COMMENT ON COLUMN MessageRevision.message
> +    IS 'The current message of this revision';
> +COMMENT ON COLUMN MessageRevision.revision
> +    IS 'The revision monotonic increasing number';
> +COMMENT ON COLUMN MessageRevision.date_created
> +    IS 'When the original message was edited and created this revision';
> +COMMENT ON COLUMN MessageRevision.date_deleted
> +    IS 'If this revision was deleted, when did that happen';
> +
> +
> +CREATE TABLE MessageRevisionChunk (
> +    id serial PRIMARY KEY,
> +    messagerevision integer NOT NULL REFERENCES MessageRevision,
> +    sequence integer NOT NULL,
> +    content text NOT NULL
> +) WITH (fillfactor='100');

Should this have some indexes? Like UNIQUE (messagerevision, sequence), to allow lookup and ensure uniqueness.

> +
> +COMMENT ON TABLE MessageRevisionChunk
> +    IS 'Old chunks of a message when a revision was created for it';
> +COMMENT ON COLUMN MessageRevisionChunk.sequence
> +    IS 'Order of this particular chunk';
> +COMMENT ON COLUMN MessageRevisionChunk.content
> +    IS 'Text content for this chunk of the message.';
> +
> +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0);


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-db-patch.