launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Pushed the requested changes with comments.

Diff comments:

> diff --git a/database/schema/patch-2210-17-0.sql b/database/schema/patch-2210-17-0.sql
> new file mode 100644
> index 0000000..ddb5ec6
> --- /dev/null
> +++ b/database/schema/patch-2210-17-0.sql
> @@ -0,0 +1,11 @@
> +-- 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;
> +
> +ALTER TABLE GitRepository ADD COLUMN status INTEGER;

It makes sense to add a SET DEFAULT here. But I was expecting to backfill the table in a hot patch, rather than a garbo job.

I was avoiding to add the DEFAULT in this migration because adding a column with default value indeed does cause the table  to rewrite (which is actually quite fast in pg 11 because it doesn't rewrite each row: https://www.depesz.com/2018/04/04/waiting-for-postgresql-11-fast-alter-table-add-column-with-a-non-null-default/).

But it turns out that adding a column with default value is not the same as altering a column to SET DEFAULT. The first causes table to rewrite, the second doesn't. I'm adding a new statement here to SET DEFAULT.

> +
> +COMMENT ON COLUMN GitRepository.status
> +    IS 'The current situation of this git repository.';

Ok

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


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/385199
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:gitrepo-status-column into launchpad:db-devel.