launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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).
-- 
https://code.launchpad.net/~cjwatson/launchpad/db-distroseries-publishing-options/+merge/278241
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/db-distroseries-publishing-options into lp:launchpad/db-devel.