1. 26 Feb, 2018 4 commits
    • Noah Misch's avatar
      Document security implications of search_path and the public schema. · 5770172c
      Noah Misch authored
      The ability to create like-named objects in different schemas opens up
      the potential for users to change the behavior of other users' queries,
      maliciously or accidentally.  When you connect to a PostgreSQL server,
      you should remove from your search_path any schema for which a user
      other than yourself or superusers holds the CREATE privilege.  If you do
      not, other users holding CREATE privilege can redefine the behavior of
      your commands, causing them to perform arbitrary SQL statements under
      your identity.  "SET search_path = ..." and "SELECT
      pg_catalog.set_config(...)" are not vulnerable to such hijacking, so one
      can use either as the first command of a session.  As special
      exceptions, the following client applications behave as documented
      regardless of search_path settings and schema privileges: clusterdb
      createdb createlang createuser dropdb droplang dropuser ecpg (not
      programs it generates) initdb oid2name pg_archivecleanup pg_basebackup
      pg_config pg_controldata pg_ctl pg_dump pg_dumpall pg_isready
      pg_receivewal pg_recvlogical pg_resetwal pg_restore pg_rewind pg_standby
      pg_test_fsync pg_test_timing pg_upgrade pg_waldump reindexdb vacuumdb
      vacuumlo.  Not included are core client programs that run user-specified
      SQL commands, namely psql and pgbench.  PostgreSQL encourages non-core
      client applications to do likewise.
      
      Document this in the context of libpq connections, psql connections,
      dblink connections, ECPG connections, extension packaging, and schema
      usage patterns.  The principal defense for applications is "SELECT
      pg_catalog.set_config('search_path', '', false)", and the principal
      defense for databases is "REVOKE CREATE ON SCHEMA public FROM PUBLIC".
      Either one is sufficient to prevent attack.  After a REVOKE, consider
      auditing the public schema for objects named like pg_catalog objects.
      
      Authors of SECURITY DEFINER functions use some of the same defenses, and
      the CREATE FUNCTION reference page already covered them thoroughly.
      This is a good opportunity to audit SECURITY DEFINER functions for
      robust security practice.
      
      Back-patch to 9.3 (all supported versions).
      
      Reviewed by Michael Paquier and Jonathan S. Katz.  Reported by Arseniy
      Sharoglazov.
      
      Security: CVE-2018-1058
      5770172c
    • Noah Misch's avatar
      Empty search_path in Autovacuum and non-psql/pgbench clients. · 582edc36
      Noah Misch authored
      This makes the client programs behave as documented regardless of the
      connect-time search_path and regardless of user-created objects.  Today,
      a malicious user with CREATE permission on a search_path schema can take
      control of certain of these clients' queries and invoke arbitrary SQL
      functions under the client identity, often a superuser.  This is
      exploitable in the default configuration, where all users have CREATE
      privilege on schema "public".
      
      This changes behavior of user-defined code stored in the database, like
      pg_index.indexprs and pg_extension_config_dump().  If they reach code
      bearing unqualified names, "does not exist" or "no schema has been
      selected to create in" errors might appear.  Users may fix such errors
      by schema-qualifying affected names.  After upgrading, consider watching
      server logs for these errors.
      
      The --table arguments of src/bin/scripts clients have been lax; for
      example, "vacuumdb -Zt pg_am\;CHECKPOINT" performed a checkpoint.  That
      now fails, but for now, "vacuumdb -Zt 'pg_am(amname);CHECKPOINT'" still
      performs a checkpoint.
      
      Back-patch to 9.3 (all supported versions).
      
      Reviewed by Tom Lane, though this fix strategy was not his first choice.
      Reported by Arseniy Sharoglazov.
      
      Security: CVE-2018-1058
      582edc36
    • Tom Lane's avatar
      Avoid using unsafe search_path settings during dump and restore. · 3d2aed66
      Tom Lane authored
      Historically, pg_dump has "set search_path = foo, pg_catalog" when
      dumping an object in schema "foo", and has also caused that setting
      to be used while restoring the object.  This is problematic because
      functions and operators in schema "foo" could capture references meant
      to refer to pg_catalog entries, both in the queries issued by pg_dump
      and those issued during the subsequent restore run.  That could
      result in dump/restore misbehavior, or in privilege escalation if a
      nefarious user installs trojan-horse functions or operators.
      
      This patch changes pg_dump so that it does not change the search_path
      dynamically.  The emitted restore script sets the search_path to what
      was used at dump time, and then leaves it alone thereafter.  Created
      objects are placed in the correct schema, regardless of the active
      search_path, by dint of schema-qualifying their names in the CREATE
      commands, as well as in subsequent ALTER and ALTER-like commands.
      
      Since this change requires a change in the behavior of pg_restore
      when processing an archive file made according to this new convention,
      bump the archive file version number; old versions of pg_restore will
      therefore refuse to process files made with new versions of pg_dump.
      
      Security: CVE-2018-1058
      3d2aed66
    • Robert Haas's avatar
      Add a new upper planner relation for partially-aggregated results. · 3bf05e09
      Robert Haas authored
      Up until now, we've abused grouped_rel->partial_pathlist as a place to
      store partial paths that have been partially aggregate, but that's
      really not correct, because a partial path for a relation is supposed
      to be one which produces the correct results with the addition of only
      a Gather or Gather Merge node, and these paths also require a Finalize
      Aggregate step.  Instead, add a new partially_group_rel which can hold
      either partial paths (which need to be gathered and then have
      aggregation finalized) or non-partial paths (which only need to have
      aggregation finalized).  This allows us to reuse generate_gather_paths
      for partially_grouped_rel instead of writing new code, so that this
      patch actually basically no net new code while making things cleaner,
      simplifying things for pending patches for partition-wise aggregate.
      
      Robert Haas and Jeevan Chalke.  The larger patch series of which this
      patch is a part was also reviewed and tested by Antonin Houska,
      Rajkumar Raghuwanshi, David Rowley, Dilip Kumar, Konstantin Knizhnik,
      Pascal Legrand, Rafia Sabih, and me.
      
      Discussion: http://postgr.es/m/CA+TgmobrzFYS3+U8a_BCy3-hOvh5UyJbC18rEcYehxhpw5=ETA@mail.gmail.com
      Discussion: http://postgr.es/m/CA+TgmoZyQEjdBNuoG9-wC5GQ5GrO4544Myo13dVptvx+uLg9uQ@mail.gmail.com
      3bf05e09
  2. 25 Feb, 2018 2 commits
    • Tom Lane's avatar
      Un-break parallel pg_upgrade. · 5b570d77
      Tom Lane authored
      Commit b3f84012 changed pg_upgrade so that it'd actually drop and
      re-create the template1 and postgres databases in the new cluster.
      That works fine, serially.  With the -j option it's not so fine, because
      other per-database jobs might be launched while the template1 database is
      dropped.  Since they attempt to connect there to start up, kaboom.
      
      This is the cause of the intermittent failures buildfarm member jacana
      has been showing for the last month; evidently it is the only BF member
      configured to run the pg_upgrade test with parallelism enabled.
      
      Fix by processing template1 separately before we get into the parallel
      sub-job launch loop.  (We could alternatively have made the postgres DB
      be the special case, but it seems likely that template1 will contain
      less stuff and so we lose less parallelism with this choice.)
      5b570d77
    • Tom Lane's avatar
      1316417b
  3. 24 Feb, 2018 7 commits
    • Peter Eisentraut's avatar
      Update headers of generated files · c4ba1bee
      Peter Eisentraut authored
      The scripts were changed in c98c35cd,
      but the output files were not updated to reflect the script changes.
      c4ba1bee
    • Peter Eisentraut's avatar
      Add current directory to Perl include path · 9ee0573e
      Peter Eisentraut authored
      Recent Perl versions don't have the current directory in the module
      include path anymore, so we need to add it here explicitly to make these
      scripts continue to work.
      9ee0573e
    • Peter Eisentraut's avatar
    • Tom Lane's avatar
      Fix thinko in in_range_float4_float8. · 32291aed
      Tom Lane authored
      I forgot the coding rule for correct use of Float8GetDatumFast.
      Per buildfarm.
      32291aed
    • Tom Lane's avatar
      Add window RANGE support for float4, float8, numeric. · 8b29e88c
      Tom Lane authored
      Commit 0a459cec left this for later, but since time's running out,
      I went ahead and took care of it.  There are more data types that
      somebody might someday want RANGE support for, but this is enough
      to satisfy all expectations of the SQL standard, which just says that
      "numeric, datetime, and interval" types should have RANGE support.
      8b29e88c
    • Peter Eisentraut's avatar
      Check error messages in SSL tests · 081bfc19
      Peter Eisentraut authored
      In tests that check whether a connection fails, also check the error
      message.  That makes sure that the connection was rejected for the right
      reason.
      
      This discovered that two tests had their connection failing for the
      wrong reason.  One test failed because pg_hba.conf was not set up to
      allow that user, one test failed because the client key file did not
      have the right permissions.  Fix those tests and add a new one that is
      really supposed to check the file permission issue.
      Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      081bfc19
    • Peter Eisentraut's avatar
      Fix filtering of unsupported relations in logical replication · bc1adc65
      Peter Eisentraut authored
      In the pgoutput plugin, skip changes for relations that are not
      publishable, per is_publishable_class().  This concerns in particular
      materialized views and information_schema tables.  While those relations
      cannot be part of a publication, per existing checks, they will be
      considered by a FOR ALL TABLES publication.  A subscription would not
      actually apply changes for those relations, again per existing checks,
      but trying to match incoming changes to local tables on the subscriber
      would lead to errors if no matching local table exists.  Skipping those
      changes on the publisher avoids sending useless changes and eliminates
      the error.
      
      Bug: #15044
      Reported-by: default avatarChad Trabant <chad@iris.washington.edu>
      Reviewed-by: default avatarPetr Jelinek <petr.jelinek@2ndquadrant.com>
      bc1adc65
  4. 23 Feb, 2018 8 commits
    • Tom Lane's avatar
      First-draft release notes for 10.3. · eec1a8cb
      Tom Lane authored
      eec1a8cb
    • Tom Lane's avatar
      Fix brown-paper-bag bug in commit 0a459cec. · 9fe802c8
      Tom Lane authored
      RANGE_OFFSET comparisons need to examine the first ORDER BY column,
      which isn't necessarily the first column in the incoming tuples.
      No idea how this slipped through initial testing.
      
      Per bug #15082 from Zhou Digoal.
      
      Discussion: https://postgr.es/m/151939899974.1461.9411971793110285476@wrigleys.postgresql.org
      9fe802c8
    • Tom Lane's avatar
      Allow auto_explain.log_min_duration to go up to INT_MAX. · 8af87f41
      Tom Lane authored
      The previous limit of INT_MAX / 1000 seems to have been cargo-culted in
      from somewhere else.  Or possibly the value was converted to microseconds
      at some point; but in all supported releases, it's just compared to other
      values, so there's no need for the restriction.  This change raises the
      effective limit from ~35 minutes to ~24 days, which conceivably is useful
      to somebody, and anyway it's more consistent with the range of the core
      log_min_duration_statement GUC.
      
      Per complaint from Kevin Bloch.  Back-patch to all supported releases.
      
      Discussion: https://postgr.es/m/8ea82d7e-cb78-8e05-0629-73aa14d2a0ca@codingthat.com
      8af87f41
    • Noah Misch's avatar
      Synchronize doc/ copies of src/test/examples/. · fe35cea7
      Noah Misch authored
      This is mostly cosmetic, but it might fix build failures, on some
      platform, when copying from the documentation.
      
      Back-patch to 9.3 (all supported versions).
      fe35cea7
    • Tom Lane's avatar
      Fix planner failures with overlapping mergejoin clauses in an outer join. · 9afd513d
      Tom Lane authored
      Given overlapping or partially redundant join clauses, for example
      	t1 JOIN t2 ON t1.a = t2.x AND t1.b = t2.x
      the planner's EquivalenceClass machinery will ordinarily refactor the
      clauses as "t1.a = t1.b AND t1.a = t2.x", so that join processing doesn't
      see multiple references to the same EquivalenceClass in a list of join
      equality clauses.  However, if the join is outer, it's incorrect to derive
      a restriction clause on the outer side from the join conditions, so the
      clause refactoring does not happen and we end up with overlapping join
      conditions.  The code that attempted to deal with such cases had several
      subtle bugs, which could result in "left and right pathkeys do not match in
      mergejoin" or "outer pathkeys do not match mergeclauses" planner errors,
      if the selected join plan type was a mergejoin.  (It does not appear that
      any actually incorrect plan could have been emitted.)
      
      The core of the problem really was failure to recognize that the outer and
      inner relations' pathkeys have different relationships to the mergeclause
      list.  A join's mergeclause list is constructed by reference to the outer
      pathkeys, so it will always be ordered the same as the outer pathkeys, but
      this cannot be presumed true for the inner pathkeys.  If the inner sides of
      the mergeclauses contain multiple references to the same EquivalenceClass
      ({t2.x} in the above example) then a simplistic rendering of the required
      inner sort order is like "ORDER BY t2.x, t2.x", but the pathkey machinery
      recognizes that the second sort column is redundant and throws it away.
      The mergejoin planning code failed to account for that behavior properly.
      One error was to try to generate cut-down versions of the mergeclause list
      from cut-down versions of the inner pathkeys in the same way as the initial
      construction of the mergeclause list from the outer pathkeys was done; this
      could lead to choosing a mergeclause list that fails to match the outer
      pathkeys.  The other problem was that the pathkey cross-checking code in
      create_mergejoin_plan treated the inner and outer pathkey lists
      identically, whereas actually the expectations for them must be different.
      That led to false "pathkeys do not match" failures in some cases, and in
      principle could have led to failure to detect bogus plans in other cases,
      though there is no indication that such bogus plans could be generated.
      
      Reported by Alexander Kuzmenkov, who also reviewed this patch.  This has
      been broken for years (back to around 8.3 according to my testing), so
      back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/5dad9160-4632-0e47-e120-8e2082000c01@postgrespro.ru
      9afd513d
    • Robert Haas's avatar
      Revise API for partition bound search functions. · f724022d
      Robert Haas authored
      Similar to what commit b0229235 for a
      different set of functions, pass the required bits of the PartitionKey
      instead of the whole thing.  This allows these functions to be used
      without needing the PartitionKey to be available.
      
      Amit Langote.  The larger patch series of which this patch is a part
      has been reviewed and tested by Ashutosh Bapat, David Rowley, Dilip
      Kumar, Jesper Pedersen, Rajkumar Raghuwanshi, Beena Emerson, Kyotaro
      Horiguchi, Álvaro Herrera, and me, but especially and in great detail
      by David Rowley.
      
      Discussion: http://postgr.es/m/098b9c71-1915-1a2a-8d52-1a7a50ce79e8@lab.ntt.co.jp
      Discussion: http://postgr.es/m/1f6498e8-377f-d077-e791-5dc84dba2c00@lab.ntt.co.jp
      f724022d
    • Robert Haas's avatar
      Revise API for partition_rbound_cmp/partition_rbound_datum_cmp. · b0229235
      Robert Haas authored
      Instead of passing the PartitionKey, pass just the required bits of
      it.  This allows these functions to be used without needing the
      PartitionKey to be available, which is important for several
      pending patches.
      
      Ashutosh Bapat, reviewed by Amit Langote, with a comment tweak
      by me.
      
      Discussion: http://postgr.es/m/3d835ed1-36ab-f06d-0ce8-a76a2bbf7677@lab.ntt.co.jp
      Discussion: http://postgr.es/m/b4d88995-094b-320c-b614-2282fae0bf6c@lab.ntt.co.jp
      b0229235
    • Peter Eisentraut's avatar
      Support parameters in CALL · 76b6aa41
      Peter Eisentraut authored
      To support parameters in CALL, move the parse analysis of the procedure
      and arguments into the global transformation phase, so that the parser
      hooks can be applied.  And then at execution time pass the parameters
      from ProcessUtility on to ExecuteCallStmt.
      76b6aa41
  5. 22 Feb, 2018 10 commits
    • Robert Haas's avatar
    • Peter Eisentraut's avatar
      Fix perlcritic warnings · abcba700
      Peter Eisentraut authored
      abcba700
    • Peter Eisentraut's avatar
      Update gratuitous use of MD5 in documentation · 0db2fc98
      Peter Eisentraut authored
      It seems some people are bothered by the outdated MD5 appearing in
      example code.  So replace it with more modern alternatives or by
      a different example function.
      Reported-by: default avatarJon Wolski <jonwolski@gmail.com>
      0db2fc98
    • Peter Eisentraut's avatar
      Add user-callable SHA-2 functions · 10cfce34
      Peter Eisentraut authored
      Add the user-callable functions sha224, sha256, sha384, sha512.  We
      already had these in the C code to support SCRAM, but there was no test
      coverage outside of the SCRAM tests.  Adding these as user-callable
      functions allows writing some tests.  Also, we have a user-callable md5
      function but no more modern alternative, which led to wide use of md5 as
      a general-purpose hash function, which leads to occasional complaints
      about using md5.
      
      Also mark the existing md5 functions as leak-proof.
      Reviewed-by: default avatarMichael Paquier <michael@paquier.xyz>
      10cfce34
    • Robert Haas's avatar
      Be lazier about partition tuple routing. · edd44738
      Robert Haas authored
      It's not necessary to fully initialize the executor data structures
      for partitions to which no tuples are ever routed.  Consider, for
      example, an INSERT statement that inserts only one row: it only cares
      about the partition to which that one row is routed.  The new function
      ExecInitPartitionInfo performs the initialization in question only
      when a particular partition is about to receive a tuple. This includes
      creating, validating, and saving a pointer to the ResultRelInfo,
      setting up for speculative insertions, translating WCOs and
      initializing the resulting expressions, translating returning lists
      and building the appropriate projection information, and setting up a
      tuple conversion map.
      
      One thing that's not deferred is locking the child partitions; that
      seems desirable but would need more thought.  Still, testing shows
      that this makes single-row inserts significantly faster on a table
      with many partitions without harming the bulk-insert case.
      
      Amit Langote, reviewed by Etsuro Fujita, with a few changes by me
      
      Discussion: http://postgr.es/m/8975331d-d961-cbdd-f862-fdd3d97dc2d0@lab.ntt.co.jp
      edd44738
    • Robert Haas's avatar
      Remove extra word from comment. · 810e7e26
      Robert Haas authored
      Etsuro Fujita
      
      Discussion: http://postgr.es/m/5A8EAF74.5010905@lab.ntt.co.jp
      810e7e26
    • Robert Haas's avatar
      postgres_fdw: Fix interaction of PHVs with child joins. · 84cb51b4
      Robert Haas authored
      Commit f49842d1 introduced the
      concept of a child join, but did not update this code accordingly.
      
      Ashutosh Bapat, with cosmetic changes by me
      
      Discussion: http://postgr.es/m/CAFjFpRf=J_KPOtw+bhZeURYkbizr8ufSaXg6gPEF6DKpgH-t6g@mail.gmail.com
      84cb51b4
    • Robert Haas's avatar
      Avoid another valgrind complaint about write() of uninitalized bytes. · de6428af
      Robert Haas authored
      Peter Geoghegan, per buildfarm member skink and Andres Freund
      
      Discussion: http://postgr.es/m/20180221053426.gp72lw67yfpzkw7a@alap3.anarazel.de
      de6428af
    • Robert Haas's avatar
      Try to stabilize EXPLAIN output in partition_check test. · 9a5c4f58
      Robert Haas authored
      Commit 7d8ac981 adjusted these
      tests in the hope of preserving the plan shape, but I failed to
      notice that the three partitions were, on my local machine, choosing
      two different plan shapes.  This is probably related to the fact
      that all three tables have exactly the same row count.  Try to
      improve the situation by making pht1_e about half as large as
      the other two.
      
      Per Tom Lane and the buildfarm.
      
      Discussion: http://postgr.es/m/25380.1519277713@sss.pgh.pa.us
      9a5c4f58
    • Robert Haas's avatar
      Charge cpu_tuple_cost * 0.5 for Append and MergeAppend nodes. · 7d8ac981
      Robert Haas authored
      Previously, Append didn't charge anything at all, and MergeAppend
      charged only cpu_operator_cost, about half the value used here.  This
      change might make MergeAppend plans slightly more likely to be chosen
      than before, since this commit increases the assumed cost for Append
      -- with default values -- by 0.005 per tuple but MergeAppend by only
      0.0025 per tuple.  Since the comparisons required by MergeAppend are
      costed separately, it's not clear why MergeAppend needs to be
      otherwise more expensive than Append, so hopefully this is OK.
      
      Prior to partition-wise join, it didn't really matter whether or not
      an Append node had any cost of its own, because every plan had to use
      the same number of Append or MergeAppend nodes and in the same places.
      Only the relative cost of Append vs. MergeAppend made a difference.
      Now, however, it is possible to avoid some of the Append nodes using a
      partition-wise join, so it's worth making an effort.  Pending patches
      for partition-wise aggregate care too, because an Append of Aggregate
      nodes will incur the Append overhead fewer times than an Aggregate
      over an Append.  Although in most cases this change will favor the use
      of partition-wise techniques, it does the opposite when the join
      cardinality is greater than the sum of the input cardinalities.  Since
      this situation arises in an existing regression test, I [rhaas]
      adjusted it to keep the overall plan shape approximately the same.
      
      Jeevan Chalke, per a suggestion from David Rowley.  Reviewed by
      Ashutosh Bapat.  Some changes by me.  The larger patch series of which
      this patch is a part was also reviewed and tested by Antonin Houska,
      Rajkumar Raghuwanshi, David Rowley, Dilip Kumar, Konstantin Knizhnik,
      Pascal Legrand, Rafia Sabih, and me.
      
      Discussion: http://postgr.es/m/CAKJS1f9UXdk6ZYyqbJnjFO9a9hyHKGW7B=ZRh-rxy9qxfPA5Gw@mail.gmail.com
      7d8ac981
  6. 21 Feb, 2018 2 commits
    • Tom Lane's avatar
      Repair pg_upgrade's failure to preserve relfrozenxid for matviews. · 38b41f18
      Tom Lane authored
      This oversight led to data corruption in matviews, manifesting as
      "could not access status of transaction" before our most recent releases,
      and "found xmin from before relfrozenxid" errors since then.
      
      The proximate cause of the problem seems to have been confusion between
      the task of preserving dropped-column status and the task of preserving
      frozenxid status.  Those are required for distinct sets of relkinds,
      and the reasoning was entirely undocumented in the source code.  In hopes
      of forestalling future errors of the same kind, try to improve the
      commentary in this area.
      
      In passing, also improve the remarkably unhelpful comments around
      pg_upgrade's set_frozenxids().  That's not actually buggy AFAICS,
      but good luck figuring out what it does from the old comments.
      
      Per report from Claudio Freire.  It appears that bug #14852 from Alexey
      Ermakov is an earlier report of the same issue, and there may be other
      cases that we failed to identify at the time.
      
      Patch by me based on analysis by Andres Freund.  The bug dates back
      to the introduction of matviews, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/CAGTBQpbrY9CdRGGhyBZ9yqY4jWaGC85rUF4X+R7d-aim=mBNsw@mail.gmail.com
      Discussion: https://postgr.es/m/20171013115320.28049.86457@wrigleys.postgresql.org
      38b41f18
    • Andres Freund's avatar
      Blindly attempt to adapt sepgsql regression tests. · 29d432e4
      Andres Freund authored
      Commit bf6c614a broke the sepgsql test
      due to a new invocation of the function access hook during grouping
      equal initialization.
      
      The new behaviour seems at least as correct as the old one, so try
      adapt the tests. As I've no working sepgsql setup here, this is just
      going from buildfarm results.
      
      Author: Andres Freund
      Discussion: https://postgr.es/m/20180217000337.lfsdvro3l6ccsksp@alap3.anarazel.de
      29d432e4
  7. 20 Feb, 2018 6 commits
  8. 19 Feb, 2018 1 commit
    • Tom Lane's avatar
      Fix misbehavior of CTE-used-in-a-subplan during EPQ rechecks. · 159efe4a
      Tom Lane authored
      An updating query that reads a CTE within an InitPlan or SubPlan could get
      incorrect results if it updates rows that are concurrently being modified.
      This is caused by CteScanNext supposing that nothing inside its recursive
      ExecProcNode call could change which read pointer is selected in the CTE's
      shared tuplestore.  While that's normally true because of scoping
      considerations, it can break down if an EPQ plan tree gets built during the
      call, because EvalPlanQualStart builds execution trees for all subplans
      whether they're going to be used during the recheck or not.  And it seems
      like a pretty shaky assumption anyway, so let's just reselect our own read
      pointer here.
      
      Per bug #14870 from Andrei Gorita.  This has been broken since CTEs were
      implemented, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/20171024155358.1471.82377@wrigleys.postgresql.org
      159efe4a