1. 22 Jun, 2022 1 commit
    • Tom Lane's avatar
      Fix SPI's handling of errors during transaction commit. · 60465188
      Tom Lane authored
      SPI_commit previously left it up to the caller to recover from any error
      occurring during commit.  Since that's complicated and requires use of
      low-level xact.c facilities, it's not too surprising that no caller got
      it right.  Let's move the responsibility for cleanup into spi.c.  Doing
      that requires redefining SPI_commit as starting a new transaction, so
      that it becomes equivalent to SPI_commit_and_chain except that you get
      default transaction characteristics instead of preserving the prior
      transaction's characteristics.  We can make this pretty transparent
      API-wise by redefining SPI_start_transaction() as a no-op.  Callers
      that expect to do something in between might be surprised, but
      available evidence is that no callers do so.
      
      Having made that API redefinition, we can fix this mess by having
      SPI_commit[_and_chain] trap errors and start a new, clean transaction
      before re-throwing the error.  Likewise for SPI_rollback[_and_chain].
      Some cleanup is also needed in AtEOXact_SPI, which was nowhere near
      smart enough to deal with SPI contexts nested inside a committing
      context.
      
      While plperl and pltcl need no changes beyond removing their now-useless
      SPI_start_transaction() calls, plpython needs some more work because it
      hadn't gotten the memo about catching commit/rollback errors in the
      first place.  Such an error resulted in longjmp'ing out of the Python
      interpreter, which leaks Python stack entries at present and is reported
      to crash Python 3.11 altogether.  Add the missing logic to catch such
      errors and convert them into Python exceptions.
      
      This is a back-patch of commit 2e517818f.  That's now aged long enough
      to reduce the concerns about whether it will break something, and we
      do need to ensure that supported branches will work with Python 3.11.
      
      Peter Eisentraut and Tom Lane
      
      Discussion: https://postgr.es/m/3375ffd8-d71c-2565-e348-a597d6e739e3@enterprisedb.com
      Discussion: https://postgr.es/m/17416-ed8fe5d7213d6c25@postgresql.org
      60465188
  2. 21 Jun, 2022 2 commits
    • Amit Kapila's avatar
      Fix stale values in partition map entries on subscribers. · f0022a77
      Amit Kapila authored
      We build the partition map entries on subscribers while applying the
      changes for update/delete on partitions. The component relation in each
      entry is closed after its use so we need to update it on successive use of
      cache entries.
      
      This problem was there since the original commit f1ac27bf that
      introduced this code but we didn't notice it till the recent commit
      26b3455afa started to use the component relation of partition map cache
      entry.
      
      Reported-by: Tom Lane, as per buildfarm
      Author: Amit Langote, Hou Zhijie
      Reviewed-by: Amit Kapila, Shi Yu
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
      f0022a77
    • Amit Kapila's avatar
      Fix partition table's REPLICA IDENTITY checking on the subscriber. · 52d5ea9a
      Amit Kapila authored
      In logical replication, we will check if the target table on the
      subscriber is updatable by comparing the replica identity of the table on
      the publisher with the table on the subscriber. When the target table is a
      partitioned table, we only check its replica identity but not for the
      partition tables. This leads to assertion failure while applying changes
      for update/delete as we expect those to succeed only when the
      corresponding partition table has a primary key or has a replica
      identity defined.
      
      Fix it by checking the replica identity of the partition table while
      applying changes.
      
      Reported-by: Shi Yu
      Author: Shi Yu, Hou Zhijie
      Reviewed-by: Amit Langote, Amit Kapila
      Backpatch-through: 13, where it was introduced
      Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
      52d5ea9a
  3. 16 Jun, 2022 1 commit
  4. 15 Jun, 2022 1 commit
  5. 14 Jun, 2022 2 commits
    • Tom Lane's avatar
      Avoid ecpglib core dump with out-of-order operations. · 7bc21ed8
      Tom Lane authored
      If an application executed operations like EXEC SQL PREPARE
      without having first established a database connection, it could
      get a core dump instead of the expected clean failure.  This
      occurred because we did "pthread_getspecific(actual_connection_key)"
      without ever having initialized the TSD key actual_connection_key.
      The results of that are probably platform-specific, but at least
      on Linux it often leads to a crash.
      
      To fix, add calls to ecpg_pthreads_init() in the code paths that
      might use actual_connection_key uninitialized.  It's harmless
      (and hopefully inexpensive) to do that more than once.
      
      Per bug #17514 from Okano Naoki.  The problem's ancient, so
      back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/17514-edd4fad547c5692c@postgresql.org
      7bc21ed8
    • Tom Lane's avatar
      Doc: clarify the default collation behavior of domains. · be35a645
      Tom Lane authored
      The previous wording was "the underlying data type's default collation
      is used", which is wrong or at least misleading.  The domain inherits
      the base type's collation behavior, which if "default" actually can
      mean that we use some non-default collation obtained from elsewhere.
      
      Per complaint from Jian He.
      
      Discussion: https://postgr.es/m/CACJufxHMR8_4WooDPjjvEdaxB2hQ5a49qthci8fpKP0MKemVRQ@mail.gmail.com
      be35a645
  6. 13 Jun, 2022 2 commits
  7. 10 Jun, 2022 4 commits
  8. 08 Jun, 2022 3 commits
    • Tom Lane's avatar
      Doc: copy-edit "jsonb Indexing" section. · 0ccef410
      Tom Lane authored
      The patch introducing jsonpath dropped a para about that between
      two related examples, and didn't bother updating the introductory
      sentences that it falsified.  The grammar was pretty shaky as well.
      0ccef410
    • Peter Eisentraut's avatar
      Fix whitespace · 804a5079
      Peter Eisentraut authored
      804a5079
    • David Rowley's avatar
      Harden Memoization code against broken data types · cbcea3b9
      David Rowley authored
      Bug #17512 highlighted that a suitably broken data type could cause the
      backend to crash if either the hash function or equality function were in
      someway non-deterministic based on their input values.  Such a data type
      could cause a crash of the backend due to some code which assumes that
      we'll always find a hash table entry corresponding to an item in the
      Memoize LRU list.
      
      Here we remove the assumption that we'll always find the entry
      corresponding to the given LRU list item and add run-time checks to verify
      we have found the given item in the cache.
      
      This is not a fix for bug #17512, but it will turn the crash reported by
      that bug report into an internal ERROR.
      
      Reported-by: Ales Zeleny
      Reviewed-by: Tom Lane
      Discussion: https://postgr.es/m/CAApHDvpxFSTwvoYWT7kmFVSZ9zLAeHb=S9vrz=RExMgSkQNWqw@mail.gmail.com
      Backpatch-through: 14, where Memoize was added.
      cbcea3b9
  9. 07 Jun, 2022 1 commit
    • Tom Lane's avatar
      Fix off-by-one loop termination condition in pg_stat_get_subscription(). · 5c3b5f7d
      Tom Lane authored
      pg_stat_get_subscription scanned one more LogicalRepWorker array entry
      than is really allocated.  In the worst case this could lead to SIGSEGV,
      if the LogicalRepCtx data structure is near the end of shared memory.
      That seems quite unlikely though (thanks to the ordering of calls in
      CreateSharedMemoryAndSemaphores) and we've heard no field reports of it.
      A more likely misbehavior is one row of garbage data in the function's
      result, but even that is not real likely because of the check that the
      pid field matches some live backend.
      
      Report and fix by Kuntal Ghosh.  This bug is old, so back-patch
      to all supported branches.
      
      Discussion: https://postgr.es/m/CAGz5QCJykEDzW6jQK6Yz7Qh_PMtD=95de_7QoocbVR2Qy8hWZA@mail.gmail.com
      5c3b5f7d
  10. 06 Jun, 2022 3 commits
    • Tom Lane's avatar
      Don't fail on libpq-generated error reports in pg_amcheck. · 32a85ee4
      Tom Lane authored
      An error PGresult generated by libpq itself, such as a report of
      connection loss, won't have broken-down error fields.
      should_processing_continue() blithely assumed that
      PG_DIAG_SEVERITY_NONLOCALIZED would always be present, and would
      dump core if it wasn't.
      
      Per grepping to see if 6d157e7cb's mistake was repeated elsewhere.
      32a85ee4
    • Tom Lane's avatar
      Don't fail on libpq-generated error reports in ecpg_raise_backend(). · a5dbca46
      Tom Lane authored
      An error PGresult generated by libpq itself, such as a report of
      connection loss, won't have broken-down error fields.
      ecpg_raise_backend() blithely assumed that PG_DIAG_MESSAGE_PRIMARY
      would always be present, and would end up passing a NULL string
      pointer to snprintf when it isn't.  That would typically crash
      before 3779ac62d, and it would fail to provide a useful error report
      in any case.  Best practice is to substitute PQerrorMessage(conn)
      in such cases, so do that.
      
      Per bug #17421 from Masayuki Hirose.  Back-patch to all supported
      branches.
      
      Discussion: https://postgr.es/m/17421-790ff887e3188874@postgresql.org
      a5dbca46
    • Michael Paquier's avatar
      Fix psql's single transaction mode on client-side errors with -c/-f switches · a04ccf6d
      Michael Paquier authored
      psql --single-transaction is able to handle multiple -c and -f switches
      in a single transaction since d5563d7d, but this had the surprising
      behavior of forcing a transaction COMMIT even if psql failed with an
      error in the client (for example incorrect path given to \copy), which
      would generate an error, but still commit any changes that were already
      applied in the backend.  This commit makes the behavior more consistent,
      by enforcing a transaction ROLLBACK if any commands fail, both
      client-side and backend-side, so as no changes are applied if one error
      happens in any of them.
      
      Some tests are added on HEAD to provide some coverage about all that.
      Backend-side errors are unreliable as IPC::Run can complain on SIGPIPE
      if psql quits before reading a query result, but that should work
      properly in the case where any errors come from psql itself, which is
      what the original report is about.
      
      Reported-by: Christoph Berg
      Author: Kyotaro Horiguchi, Michael Paquier
      Discussion: https://postgr.es/m/17504-76b68018e130415e@postgresql.org
      Backpatch-through: 10
      a04ccf6d
  11. 03 Jun, 2022 2 commits
  12. 02 Jun, 2022 1 commit
  13. 01 Jun, 2022 4 commits
  14. 31 May, 2022 4 commits
  15. 30 May, 2022 1 commit
  16. 29 May, 2022 2 commits
  17. 28 May, 2022 1 commit
  18. 26 May, 2022 2 commits
    • Tom Lane's avatar
      Remove misguided SSL key file ownership check in libpq. · b4be4a08
      Tom Lane authored
      Commits a59c79564 et al. tried to sync libpq's SSL key file
      permissions checks with what we've used for years in the backend.
      We did not intend to create any new failure cases, but it turns out
      we did: restricting the key file's ownership breaks cases where the
      client is allowed to read a key file despite not having the identical
      UID.  In particular a client running as root used to be able to read
      someone else's key file; and having seen that I suspect that there are
      other, less-dubious use cases that this restriction breaks on some
      platforms.
      
      We don't really need an ownership check, since if we can read the key
      file despite its having restricted permissions, it must have the right
      ownership --- under normal conditions anyway, and the point of this
      patch is that any additional corner cases where that works should be
      deemed allowable, as they have been historically.  Hence, just drop
      the ownership check, and rearrange the permissions check to get rid
      of its faulty assumption that geteuid() can't be zero.  (Note that the
      comparable backend-side code doesn't have to cater for geteuid() == 0,
      since the server rejects that very early on.)
      
      This does have the end result that the permissions safety check used
      for a root user's private key file is weaker than that used for
      anyone else's.  While odd, root really ought to know what she's doing
      with file permissions, so I think this is acceptable.
      
      Per report from Yogendra Suralkar.  Like the previous patch,
      back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/MW3PR15MB3931DF96896DC36D21AFD47CA3D39@MW3PR15MB3931.namprd15.prod.outlook.com
      b4be4a08
    • Robert Haas's avatar
      In CREATE FOREIGN TABLE syntax synopsis, fix partitioning stuff. · a5fc06bf
      Robert Haas authored
      Foreign tables can be partitioned, but previous documentation commits
      left the syntax synopsis both incomplete and incorrect.
      
      Justin Pryzby and Amit Langote
      
      Discussion: http://postgr.es/m/20220521130922.GX19626@telsasoft.com
      a5fc06bf
  19. 21 May, 2022 2 commits
    • Tom Lane's avatar
      Show 'AS "?column?"' explicitly when it's important. · 6f7eec11
      Tom Lane authored
      ruleutils.c was coded to suppress the AS label for a SELECT output
      expression if the column name is "?column?", which is the parser's
      fallback if it can't think of something better.  This is fine, and
      avoids ugly clutter, so long as (1) nothing further up in the parse
      tree relies on that column name or (2) the same fallback would be
      assigned when the rule or view definition is reloaded.  Unfortunately
      (2) is far from certain, both because ruleutils.c might print the
      expression in a different form from how it was originally written
      and because FigureColname's rules might change in future releases.
      So we shouldn't rely on that.
      
      Detecting exactly whether there is any outer-level use of a SELECT
      column name would be rather expensive.  This patch takes the simpler
      approach of just passing down a flag indicating whether there *could*
      be any outer use; for example, the output column names of a SubLink
      are not referenceable, and we also do not care about the names exposed
      by the right-hand side of a setop.  This is sufficient to suppress
      unwanted clutter in all but one case in the regression tests.  That
      seems like reasonable evidence that it won't be too much in users'
      faces, while still fixing the cases we need to fix.
      
      Per bug #17486 from Nicolas Lutic.  This issue is ancient, so
      back-patch to all supported branches.
      
      Discussion: https://postgr.es/m/17486-1ad6fd786728b8af@postgresql.org
      6f7eec11
    • Michael Paquier's avatar
      doc: Mention pg_read_all_stats in description of track_activities · 7f798e89
      Michael Paquier authored
      The description of track_activities mentioned that it is visible to
      superusers and that the information related to the current session can
      be seen, without telling about pg_read_all_stats.  Roles that are
      granted the privileges of pg_read_all_stats can also see this
      information, so mention it in the docs.
      
      Author: Ian Barwick
      Reviewed-by: Nathan Bossart
      Discussion: https://postgr.es/m/CAB8KJ=jhPyYFu-A5r-ZGP+Ax715mUKsMxAGcEQ9Cx_mBAmrPow@mail.gmail.com
      Backpatch-through: 10
      7f798e89
  20. 20 May, 2022 1 commit