1. 23 Mar, 2021 12 commits
    • Peter Geoghegan's avatar
      nbtree VACUUM: Cope with buggy opclasses. · 5b861baa
      Peter Geoghegan authored
      Teach nbtree VACUUM to press on with vacuuming in the event of a page
      deletion attempt that fails to "re-find" a downlink for its child/target
      page.
      
      There is no good reason to treat this as an irrecoverable error.  But
      there is a good reason not to: pressing on at this point removes any
      question of VACUUM not making progress solely due to misbehavior from
      user-defined operator class code.
      
      Discussion: https://postgr.es/m/CAH2-Wzma5G9CTtMjbrXTwOym+U=aWg-R7=-htySuztgoJLvZXg@mail.gmail.com
      5b861baa
    • Robert Haas's avatar
      Improve pg_amcheck's TAP test 003_check.pl. · 87d90ac6
      Robert Haas authored
      Disable autovacuum, because we don't want it to run against
      intentionally corrupted tables. Also, before corrupting the tables,
      run pg_amcheck and ensure that it passes. Otherwise, if something
      unexpected happens when we check the corrupted tables, it's not so
      clear whether it would have also happened before we corrupted
      them.
      
      Mark Dilger
      
      Discussion: http://postgr.es/m/AA5506CE-7D2A-42E4-A51D-358635E3722D@enterprisedb.com
      87d90ac6
    • Tom Lane's avatar
      Fix psql's \connect command some more. · ea801385
      Tom Lane authored
      Jasen Betts reported yet another unintended side effect of commit
      85c54287: reconnecting with "\c service=whatever" did not have the
      expected results.  The reason is that starting from the output of
      PQconndefaults() effectively allows environment variables (such
      as PGPORT) to override entries in the service file, whereas the
      normal priority is the other way around.
      
      Not using PQconndefaults at all would require yet a third main code
      path in do_connect's parameter setup, so I don't really want to fix
      it that way.  But we can have the logic effectively ignore all the
      default values for just a couple more lines of code.
      
      This patch doesn't change the behavior for "\c -reuse-previous=on
      service=whatever".  That remains significantly different from before
      85c54287, because many more parameters will be re-used, and thus
      not be possible for service entries to replace.  But I think this
      is (mostly?) intentional.  In any case, since libpq does not report
      where it got parameter values from, it's hard to do differently.
      
      Per bug #16936 from Jasen Betts.  As with the previous patches,
      back-patch to all supported branches.  (9.5 is unfortunately now
      out of support, so this won't get fixed there.)
      
      Discussion: https://postgr.es/m/16936-3f524322a53a29f0@postgresql.org
      ea801385
    • Tom Lane's avatar
      Avoid possible crash while finishing up a heap rewrite. · 9d523119
      Tom Lane authored
      end_heap_rewrite was not careful to ensure that the target relation
      is open at the smgr level before performing its final smgrimmedsync.
      In ordinary cases this is no problem, because it would have been
      opened earlier during the rewrite.  However a crash can be reproduced
      by re-clustering an empty table with CLOBBER_CACHE_ALWAYS enabled.
      
      Although that exact scenario does not crash in v13, I think that's
      a chance result of unrelated planner changes, and the problem is
      likely still reachable with other test cases.  The true proximate
      cause of this failure is commit c6b92041, which replaced a call to
      heap_sync (which was careful about opening smgr) with a direct call
      to smgrimmedsync.  Hence, back-patch to v13.
      
      Amul Sul, per report from Neha Sharma; cosmetic changes
      and test case by me.
      
      Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
      9d523119
    • Peter Eisentraut's avatar
      pgcrypto: Check for error return of px_cipher_decrypt() · 22e1943f
      Peter Eisentraut authored
      This has previously not been a problem (that anyone ever reported),
      but in future OpenSSL versions (3.0.0), where legacy ciphers are/can
      be disabled, this is the place where this is reported.  So we need to
      catch the error here, otherwise the higher-level functions would
      return garbage.  The nearby encryption code already handled errors
      similarly.
      Reviewed-by: default avatarDaniel Gustafsson <daniel@yesql.se>
      Discussion: https://www.postgresql.org/message-id/9e9c431c-0adc-7a6d-9b1a-915de1ba3fe7@enterprisedb.com
      22e1943f
    • Peter Eisentraut's avatar
      Add bit_count SQL function · a6715af1
      Peter Eisentraut authored
      This function for bit and bytea counts the set bits in the bit or byte
      string.  Internally, we use the existing popcount functionality.
      
      For the name, after some discussion, we settled on bit_count, which
      also exists with this meaning in MySQL, Java, and Python.
      
      Author: David Fetter <david@fetter.org>
      Discussion: https://www.postgresql.org/message-id/flat/20201230105535.GJ13234@fetter.org
      a6715af1
    • Michael Paquier's avatar
      Add per-index stats information in verbose logs of autovacuum · 5aed6a1f
      Michael Paquier authored
      Once a relation's autovacuum is completed, the logs include more
      information about this relation state if the threshold of
      log_autovacuum_min_duration (or its relation option) is reached, with
      for example contents about the statistics of the VACUUM operation for
      the relation, WAL and system usage.
      
      This commit adds more information about the statistics of the relation's
      indexes, with one line of logs generated for each index.  The index
      stats were already calculated, but not printed in the context of
      autovacuum yet.  While on it, some refactoring is done to keep track of
      the index statistics directly within LVRelStats, simplifying some
      routines related to parallel VACUUMs.
      
      Author: Masahiko Sawada
      Reviewed-by: Michael Paquier, Euler Taveira
      Discussion: https://postgr.es/m/CAD21AoAy6SxHiTivh5yAPJSUE4S=QRPpSZUdafOSz0R+fRcM6Q@mail.gmail.com
      5aed6a1f
    • Amit Kapila's avatar
      Fix dangling pointer reference in stream_cleanup_files. · 4b82ed6e
      Amit Kapila authored
      We can't access the entry after it is removed from dynahash.
      
      Author: Peter Smith
      Discussion: https://postgr.es/m/CAHut+Ps-pL++f6CJwPx2+vUqXuew=Xt-9Bi-6kCyxn+Fwi2M7w@mail.gmail.com
      4b82ed6e
    • Tomas Vondra's avatar
      Use correct spelling of statistics kind · a5f002ad
      Tomas Vondra authored
      A couple error messages and comments used 'statistic kind', not the
      correct 'statistics kind'. Fix and backpatch all the way back to 10,
      where extended statistics were introduced.
      
      Backpatch-through: 10
      a5f002ad
    • Fujii Masao's avatar
      Change the type of WalReceiverWaitStart wait event from Client to IPC. · 1e3e8b51
      Fujii Masao authored
      Previously the type of this wait event was Client. But while this
      wait event is being reported, walreceiver process is waiting for
      the startup process to set initial data for streaming replication.
      It's not waiting for any activity on a socket connected to a user
      application or walsender. So this commit changes the type for
      WalReceiverWaitStart wait event to IPC.
      
      Author: Fujii Masao
      Reviewed-by: Kyotaro Horiguchi
      Discussion: https://postgr.es/m/cdacc27c-37ff-f1a4-20e2-ce19933abfcc@oss.nttdata.com
      1e3e8b51
    • Fujii Masao's avatar
      pg_waldump: Fix bug in per-record statistics. · 51893c84
      Fujii Masao authored
      pg_waldump --stats=record identifies a record by a combination
      of the RmgrId and the four bits of the xl_info field of the record.
      But XACT records use the first bit of those four bits for an optional
      flag variable, and the following three bits for the opcode to
      identify a record. So previously the same type of XACT record
      could have different four bits (three bits are the same but the
      first one bit is different), and which could cause
      pg_waldump --stats=record to show two lines of per-record statistics
      for the same XACT record. This is a bug.
      
      This commit changes pg_waldump --stats=record so that it processes
      only XACT record differently, i.e., filters the opcode out of xl_info
      and uses a combination of the RmgrId and those three bits as
      the identifier of a record, only for XACT record. For other records,
      the four bits of the xl_info field are still used.
      
      Back-patch to all supported branches.
      
      Author: Kyotaro Horiguchi
      Reviewed-by: Shinya Kato, Fujii Masao
      Discussion: https://postgr.es/m/2020100913412132258847@highgo.ca
      51893c84
    • Bruce Momjian's avatar
      Add macro RelationIsPermanent() to report relation permanence · 95d77149
      Bruce Momjian authored
      Previously, to check relation permanence, the Relation's Form_pg_class
      structure member relpersistence was compared to the value
      RELPERSISTENCE_PERMANENT ("p"). This commit adds the macro
      RelationIsPermanent() and is used in appropirate places to simplify the
      code.  This matches other RelationIs* macros.
      
      This macro will be used in more places in future cluster file encryption
      patches.
      
      Discussion: https://postgr.es/m/20210318153134.GH20766@tamriel.snowman.net
      95d77149
  2. 22 Mar, 2021 14 commits
  3. 21 Mar, 2021 11 commits
    • Michael Paquier's avatar
      Simplify TAP tests of kerberos with expected log file contents · 11e1577a
      Michael Paquier authored
      The TAP tests of kerberos rely on the logs generated by the backend to
      check various connection scenarios.  In order to make sure that a given
      test does not overlap with the log contents generated by a previous
      test, the test suite relied on a logic with the logging collector and a
      rotation of the log files to ensure the uniqueness of the log generated
      with a wait phase.
      
      Parsing the log contents for expected patterns is a problem that has
      been solved in a simpler way by PostgresNode::issues_sql_like() where
      the log file is truncated before checking for the contents generated,
      with the backend sending its output to a log file given by pg_ctl
      instead.  This commit switches the kerberos test suite to use such a
      method, removing any wait phase and simplifying the whole logic,
      resulting in less code.  If a failure happens in the tests, the contents
      of the logs are still showed to the user at the moment of the failure
      thanks to like(), so this has no impact on debugging capabilities.
      
      I have bumped into this issue while reviewing a different patch set
      aiming at extending the kerberos test suite to check for multiple
      log patterns instead of one now.
      
      Author: Michael Paquier
      Reviewed-by: Stephen Frost, Bharath Rupireddy
      Discussion: https://postgr.es/m/YFXcq2vBTDGQVBNC@paquier.xyz
      11e1577a
    • Michael Paquier's avatar
      Fix timeline assignment in checkpoints with 2PC transactions · 595b9cba
      Michael Paquier authored
      Any transactions found as still prepared by a checkpoint have their
      state data read from the WAL records generated by PREPARE TRANSACTION
      before being moved into their new location within pg_twophase/.  While
      reading such records, the WAL reader uses the callback
      read_local_xlog_page() to read a page, that is shared across various
      parts of the system.  This callback, since 1148e22a, has introduced an
      update of ThisTimeLineID when reading a record while in recovery, which
      is potentially helpful in the context of cascading WAL senders.
      
      This update of ThisTimeLineID interacts badly with the checkpointer if a
      promotion happens while some 2PC data is read from its record, as, by
      changing ThisTimeLineID, any follow-up WAL records would be written to
      an timeline older than the promoted one.  This results in consistency
      issues.  For instance, a subsequent server restart would cause a failure
      in finding a valid checkpoint record, resulting in a PANIC, for
      instance.
      
      This commit changes the code reading the 2PC data to reset the timeline
      once the 2PC record has been read, to prevent messing up with the static
      state of the checkpointer.  It would be tempting to do the same thing
      directly in read_local_xlog_page().  However, based on the discussion
      that has led to 1148e22a, users may rely on the updates of
      ThisTimeLineID when a WAL record page is read in recovery, so changing
      this callback could break some cases that are working currently.
      
      A TAP test reproducing the issue is added, relying on a PITR to
      precisely trigger a promotion with a prepared transaction still
      tracked.
      
      Per discussion with Heikki Linnakangas, Kyotaro Horiguchi, Fujii Masao
      and myself.
      
      Author: Soumyadeep Chakraborty, Jimmy Yih, Kevin Yeap
      Discussion: https://postgr.es/m/CAE-ML+_EjH_fzfq1F3RJ1=XaaNG=-Jz-i3JqkNhXiLAsM3z-Ew@mail.gmail.com
      Backpatch-through: 10
      595b9cba
    • Tom Lane's avatar
      Fix assorted silliness in ATExecSetCompression(). · ac897c48
      Tom Lane authored
      It's not okay to scribble directly on a syscache entry.
      Nor to continue accessing said entry after releasing it.
      
      Also get rid of not-used local variables.
      
      Per valgrind testing.
      ac897c48
    • Peter Geoghegan's avatar
      Recycle nbtree pages deleted during same VACUUM. · 9dd963ae
      Peter Geoghegan authored
      Maintain a simple array of metadata about pages that were deleted during
      nbtree VACUUM's current btvacuumscan() call.  Use this metadata at the
      end of btvacuumscan() to attempt to place newly deleted pages in the FSM
      without further delay.  It might not yet be safe to place any of the
      pages in the FSM by then (they may not be deemed recyclable), but we
      have little to lose and plenty to gain by trying.  In practice there is
      a very good chance that this will work out when vacuuming larger
      indexes, where scanning the index naturally takes quite a while.
      
      This commit doesn't change the page recycling invariants; it merely
      improves the efficiency of page recycling within the confines of the
      existing design.  Recycle safety is a part of nbtree's implementation of
      what Lanin & Shasha call "the drain technique".  The design happens to
      use transaction IDs (they're stored in deleted pages), but that in
      itself doesn't align the cutoff for recycle safety to any of the
      XID-based cutoffs used by VACUUM (e.g., OldestXmin).  All that matters
      is whether or not _other_ backends might be able to observe various
      inconsistencies in the tree structure (that they cannot just detect and
      recover from by moving right).  Recycle safety is purely a question of
      maintaining the consistency (or the apparent consistency) of a physical
      data structure.
      
      Note that running a simple serial test case involving a large range
      DELETE followed by a VACUUM VERBOSE will probably show that any newly
      deleted nbtree pages are not yet reusable/recyclable.  This is expected
      in the absence of even one concurrent XID assignment.  It is an old
      implementation restriction.  In practice it's unlikely to be the thing
      that makes recycling remain unsafe, at least with larger indexes, where
      recycling newly deleted pages during the same VACUUM actually matters.
      
      An important high-level goal of this commit (as well as related recent
      commits e5d8a999 and 9f3665fb) is to make expensive deferred cleanup
      operations in index AMs rare in general.  If index vacuuming frequently
      depends on the next VACUUM operation finishing off work that the current
      operation started, then the general behavior of index vacuuming is hard
      to predict.  This is relevant to ongoing work that adds a vacuumlazy.c
      mechanism to skip index vacuuming in certain cases.  Anything that makes
      the real world behavior of index vacuuming simpler and more linear will
      also make top-down modeling in vacuumlazy.c more robust.
      
      Author: Peter Geoghegan <pg@bowt.ie>
      Reviewed-By: default avatarMasahiko Sawada <sawada.mshk@gmail.com>
      Discussion: https://postgr.es/m/CAH2-Wzk76_P=67iUscb1UN44-gyZL-KgpsXbSxq_bdcMa7Q+wQ@mail.gmail.com
      9dd963ae
    • Tom Lane's avatar
      Bring configure support for LZ4 up to snuff. · 4d399a6f
      Tom Lane authored
      It's not okay to just shove the pkg_config results right into our
      build flags, for a couple different reasons:
      
      * This fails to maintain the separation between CPPFLAGS and CFLAGS,
      as well as that between LDFLAGS and LIBS.  (The CPPFLAGS angle is,
      I believe, the reason for warning messages reported when building
      with MacPorts' liblz4.)
      
      * If pkg_config emits anything other than -I/-D/-L/-l switches,
      it's highly unlikely that we want to absorb those.  That'd be more
      likely to break the build than do anything helpful.  (Even the -D
      case is questionable; but we're doing that for libxml2, so I kept it.)
      
      Also, it's not okay to skip doing an AC_CHECK_LIB probe, as
      evidenced by recent build failure on topminnow; that should
      have been caught at configure time.
      
      Model fixes for this on configure's libxml2 support.
      
      It appears that somebody overlooked an autoheader run, too.
      
      Discussion: https://postgr.es/m/20210119190720.GL8560@telsasoft.com
      4d399a6f
    • Tom Lane's avatar
      Make compression.sql regression test independent of default. · fd1ac9a5
      Tom Lane authored
      This test will fail in "make installcheck" if the installation's
      default_toast_compression setting is not 'pglz'.  Make it robust
      against that situation.
      
      Dilip Kumar
      
      Discussion: https://postgr.es/m/CAFiTN-t0w+Rc2U3S+y=7KWcLuOYNB5MfWeGdNa7+pg0UovVdcQ@mail.gmail.com
      fd1ac9a5
    • Andrew Dunstan's avatar
      Don't run recover crash_temp_files test in Windows perl · ef823873
      Andrew Dunstan authored
      This reverts commit 677271a3.
      "Unbreak recovery test on Windows"
      
      The test hangs on Windows, and attempts to remedy the problem have
      proved fragile at best. So we simply disable the test on Windows perl.
      (Msys perl seems perfectly happy).
      
      Discussion: https://postgr.es/m/5b748470-7335-5439-e876-6a88c951e1c5@dunslane.net
      ef823873
    • Alvaro Herrera's avatar
      Fix new memory leaks in libpq · 2b526ed2
      Alvaro Herrera authored
      My oversight in commit 9aa491ab.
      
      Per coverity.
      2b526ed2
    • Andrew Dunstan's avatar
      Unbreak recovery test on Windows · 677271a3
      Andrew Dunstan authored
      On Windows we need to send explicit quit messages to psql or the TAP tests
      can hang.
      677271a3
    • Tom Lane's avatar
      Suppress various new compiler warnings. · 9fb9691a
      Tom Lane authored
      Compilers that don't understand that elog(ERROR) doesn't return
      issued warnings here.  In the cases in libpq_pipeline.c, we were
      not exactly helping things by failing to mark pg_fatal() as noreturn.
      
      Per buildfarm.
      9fb9691a
    • Peter Eisentraut's avatar
      Move lwlock-release probe back where it belongs · 96ae658e
      Peter Eisentraut authored
      The documentation specifically states that lwlock-release fires before
      any released waiters have been awakened.  It worked that way until
      ab5194e6, where is seems to have been
      misplaced accidentally.  Move it back where it belongs.
      
      Author: Craig Ringer <craig.ringer@enterprisedb.com>
      Discussion: https://www.postgresql.org/message-id/CAGRY4nwxKUS_RvXFW-ugrZBYxPFFM5kjwKT5O+0+Stuga5b4+Q@mail.gmail.com
      96ae658e
  4. 20 Mar, 2021 3 commits
    • Tomas Vondra's avatar
      Use valid compression method in brin_form_tuple · 882b2cdc
      Tomas Vondra authored
      When compressing the BRIN summary, we can't simply use the compression
      method from the indexed attribute.  The summary may use a different data
      type, e.g. fixed-length attribute may have varlena summary, leading to
      compression failures.  For the built-in BRIN opclasses this happens to
      work, because the summary uses the same data type as the attribute.
      
      When the data types match, we can inherit use the compression method
      specified for the attribute (it's copied into the index descriptor).
      Otherwise we don't have much choice and have to use the default one.
      
      Author: Tomas Vondra
      Reviewed-by: default avatarJustin Pryzby <pryzby@telsasoft.com>
      Discussion: https://postgr.es/m/e0367f27-392c-321a-7411-a58e1a7e4817%40enterprisedb.com
      882b2cdc
    • Tom Lane's avatar
      Fix up pg_dump's handling of per-attribute compression options. · aa25d108
      Tom Lane authored
      The approach used in commit bbe0a81d would've been disastrous for
      portability of dumps.  Instead handle non-default compression options
      in separate ALTER TABLE commands.  This reduces chatter for the
      common case where most columns are compressed the same way, and it
      makes it possible to restore the dump to a server that lacks any
      knowledge of per-attribute compression options (so long as you're
      willing to ignore syntax errors from the ALTER TABLE commands).
      
      There's a whole lot left to do to mop up after bbe0a81d, but
      I'm fast-tracking this part because we need to see if it's
      enough to make the buildfarm's cross-version-upgrade tests happy.
      
      Justin Pryzby and Tom Lane
      
      Discussion: https://postgr.es/m/20210119190720.GL8560@telsasoft.com
      aa25d108
    • Tom Lane's avatar
      Fix memory leak when rejecting bogus DH parameters. · e835e89a
      Tom Lane authored
      While back-patching e0e569e1, I noted that there were some other
      places where we ought to be applying DH_free(); namely, where we
      load some DH parameters from a file and then reject them as not
      being sufficiently secure.  While it seems really unlikely that
      anybody would hit these code paths in production, let alone do
      so repeatedly, let's fix it for consistency.
      
      Back-patch to v10 where this code was introduced.
      
      Discussion: https://postgr.es/m/16160-18367e56e9a28264@postgresql.org
      e835e89a