1. 10 Aug, 2021 1 commit
    • Tom Lane's avatar
      Fix failure of btree_gin indexscans with "char" type and </<= operators. · a4957b5a
      Tom Lane authored
      As a result of confusion about whether the "char" type is signed or
      unsigned, scans for index searches like "col < 'x'" or "col <= 'x'"
      would start at the middle of the index not the left end, thus missing
      many or all of the entries they should find.  Fortunately, this
      is not a symptom of index corruption.  It's only the search logic
      that is broken, and we can fix it without unpleasant side-effects.
      
      Per report from Jason Kim.  This has been wrong since btree_gin's
      beginning, so back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/20210810001649.htnltbh7c63re42p@jasonk.me
      a4957b5a
  2. 09 Aug, 2021 5 commits
  3. 08 Aug, 2021 4 commits
    • Tom Lane's avatar
      Doc: remove bogus <indexterm> items. · c905e64d
      Tom Lane authored
      Copy-and-pasteo in 665c5855e, evidently.  The 9.6 docs toolchain
      whined about duplicate index entries, though our modern toolchain
      doesn't.  In any case, these GUCs surely are not about the
      default settings of these values.
      c905e64d
    • Tom Lane's avatar
      Rethink regexp engine's backref-related compilation state. · 5227d998
      Tom Lane authored
      I had committer's remorse almost immediately after pushing cb76fbd7e,
      upon finding that removing capturing subexpressions' subREs from the
      data structure broke my proposed patch for REG_NOSUB optimization.
      Revert that data structure change.  Instead, address the concern
      about not changing capturing subREs' endpoints by not changing the
      endpoints.  We don't need to, because the point of that bit was just
      to ensure that the atom has endpoints distinct from the outer state
      pair that we're stringing the branch between.  We already made
      suitable states in the parenthesized-subexpression case, so the
      additional ones were just useless overhead.  This seems more
      understandable than Spencer's original coding, and it ought to be
      a shade faster too by saving a few state creations and arc changes.
      (I actually see a couple percent improvement on Jacobson's web
      corpus, though that's barely above the noise floor so I wouldn't
      put much stock in that result.)
      
      Also, fix the logic added by ea1268f6 to ensure that the subRE
      recorded in v->subs[subno] is exactly the one with capno == subno.
      Spencer's original coding recorded the child subRE of the capture
      node, which is okay so far as having the right endpoint states is
      concerned, but as of cb76fbd7e the capturing subRE itself always
      has those endpoints too.  I think the inconsistency is confusing
      for the REG_NOSUB optimization.
      
      As before, backpatch to v14.
      
      Discussion: https://postgr.es/m/0203588E-E609-43AF-9F4F-902854231EE7@enterprisedb.com
      5227d998
    • Tom Lane's avatar
      Make regexp engine's backref-related compilation state more bulletproof. · 5e6ad63c
      Tom Lane authored
      Up to now, we remembered the definition of a capturing parenthesis
      subexpression by storing a pointer to the associated subRE node.
      That was okay before, because that subRE didn't get modified anymore
      while parsing the rest of the regexp.  However, in the wake of
      commit ea1268f6, that's no longer true: the outer invocation of
      parseqatom() feels free to scribble on that subRE.  This seems to
      work anyway, because the states we jam into the child atom in the
      "prepare a general-purpose state skeleton" stanza aren't really
      semantically different from the original endpoints of the child atom.
      But that would be mighty easy to break, and it's definitely not how
      things worked before.
      
      Between this and the issue fixed in the prior commit, it seems best
      to get rid of this dependence on subRE nodes entirely.  We don't need
      the whole child subRE for future backrefs, only its starting and ending
      NFA states; so let's just store pointers to those.
      
      Also, in the corner case where we make an extra subRE to handle
      immediately-nested capturing parentheses, it seems like it'd be smart
      to have the extra subRE have the same begin/end states as the original
      child subRE does (s/s2 not lp/rp).  I think that linking it from lp to
      rp might actually be semantically wrong, though since Spencer's original
      code did it that way, I'm not totally certain.  Using s/s2 is certainly
      not wrong, in any case.
      
      Per report from Mark Dilger.  Back-patch to v14 where the problematic
      patches came in.
      
      Discussion: https://postgr.es/m/0203588E-E609-43AF-9F4F-902854231EE7@enterprisedb.com
      5e6ad63c
    • Tom Lane's avatar
      Fix use-after-free issue in regexp engine. · f42ea835
      Tom Lane authored
      Commit cebc1d34 taught parseqatom() to optimize cases where a branch
      contains only one, "messy", atom by getting rid of excess subRE nodes.
      The way we really should do that is to keep the subRE built for the
      "messy" child atom; but to avoid changing parseqatom's nominal API,
      I made it delete that node after copying its fields to the outer subRE
      made by parsebranch().  It seems that that actually worked at the time;
      but it became dangerous after ea1268f6, because that later commit
      allowed the lower invocation of parse() to return a subRE that was also
      pointed to by some v->subs[] entry.  This meant we could wind up with a
      dangling pointer in v->subs[], allowing a later backref to misbehave,
      but only if that subRE struct had been reused in between.  So the damage
      seems confined to cases like '((...))...(...\2'.
      
      To fix, do what I should have done before and modify parseqatom's API
      to make it possible for it to remove the caller's subRE instead of the
      callee's.  That's safer because we know that subRE isn't complete yet,
      so noplace else will have a pointer to it.
      
      Per report from Mark Dilger.  Back-patch to v14 where the problematic
      patches came in.
      
      Discussion: https://postgr.es/m/0203588E-E609-43AF-9F4F-902854231EE7@enterprisedb.com
      f42ea835
  4. 07 Aug, 2021 4 commits
  5. 06 Aug, 2021 4 commits
    • Tom Lane's avatar
      Don't elide casting to typmod -1. · e5f6493e
      Tom Lane authored
      Casting a value that's already of a type with a specific typmod
      to an unspecified typmod doesn't do anything so far as run-time
      behavior is concerned.  However, it really ought to change the
      exposed type of the expression to match.  Up to now,
      coerce_type_typmod hasn't bothered with that, which creates gotchas
      in contexts such as recursive unions.  If for example one side of
      the union is numeric(18,3), but it needs to be plain numeric to
      match the other side, there's no direct way to express that.
      
      This is easy enough to fix, by inserting a RelabelType to update the
      exposed type of the expression.  However, it's a bit nervous-making
      to change this behavior, because it's stood for a really long time.
      (I strongly suspect that it's like this in part because the logic
      pre-dates the introduction of RelabelType in 7.0.  The commit log
      message for 57b30e8e is interesting reading here.)  As a compromise,
      we'll sneak the change into 14beta3, and consider back-patching to
      stable branches if no complaints emerge in the next three months.
      
      Discussion: https://postgr.es/m/CABNQVagu3bZGqiTjb31a8D5Od3fUMs7Oh3gmZMQZVHZ=uWWWfQ@mail.gmail.com
      e5f6493e
    • Dean Rasheed's avatar
      Adjust the integer overflow tests in the numeric code. · 03255657
      Dean Rasheed authored
      Formerly, the numeric code tested whether an integer value of a larger
      type would fit in a smaller type by casting it to the smaller type and
      then testing if the reverse conversion produced the original value.
      That's perfectly fine, except that it caused a test failure on
      buildfarm animal castoroides, most likely due to a compiler bug.
      
      Instead, do these tests by comparing against PG_INT16/32_MIN/MAX. That
      matches existing code in other places, such as int84(), which is more
      widely tested, and so is less likely to go wrong.
      
      While at it, add regression tests covering the numeric-to-int8/4/2
      conversions, and adjust the recently added tests to the style of
      434ddfb79a (on the v11 branch) to make failures easier to diagnose.
      
      Per buildfarm via Tom Lane, reviewed by Tom Lane.
      
      Discussion: https://postgr.es/m/2394813.1628179479%40sss.pgh.pa.us
      03255657
    • Peter Eisentraut's avatar
      Add missing message punctuation · c3a135b4
      Peter Eisentraut authored
      c3a135b4
    • Peter Eisentraut's avatar
      Fix wording · acd6b6e6
      Peter Eisentraut authored
      acd6b6e6
  6. 05 Aug, 2021 4 commits
    • Tom Lane's avatar
      Doc: remove commit 2945a488 from v14 release notes. · 64b7a835
      Tom Lane authored
      Now that this has been back-patched, it's no longer a new feature
      for v14.
      64b7a835
    • Etsuro Fujita's avatar
      postgres_fdw: Fix issues with generated columns in foreign tables. · 588d3f59
      Etsuro Fujita authored
      postgres_fdw imported generated columns from the remote tables as plain
      columns, and caused failures like "ERROR: cannot insert a non-DEFAULT
      value into column "foo"" when inserting into the foreign tables, as it
      tried to insert values into the generated columns.  To fix, we do the
      following under the assumption that generated columns in a postgres_fdw
      foreign table are defined so that they represent generated columns in
      the underlying remote table:
      
      * Send DEFAULT for the generated columns to the foreign server on insert
        or update, not generated column values computed on the local server.
      * Add to postgresImportForeignSchema() an option "import_generated" to
        include column generated expressions in the definitions of foreign
        tables imported from a foreign server.  The option is true by default.
      
      The assumption seems reasonable, because that would make a query of the
      postgres_fdw foreign table return values for the generated columns that
      are consistent with the generated expression.
      
      While here, fix another issue in postgresImportForeignSchema(): it tried
      to include column generated expressions as column default expressions in
      the foreign table definitions when the import_default option was enabled.
      
      Per bug #16631 from Daniel Cherniy.  Back-patch to v12 where generated
      columns were added.
      
      Discussion: https://postgr.es/m/16631-e929fe9db0ffc7cf%40postgresql.org
      588d3f59
    • Dean Rasheed's avatar
      Fix division-by-zero error in to_char() with 'EEEE' format. · ecbdbdfd
      Dean Rasheed authored
      This fixes a long-standing bug when using to_char() to format a
      numeric value in scientific notation -- if the value's exponent is
      less than -NUMERIC_MAX_DISPLAY_SCALE-1 (-1001), it produced a
      division-by-zero error.
      
      The reason for this error was that get_str_from_var_sci() divides its
      input by 10^exp, which it produced using power_var_int(). However, the
      underflow test in power_var_int() causes it to return zero if the
      result scale is too small. That's not a problem for power_var_int()'s
      only other caller, power_var(), since that limits the rscale to 1000,
      but in get_str_from_var_sci() the exponent can be much smaller,
      requiring a much larger rscale. Fix by introducing a new function to
      compute 10^exp directly, with no rscale limit. This also allows 10^exp
      to be computed more efficiently, without any numeric multiplication,
      division or rounding.
      
      Discussion: https://postgr.es/m/CAEZATCWhojfH4whaqgUKBe8D5jNHB8ytzemL-PnRx+KCTyMXmg@mail.gmail.com
      ecbdbdfd
    • Andres Freund's avatar
      pgbench: When using pipelining only do PQconsumeInput() when necessary. · fa604e0d
      Andres Freund authored
      Up to now we did a PQconsumeInput() for each pipelined query, asking the OS
      for more input - which it often won't have, as all results might already have
      been sent. That turns out to have a noticeable performance impact.
      
      Alvaro Herrera reviewed the idea to add the PQisBusy() check, but not this
      concrete patch.
      
      Author: Andres Freund <andres@anarazel.de>
      Discussion: https://postgr.es/m/20210720180039.23rivhdft3l4mayn@alap3.anarazel.de
      Backpatch: 14, where libpq/pgbench pipelining was introduced.
      fa604e0d
  7. 04 Aug, 2021 1 commit
  8. 03 Aug, 2021 6 commits
  9. 02 Aug, 2021 1 commit
    • Etsuro Fujita's avatar
      Fix oversight in commit 1ec7fca8592178281cd5cdada0f27a340fb813fc. · fb234086
      Etsuro Fujita authored
      I failed to account for the possibility that when
      ExecAppendAsyncEventWait() notifies multiple async-capable nodes using
      postgres_fdw, a preceding node might invoke process_pending_request() to
      process a pending asynchronous request made by a succeeding node.  In
      that case the succeeding node should produce a tuple to return to the
      parent Append node from tuples fetched by process_pending_request() when
      notified.  Repair.
      
      Per buildfarm via Michael Paquier.  Back-patch to v14, like the previous
      commit.
      
      Thanks to Tom Lane for testing.
      
      Discussion: https://postgr.es/m/YQP0UPT8KmPiHTMs%40paquier.xyz
      fb234086
  10. 31 Jul, 2021 2 commits
    • Tom Lane's avatar
      Use elog, not Assert, to report failure to provide an outer snapshot. · ec410c98
      Tom Lane authored
      As of commit 84f5c290, executing SQL commands (via SPI or otherwise)
      requires having either an active Portal, or a caller-established
      active snapshot.  We were simply Assert'ing that that's the case.
      But we've now had a couple different reports of people testing
      extensions that didn't meet this requirement, and were confused by
      the resulting crash.  Let's convert the Assert to a test-and-elog,
      in hopes of making the issue clearer for extension authors.
      
      Per gripes from Liu Huailing and RekGRpth.  Back-patch to v11,
      like the prior commit.
      
      Discussion: https://postgr.es/m/OSZPR01MB6215671E3C5956A034A080DFBEEC9@OSZPR01MB6215.jpnprd01.prod.outlook.com
      Discussion: https://postgr.es/m/17035-14607d308ac8643c@postgresql.org
      ec410c98
    • Dean Rasheed's avatar
      Fix corner-case errors and loss of precision in numeric_power(). · 0d6b8749
      Dean Rasheed authored
      This fixes a couple of related problems that arise when raising
      numbers to very large powers.
      
      Firstly, when raising a negative number to a very large integer power,
      the result should be well-defined, but the previous code would only
      cope if the exponent was small enough to go through power_var_int().
      Otherwise it would throw an internal error, attempting to take the
      logarithm of a negative number. Fix this by adding suitable handling
      to the general case in power_var() to cope with negative bases,
      checking for integer powers there.
      
      Next, when raising a (positive or negative) number whose absolute
      value is slightly less than 1 to a very large power, the result should
      approach zero as the power is increased. However, in some cases, for
      sufficiently large powers, this would lose all precision and return 1
      instead of 0. This was due to the way that the local_rscale was being
      calculated for the final full-precision calculation:
      
        local_rscale = rscale + (int) val - ln_dweight + 8
      
      The first two terms on the right hand side are meant to give the
      number of significant digits required in the result ("val" being the
      estimated result weight). However, this failed to account for the fact
      that rscale is clipped to a maximum of NUMERIC_MAX_DISPLAY_SCALE
      (1000), and the result weight might be less then -1000, causing their
      sum to be negative, leading to a loss of precision. Fix this by
      forcing the number of significant digits calculated to be nonnegative.
      It's OK for it to be zero (when the result weight is less than -1000),
      since the local_rscale value then includes a few extra digits to
      ensure an accurate result.
      
      Finally, add additional underflow checks to exp_var() and power_var(),
      so that they consistently return zero for cases like this where the
      result is indistinguishable from zero. Some paths through this code
      already returned zero in such cases, but others were throwing overflow
      errors.
      
      Dean Rasheed, reviewed by Yugo Nagata.
      
      Discussion: http://postgr.es/m/CAEZATCW6Dvq7+3wN3tt5jLj-FyOcUgT5xNoOqce5=6Su0bCR0w@mail.gmail.com
      0d6b8749
  11. 30 Jul, 2021 4 commits
    • John Naylor's avatar
      Fix range check in ECPG numeric to int conversion · f051b87a
      John Naylor authored
      The previous coding guarded against -INT_MAX instead of INT_MIN,
      leading to -2147483648 being rejected as out of range.
      
      Per bug #17128 from Kevin Sweet
      
      Discussion: https://www.postgresql.org/message-id/flat/17128-55a8a879727a3e3a%40postgresql.org
      Reviewed-by: Tom Lane
      Backpatch to all supported branches
      f051b87a
    • Heikki Linnakangas's avatar
      Update obsolete comment that still referred to CheckpointLock · 99da905d
      Heikki Linnakangas authored
      CheckpointLock was removed in commit d18e7566, and commit ce197e91d0
      updated a leftover comment in CreateCheckPoint, but there was another
      copy of it in CreateRestartPoint still.
      99da905d
    • Etsuro Fujita's avatar
      postgres_fdw: Fix handling of pending asynchronous requests. · 1cf7fb37
      Etsuro Fujita authored
      A pending asynchronous request is handled by process_pending_request(),
      which previously not only processed an in-progress remote query but
      performed ExecForeignScan() to produce a tuple to return to the local
      server asynchronously from the result of the remote query.  But that led
      to a server crash when executing a query or led to an "InstrStartNode
      called twice in a row" or "InstrEndLoop called on running node" failure
      when doing EXPLAIN ANALYZE of it, in cases where the plan tree for it
      contained multiple async-capable nodes accessing the same
      initplan/subplan that contained multiple async-capable nodes scanning
      the same foreign tables as for the parent async-capable nodes, as
      reported by Andrey Lepikhov.  The reason is that the second step in
      process_pending_request() invoked when executing the initplan/subplan
      for one of the parent async-capable nodes caused recursive execution of
      the initplan/subplan for another of the parent async-capable nodes.
      
      To fix, split process_pending_request() into the two steps and postpone
      the second step until ForeignAsyncConfigureWait() is called for each of
      the pending asynchronous requests.  Also, in ExecAppendAsyncEventWait()
      we assumed that FDWs would register at least one wait event in a
      WaitEventSet created there when they were called from
      ForeignAsyncConfigureWait() in that function, but allow FDWs to register
      zero wait events in the WaitEventSet; modify ExecAppendAsyncEventWait()
      to just return in that case.
      
      Oversight in commit 27e1f145.  Back-patch to v14 where that commit went
      in.
      
      Andrey Lepikhov and Etsuro Fujita
      
      Discussion: https://postgr.es/m/fe5eaa19-1704-e4a4-76ee-3b9d37ade399@postgrespro.ru
      1cf7fb37
    • Amit Kapila's avatar
      Remove unused argument in apply_handle_commit_internal(). · f4b939f1
      Amit Kapila authored
      Oversight in commit 0926e96c.
      
      Author: Masahiko Sawada
      Reviewed-By: Amit Kapila
      Backpatch-through: 14, where it was introduced
      Discussion: https://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
      f4b939f1
  12. 29 Jul, 2021 4 commits