1. 17 May, 2021 8 commits
  2. 15 May, 2021 5 commits
  3. 14 May, 2021 9 commits
    • Peter Geoghegan's avatar
      Harden nbtree deduplication posting split code. · 8f72bbac
      Peter Geoghegan authored
      Add a defensive "can't happen" error to code that handles nbtree posting
      list splits (promote an existing assertion).  This avoids a segfault in
      the event of an insertion of a newitem that is somehow identical to an
      existing non-pivot tuple in the index.  An nbtree index should never
      have two index tuples with identical TIDs.
      
      This scenario is not particular unlikely in the event of any kind of
      corruption that leaves the index in an inconsistent state relative to
      the heap relation that is indexed.  There are two known reports of
      preventable hard crashes.  Doing nothing seems unacceptable given the
      general expectation that nbtree will cope reasonably well with corrupt
      data.
      
      Discussion: https://postgr.es/m/CAH2-Wz=Jr_d-dOYEEmwz0-ifojVNWho01eAqewfQXgKfoe114w@mail.gmail.com
      Backpatch: 13-, where nbtree deduplication was introduced.
      8f72bbac
    • Tom Lane's avatar
      Prevent infinite insertion loops in spgdoinsert(). · c3c35a73
      Tom Lane authored
      Formerly we just relied on operator classes that assert longValuesOK
      to eventually shorten the leaf value enough to fit on an index page.
      That fails since the introduction of INCLUDE-column support (commit
      09c1c6ab), because the INCLUDE columns might alone take up more
      than a page, meaning no amount of leaf-datum compaction will get
      the job done.  At least with spgtextproc.c, that leads to an infinite
      loop, since spgtextproc.c won't throw an error for not being able
      to shorten the leaf datum anymore.
      
      To fix without breaking cases that would otherwise work, add logic
      to spgdoinsert() to verify that the leaf tuple size is decreasing
      after each "choose" step.  Some opclasses might not decrease the
      size on every single cycle, and in any case, alignment roundoff
      of the tuple size could obscure small gains.  Therefore, allow
      up to 10 cycles without additional savings before throwing an
      error.  (Perhaps this number will need adjustment, but it seems
      quite generous right now.)
      
      As long as we've developed this logic, let's back-patch it.
      The back branches don't have INCLUDE columns to worry about, but
      this seems like a good defense against possible bugs in operator
      classes.  We already know that an infinite loop here is pretty
      unpleasant, so having a defense seems to outweigh the risk of
      breaking things.  (Note that spgtextproc.c is actually the only
      known opclass with longValuesOK support, so that this is all moot
      for known non-core opclasses anyway.)
      
      Per report from Dilip Kumar.
      
      Discussion: https://postgr.es/m/CAFiTN-uxP_soPhVG840tRMQTBmtA_f_Y8N51G7DKYYqDh7XN-A@mail.gmail.com
      c3c35a73
    • Tom Lane's avatar
      Fix query-cancel handling in spgdoinsert(). · eb7a6b92
      Tom Lane authored
      Knowing that a buggy opclass could cause an infinite insertion loop,
      spgdoinsert() intended to allow its loop to be interrupted by query
      cancel.  However, that never actually worked, because in iterations
      after the first, we'd be holding buffer lock(s) which would cause
      InterruptHoldoffCount to be positive, preventing servicing of the
      interrupt.
      
      To fix, check if an interrupt is pending, and if so fall out of
      the insertion loop and service the interrupt after we've released
      the buffers.  If it was indeed a query cancel, that's the end of
      the matter.  If it was a non-canceling interrupt reason, make use
      of the existing provision to retry the whole insertion.  (This isn't
      as wasteful as it might seem, since any upper-level index tuples we
      already created should be usable in the next attempt.)
      
      While there's no known instance of such a bug in existing release
      branches, it still seems like a good idea to back-patch this to
      all supported branches, since the behavior is fairly nasty if a
      loop does happen --- not only is it uncancelable, but it will
      quickly consume memory to the point of an OOM failure.  In any
      case, this code is certainly not working as intended.
      
      Per report from Dilip Kumar.
      
      Discussion: https://postgr.es/m/CAFiTN-uxP_soPhVG840tRMQTBmtA_f_Y8N51G7DKYYqDh7XN-A@mail.gmail.com
      eb7a6b92
    • Tom Lane's avatar
      Refactor CHECK_FOR_INTERRUPTS() to add flexibility. · e47f93f9
      Tom Lane authored
      Split up CHECK_FOR_INTERRUPTS() to provide an additional macro
      INTERRUPTS_PENDING_CONDITION(), which just tests whether an
      interrupt is pending without attempting to service it.  This is
      useful in situations where the caller knows that interrupts are
      blocked, and would like to find out if it's worth the trouble
      to unblock them.
      
      Also add INTERRUPTS_CAN_BE_PROCESSED(), which indicates whether
      CHECK_FOR_INTERRUPTS() can be relied on to clear the pending interrupt.
      
      This commit doesn't actually add any uses of the new macros,
      but a follow-on bug fix will do so.  Back-patch to all supported
      branches to provide infrastructure for that fix.
      
      Alvaro Herrera and Tom Lane
      
      Discussion: https://postgr.es/m/20210513155351.GA7848@alvherre.pgsql
      e47f93f9
    • Alvaro Herrera's avatar
      Describe (auto-)analyze behavior for partitioned tables · 1b5617eb
      Alvaro Herrera authored
      This explains the new behavior introduced by 0827e8af as well as
      preexisting.
      
      Author: Justin Pryzby <pryzby@telsasoft.com>
      Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
      Discussion: https://postgr.es/m/20210423180152.GA17270@telsasoft.com
      1b5617eb
    • Bruce Momjian's avatar
      5eb1b27d
    • Peter Eisentraut's avatar
      Message style improvements · 09ae3299
      Peter Eisentraut authored
      09ae3299
    • Bruce Momjian's avatar
      521d08a2
    • David Rowley's avatar
      Convert misleading while loop into an if condition · 6cb93bed
      David Rowley authored
      This seems to be leftover from ea15e186 and from when we used to evaluate
      SRFs at each node.
      
      Since there is an unconditional "return" at the end of the loop body, only
      1 loop is ever possible, so we can just change this into an if condition.
      
      There is no actual bug being fixed here so no back-patch. It seems fine to
      just fix this anomaly in master only.
      
      Author: Greg Nancarrow
      Discussion: https://postgr.es/m/CAJcOf-d7T1q0az-D8evWXnsuBZjigT04WkV5hCAOEJQZRWy28w@mail.gmail.com
      6cb93bed
  4. 13 May, 2021 8 commits
  5. 12 May, 2021 10 commits
    • Alvaro Herrera's avatar
      Rename the logical replication global "wrconn" · db16c656
      Alvaro Herrera authored
      The worker.c global wrconn is only meant to be used by logical apply/
      tablesync workers, but there are other variables with the same name. To
      reduce future confusion rename the global from "wrconn" to
      "LogRepWorkerWalRcvConn".
      
      While this is just cosmetic, it seems better to backpatch it all the way
      back to 10 where this code appeared, to avoid future backpatching
      issues.
      
      Author: Peter Smith <smithpb2250@gmail.com>
      Discussion: https://postgr.es/m/CAHut+Pu7Jv9L2BOEx_Z0UtJxfDevQSAUW2mJqWU+CtmDrEZVAg@mail.gmail.com
      db16c656
    • Tom Lane's avatar
      Double-space commands in system_constraints.sql/system_functions.sql. · 7dde9872
      Tom Lane authored
      Previously, any error reported by the backend while reading
      system_constraints.sql would report the entire file, not just the
      particular command it was working on.  (Ask me how I know.)  Likewise,
      there were chunks of system_functions.sql that would be read as one
      command, which would be annoying if anything failed there.
      
      The issue for system_constraints.sql is an oversight in commit
      dfb75e47.  I didn't try to trace down where the poor formatting
      in system_functions.sql started, but it's certainly contrary to
      the advice at the head of that file.
      7dde9872
    • Tom Lane's avatar
      Doc: update bki.sgml's statements about OID ranges. · 1f9b0e69
      Tom Lane authored
      Commit ab596105 neglected to make the docs match the code.
      1f9b0e69
    • Tom Lane's avatar
      Do pre-release housekeeping on catalog data. · 14472442
      Tom Lane authored
      Run renumber_oids.pl to move high-numbered OIDs down, as per pre-beta
      tasks specified by RELEASE_CHANGES.  For reference, the command was
      ./renumber_oids.pl --first-mapped-oid 8000 --target-oid 6150
      14472442
    • Tom Lane's avatar
      Initial pgindent and pgperltidy run for v14. · def5b065
      Tom Lane authored
      Also "make reformat-dat-files".
      
      The only change worthy of note is that pgindent messed up the formatting
      of launcher.c's struct LogicalRepWorkerId, which led me to notice that
      that struct wasn't used at all anymore, so I just took it out.
      def5b065
    • Michael Paquier's avatar
      Simplify one use of ScanKey in pg_subscription.c · e6ccd1ce
      Michael Paquier authored
      The section of the code in charge of returning all the relations
      associated to a subscription only need one ScanKey, but allocated two of
      them.  This code was introduced as a copy-paste from a different area on
      the same file by 7c4f5240, making the result confusing to follow.
      
      Author: Peter Smith
      Reviewed-by: Tom Lane, Julien Rouhaud, Bharath Rupireddy
      Discussion: https://postgr.es/m/CAHut+PsLKe+rN3FjchoJsd76rx2aMsFTB7CTFxRgUP05p=kcpQ@mail.gmail.com
      e6ccd1ce
    • Peter Eisentraut's avatar
      ec6e70c7
    • Etsuro Fujita's avatar
      Fix EXPLAIN ANALYZE for async-capable nodes. · a363bc6d
      Etsuro Fujita authored
      EXPLAIN ANALYZE for an async-capable ForeignScan node associated with
      postgres_fdw is done just by using instrumentation for ExecProcNode()
      called from the node's callbacks, causing the following problems:
      
      1) If the remote table to scan is empty, the node is incorrectly
         considered as "never executed" by the command even if the node is
         executed, as ExecProcNode() isn't called from the node's callbacks at
         all in that case.
      2) The command fails to collect timings for things other than
         ExecProcNode() done in the node, such as creating a cursor for the
         node's remote query.
      
      To fix these problems, add instrumentation for async-capable nodes, and
      modify postgres_fdw accordingly.
      
      My oversight in commit 27e1f145.
      
      While at it, update a comment for the AsyncRequest struct in execnodes.h
      and the documentation for the ForeignAsyncRequest API in fdwhandler.sgml
      to match the code in ExecAsyncAppendResponse() in nodeAppend.c, and fix
      typos in comments in nodeAppend.c.
      
      Per report from Andrey Lepikhov, though I didn't use his patch.
      
      Reviewed-by: Andrey Lepikhov
      Discussion: https://postgr.es/m/2eb662bb-105d-fc20-7412-2f027cc3ca72%40postgrespro.ru
      a363bc6d
    • Tom Lane's avatar
      Reduce runtime of privileges.sql test under CLOBBER_CACHE_ALWAYS. · e135743e
      Tom Lane authored
      Several queries in the privileges regression test cause the planner
      to apply the plpgsql function "leak()" to every element of the
      histogram for atest12.b.  Since commit 0c882e52 increased the size
      of that histogram to 10000 entries, the test invokes that function
      over 100000 times, which takes an absolutely unreasonable amount of
      time in clobber-cache-always mode.
      
      However, there's no real reason why that has to be a plpgsql
      function: for the purposes of this test, all that matters is that
      it not be marked leakproof.  So we can replace the plpgsql
      implementation with a direct call of int4lt, which has the same
      behavior and is orders of magnitude faster.  This is expected to
      cut several hours off the buildfarm cycle time for CCA animals.
      It has some positive impact in normal builds too, though that's
      probably lost in the noise.
      
      Back-patch to v13 where 0c882e52 came in.
      
      Discussion: https://postgr.es/m/575884.1620626638@sss.pgh.pa.us
      e135743e
    • Fujii Masao's avatar
      Change data type of counters in BufferUsage and WalUsage from long to int64. · d780d7c0
      Fujii Masao authored
      Previously long was used as the data type for some counters in BufferUsage
      and WalUsage. But long is only four byte, e.g., on Windows, and it's entirely
      possible to wrap a four byte counter. For example, emitting more than
      four billion WAL records in one transaction isn't actually particularly rare.
      
      To avoid the overflows of those counters, this commit changes the data type
      of them from long to int64.
      
      Suggested-by: Andres Freund
      Author: Masahiro Ikeda
      Reviewed-by: Fujii Masao
      Discussion: https://postgr.es/m/20201221211650.k7b53tcnadrciqjo@alap3.anarazel.de
      Discussion: https://postgr.es/m/af0964ac-7080-1984-dc23-513754987716@oss.nttdata.com
      d780d7c0