Merge ~lgp171188/launchpad:add-locked-column-to-bug-table into launchpad:db-devel

Proposed by Guruprasad
Status: Merged
Approved by: Guruprasad
Approved revision: 39170af1528575fb35bb06c20ff3decb9b12f431
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~lgp171188/launchpad:add-locked-column-to-bug-table
Merge into: launchpad:db-devel
Diff against target: 30 lines (+24/-0)
1 file modified
database/schema/patch-2210-38-0.sql (+24/-0)
Reviewer Review Type Date Requested Status
William Grant db Approve
Colin Watson db Approve
Review via email: mp+413934@code.launchpad.net

Commit message

Add 'lock_status' field to Bug

This will be used to store the values of an enum containing the possible values.

To post a comment you must log in.
Revision history for this message
Guruprasad (lgp171188) wrote :

I think I misread Colin's comment about this field.

> An enum is better than a boolean here, since for now we want "unlocked" and "comment-only" lock states, but it's conceivable that we might want others in future.

So I am updating the status to 'Work in progress' and no review is needed right now.

Revision history for this message
Guruprasad (lgp171188) wrote :

Updated the field to be an integer to store the values of an enum that will contain values for the various possible states.

Revision history for this message
Colin Watson (cjwatson) :
Revision history for this message
Guruprasad (lgp171188) :
Revision history for this message
Guruprasad (lgp171188) wrote :

I have pushed a new revision with the changes to address the review comments on the previous revision.

Revision history for this message
Colin Watson (cjwatson) :
Revision history for this message
Colin Watson (cjwatson) wrote :

LGTM, but adding William for a second DB review.

review: Approve (db)
Revision history for this message
Colin Watson (cjwatson) wrote :

As discussed on Mattermost, I think we probably additionally want a nullable text `lock_reason` column, so that it's possible for somebody to explain why they're locking a bug.

Revision history for this message
Guruprasad (lgp171188) wrote :

Colin, I have updated the patch to add a 'lock_reason' column.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve (db)
Revision history for this message
Guruprasad (lgp171188) :
Revision history for this message
William Grant (wgrant) :
review: Approve (db)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/database/schema/patch-2210-38-0.sql b/database/schema/patch-2210-38-0.sql
2new file mode 100644
3index 0000000..eef88ec
4--- /dev/null
5+++ b/database/schema/patch-2210-38-0.sql
6@@ -0,0 +1,24 @@
7+-- Copyright 2022 Canonical Ltd. This software is licensed under the
8+-- GNU Affero General Public License version 3 (see the file LICENSE).
9+
10+SET client_min_messages=ERROR;
11+
12+ALTER TABLE Bug
13+ ADD COLUMN lock_status integer,
14+ ADD COLUMN lock_reason text;
15+
16+-- ALTER COLUMN ... SET DEFAULT doesn't trigger a table rewrite,
17+-- while ADD COLUMN ... DEFAULT xx does. In pg <11 this operation is slow.
18+-- That's why we first create, and then we set the default value.
19+-- Data backfilling will be done in a garbo job instead of a fast downtime.
20+
21+ALTER TABLE Bug
22+ ALTER COLUMN lock_status
23+ -- 0 = UNLOCKED
24+ SET DEFAULT 0;
25+
26+COMMENT ON COLUMN Bug.lock_status IS 'The current lock status of this bug.';
27+
28+COMMENT ON COLUMN Bug.lock_reason IS 'The reason for locking this bug.';
29+
30+INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 38, 0);

Subscribers

People subscribed via source and target branches

to status/vote changes: