1. 01 Oct, 2021 4 commits
    • Daniel Gustafsson's avatar
      Fix memory leak in pg_hmac · a5e83ad7
      Daniel Gustafsson authored
      The intermittent h buffer was not freed, causing it to leak. Backpatch
      through 14 where HMAC was refactored to the current API.
      
      Author: Sergey Shinderuk <s.shinderuk@postgrespro.ru>
      Discussion: https://postgr.es/m/af07e620-7e28-a742-4637-2bc44aa7c2be@postgrespro.ru
      Backpatch-through: 14
      a5e83ad7
    • Tom Lane's avatar
      Avoid believing incomplete MCV-only stats in get_variable_range(). · a54509bf
      Tom Lane authored
      get_variable_range() would incautiously believe that statistics
      containing only an MCV list are sufficient to derive a range estimate.
      That's okay for an enum-like column that contains only MCVs, but
      otherwise the estimate could be pretty bad.  Make it report that the
      range is indeterminate unless the MCVs plus nullfrac account for
      the whole table.
      
      I don't think this needs a dedicated test case, since a quick code
      coverage check verifies that the existing regression tests traverse
      all the alternatives.  There is room to doubt that a future-proof
      test case could be built anyway, given that the submitted example
      accidentally doesn't fail before v11.
      
      Per bug #17207 from Simon Perepelitsa.  Back-patch to v10.
      In principle this has been broken all along, but I'm hesitant to
      make such changes in 9.6, since if anyone is unhappy with 9.6.24's
      behavior there will be no second chance to fix it.
      
      Discussion: https://postgr.es/m/17207-5265aefa79e333b4@postgresql.org
      a54509bf
    • Tom Lane's avatar
      Fix Portal snapshot tracking to handle subtransactions properly. · e6adaa17
      Tom Lane authored
      Commit 84f5c290 forgot to consider the possibility that
      EnsurePortalSnapshotExists could run inside a subtransaction with
      lifespan shorter than the Portal's.  In that case, the new active
      snapshot would be popped at the end of the subtransaction, leaving
      a dangling pointer in the Portal, with mayhem ensuing.
      
      To fix, make sure the ActiveSnapshot stack entry is marked with
      the same subtransaction nesting level as the associated Portal.
      It's certainly safe to do so since we won't be here at all unless
      the stack is empty; hence we can't create an out-of-order stack.
      
      Let's also apply this logic in the case where PortalRunUtility
      sets portalSnapshot, just to be sure that path can't cause similar
      problems.  It's slightly less clear that that path can't create
      an out-of-order stack, so add an assertion guarding it.
      
      Report and patch by Bertrand Drouvot (with kibitzing by me).
      Back-patch to v11, like the previous commit.
      
      Discussion: https://postgr.es/m/ff82b8c5-77f4-3fe7-6028-fcf3303e82dd@amazon.com
      e6adaa17
    • Amit Kapila's avatar
      Doc: Move pg_stat_replication_slots view to "Collected Statistics Views" section. · 8de4a317
      Amit Kapila authored
      Commit 98681675 added pg_stat_replication_slots view to monitor
      ReorderBuffer stats but mistakenly added it under
      "Dynamic Statistics Views" section in the docs whereas it belongs to
      "Collected Statistics Views" section.
      
      Author: Amit Kapila
      Reviewed-by: Masahiko Sawada
      Backpatch-through: 14, where it was introduced
      Discussion: https://postgr.es/m/CAA4eK1Kb5ur=OC-G4cAsqPOjoVe+S8LNw1WmUY8Owasjk8o5WQ@mail.gmail.com
      8de4a317
  2. 30 Sep, 2021 2 commits
    • Tom Lane's avatar
      Remove gratuitous environment dependency in 002_types.pl test. · afc6081f
      Tom Lane authored
      Computing related timestamps by subtracting "N days" is sensitive
      to the prevailing timezone, since we interpret that as "same local
      time on the N'th prior day".  Even though the intervals in question
      are only two to four days, through remarkable bad luck they managed
      to cross the end of Ramadan in 2014, causing the test's output to
      change if timezone is set to Africa/Casablanca.  (Maybe in other
      Muslim areas as well; I didn't check.)  There's absolutely no reason
      for this test to exercise interval subtraction, so just get rid of
      that and use plain timestamptz constants representing the intended
      values.
      
      Per report from Andres Freund.  Back-patch to v10 where this test
      script came in.
      
      Discussion: https://postgr.es/m/20210930183641.7lh4jhvpipvromca@alap3.anarazel.de
      afc6081f
    • Alvaro Herrera's avatar
      Repair two portability oversights of new test · e3731bac
      Alvaro Herrera authored
      First, as pointed out by Tom Lane and Michael Paquier, I failed to
      realize that Windows' PostgresNode needs an extra pg_hba.conf line
      (added by PostgresNode->set_replication_conf, called internally by
      ->init() when 'allows_streaming=>1' is given -- but I purposefully
      omitted that).  I think a good fix should be to have nodes with only
      'has_archiving=>1' set up for replication too, but that's a bigger
      discussion.  Fix it by calling ->set_replication_conf, which is not
      unprecedented, as pointed out by Andrew Dunstan.
      
      I also forgot to uncomment a ->finish() call for a pumpable IPC::Run
      file descriptor.  Apparently this is innocuous in almost all platforms.
      
      Backpatch to 14.  The older branches were added this file too, but not
      this particular part of the test.
      
      Discussion: https://postgr.es/m/3000074.1632947632@sss.pgh.pa.us
      Discussion: https://postgr.es/m/YVT7qwhR8JmC2kfz@paquier.xyz
      e3731bac
  3. 29 Sep, 2021 6 commits
    • Alvaro Herrera's avatar
      Fix WAL replay in presence of an incomplete record · 64a8687a
      Alvaro Herrera authored
      Physical replication always ships WAL segment files to replicas once
      they are complete.  This is a problem if one WAL record is split across
      a segment boundary and the primary server crashes before writing down
      the segment with the next portion of the WAL record: WAL writing after
      crash recovery would happily resume at the point where the broken record
      started, overwriting that record ... but any standby or backup may have
      already received a copy of that segment, and they are not rewinding.
      This causes standbys to stop following the primary after the latter
      crashes:
        LOG:  invalid contrecord length 7262 at A8/D9FFFBC8
      because the standby is still trying to read the continuation record
      (contrecord) for the original long WAL record, but it is not there and
      it will never be.  A workaround is to stop the replica, delete the WAL
      file, and restart it -- at which point a fresh copy is brought over from
      the primary.  But that's pretty labor intensive, and I bet many users
      would just give up and re-clone the standby instead.
      
      A fix for this problem was already attempted in commit 515e3d84a0b5, but
      it only addressed the case for the scenario of WAL archiving, so
      streaming replication would still be a problem (as well as other things
      such as taking a filesystem-level backup while the server is down after
      having crashed), and it had performance scalability problems too; so it
      had to be reverted.
      
      This commit fixes the problem using an approach suggested by Andres
      Freund, whereby the initial portion(s) of the split-up WAL record are
      kept, and a special type of WAL record is written where the contrecord
      was lost, so that WAL replay in the replica knows to skip the broken
      parts.  With this approach, we can continue to stream/archive segment
      files as soon as they are complete, and replay of the broken records
      will proceed across the crash point without a hitch.
      
      Because a new type of WAL record is added, users should be careful to
      upgrade standbys first, primaries later. Otherwise they risk the standby
      being unable to start if the primary happens to write such a record.
      
      A new TAP test that exercises this is added, but the portability of it
      is yet to be seen.
      
      This has been wrong since the introduction of physical replication, so
      backpatch all the way back.  In stable branches, keep the new
      XLogReaderState members at the end of the struct, to avoid an ABI
      break.
      
      Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
      Reviewed-by: default avatarKyotaro Horiguchi <horikyota.ntt@gmail.com>
      Reviewed-by: default avatarNathan Bossart <bossartn@amazon.com>
      Discussion: https://postgr.es/m/202108232252.dh7uxf6oxwcy@alvherre.pgsql
      64a8687a
    • Bruce Momjian's avatar
      doc: PG 14 relnotes, improve cache invalidation wording · 4f2c7531
      Bruce Momjian authored
      Reported-by: Simon Riggs (privately)
      
      Backpatch-through: 14 only
      4f2c7531
    • Fujii Masao's avatar
      pgbench: Fix handling of socket errors during benchmark. · 8231c500
      Fujii Masao authored
      Previously socket errors such as invalid socket or socket wait method failures
      during benchmark caused pgbench to exit with status 0. Instead, errors during
      the run should result in exit status 2.
      
      Back-patch to v12 where pgbench started reporting exit status.
      
      Original complaint and patch by Hayato Kuroda.
      
      Author: Yugo Nagata, Fabien COELHO
      Reviewed-by: Kyotaro Horiguchi, Fujii Masao
      Discussion: https://postgr.es/m/TYCPR01MB5870057375ACA8A73099C649F5349@TYCPR01MB5870.jpnprd01.prod.outlook.com
      8231c500
    • Fujii Masao's avatar
      pgbench: Correct log level of message output when socket wait method fails. · 8021334d
      Fujii Masao authored
      The failure of socket wait method like "select()" doesn't terminate pgbench.
      So the log level of error message when that failure happens should be ERROR.
      But previously FATAL was used in that case.
      
      Back-patch to v13 where pgbench started using common logging API.
      
      Author: Yugo Nagata, Fabien COELHO
      Reviewed-by: Kyotaro Horiguchi, Fujii Masao
      Discussion: https://postgr.es/m/20210617005934.8bd37bf72efd5f1b38e6f482@sraoss.co.jp
      8021334d
    • Michael Paquier's avatar
      Clarify use of "statistics objects" in the code · 2cf9cf5d
      Michael Paquier authored
      The code inconsistently used "statistic object" or "statistics" where
      the correct term, as discussed, is actually "statistics object".  This
      improves the state of the code to be more consistent.
      
      While on it, fix an incorrect error message introduced in a4d75c86.  This
      error should never happen, as the code states, but it would be
      misleading.
      
      Author: Justin Pryzby
      Reviewed-by: Álvaro Herrera, Michael Paquier
      Discussion: https://postgr.es/m/20210924215827.GS831@telsasoft.com
      Backpatch-through: 14
      2cf9cf5d
    • Michael Paquier's avatar
      doc: Fix some typos and markups · 2a27dbae
      Michael Paquier authored
      Author: Ekaterina Kiryanova
      Discussion: https://postgr.es/m/8a14e78f-6991-7a6e-4711-fe376635f2ad@postgrespro.ru
      Backpatch-through: 14
      2a27dbae
  4. 28 Sep, 2021 3 commits
  5. 27 Sep, 2021 3 commits
  6. 26 Sep, 2021 1 commit
  7. 25 Sep, 2021 6 commits
  8. 23 Sep, 2021 4 commits
  9. 22 Sep, 2021 2 commits
    • Amit Kapila's avatar
      Invalidate all partitions for a partitioned table in publication. · 9eff8593
      Amit Kapila authored
      Updates/Deletes on a partition were allowed even without replica identity
      after the parent table was added to a publication. This would later lead
      to an error on subscribers. The reason was that we were not invalidating
      the partition's relcache and the publication information for partitions
      was not getting rebuilt. Similarly, we were not invalidating the
      partitions' relcache after dropping a partitioned table from a publication
      which will prohibit Updates/Deletes on its partition without replica
      identity even without any publication.
      
      Reported-by: Haiying Tang
      Author: Hou Zhijie and Vignesh C
      Reviewed-by: Vignesh C and Amit Kapila
      Backpatch-through: 13
      Discussion: https://postgr.es/m/OS0PR01MB6113D77F583C922F1CEAA1C3FBD29@OS0PR01MB6113.jpnprd01.prod.outlook.com
      9eff8593
    • Peter Geoghegan's avatar
      Fix "single value strategy" index deletion issue. · e665129c
      Peter Geoghegan authored
      It is not appropriate for deduplication to apply single value strategy
      when triggered by a bottom-up index deletion pass.  This wastes cycles
      because later bottom-up deletion passes will overinterpret older
      duplicate tuples that deduplication actually just skipped over "by
      design".  It also makes bottom-up deletion much less effective for low
      cardinality indexes that happen to cross a meaningless "index has single
      key value per leaf page" threshold.
      
      To fix, slightly narrow the conditions under which deduplication's
      single value strategy is considered.  We already avoided the strategy
      for a unique index, since our high level goal must just be to buy time
      for VACUUM to run (not to buy space).  We'll now also avoid it when we
      just had a bottom-up pass that reported failure.  The two cases share
      the same high level goal, and already overlapped significantly, so this
      approach is quite natural.
      
      Oversight in commit d168b666, which added bottom-up index deletion.
      
      Author: Peter Geoghegan <pg@bowt.ie>
      Discussion: https://postgr.es/m/CAH2-WznaOvM+Gyj-JQ0X=JxoMDxctDTYjiEuETdAGbF5EUc3MA@mail.gmail.com
      Backpatch: 14-, where bottom-up deletion was introduced.
      e665129c
  10. 21 Sep, 2021 3 commits
    • Michael Paquier's avatar
      Fix places in TestLib.pm in need of adaptation to the output of Msys perl · 90251ff1
      Michael Paquier authored
      Contrary to the output of native perl, Msys perl generates outputs with
      CRLFs characters.  There are already places in the TAP code where CRLFs
      (\r\n) are automatically converted to LF (\n) on Msys, but we missed a
      couple of places when running commands and using their output for
      comparison, that would lead to failures.
      
      This problem has been found thanks to the test added in 5adb067 using
      TestLib::command_checks_all(), but after a closer look more code paths
      were missing a filter.
      
      This is backpatched all the way down to prevent any surprises if a new
      test is introduced in stable branches.
      
      Reviewed-by: Andrew Dunstan, Álvaro Herrera
      Discussion: https://postgr.es/m/1252480.1631829409@sss.pgh.pa.us
      Backpatch-through: 9.6
      90251ff1
    • Tom Lane's avatar
      Fix misevaluation of STABLE parameters in CALL within plpgsql. · 2ad5f963
      Tom Lane authored
      Before commit 84f5c290, a STABLE function in a plpgsql CALL
      statement's argument list would see an up-to-date snapshot,
      because exec_stmt_call would push a new snapshot.  I got rid of
      that because the possibility of the snapshot disappearing within
      COMMIT made it too hard to manage a snapshot across the CALL
      statement.  That's fine so far as the procedure itself goes,
      but I forgot to think about the possibility of STABLE functions
      within the CALL argument list.  As things now stand, those'll
      be executed with the Portal's snapshot as ActiveSnapshot,
      keeping them from seeing updates more recent than Portal startup.
      
      (VOLATILE functions don't have a problem because they take their
      own snapshots; which indeed is also why the procedure itself
      doesn't have a problem.  There are no STABLE procedures.)
      
      We can fix this by pushing a new snapshot transiently within
      ExecuteCallStmt itself.  Popping the snapshot before we get
      into the procedure proper eliminates the management problem.
      The possibly-useless extra snapshot-grab is slightly annoying,
      but it's no worse than what happened before 84f5c290.
      
      Per bug #17199 from Alexander Nawratil.  Back-patch to v11,
      like the previous patch.
      
      Discussion: https://postgr.es/m/17199-1ab2561f0d94af92@postgresql.org
      2ad5f963
    • Alvaro Herrera's avatar
      Document XLOG_INCLUDE_XID a little better · c1d1ae1d
      Alvaro Herrera authored
      I noticed that commit 0bead9af left this flag undocumented in
      XLogSetRecordFlags, which led me to discover that the flag doesn't
      actually do what the one comment on it said it does.  Improve the
      situation by adding some more comments.
      
      Backpatch to 14, where the aforementioned commit appears.
      
      Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
      Discussion: https://postgr.es/m/202109212119.c3nhfp64t2ql@alvherre.pgsql
      c1d1ae1d
  11. 20 Sep, 2021 5 commits
  12. 19 Sep, 2021 1 commit
    • Tomas Vondra's avatar
      Disallow extended statistics on system columns · 66061077
      Tomas Vondra authored
      Since introduction of extended statistics, we've disallowed references
      to system columns. So for example
      
          CREATE STATISTICS s ON ctid FROM t;
      
      would fail. But with extended statistics on expressions, it was possible
      to work around this limitation quite easily
      
          CREATE STATISTICS s ON (ctid::text) FROM t;
      
      This is an oversight in a4d75c86, fixed by adding a simple check.
      Backpatch to PostgreSQL 14, where support for extended statistics on
      expressions was introduced.
      
      Backpatch-through: 14
      Discussion: https://postgr.es/m/20210816013255.GS10479%40telsasoft.com
      66061077