1. 06 Jan, 2021 7 commits
    • Tom Lane's avatar
      Add a test module for the regular expression package. · ca8217c1
      Tom Lane authored
      This module provides a function test_regex() that is functionally
      rather like regexp_matches(), but with additional debugging-oriented
      options and additional output.  The debug options are somewhat obscure;
      they are chosen to match the API of the test harness that Henry Spencer
      wrote way-back-when for use in Tcl.  With this, we can import all the
      test cases that Spencer wrote originally, even for regex functionality
      that we don't currently expose in Postgres.  This seems necessary
      because we can no longer rely on Tcl to act as upstream and verify
      any fixes or improvements that we make.
      
      In addition to Spencer's tests, I added a few for lookbehind
      constraints (which we added in 2015, and Tcl still hasn't absorbed)
      that are modeled on his tests for lookahead constraints.  After looking
      at code coverage reports, I also threw in a couple of tests to more
      fully exercise our "high colormap" logic.
      
      According to my testing, this brings the check-world coverage
      for src/backend/regex/ from 71.1% to 86.7% of lines.
      (coverage.postgresql.org shows a slightly different number,
      which I think is because it measures a non-assert build.)
      
      Discussion: https://postgr.es/m/2873268.1609732164@sss.pgh.pa.us
      ca8217c1
    • Peter Eisentraut's avatar
      Replace CLOBBER_CACHE_ALWAYS with run-time GUC · 4656e3d6
      Peter Eisentraut authored
      Forced cache invalidation (CLOBBER_CACHE_ALWAYS) has been impractical
      to use for testing in PostgreSQL because it's so slow and because it's
      toggled on/off only at build time.  It is helpful when hunting bugs in
      any code that uses the sycache/relcache because causes cache
      invalidations to be injected whenever it would be possible for an
      invalidation to occur, whether or not one was really pending.
      
      Address this by providing run-time control over cache clobber
      behaviour using the new debug_invalidate_system_caches_always GUC.
      Support is not compiled in at all unless assertions are enabled or
      CLOBBER_CACHE_ENABLED is explicitly defined at compile time.  It
      defaults to 0 if compiled in, so it has negligible effect on assert
      build performance by default.
      
      When support is compiled in, test code can now set
      debug_invalidate_system_caches_always=1 locally to a backend to test
      specific queries, functions, extensions, etc.  Or tests can toggle it
      globally for a specific test case while retaining normal performance
      during test setup and teardown.
      
      For backwards compatibility with existing test harnesses and scripts,
      debug_invalidate_system_caches_always defaults to 1 if
      CLOBBER_CACHE_ALWAYS is defined, and to 3 if CLOBBER_CACHE_RECURSIVE
      is defined.
      
      CLOBBER_CACHE_ENABLED is now visible in pg_config_manual.h, as is the
      related RECOVER_RELATION_BUILD_MEMORY setting for the relcache.
      
      Author: Craig Ringer <craig.ringer@2ndquadrant.com>
      Discussion: https://www.postgresql.org/message-id/flat/CAMsr+YF=+ctXBZj3ywmvKNUjWpxmuTuUKuv-rgbHGX5i5pLstQ@mail.gmail.com
      4656e3d6
    • Fujii Masao's avatar
      Detect the deadlocks between backends and the startup process. · 8900b5a9
      Fujii Masao authored
      The deadlocks that the recovery conflict on lock is involved in can
      happen between hot-standby backends and the startup process.
      If a backend takes an access exclusive lock on the table and which
      finally triggers the deadlock, that deadlock can be detected
      as expected. On the other hand, previously, if the startup process
      took an access exclusive lock and which finally triggered the deadlock,
      that deadlock could not be detected and could remain even after
      deadlock_timeout passed. This is a bug.
      
      The cause of this bug was that the code for handling the recovery
      conflict on lock didn't take care of deadlock case at all. It assumed
      that deadlocks involving the startup process and backends were able
      to be detected by the deadlock detector invoked within backends.
      But this assumption was incorrect. The startup process also should
      have invoked the deadlock detector if necessary.
      
      To fix this bug, this commit makes the startup process invoke
      the deadlock detector if deadlock_timeout is reached while handling
      the recovery conflict on lock. Specifically, in that case, the startup
      process requests all the backends holding the conflicting locks to
      check themselves for deadlocks.
      
      Back-patch to v9.6. v9.5 has also this bug, but per discussion we decided
      not to back-patch the fix to v9.5. Because v9.5 doesn't have some
      infrastructure codes (e.g., 37c54863) that this bug fix patch depends on.
      We can apply those codes for the back-patch, but since the next minor
      version release is the final one for v9.5, it's risky to do that. If we
      unexpectedly introduce new bug to v9.5 by the back-patch, there is no
      chance to fix that. We determined that the back-patch to v9.5 would give
      more risk than gain.
      
      Author: Fujii Masao
      Reviewed-by: Bertrand Drouvot, Masahiko Sawada, Kyotaro Horiguchi
      Discussion: https://postgr.es/m/4041d6b6-cf24-a120-36fa-1294220f8243@oss.nttdata.com
      8900b5a9
    • Amit Kapila's avatar
    • Fujii Masao's avatar
      doc: Fix description about default behavior of recovery_target_timeline. · 25dde583
      Fujii Masao authored
      The default value of recovery_target_timeline was changed in v12,
      but the description about the default behavior of that was not updated.
      
      Back-patch to v12 where the default behavior of recovery_target_timeline
      was changed.
      
      Author: Benoit Lobréau
      Reviewed-by: Fujii Masao
      Discussion: https://postgr.es/m/CAPE8EZ7c3aruEmM24GYkj8y8WmHKD1m9TtPtgCF0nQ3zw4LCkQ@mail.gmail.com
      25dde583
    • Michael Paquier's avatar
      Promote --data-checksums to the common set of options in initdb --help · bc08f797
      Michael Paquier authored
      This was previously part of the section dedicated to less common
      options, but it is an option commonly used these days.
      
      Author: Michael Banck
      Reviewed-by: Stephen Frost, Michael Paquier
      Discussion: https://postgr.es/m/d7938aca4d4ea8e8c72c33bd75efe9f8218fe390.camel@credativ.de
      bc08f797
    • Tom Lane's avatar
      Revert unstable test cases from commit 7d80441d. · 14d49f48
      Tom Lane authored
      I momentarily forgot that the "owner" column wouldn't be stable
      in the buildfarm.  Oh well, these tests weren't very valuable
      anyway.
      
      Discussion: https://postgr.es/m/20201130165436.GX24052@telsasoft.com
      14d49f48
  2. 05 Jan, 2021 12 commits
  3. 04 Jan, 2021 9 commits
    • Thomas Munro's avatar
      Rename "enum blacklist" to "uncommitted enums". · c0d4f6d8
      Thomas Munro authored
      We agreed to remove this terminology and use something more descriptive.
      
      Discussion: https://postgr.es/m/20200615182235.x7lch5n6kcjq4aue%40alap3.anarazel.de
      c0d4f6d8
    • Tom Lane's avatar
      Fix integer-overflow corner cases in substring() functions. · 4bd3fad8
      Tom Lane authored
      If the substring start index and length overflow when added together,
      substring() misbehaved, either throwing a bogus "negative substring
      length" error on a case that should succeed, or failing to complain that
      a negative length is negative (and instead returning the whole string,
      in most cases).  Unsurprisingly, the text, bytea, and bit variants of
      the function all had this issue.  Rearrange the logic to ensure that
      negative lengths are always rejected, and add an overflow check to
      handle the other case.
      
      Also install similar guards into detoast_attr_slice() (nee
      heap_tuple_untoast_attr_slice()), since it's far from clear that
      no other code paths leading to that function could pass it values
      that would overflow.
      
      Patch by myself and Pavel Stehule, per bug #16804 from Rafi Shamim.
      
      Back-patch to v11.  While these bugs are old, the common/int.h
      infrastructure for overflow-detecting arithmetic didn't exist before
      commit 4d6ad312, and it doesn't seem like these misbehaviors are bad
      enough to justify developing a standalone fix for the older branches.
      
      Discussion: https://postgr.es/m/16804-f4eeeb6c11ba71d4@postgresql.org
      4bd3fad8
    • Thomas Munro's avatar
    • Tom Lane's avatar
      Rethink the "read/write parameter" mechanism in pl/pgsql. · 1c1cbe27
      Tom Lane authored
      Performance issues with the preceding patch to re-implement array
      element assignment within pl/pgsql led me to realize that the read/write
      parameter mechanism is misdesigned.  Instead of requiring the assignment
      source expression to be such that *all* its references to the target
      variable could be passed as R/W, we really want to identify *one*
      reference to the target variable to be passed as R/W, allowing any other
      ones to be passed read/only as they would be by default.  As long as the
      R/W reference is a direct argument to the top-level (hence last to be
      executed) function in the expression, there is no harm in R/O references
      being passed to other lower parts of the expression.  Nor is there any
      use-case for more than one argument of the top-level function being R/W.
      
      Hence, rewrite that logic to identify one single Param that references
      the target variable, and make only that Param pass a read/write
      reference, not any other Params referencing the target variable.
      
      Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
      1c1cbe27
    • Tom Lane's avatar
      Remove PLPGSQL_DTYPE_ARRAYELEM datum type within pl/pgsql. · 1788828d
      Tom Lane authored
      In the wake of the previous commit, we don't really need this anymore,
      since array assignment is primarily handled by the core code.
      
      The only way that that code could still be reached is that a GET
      DIAGNOSTICS target variable could be an array element.  But that
      doesn't seem like a particularly essential feature.  I'd added it
      in commit 55caaaeb, but just because it was easy not because
      anyone had actually asked for it.  Hence, revert that patch and
      then remove the now-unreachable stuff.  (If we really had to,
      we could probably reimplement GET DIAGNOSTICS using the new
      assignment machinery; but the cost/benefit ratio looks very poor,
      and it'd likely be a bit slower.)
      
      Note that PLPGSQL_DTYPE_RECFIELD remains.  It's possible that we
      could get rid of that too, but maintaining the existing behaviors
      for RECORD-type variables seems like it might be difficult.  Since
      there's not any functional limitation in those code paths as there
      was in the ARRAYELEM code, I've not pursued the idea.
      
      Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
      1788828d
    • Tom Lane's avatar
      Re-implement pl/pgsql's expression and assignment parsing. · c9d52984
      Tom Lane authored
      Invent new RawParseModes that allow the core grammar to handle
      pl/pgsql expressions and assignments directly, and thereby get rid
      of a lot of hackery in pl/pgsql's parser.  This moves a good deal
      of knowledge about pl/pgsql into the core code: notably, we have to
      invent a CoercionContext that matches pl/pgsql's (rather dubious)
      historical behavior for assignment coercions.  That's getting away
      from the original idea of pl/pgsql as an arm's-length extension of
      the core, but really we crossed that bridge a long time ago.
      
      The main advantage of doing this is that we can now use the core
      parser to generate FieldStore and/or SubscriptingRef nodes to handle
      assignments to pl/pgsql variables that are records or arrays.  That
      fixes a number of cases that had never been implemented in pl/pgsql
      assignment, such as nested records and array slicing, and it allows
      pl/pgsql assignment to support the datatype-specific subscripting
      behaviors introduced in commit c7aba7c1.
      
      There are cosmetic benefits too: when a syntax error occurs in a
      pl/pgsql expression, the error report no longer includes the confusing
      "SELECT" keyword that used to get prefixed to the expression text.
      Also, there seem to be some small speed gains.
      
      Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
      c9d52984
    • Tom Lane's avatar
      Add the ability for the core grammar to have more than one parse target. · 844fe9f1
      Tom Lane authored
      This patch essentially allows gram.y to implement a family of related
      syntax trees, rather than necessarily always parsing a list of SQL
      statements.  raw_parser() gains a new argument, enum RawParseMode,
      to say what to do.  As proof of concept, add a mode that just parses
      a TypeName without any other decoration, and use that to greatly
      simplify typeStringToTypeName().
      
      In addition, invent a new SPI entry point SPI_prepare_extended() to
      allow SPI users (particularly plpgsql) to get at this new functionality.
      In hopes of making this the last variant of SPI_prepare(), set up its
      additional arguments as a struct rather than direct arguments, and
      promise that future additions to the struct can default to zero.
      SPI_prepare_cursor() and SPI_prepare_params() can perhaps go away at
      some point.
      
      Discussion: https://postgr.es/m/4165684.1607707277@sss.pgh.pa.us
      844fe9f1
    • Michael Paquier's avatar
      Simplify some comments in xml.c · b49154b3
      Michael Paquier authored
      Author: Justin Pryzby
      Discussion: https://postgr.es/m/X/Ff7jfnvJUab013@paquier.xyz
      b49154b3
    • Amit Kapila's avatar
      Allow decoding at prepare time in ReorderBuffer. · a271a1b5
      Amit Kapila authored
      This patch allows PREPARE-time decoding of two-phase transactions (if the
      output plugin supports this capability), in which case the transactions
      are replayed at PREPARE and then committed later when COMMIT PREPARED
      arrives.
      
      Now that we decode the changes before the commit, the concurrent aborts
      may cause failures when the output plugin consults catalogs (both system
      and user-defined).
      
      We detect such failures with a special sqlerrcode
      ERRCODE_TRANSACTION_ROLLBACK introduced by commit 7259736a and stop
      decoding the remaining changes. Then we rollback the changes when rollback
      prepared is encountered.
      
      Author: Ajin Cherian and Amit Kapila based on previous work by Nikhil Sontakke and Stas Kelvich
      Reviewed-by: Amit Kapila, Peter Smith, Sawada Masahiko, Arseny Sher, and Dilip Kumar
      Tested-by: Takamichi Osumi
      Discussion:
      https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru
      https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com
      a271a1b5
  4. 02 Jan, 2021 1 commit
  5. 01 Jan, 2021 1 commit
  6. 31 Dec, 2020 2 commits
    • Peter Geoghegan's avatar
      Get heap page max offset with buffer lock held. · 32d6287d
      Peter Geoghegan authored
      On further reflection it seems better to call PageGetMaxOffsetNumber()
      after acquiring a buffer lock on the page.  This shouldn't really
      matter, but doing it this way is cleaner.
      
      Follow-up to commit 42288174.
      
      Backpatch: 12-, just like commit 42288174
      32d6287d
    • Peter Geoghegan's avatar
      Fix index deletion latestRemovedXid bug. · 42288174
      Peter Geoghegan authored
      The logic for determining the latest removed XID for the purposes of
      generating recovery conflicts in REDO routines was subtly broken.  It
      failed to follow links from HOT chains, and so failed to consider all
      relevant heap tuple headers in some cases.
      
      To fix, expand the loop that deals with LP_REDIRECT line pointers to
      also deal with HOT chains.  The new version of the loop is loosely based
      on a similar loop from heap_prune_chain().
      
      The impact of this bug is probably quite limited, since the horizon code
      necessarily deals with heap tuples that are pointed to by LP_DEAD-set
      index tuples.  The process of setting LP_DEAD index tuples (e.g. within
      the kill_prior_tuple mechanism) is highly correlated with opportunistic
      pruning of pointed-to heap tuples.  Plus the question of generating a
      recovery conflict usually comes up some time after index tuple LP_DEAD
      bits were initially set, unlike heap pruning, where a latestRemovedXid
      is generated at the point of the pruning operation (heap pruning has no
      deferred "would-be page split" style processing that produces conflicts
      lazily).
      
      Only backpatch to Postgres 12, the first version where this logic runs
      during original execution (following commit 558a9165).  The index
      latestRemovedXid mechanism has had the same bug since it first appeared
      over 10 years ago (in commit a760893d), but backpatching to all
      supported versions now seems like a bad idea on balance.  Running the
      new improved code during recovery seems risky, especially given the lack
      of complaints from the field.
      
      Author: Peter Geoghegan <pg@bowt.ie>
      Discussion: https://postgr.es/m/CAH2-Wz=Eib393+HHcERK_9MtgNS7Ew1HY=RDC_g6GL46zM5C6Q@mail.gmail.com
      Backpatch: 12-
      42288174
  7. 30 Dec, 2020 8 commits
    • Tom Lane's avatar
      Doc: spell out comparison behaviors for the date/time types. · 319f4d54
      Tom Lane authored
      The behavior of cross-type comparisons among date/time data types was
      not really explained anywhere.  You could probably infer it if you
      recognized the applicability of comments elsewhere about datatype
      conversions, but it seems worthy of explicit documentation.
      
      Per bug #16797 from Dana Burd.
      
      Discussion: https://postgr.es/m/16797-f264b0b980b53b8b@postgresql.org
      319f4d54
    • Tom Lane's avatar
      More fixups for pg_upgrade cross-version tests. · 09186672
      Tom Lane authored
      Commit 7ca37fb0 removed regress_putenv from the regress.so library,
      so reloading a SQL function dependent on that would not work.
      Fix similarly to 52202bb3.
      
      Per buildfarm.
      09186672
    • Alexander Korotkov's avatar
      Refactor multirange_in() · 16d531a3
      Alexander Korotkov authored
      This commit preserves the logic of multirange_in() but makes it more clear
      what's going on.  Also, this commit fixes the compiler warning spotted by the
      buildfarm.
      
      Reported-by: Tom Lane
      Discussion: https://postgr.es/m/2246043.1609290699%40sss.pgh.pa.us
      16d531a3
    • Tom Lane's avatar
      Use setenv() in preference to putenv(). · 7ca37fb0
      Tom Lane authored
      Since at least 2001 we've used putenv() and avoided setenv(), on the
      grounds that the latter was unportable and not in POSIX.  However,
      POSIX added it that same year, and by now the situation has reversed:
      setenv() is probably more portable than putenv(), since POSIX now
      treats the latter as not being a core function.  And setenv() has
      cleaner semantics too.  So, let's reverse that old policy.
      
      This commit adds a simple src/port/ implementation of setenv() for
      any stragglers (we have one in the buildfarm, but I'd not be surprised
      if that code is never used in the field).  More importantly, extend
      win32env.c to also support setenv().  Then, replace usages of putenv()
      with setenv(), and get rid of some ad-hoc implementations of setenv()
      wannabees.
      
      Also, adjust our src/port/ implementation of unsetenv() to follow the
      POSIX spec that it returns an error indicator, rather than returning
      void as per the ancient BSD convention.  I don't feel a need to make
      all the call sites check for errors, but the portability stub ought
      to match real-world practice.
      
      Discussion: https://postgr.es/m/2065122.1609212051@sss.pgh.pa.us
      7ca37fb0
    • Alexander Korotkov's avatar
      Fix selectivity estimation @> (anymultirange, anyrange) operator · 62097a4c
      Alexander Korotkov authored
      Attempt to get selectivity estimation for @> (anymultirange, anyrange) operator
      caused an error in buildfarm, because this operator was missed in switch()
      of calc_hist_selectivity().  Fix that and also make regression tests reliably
      check that selectivity estimation for (multi)ranges doesn't fall.  Previously,
      whether we test selectivity estimation for (multi)ranges depended on
      whether autovacuum managed to gather concurrently to the test.
      
      Reported-by: Michael Paquier
      Discussion: https://postgr.es/m/X%2BwmgjRItuvHNBeV%40paquier.xyz
      62097a4c
    • Tom Lane's avatar
      Fix up usage of krb_server_keyfile GUC parameter. · 860fe27e
      Tom Lane authored
      secure_open_gssapi() installed the krb_server_keyfile setting as
      KRB5_KTNAME unconditionally, so long as it's not empty.  However,
      pg_GSS_recvauth() only installed it if KRB5_KTNAME wasn't set already,
      leading to a troubling inconsistency: in theory, clients could see
      different sets of server principal names depending on whether they
      use GSSAPI encryption.  Always using krb_server_keyfile seems like
      the right thing, so make both places do that.  Also fix up
      secure_open_gssapi()'s lack of a check for setenv() failure ---
      it's unlikely, surely, but security-critical actions are no place
      to be sloppy.
      
      Also improve the associated documentation.
      
      This patch does nothing about secure_open_gssapi()'s use of setenv(),
      and indeed causes pg_GSS_recvauth() to use it too.  That's nominally
      against project portability rules, but since this code is only built
      with --with-gssapi, I do not feel a need to do something about this
      in the back branches.  A fix will be forthcoming for HEAD though.
      
      Back-patch to v12 where GSSAPI encryption was introduced.  The
      dubious behavior in pg_GSS_recvauth() goes back further, but it
      didn't have anything to be inconsistent with, so let it be.
      
      Discussion: https://postgr.es/m/2187460.1609263156@sss.pgh.pa.us
      860fe27e
    • Michael Paquier's avatar
      Sanitize IF NOT EXISTS in EXPLAIN for CTAS and matviews · e665769e
      Michael Paquier authored
      IF NOT EXISTS was ignored when specified in an EXPLAIN query for CREATE
      MATERIALIZED VIEW or CREATE TABLE AS.  Hence, if this clause was
      specified, the caller would get a failure if the relation already
      exists instead of a success with a NOTICE message.
      
      This commit makes the behavior of IF NOT EXISTS in EXPLAIN consistent
      with the non-EXPLAIN'd DDL queries, preventing a failure with IF NOT
      EXISTS if the relation to-be-created already exists.  The skip is done
      before the SELECT query used for the relation is planned or executed,
      and a "dummy" plan is generated instead depending on the format used by
      EXPLAIN.
      
      Author: Bharath Rupireddy
      Reviewed-by: Zhijie Hou, Michael Paquier
      Discussion: https://postgr.es/m/CALj2ACVa3oJ9O_wcGd+FtHWZds04dEKcakxphGz5POVgD4wC7Q@mail.gmail.com
      e665769e
    • Amit Kapila's avatar
      Extend the output plugin API to allow decoding of prepared xacts. · 0aa8a01d
      Amit Kapila authored
      This adds six methods to the output plugin API, adding support for
      streaming changes of two-phase transactions at prepare time.
      
      * begin_prepare
      * filter_prepare
      * prepare
      * commit_prepared
      * rollback_prepared
      * stream_prepare
      
      Most of this is a simple extension of the existing methods, with the
      semantic difference that the transaction is not yet committed and maybe
      aborted later.
      
      Until now two-phase transactions were translated into regular transactions
      on the subscriber, and the GID was not forwarded to it. None of the
      two-phase commands were communicated to the subscriber.
      
      This patch provides the infrastructure for logical decoding plugins to be
      informed of two-phase commands Like PREPARE TRANSACTION, COMMIT PREPARED
      and ROLLBACK PREPARED commands with the corresponding GID.
      
      This also extends the 'test_decoding' plugin, implementing these new
      methods.
      
      This commit simply adds these new APIs and the upcoming patch to "allow
      the decoding at prepare time in ReorderBuffer" will use these APIs.
      
      Author: Ajin Cherian and Amit Kapila based on previous work by Nikhil Sontakke and Stas Kelvich
      Reviewed-by: Amit Kapila, Peter Smith, Sawada Masahiko, and Dilip Kumar
      Discussion:
      https://postgr.es/m/02DA5F5E-CECE-4D9C-8B4B-418077E2C010@postgrespro.ru
      https://postgr.es/m/CAMGcDxeqEpWj3fTXwqhSwBdXd2RS9jzwWscO-XbeCfso6ts3+Q@mail.gmail.com
      0aa8a01d