1. 07 Jun, 2021 5 commits
    • Tom Lane's avatar
      Stabilize contrib/seg regression test. · d16ebfbf
      Tom Lane authored
      If autovacuum comes along just after we fill table test_seg with
      some data, it will update the stats to the point where we prefer
      a plain indexscan over a bitmap scan, breaking the expected
      output (as well as the point of the test case).  To fix, just
      force a bitmap scan to be chosen here.
      
      This has evidently been wrong since commit de1d042f.  It's not
      clear why we just recently saw any buildfarm failures due to it;
      but prairiedog has failed twice on this test in the past week.
      Hence, backpatch to v11 where this test case came in.
      d16ebfbf
    • Tom Lane's avatar
      Fix incautious handling of possibly-miscoded strings in client code. · 42f94f56
      Tom Lane authored
      An incorrectly-encoded multibyte character near the end of a string
      could cause various processing loops to run past the string's
      terminating NUL, with results ranging from no detectable issue to
      a program crash, depending on what happens to be in the following
      memory.
      
      This isn't an issue in the server, because we take care to verify
      the encoding of strings before doing any interesting processing
      on them.  However, that lack of care leaked into client-side code
      which shouldn't assume that anyone has validated the encoding of
      its input.
      
      Although this is certainly a bug worth fixing, the PG security team
      elected not to regard it as a security issue, primarily because
      any untrusted text should be sanitized by PQescapeLiteral or
      the like before being incorporated into a SQL or psql command.
      (If an app fails to do so, the same technique can be used to
      cause SQL injection, with probably much more dire consequences
      than a mere client-program crash.)  Those functions were already
      made proof against this class of problem, cf CVE-2006-2313.
      
      To fix, invent PQmblenBounded() which is like PQmblen() except it
      won't return more than the number of bytes remaining in the string.
      In HEAD we can make this a new libpq function, as PQmblen() is.
      It seems imprudent to change libpq's API in stable branches though,
      so in the back branches define PQmblenBounded as a macro in the files
      that need it.  (Note that just changing PQmblen's behavior would not
      be a good idea; notably, it would completely break the escaping
      functions' defense against this exact problem.  So we just want a
      version for those callers that don't have any better way of handling
      this issue.)
      
      Per private report from houjingyi.  Back-patch to all supported branches.
      42f94f56
    • Michael Paquier's avatar
      Fix portability issue in test indirect_toast · 68a6d8a8
      Michael Paquier authored
      When run on a server using default_toast_compression set to LZ4, this
      test would fail because of a consistency issue with the order of the
      tuples treated.  LZ4 causes one tuple to be stored inline instead of
      getting externalized.  As the goal of this test is to check after data
      stored externally, stick to pglz as the compression algorithm used, so
      as all data of this test is stored the way it should.
      
      Analyzed-by: Dilip Kumar
      Discussion: https://postgr.es/m/YLrDWxJgM8WWMoCg@paquier.xyz
      68a6d8a8
    • Amit Kapila's avatar
      Remove two_phase variable from CreateReplicationSlotCmd struct. · be57f216
      Amit Kapila authored
      Commit 19890a06 added the option to enable two_phase commits via
      pg_create_logical_replication_slot but didn't extend the support of same
      in replication protocol. However, by mistake, it added the two_phase
      variable in CreateReplicationSlotCmd which is required only when we extend
      the replication protocol.
      
      Reported-by: Jeff Davis
      Author: Ajin Cherian
      Reviewed-by: Amit Kapila
      Discussion: https://postgr.es/m/64b9f783c6e125f18f88fbc0c0234e34e71d8639.camel@j-davis.com
      be57f216
    • Etsuro Fujita's avatar
      Fix rescanning of async-aware Append nodes. · f3baaf28
      Etsuro Fujita authored
      In cases where run-time pruning isn't required, the synchronous and
      asynchronous subplans for an async-aware Append node determined using
      classify_matching_subplans() should be re-used when rescanning the node,
      but the previous code re-determined them using that function repeatedly
      each time when rescanning the node, leading to incorrect results in a
      normal build and an Assert failure in an Assert-enabled build as that
      function doesn't assume that it's called repeatedly in such cases.  Fix
      the code as mentioned above.
      
      My oversight in commit 27e1f145.
      
      While at it, initialize async-related pointers/variables to NULL/zero
      explicitly in ExecInitAppend() and ExecReScanAppend(), just to be sure.
      (The variables would have been set to zero before we get to the latter
      function, but let's do so.)
      
      Reviewed-by: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/CAPmGK16Q4B2_KY%2BJH7rb7wQbw54AUprp7TMekGTd2T1B62yysQ%40mail.gmail.com
      f3baaf28
  2. 06 Jun, 2021 3 commits
  3. 05 Jun, 2021 5 commits
  4. 04 Jun, 2021 6 commits
  5. 03 Jun, 2021 6 commits
    • Andrew Dunstan's avatar
      In PostgresNode.pm, don't pass SQL to psql on the command line · 11e9caff
      Andrew Dunstan authored
      The Msys shell mangles certain patterns in its command line, so avoid
      handing arbitrary SQL to psql on the command line and instead use
      IPC::Run's redirection facility for stdin. This pattern is already
      mostly whats used, but query_poll_until() was not doing the right thing.
      
      Problem discovered on the buildfarm when a new TAP test failed on msys.
      11e9caff
    • Tom Lane's avatar
      Fix incorrect permissions on pg_subscription. · 3590680b
      Tom Lane authored
      The documented intent is for all columns except subconninfo to be
      publicly readable.  However, this has been overlooked twice.
      subsynccommit has never been readable since it was introduced,
      nor has the oid column (which is important for joining).
      
      Given the lack of previous complaints, it's not clear that it's
      worth doing anything about this in the back branches.  But there's
      still time to fix it inexpensively for v14.
      
      Per report from Israel Barth (via Euler Taveira).
      
      Patch by Euler Taveira, possibly-vain comment updates by me.
      
      Discussion: https://postgr.es/m/b8f7c17c-0041-46b6-acfe-2d1f5a985ab4@www.fastmail.com
      3590680b
    • Michael Paquier's avatar
      Reduce risks of conflicts in internal queries of REFRESH MATVIEW CONCURRENTLY · 187682c3
      Michael Paquier authored
      The internal SQL queries used by REFRESH MATERIALIZED VIEW CONCURRENTLY
      include some aliases for its diff and temporary relations with
      rather-generic names: diff, newdata, newdata2 and mv.  Depending on the
      queries used for the materialized view, using CONCURRENTLY could lead to
      some internal failures if the matview query and those internal aliases
      conflict.
      
      Those names have been chosen in 841c29c8.  This commit switches instead
      to a naming pattern which is less likely going to cause conflicts, based
      on an idea from Thomas Munro, by appending _$ to those aliases.  This is
      not perfect as those new names could still conflict, but at least it has
      the advantage to keep the code readable and simple while reducing the
      likelihood of conflicts to be close to zero.
      
      Reported-by: Mathis Rudolf
      Author: Bharath Rupireddy
      Reviewed-by: Bernd Helmle, Thomas Munro, Michael Paquier
      Discussion: https://postgr.es/m/109c267a-10d2-3c53-b60e-720fcf44d9e8@credativ.de
      Backpatch-through: 9.6
      187682c3
    • Peter Eisentraut's avatar
      doc: Group options in pg_amcheck reference page · cb3cffe6
      Peter Eisentraut authored
      The previous arrangement was just one big list, and the internal order
      was not all consistent either.  Now arrange the options by group and
      sort them, the way it's already done in the --help output and one
      other reference pages.  Also fix some ordering in the --help output.
      cb3cffe6
    • David Rowley's avatar
      Standardize usages of appendStringInfo and appendPQExpBuffer · f736e188
      David Rowley authored
      Fix a few places that were using appendStringInfo() when they should have
      been using appendStringInfoString().  Also some cases of
      appendPQExpBuffer() that would have been better suited to use
      appendPQExpBufferChar(), and finally, some places that used
      appendPQExpBuffer() when appendPQExpBufferStr() would have suited better.
      
      There are no bugs are being fixed here.  The aim is just to make the code
      use the most optimal function for the job.
      
      All the code being changed here is new to PG14.  It makes sense to fix
      these before we branch for PG15.  There are a few other places that we
      could fix, but those cases are older code so fixing those seems less
      worthwhile as it may cause unnecessary back-patching pain in the future.
      
      Author: Hou Zhijie
      Discussion: https://postgr.es/m/OS0PR01MB5716732158B1C4142C6FE375943D9@OS0PR01MB5716.jpnprd01.prod.outlook.com
      f736e188
    • Michael Paquier's avatar
      Ignore more environment variables in TAP tests · 8279f68a
      Michael Paquier authored
      Various environment variables were not getting reset in the TAP tests,
      which would cause failures depending on the tests or the environment
      variables involved.  For example, PGSSL{MAX,MIN}PROTOCOLVERSION could
      cause failures in the SSL tests.  Even worse, a junk value of
      PGCLIENTENCODING makes a server startup fail.  The list of variables
      reset is adjusted in each stable branch depending on what is supported.
      
      While on it, simplify a bit the code per a suggestion from Andrew
      Dunstan, using a list of variables instead of doing single deletions.
      
      Reviewed-by: Andrew Dunstan, Daniel Gustafsson
      Discussion: https://postgr.es/m/YLbjjRpucIeZ78VQ@paquier.xyz
      Backpatch-through: 9.6
      8279f68a
  6. 02 Jun, 2021 9 commits
  7. 01 Jun, 2021 2 commits
    • Tom Lane's avatar
      Reject SELECT ... GROUP BY GROUPING SETS (()) FOR UPDATE. · 1103033a
      Tom Lane authored
      This case should be disallowed, just as FOR UPDATE with a plain
      GROUP BY is disallowed; FOR UPDATE only makes sense when each row
      of the query result can be identified with a single table row.
      However, we missed teaching CheckSelectLocking() to check
      groupingSets as well as groupClause, so that it would allow
      degenerate grouping sets.  That resulted in a bad plan and
      a null-pointer dereference in the executor.
      
      Looking around for other instances of the same bug, the only one
      I found was in examine_simple_variable().  That'd just lead to
      silly estimates, but it should be fixed too.
      
      Per private report from Yaoguang Chen.
      Back-patch to all supported branches.
      1103033a
    • Amit Kapila's avatar
      pgoutput: Fix memory leak due to RelationSyncEntry.map. · eb89cb43
      Amit Kapila authored
      Release memory allocated when creating the tuple-conversion map and its
      component TupleDescs when its owning sync entry is invalidated.
      TupleDescs must also be freed when no map is deemed necessary, to begin
      with.
      
      Reported-by: Andres Freund
      Author: Amit Langote
      Reviewed-by: Takamichi Osumi, Amit Kapila
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/MEYP282MB166933B1AB02B4FE56E82453B64D9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
      eb89cb43
  8. 31 May, 2021 4 commits
    • Thomas Munro's avatar
      Fix error handling in replacement pthread_barrier_init(). · a40646e3
      Thomas Munro authored
      Commit 44bf3d50 incorrectly used an errno-style interface when supplying
      missing pthread functionality (i.e. on macOS), but it should check for
      and return error numbers directly.
      a40646e3
    • Peter Eisentraut's avatar
      Fix RADIUS error reporting in hba file parsing · 7c544ecd
      Peter Eisentraut authored
      The RADIUS-related checks in parse_hba_line() did not respect elevel
      and did not fill in *err_msg.  Also, verify_option_list_length()
      pasted together error messages in an untranslatable way.  To fix the
      latter, remove the function and do the error checking inline.  It's a
      bit more verbose but only minimally longer, and it makes fixing the
      first two issues straightforward.
      Reviewed-by: default avatarMagnus Hagander <magnus@hagander.net>
      Discussion: https://www.postgresql.org/message-id/flat/8381e425-8c23-99b3-15ec-3115001db1b2%40enterprisedb.com
      7c544ecd
    • Tom Lane's avatar
      Fix mis-planning of repeated application of a projection. · 6ee41a30
      Tom Lane authored
      create_projection_plan contains a hidden assumption (here made
      explicit by an Assert) that a projection-capable Path will yield a
      projection-capable Plan.  Unfortunately, that assumption is violated
      only a few lines away, by create_projection_plan itself.  This means
      that two stacked ProjectionPaths can yield an outcome where we try to
      jam the upper path's tlist into a non-projection-capable child node,
      resulting in an invalid plan.
      
      There isn't any good reason to have stacked ProjectionPaths; indeed the
      whole concept is faulty, since the set of Vars/Aggs/etc needed by the
      upper one wouldn't necessarily be available in the output of the lower
      one, nor could the lower one create such values if they weren't
      available from its input.  Hence, we can fix this by adjusting
      create_projection_path to strip any top-level ProjectionPath from the
      subpath it's given.  (This amounts to saying "oh, we changed our
      minds about what we need to project here".)
      
      The test case added here only fails in v13 and HEAD; before that, we
      don't attempt to shove the Sort into the parallel part of the plan,
      for reasons that aren't entirely clear to me.  However, all the
      directly-related code looks generally the same as far back as v11,
      where the hazard was introduced (by d7c19e62).  So I've got no faith
      that the same type of bug doesn't exist in v11 and v12, given the
      right test case.  Hence, back-patch the code changes, but not the
      irrelevant test case, into those branches.
      
      Per report from Bas Poot.
      
      Discussion: https://postgr.es/m/534fca83789c4a378c7de379e9067d4f@politie.nl
      6ee41a30
    • Noah Misch's avatar
      Raise a timeout to 180s, in test 010_logical_decoding_timelines.pl. · d03eeab8
      Noah Misch authored
      Per buildfarm member hornet.  Also, update Pod documentation showing the
      lower value.  Back-patch to v10, where the test first appeared.
      d03eeab8