Merge lp:~cjwatson/launchpad/db-distroseries-publishing-options into lp:launchpad/db-devel

[Colin Watson]

Add DistroSeries.publishing_options column.

Add DistroSeries.publishing_options column, which will contain a JSON object. I plan to move backports_not_automatic and include_long_descriptions into this shortly. This also makes it easier to enable xz indexes and disable bz2 indexes on a per-series basis, both of which I'd like to do shortly. In general it seems useful to have a reasonably flexible way to change publisher behaviour on a per-series basis.

[Stuart Bishop (stub)]

Consider using the json type available in PostgreSQL 9.3 rather than text. Storm or the mapping to the Launchpad API will likely nix this, but it would be nice to start storing json as json.

[Colin Watson (cjwatson)]

With a json column, queries that involve SELECT DISTINCT on a whole DistroSeries row now result in:

  ProgrammingError: could not identify an equality operator for type json

It's at least slightly arguable that that's a Storm bug; it would surely be more efficient for it to turn it into DISTINCT ON (primary key). But it's not too difficult to work around this by using ResultSet.config(distinct=(DistroSeries.id, ...)), and in the meantime this does in fact point out a real bug: we can't rely on the order of JSON serialisation. So I'd definitely like to move to the json type because it makes it harder to make that kind of mistake.

However, moving to the json type requires upgrading to psycopg2 >= 2.5, which knows how to deserialise it; and that causes us to run into https://bugs.launchpad.net/storm/+bug/1170063. I've proposed a patch for that and am currently running the Launchpad test suite with psycopg2 2.6.1 and a cherry-pick of that patch onto the Launchpad branch of Storm.

[Colin Watson (cjwatson)]

Right. I think that upgrading psycopg2 etc. is still a good idea on general principles, and I figured out the small amount of remaining glue to be able to use a json column with Storm. However, there's still one difficult roadblock. The approach I suggested in my previous comment of transforming ResultSet.config(distinct=True) into ResultSet.config(distinct=(DistroSeries.id,)) doesn't work in all cases when ORDER BY is also involved. In such cases, PostgreSQL wants the DISTINCT ON expressions to match the initial ORDER BY expressions, but then we have cases like this in Distribution.derivatives:

        ret = Store.of(self).find(
            DistroSeries,
            ParentDistroSeries.id == DistroSeries.previous_seriesID,
            ParentDistroSeries.distributionID == self.id,
            DistroSeries.distributionID != self.id)
        return ret.config(
            distinct=True).order_by(Desc(DistroSeries.date_created))

With Storm we can only easily have one ORDER BY, and they can't match in this case. So it's very cumbersome to use DISTINCT ON here.

https://stackoverflow.com/a/24378258/989530 explains why this is hard. If we had jsonb, we could use it and not have this problem. But we're still on PostgreSQL 9.3, and lacking 9.4 or better, I think we should stick with text.

We still need to address the fact that DISTINCT will try to compare potentially different serialisations of DistroSeries.publishing_options. I think the simplest way to do this with 9.3 is just to add a patched version of JSONVariable that uses json.dumps(sort_keys=True).

[Colin Watson (cjwatson)]

William points out that my concern about DISTINCT and serialisation is the result of fuzzy thinking, so we don't need to worry about that.

[William Grant (wgrant)]

review: Approve (db)

=== added file 'database/schema/patch-2209-71-0.sql'
--- database/schema/patch-2209-71-0.sql	1970-01-01 00:00:00 +0000
+++ database/schema/patch-2209-71-0.sql	2015-11-21 11:08:55 +0000
@@ -0,0 +1,10 @@
+-- Copyright 2015 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 DistroSeries ADD COLUMN publishing_options text;
+
+COMMENT ON COLUMN DistroSeries.publishing_options IS 'A JSON object containing options modifying the publisher''s behaviour for this series.';
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 71, 0);