sg-report-flight: Use WITH OFFSET 0 for optimisation fence

I am not best pleased.

CC: George Dunlap <George.Dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/sg-report-flight b/sg-report-flight
index 3e0019b07084c0a65952e07f79f9168b250866c1..0783458182ba6818321a783e09470a448b05de4b 100755 (executable)
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -282,10 +282,35 @@ END
 END
     }
 
-    # In psql 9.6 this WITH clause makes postgresql do the flights
-    # query first.  This is good because our built revision index finds
-    # relevant flights very quickly.  Without this, postgresql seems
-    # to like to scan the jobs table.
+    # We want to do the flights query first because our built revision
+    # index finds relevant flights very quickly.  So we need to force
+    # an optimisation fence after the WITH.  Otherwise (at least in
+    # 9.6) postgresql seems to like to scan the jobs table.
+    #
+    # In pgsql 9.6 a WITH common table expression clause is sufficient
+    # (This is evident from a careful reading of the 9.6
+    # docs for WITH.)
+    #
+    # In pgsql 12 they changed WITH to no longer always be an
+    # optimisation fence, and they have documented WITH MATERIALIZED
+    # as a way to get this effect.
+    #
+    # But they have refused to backport that syntax change as a no-op
+    # to previous versions,
+    #   https://www.postgresql.org/message-id/20191018132130.GM16234%40riva.ucam.org
+    # so there is no documented way that works
+    # both with old and new pgsql.
+    #
+    # However, adding OFFSET 0 (a no-op) restores the optimisation
+    # fence in pgsql 12 and works with earlier version too.  This was
+    # suggested in the thread in which the MATERIALIZED backport was
+    # rejected, and has also been suggested previously by pgsql devs
+    # on stackmumble:
+    #   https://stackoverflow.com/questions/14897816/how-can-i-prevent-postgres-from-inlining-a-subquery
+    #
+    # Sadly this trick is not formally documented but it is our
+    # best option.
+
     my $flightsq= <<END;
       WITH sub AS (
         SELECT DISTINCT flight, blessing
@@ -295,6 +320,7 @@ $runvars_joins
               AND $blessingscond
 $runvars_conds
             ORDER BY flight DESC
+            OFFSET 0
             LIMIT 1000
       )
       SELECT flight, jobs.status
@@ -890,17 +916,21 @@ sub justifyfailures ($;$) {
 
     my @failures= values %{ $fi->{Failures} };
 
-    # In psql 9.6 this WITH clause makes postgresql do the steps query
-    # first.  This is good because if this test never passed we can
-    # determine that really quickly using the new index, without
-    # having to scan the flights table.  (If the test passed we will
-    # probably not have to look at many flights to find one, so in
-    # that case this is not much worse.)
+    # We want to do the steps query first.  This is good because if
+    # this test never passed we can determine that really quickly
+    # using the steps_job_testid_status_idx index, without having to
+    # scan the flights table.  (If the test passed we will probably
+    # not have to look at many flights to find one, so in that case
+    # this is not much worse.)
+    #
+    # This WITH OFFSET 0 clause has this effect in pgsql versions
+    # 9.6 - 12 inclusive.  See the discussion by $flightsq.
     my $anypassq= <<END;
         WITH s AS
         (
         SELECT * FROM steps
          WHERE job=? and testid=? and status='pass'
+         OFFSET 0
         )
         SELECT * FROM flights JOIN s USING (flight)
             WHERE $branches_cond_q