Code review comment for lp:~michael.nelson/launchpad/distro-series-difference-schema
- distro-series-difference-schema
- Merge into db-devel
Revision history for this message
| Michael Nelson (michael.nelson) wrote : | # |
| 1 | === modified file 'database/schema/comments.sql' |
| 2 | --- database/schema/comments.sql 2010-08-27 06:50:09 +0000 |
| 3 | +++ database/schema/comments.sql 2010-08-27 10:45:34 +0000 |
| 4 | @@ -508,10 +508,14 @@ |
| 5 | COMMENT ON COLUMN DistroSeriesDifference.derived_series IS 'The derived distroseries with the difference from its parent.'; |
| 6 | COMMENT ON COLUMN DistroSeriesDifference.source_package_name IS 'The name of the source package which is different in the two series.'; |
| 7 | COMMENT ON COLUMN DistroSeriesDifference.last_package_diff IS 'The most recent package diff that was created for this difference.'; |
| 8 | -COMMENT ON COLUMN DistroSeriesDifference.activity_log IS 'A log of actions and/or comments by users regarding action to be taken.'; |
| 9 | COMMENT ON COLUMN DistroSeriesDifference.status IS 'A distroseries difference can be needing attention, ignored or resolved.'; |
| 10 | COMMENT ON COLUMN DistroSeriesDifference.difference_type IS 'The type of difference that this record represents - a package unique to the derived series, or missing, or in both.'; |
| 11 | |
| 12 | +-- DistroSeriesDifferenceMessage |
| 13 | +COMMENT ON TABLE DistroSeriesDifferenceMessage IS 'A message/comment on a distro series difference.'; |
| 14 | +COMMENT ON COLUMN DistroSeriesDifferenceMessage.distro_series_difference IS 'The distro series difference for this comment.'; |
| 15 | +COMMENT ON COLUMN DistroSeriesDifferenceMessage.message IS 'The comment for the distro series difference.'; |
| 16 | + |
| 17 | -- DistroSeriesPackageCache |
| 18 | |
| 19 | COMMENT ON TABLE DistroSeriesPackageCache IS 'A cache of the text associated with binary packages in the distroseries. This table allows for fast queries to find a binary packagename that matches a given text.'; |
| 20 | |
| 21 | === renamed file 'database/schema/patch-2208-99-0.sql' => 'database/schema/patch-2208-07-0.sql' |
| 22 | --- database/schema/patch-2208-99-0.sql 2010-08-24 14:55:03 +0000 |
| 23 | +++ database/schema/patch-2208-07-0.sql 2010-08-27 10:52:17 +0000 |
| 24 | @@ -5,7 +5,6 @@ |
| 25 | derived_series integer NOT NULL CONSTRAINT distroseriesdifference__derived_series__fk REFERENCES distroseries, |
| 26 | source_package_name integer NOT NULL CONSTRAINT distroseriesdifference__source_package_name__fk REFERENCES sourcepackagename, |
| 27 | last_package_diff integer CONSTRAINT distroseriesdifference__last_package_diff__fk REFERENCES packagediff, |
| 28 | - activity_log text, |
| 29 | status integer NOT NULL, |
| 30 | difference_type integer NOT NULL |
| 31 | ); |
| 32 | @@ -13,5 +12,15 @@ |
| 33 | CREATE INDEX distroseriesdifference__source_package_name__idx ON distroseriesdifference(source_package_name); |
| 34 | CREATE INDEX distroseriesdifference__status__idx ON distroseriesdifference(status); |
| 35 | CREATE INDEX distroseriesdifference__difference_type__idx ON distroseriesdifference(difference_type); |
| 36 | - |
| 37 | -INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 99, 0); |
| 38 | +CREATE INDEX distroseriesdifference__last_package_diff__idx ON distroseriesdifference(last_package_diff); |
| 39 | + |
| 40 | +CREATE TABLE DistroSeriesDifferenceMessage( |
| 41 | + id serial PRIMARY KEY, |
| 42 | + distro_series_difference integer NOT NULL CONSTRAINT distroseriesdifferencemessage__distro_series_difference__fk REFERENCES distroseriesdifference, |
| 43 | + message integer NOT NULL CONSTRAINT distroseriesdifferencemessage__message__fk REFERENCES message, |
| 44 | + UNIQUE (distro_series_difference, message) |
| 45 | +); |
| 46 | +CREATE INDEX distroseriesdifferencemessage__distroseriesdifference__idx ON distroseriesdifferencemessage(distro_series_difference); |
| 47 | +CREATE INDEX distroseriesdifferencemessage__message__idx ON distroseriesdifferencemessage(message); |
| 48 | + |
| 49 | +INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 07, 0); |
| 50 | |
| 51 | === modified file 'database/schema/security.cfg' |
| 52 | --- database/schema/security.cfg 2010-08-27 06:50:09 +0000 |
| 53 | +++ database/schema/security.cfg 2010-08-27 10:45:11 +0000 |
| 54 | @@ -135,6 +135,7 @@ |
| 55 | public.distributionsourcepackage = SELECT, INSERT, UPDATE, DELETE |
| 56 | public.distributionsourcepackagecache = SELECT |
| 57 | public.distroseriesdifference = SELECT, INSERT, UPDATE |
| 58 | +public.distroseriesdifferencemessage = SELECT, INSERT, UPDATE |
| 59 | public.distroserieslanguage = SELECT, INSERT, UPDATE |
| 60 | public.distroseriespackagecache = SELECT |
| 61 | public.emailaddress = SELECT, INSERT, UPDATE, DELETE |
On Fri, Aug 27, 2010 at 12:03 PM, Stuart Bishop
<email address hidden> wrote:
> Review: Approve db
> Confused myself. This is all fine. The difference references the derived distro series it is for. The derived distro series references its parent distro series.
>
> Add the index and you are good to go from my pov.
Thanks Stuart. Based on our conversation [1] I've updated to remove
the activity_log column and instead added a joining table for messages
for the DistroSeriesDif
based it on the similar BugMessage table, but please check it in case
I've missed something.
[1] 12:04 < noodles775> stub: Why would we want delete permissions? At
the moment I'm planning that these differences will end in a resolved
state, but we wouldn't want to delete them until the derived
distroseries is itself deleted (if it ever is).
12:05 < stub> The derived distribution has a patched version of
firefox. Tomorrow it decides that it should just use the parents
version of firefox.
12:07 < noodles775> Great, so assuming the parents version has a
higher revno, they upload it, this record is marked as resolved and
the activity log is updated, but the record is not deleted, as
tomorrow the parent might upload a new version, and the previous
comments/activity is still useful.
12:07 < stub> ok
12:08 < stub> How big do you think the comments and activity will grow?
12:08 < stub> It will be a pain to split out into a structured format later.
12:10 < stub> If it is append only, storing it as a single text blob
isn't ideal. If it is more a whiteboard or a copy of a text file
stored in a branch, that would be fine I guess.
12:10 < noodles775> On average 4 or 5 lines. If a particular
difference goes through resolved->needs attention-> resolved it would
be a few more.
12:10 < stub> But over time
12:10 < noodles775> Yes, it is just like a whiteboard.
12:11 < stub> ok
12:11 < noodles775> Yep, so if overtime it went through that cycle
(resolved->needs attention-
for each change.
12:11 < noodles775> Do you think its worth adding a separate comment model?
12:12 < noodles775> (from memory the code/bugs guys made the comment
model re-usable... I'll check).
12:14 < noodles775> stub: oh, and I think I misunderstood your comment
above. I am making the activity_log append only.
12:17 < stub> So we would probably be better off with a separate table
containing the activity, one row per addition. Or even a link table
between distroseriesdif
spools.
12:18 < noodles775> Yep, just looking at the message table... I'll do that then.
12:18 < noodles775> Thanks stub.
12:19 < stub> That gives you the infrastructure for attachments, email
interfaces etc. if we want them in the future.
12:20 < noodles775> Yeah, given that the table already exists, it
sounds crazy not to use it.