1. 06 Jan, 2021 3 commits
  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 12 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
    • Noah Misch's avatar
      In pg_upgrade cross-version test, handle postfix operators. · fa744697
      Noah Misch authored
      Commit 1ed6b895 eliminated support for
      them, so drop them from regression databases before upgrading.  This is
      necessary but not sufficient for testing v13 -> v14 upgrades.
      
      Discussion: https://postgr.es/m/449144.1600439950@sss.pgh.pa.us
      fa744697
    • Noah Misch's avatar
      In pg_upgrade cross-version test, handle lack of oldstyle_length(). · 52202bb3
      Noah Misch authored
      This suffices for testing v12 -> v13; some other version pairs need more
      changes.  Back-patch to v10, which removed the function.
      52202bb3
    • Michael Paquier's avatar
      Remove references to libpq_srcdir in adminpack and old_snapshot · 107a2d42
      Michael Paquier authored
      Those two modules included references to libpq's source path, without
      using anything from libpq.  Some copy-pastos done when each module was
      created are likely at the origin of those useless references (aecf5ee2
      for old_snapshot, fe59e566 for adminpack).
      
      Reviewed-by: Tom Lane, David Rowley
      Discussion: https://postgr.es/m/X+LQpfLyk7jgzUki@paquier.xyz
      107a2d42
    • Tom Lane's avatar
      Doc: fix up PDF build warnings from over-width table columns. · f20dc2c8
      Tom Lane authored
      Addition of multirange info to tables 8.27 and 65.1 made them start
      throwing "exceed the available area" warnings in PDF docs builds.
      
      For 8.27, twiddling the existing column width hints was enough to
      fix this.  For 65.1, I twiddled the widths a little, but to really
      fix it I had to insert a space after each comma in the table, to
      allow a line break to occur there.  (This seemed easier to read
      and maintain than the alternative of inserting &zwsp; entities.)
      
      Per buildfarm.
      f20dc2c8