1. 13 Aug, 2021 3 commits
    • Michael Meskes's avatar
      Fix connection handling for DEALLOCATE and DESCRIBE statements · a945f552
      Michael Meskes authored
      After binding a statement to a connection with DECLARE STATEMENT the connection
      was still not used for DEALLOCATE and DESCRIBE statements. This patch fixes
      that, adds a missing warning and cleans up the code.
      
      Author: Hayato Kuroda
      Reviewed-by: Kyotaro Horiguchi, Michael Paquier
      Discussion: https://postgr.es/m/TYAPR01MB5866BA57688DF2770E2F95C6F5069%40TYAPR01MB5866.jpnprd01.prod.outlook.com
      a945f552
    • Daniel Gustafsson's avatar
      Fix sslsni connparam boolean check · ffff00a3
      Daniel Gustafsson authored
      The check for sslsni only checked for existence of the parameter
      but not for the actual value of the param.  This meant that the
      SNI extension was always turned on.  Fix by inspecting the value
      of sslsni and only activate the SNI extension iff sslsni has been
      enabled.  Also update the docs to be more in line with how other
      boolean params are documented.
      
      Backpatch to 14 where sslsni was first implemented.
      
      Reviewed-by: Tom Lane
      Backpatch-through: 14, where sslni was added
      ffff00a3
    • David Rowley's avatar
      Fix incorrect hash table resizing code in simplehash.h · dc23c77d
      David Rowley authored
      This fixes a bug in simplehash.h which caused an incorrect size mask to be
      used when the hash table grew to SH_MAX_SIZE (2^32).  The code was
      incorrectly setting the size mask to 0 when the hash tables reached the
      maximum possible number of buckets.  This would result always trying to
      use the 0th bucket causing an  infinite loop of trying to grow the hash
      table due to there being too many collisions.
      
      Seemingly it's not that common for simplehash tables to ever grow this big
      as this bug dates back to v10 and nobody seems to have noticed it before.
      However, probably the most likely place that people would notice it would
      be doing a large in-memory Hash Aggregate with something close to at least
      2^31 groups.
      
      After this fix, the code now works correctly with up to within 98% of 2^32
      groups and will fail with the following error when trying to insert any
      more items into the hash table:
      
      ERROR:  hash table size exceeded
      
      However, the work_mem (or hash_mem_multiplier in newer versions) settings
      will generally cause Hash Aggregates to spill to disk long before reaching
      that many groups.  The minimal test case I did took a work_mem setting of
      over 192GB to hit the bug.
      
      simplehash hash tables are used in a few other places such as Bitmap Index
      Scans, however, again the size that the hash table can become there is
      also limited to work_mem and it would take a relation of around 16TB
      (2^31) pages and a very large work_mem setting to hit this.  With smaller
      work_mem values the table would become lossy and never grow large enough
      to hit the problem.
      
      Author: Yura Sokolov
      Reviewed-by: David Rowley, Ranier Vilela
      Discussion: https://postgr.es/m/b1f7f32737c3438136f64b26f4852b96@postgrespro.ru
      Backpatch-through: 10, where simplehash.h was added
      dc23c77d
  2. 12 Aug, 2021 3 commits
    • Thomas Munro's avatar
      Make EXEC_BACKEND more convenient on macOS. · 8201b605
      Thomas Munro authored
      It's hard to disable ASLR on current macOS releases, for testing with
      -DEXEC_BACKEND.  You could already set the environment variable
      PG_SHMEM_ADDR to something not likely to collide with mappings created
      earlier in process startup.  Let's also provide a default value that
      works on current releases and architectures, for developer convenience.
      
      As noted in the pre-existing comment, this is a horrible hack, but
      -DEXEC_BACKEND is only used by Unix-based PostgreSQL developers for
      testing some otherwise Windows-only code paths, so it seems excusable.
      
      Back-patch to all supported branches.
      Reviewed-by: default avatarTom Lane <tgl@sss.pgh.pa.us>
      Discussion: https://postgr.es/m/20210806032944.m4tz7j2w47mant26%40alap3.anarazel.de
      8201b605
    • Tomas Vondra's avatar
      Use appropriate tuple descriptor in FDW batching · 886531f3
      Tomas Vondra authored
      The FDW batching code was using the same tuple descriptor both for all
      slots (regular and plan slots), but that's incorrect - the subplan may
      use a different descriptor. Currently this is benign, because batching
      is used only for INSERTs, and in that case the descriptors always match.
      But that would change if we allow batching UPDATEs.
      
      Fix by copying the appropriate tuple descriptor. Backpatch to 14, where
      the FDW batching was implemented.
      
      Author: Amit Langote
      Backpatch-through: 14, where FDW batching was added
      Discussion: https://postgr.es/m/CA%2BHiwqEWd5B0-e-RvixGGUrNvGkjH2s4m95%3DJcwUnyV%3Df0rAKQ%40mail.gmail.com
      886531f3
    • Heikki Linnakangas's avatar
      Fix segfault during EvalPlanQual with mix of local and foreign partitions. · 6458ed18
      Heikki Linnakangas authored
      It's not sensible to re-evaluate a direct-modify Foreign Update or Delete
      during EvalPlanQual. However, ExecInitForeignScan() can still get called
      if a table mixes local and foreign partitions. EvalPlanQualStart() left
      the es_result_relations array uninitialized in the child EPQ EState, but
      ExecInitForeignScan() still expected to find it. That caused a segfault.
      
      Fix by skipping the es_result_relations lookup during EvalPlanQual
      processing. To make things a bit more robust, also skip the
      BeginDirectModify calls, and add a runtime check that ExecForeignScan()
      is not called on direct-modify foreign scans during EvalPlanQual
      processing.
      
      This is new in v14, commit 1375422c. Before that, EvalPlanQualStart()
      copied the whole ResultRelInfo array to the EPQ EState. Backpatch to v14.
      
      Report and diagnosis by Andrey Lepikhov.
      
      Discussion: https://www.postgresql.org/message-id/cb2b808d-cbaa-4772-76ee-c8809bafcf3d%40postgrespro.ru
      6458ed18
  3. 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
  4. 09 Aug, 2021 5 commits
  5. 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
  6. 07 Aug, 2021 4 commits
  7. 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
  8. 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
  9. 04 Aug, 2021 1 commit
  10. 03 Aug, 2021 6 commits
  11. 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
  12. 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
  13. 30 Jul, 2021 2 commits