53803 Commits (0c67dbcc4e39e160bc5c9d7db3b3f011292d5a85)

 

All Branches   

Search

Author	SHA1	Message	Date
Tom Lane	7fb8c92404	Fix resource leaks in PL/Python error reporting, redux. ... Commit c6f7f11d8 intended to prevent leaking any PyObject reference counts in edge cases (such as out-of-memory during string construction), but actually it introduced a leak in the normal case. Repeating an error-trapping operation often enough would lead to session-lifespan memory bloat. The problem is that I failed to think about the fact that PyObject_GetAttrString() increments the refcount of the returned PyObject, so that simply walking down the list of error frame objects causes all but the first one to have their refcount incremented. I experimented with several more-or-less-complex ways around that, and eventually concluded that the right fix is simply to drop the newly-obtained refcount as soon as we walk to the next frame object in PLy_traceback. This sounds unsafe, but it's perfectly okay because the caller holds a refcount on the first frame object and each frame object holds a refcount on the next one; so the current frame object can't disappear underneath us. By the same token, we can simplify the caller's cleanup back to simply dropping its refcount on the first object. Cleanup of each frame object will lead in turn to the refcount of the next one going to zero. I also added a couple of comments explaining why PLy_elog_impl() doesn't try to free the strings acquired from PLy_get_spi_error_data() or PLy_get_error_data(). That's because I got here by looking at a Coverity complaint about how those strings might get leaked. They are not leaked, but in testing that I discovered this other leak. Back-patch, as c6f7f11d8 was. It's a bit nervous-making to be putting such a fix into v13, which is only a couple weeks from its final release; but I can't see that leaving a recently-introduced leak in place is a better idea. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1203918.1761184159@sss.pgh.pa.us Backpatch-through: 13	2 months ago
Fujii Masao	10799d025f	Add comments explaining overflow entries in the replication lag tracker. ... Commit 883a95646a introduced overflow entries in the replication lag tracker to fix an issue where lag columns in pg_stat_replication could stall when the replay LSN stopped advancing. This commit adds comments clarifying the purpose and behavior of overflow entries to improve code readability and understanding. Since commit 883a95646a was recently applied and backpatched to all supported branches, this follow-up commit is also backpatched accordingly. Author: Xuneng Zhou <xunengzhou@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CABPTF7VxqQA_DePxyZ7Y8V+ErYyXkmwJ1P6NC+YC+cvxMipWKw@mail.gmail.com Backpatch-through: 13	2 months ago
Masahiko Sawada	c4a6444a65	Add copyright notice to vacuum_horizon_floor.pl test. ... Fix oversight in commit 303ba0573, which was backpatched through 14. Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAD21AoBeFdTJcwUfUYPcEgONab3TS6i1PB9S5cSXcBAmdAdQKw%40mail.gmail.com Backpatch-through: 14	2 months ago
David Rowley	e4316ec4dd	Fix incorrect zero extension of Datum in JIT tuple deform code ... When JIT deformed tuples (controlled via the jit_tuple_deforming GUC), types narrower than sizeof(Datum) would be zero-extended up to Datum width. This wasn't the same as what fetch_att() does in the standard tuple deforming code. Logically the values are the same when fetching via the DatumGet*() marcos, but negative numbers are not the same in binary form. In the report, the problem was manifesting itself with: ERROR: could not find memoization table entry in a query which had a "Cache Mode: binary" Memoize node. However, it's currently unclear what else is affected. Anything that uses datum_image_eq() or datum_image_hash() on a Datum from a tuple deformed by JIT could be affected, but it may not be limited to that. The fix for this is simple: use signed extension instead of zero extension. Many thanks to Emmanuel Touzery for reporting this issue and providing steps and backup which allowed the problem to easily be recreated. Reported-by: Emmanuel Touzery <emmanuel.touzery@plandela.si> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/DB8P194MB08532256D5BAF894F241C06393F3A@DB8P194MB0853.EURP194.PROD.OUTLOOK.COM Backpatch-through: 13	2 months ago
Fujii Masao	f106f53df6	Make invalid primary_slot_name follow standard GUC error reporting. ... Previously, if primary_slot_name was set to an invalid slot name and the configuration file was reloaded, both the postmaster and all other backend processes reported a WARNING. With many processes running, this could produce a flood of duplicate messages. The problem was that the GUC check hook for primary_slot_name reported errors at WARNING level via ereport(). This commit changes the check hook to use GUC_check_errdetail() and GUC_check_errhint() for error reporting. As with other GUC parameters, this causes non-postmaster processes to log the message at DEBUG3, so by default, only the postmaster's message appears in the log file. Backpatch to all supported versions. Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Chao Li <lic@highgo.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/CAHGQGwFud-cvthCTfusBfKHBS6Jj6kdAPTdLWKvP2qjUX6L_wA@mail.gmail.com Backpatch-through: 13	2 months ago
Fujii Masao	a7ab6ce6a0	Fix stalled lag columns in pg_stat_replication when replay LSN stops advancing. ... Previously, when the replay LSN reported in feedback messages from a standby stopped advancing, for example, due to a recovery conflict, the write_lag and flush_lag columns in pg_stat_replication would initially update but then stop progressing. This prevented users from correctly monitoring replication lag. The problem occurred because when any LSN stopped updating, the lag tracker's cyclic buffer became full (the write head reached the slowest read head). In that state, the lag tracker could no longer compute round-trip lag values correctly. This commit fixes the issue by handling the slowest read entry (the one causing the buffer to fill) as a separate overflow entry and freeing space so the write and other read heads can continue advancing in the buffer. As a result, write_lag and flush_lag now continue updating even if the reported replay LSN remains stalled. Backpatch to all supported versions. Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Chao Li <lic@highgo.com> Reviewed-by: Shinya Kato <shinya11.kato@gmail.com> Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com> Discussion: https://postgr.es/m/CAHGQGwGdGQ=1-X-71Caee-LREBUXSzyohkoQJd4yZZCMt24C0g@mail.gmail.com Backpatch-through: 13	2 months ago
Nathan Bossart	58ba7e5ebf	Add .abi-compliance-history to back-branches. ... This file was previously added to v18 by commits a72f7d97be and 93fb76ca4e. Unlike the v18 version of the file, the back-branch versions set the original baseline point to the most recent ABI break documented in the git commit history. While we'd ordinarily set it to something just before the .0 release, we're unlikely to act upon ABI breaks in released minor versions, so it doesn't seem worth the trouble to construct a comprehensive history. Discussion: https://postgr.es/m/aPfDOD6F4FaJJd7M%40nathan Backpatch-through: 13-17	2 months ago
Nathan Bossart	0a358eb45e	Add previous commit to .git-blame-ignore-revs. ... Backpatch-through: 13	2 months ago
Nathan Bossart	e95010a9c0	Re-pgindent brin.c. ... Backpatch-through: 13	2 months ago
David Rowley	eea24eb0ac	Fix BRIN 32-bit counter wrap issue with huge tables ... A BlockNumber (32-bit) might not be large enough to add bo_pagesPerRange to when the table contains close to 2^32 pages. At worst, this could result in a cancellable infinite loop during the BRIN index scan with power-of-2 pagesPerRange, and slow (inefficient) BRIN index scans and scanning of unneeded heap blocks for non power-of-2 pagesPerRange. Backpatch to all supported versions. Author: sunil s <sunilfeb26@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAOG6S4-tGksTQhVzJM19NzLYAHusXsK2HmADPZzGQcfZABsvpA@mail.gmail.com Backpatch-through: 13	2 months ago
Michael Paquier	5274f11c17	Fix POSIX compliance in pgwin32_unsetenv() for "name" argument ... pgwin32_unsetenv() (compatibility routine of unsetenv() on Windows) lacks the input validation that its sibling pgwin32_setenv() has. Without these checks, calling unsetenv() with incorrect names crashes on WIN32. However, invalid names should be handled, failing on EINVAL. This commit adds the same checks as setenv() to fail with EINVAL for a "name" set to NULL, an empty string, or if '=' is included in the value, per POSIX requirements. Like 7ca37fb040, backpatch down to v14. pgwin32_unsetenv() is defined on REL_13_STABLE, but with the branch going EOL soon and the lack of setenv() there for WIN32, nothing is done for v13. Author: Bryan Green <dbryan.green@gmail.com> Discussion: https://postgr.es/m/b6a1e52b-d808-4df7-87f7-2ff48d15003e@gmail.com Backpatch-through: 14	2 months ago
Tom Lane	40dc710a61	Don't rely on zlib's gzgetc() macro. ... It emerges that zlib's configuration logic is not robust enough to guarantee that the macro will have the same ideas about struct field layout as the library itself does, leading to corruption of zlib's state struct followed by unintelligible failure messages. This hazard has existed for a long time, but we'd not noticed for several reasons: (1) We only use gzgetc() when trying to read a manually-compressed TOC file within a directory-format dump, which is a rarely-used scenario that we weren't even testing before 20ec99589. (2) No corruption actually occurs unless sizeof(long) is different from sizeof(off_t) and the platform is big-endian. (3) Some platforms have already fixed the configuration instability, at least sufficiently for their environments. Despite (3), it seems foolish to assume that the problem isn't going to be present in some environments for a long time to come. Hence, avoid relying on this macro. We can just #undef it and fall back on the underlying function of the same name. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/2122679.1760846783@sss.pgh.pa.us Backpatch-through: 13	2 months ago
Álvaro Herrera	6062c3db30	Fix pg_dump sorting of foreign key constraints ... Apparently, commit 04bc2c42f7 failed to notice that DO_FK_CONSTRAINT objects require identical handling as DO_CONSTRAINT ones, which causes some pg_upgrade tests in debug builds to fail spuriously. Add that. Author: Álvaro Herrera <alvherre@kurilemu.de> Backpatch-through: 13 Discussion: https://postgr.es/m/202510181201.k6y75v2tpf5r@alvherre.pgsql	2 months ago
Nathan Bossart	f146eb45cb	Fix privilege checks for pg_prewarm() on indexes. ... pg_prewarm() currently checks for SELECT privileges on the target relation. However, indexes do not have access rights of their own, so a role may be denied permission to prewarm an index despite having the SELECT privilege on its parent table. This commit fixes this by locking the parent table before the index (to avoid deadlocks) and checking for SELECT on the parent table. Note that the code is largely borrowed from amcheck_lock_relation_and_check(). An obvious downside of this change is the extra AccessShareLock on the parent table during prewarming, but that isn't expected to cause too much trouble in practice. Author: Ayush Vatsa <ayushvatsa1810@gmail.com> Co-authored-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/CACX%2BKaMz2ZoOojh0nQ6QNBYx8Ak1Dkoko%3DD4FSb80BYW%2Bo8CHQ%40mail.gmail.com Backpatch-through: 13	2 months ago
Álvaro Herrera	56cd332f80	Fix update-po for the PGXS case ... The original formulation failed to take into account the fact that for the PGXS case, the source dir is not $(top_srcdir), so it ended up not doing anything. Handle it explicitly. Author: Ryo Matsumura <matsumura.ryo@fujitsu.com> Reviewed-by: Bryan Green <dbryan.green@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/TYCPR01MB113164770FB0B0BE6ED21E68EE8DCA@TYCPR01MB11316.jpnprd01.prod.outlook.com	2 months ago
Etsuro Fujita	24e74b9621	Fix EvalPlanQual handling of foreign/custom joins in ExecScanFetch. ... If inside an EPQ recheck, ExecScanFetch would run the recheck method function for foreign/custom joins even if they aren't descendant nodes in the EPQ recheck plan tree, which is problematic at least in the foreign-join case, because such a foreign join isn't guaranteed to have an alternative local-join plan required for running the recheck method function; in the postgres_fdw case this could lead to a segmentation fault or an assert failure in an assert-enabled build when running the recheck method function. Even if inside an EPQ recheck, any scan nodes that aren't descendant ones in the EPQ recheck plan tree should be normally processed by using the access method function; fix by modifying ExecScanFetch so that if inside an EPQ recheck, it runs the recheck method function for foreign/custom joins that are descendant nodes in the EPQ recheck plan tree as before and runs the access method function for foreign/custom joins that aren't. This fix also adds to postgres_fdw an isolation test for an EPQ recheck that caused issues stated above. Oversight in commit 385f337c9. Reported-by: Kristian Lejao <kristianlejao@gmail.com> Author: Masahiko Sawada <sawada.mshk@gmail.com> Co-authored-by: Etsuro Fujita <etsuro.fujita@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Etsuro Fujita <etsuro.fujita@gmail.com> Discussion: https://postgr.es/m/CAD21AoBpo6Gx55FBOW+9s5X=nUw3Xpq64v35fpDEKsTERnc4TQ@mail.gmail.com Backpatch-through: 13	2 months ago
Tom Lane	e91eb4ad66	Fix incorrect message-printing in win32security.c. ... log_error() would probably fail completely if used, and would certainly print garbage for anything that needed to be interpolated into the message, because it was failing to use the correct printing subroutine for a va_list argument. This bug likely went undetected because the error cases this code is used for are rarely exercised - they only occur when Windows security API calls fail catastrophically (out of memory, security subsystem corruption, etc). The FRONTEND variant can be fixed just by calling vfprintf() instead of fprintf(). However, there was no va_list variant of write_stderr(), so create one by refactoring that function. Following the usual naming convention for such things, call it vwrite_stderr(). Author: Bryan Green <dbryan.green@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAF+pBj8goe4fRmZ0V3Cs6eyWzYLvK+HvFLYEYWG=TzaM+tWPnw@mail.gmail.com Backpatch-through: 13	2 months ago
David Rowley	715eb62e30	Doc: clarify n_distinct_inherited setting ... There was some confusion around how to adjust the n_distinct estimates for partitioned tables. Here we try and clarify that n_distinct_inherited needs to be adjusted rather than n_distinct. Also fix some slightly misleading text which was talking about table size rather than table rows, fix a grammatical error, and adjust some text which indicated that ANALYZE was performing calculations based on the n_distinct settings. Really it's the query planner that does this and ANALYZE only stores the overridden n_distinct estimate value in pg_statistic. Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: David G. Johnston <david.g.johnston@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/CAApHDvrL7a-ZytM1SP8Uk9nEw9bR2CPzVb+uP+bcNj=_q-ZmVw@mail.gmail.com	2 months ago
Álvaro Herrera	b753be38a4	Stop creating constraints during DETACH CONCURRENTLY ... Commit 71f4c8c6f7 (which implemented DETACH CONCURRENTLY) added code to create a separate table constraint when a table is detached concurrently, identical to the partition constraint, on the theory that such a constraint was needed in case the optimizer had constructed any query plans that depended on the constraint being there. However, that theory was apparently bogus because any such plans would be invalidated. For hash partitioning, those constraints are problematic, because their expressions reference the OID of the parent partitioned table, to which the detached table is no longer related; this causes all sorts of problems (such as inability of restoring a pg_dump of that table, and the table no longer working properly if the partitioned table is later dropped). We'd like to get rid of all those constraints. In fact, for branch master, do that -- no longer create any substitute constraints. However, out of fear that some users might somehow depend on these constraints for other partitioning strategies, for stable branches (back to 14, which added DETACH CONCURRENTLY), only do it for hash partitioning. (If you repeatedly DETACH CONCURRENTLY and then ATTACH a partition, then with this constraint addition you don't need to scan the table in the ATTACH step, which presumably is good. But if users really valued this feature, they would have requested that it worked for non-concurrent DETACH also.) Author: Haiyang Li <mohen.lhy@alibaba-inc.com> Reported-by: Fei Changhong <feichanghong@qq.com> Reported-by: Haiyang Li <mohen.lhy@alibaba-inc.com> Backpatch-through: 14 Discussion: https://postgr.es/m/18371-7fef49f63de13f02@postgresql.org Discussion: https://postgr.es/m/19070-781326347ade7c57@postgresql.org	2 months ago
Peter Geoghegan	314004021e	Remove overzealous _bt_killitems assertion. ... An assertion in _bt_killitems expected the scan's currPos state to contain a valid LSN, saved from when currPos's page was initially read. The assertion failed to account for the fact that even logged relations can have leaf pages with an invalid LSN when built with wal_level set to "minimal". Remove the faulty assertion. Oversight in commit e6eed40e (though note that the assertion was backpatched to stable branches before 18 by commit 7c319f54). Author: Peter Geoghegan <pg@bowt.ie> Reported-By: Matthijs van der Vleuten <postgresql@zr40.nl> Bug: #19082 Discussion: https://postgr.es/m/19082-628e62160dbbc1c1@postgresql.org Backpatch-through: 13	2 months ago
Michael Paquier	f57f2760e6	Remove state.tmp when failing to save a replication slot ... An error happening while a slot data is saved on disk in SaveSlotToPath() could cause a state.tmp file (temporary file holding the slot state data, renamed to its permanent name at the end of the function) to remain around after it has been created. This temporary file is created with O_EXCL, meaning that if an existing state.tmp is found, its creation would fail. This would prevent the slot data to be saved, requiring a manual intervention to remove state.tmp before being able to save again a slot. Possible scenarios where this temporary file could remain on disk is for example a ENOSPC case (no disk space) while writing, syncing or renaming it. The bug reports point to a write failure as the principal cause of the problems. Using O_TRUNC has been argued back in 2019 as a potential solution to discard any temporary file that could exist. This solution was rejected as O_EXCL can also act as a safety measure when saving the slot state, crash recovery offering cleanup guarantees post-crash. This commit uses the alternative approach that has been suggested by Andres Freund back in 2019. When the temporary state file cannot be written, synced, closed or renamed (note: not when created!), an unlink() is used to remove the temporary state file while holding the in-progress I/O LWLock, so as any follow-up attempts to save a slot's data would not choke on an existing file that remained around because of a previous failure. This problem has been reported a few times across the years, going back to 2019, but for some reason I have never come back to do something about it and it has been forgotten. A recent report has reminded me that this was still a problem. Reported-by: Kevin K Biju <kevinkbiju@gmail.com> Reported-by: Sergei Kornilov <sk@zsrv.org> Reported-by: Grigory Smolkin <g.smolkin@postgrespro.ru> Discussion: https://postgr.es/m/CAM45KeHa32soKL_G8Vk38CWvTBeOOXcsxAPAs7Jt7yPRf2mbVA@mail.gmail.com Discussion: https://postgr.es/m/3559061693910326@qy4q4a6esb2lebnz.sas.yp-c.yandex.net Discussion: https://postgr.es/m/08bbfab1-a61d-3750-fc18-4ab2c1aa7f09@postgrespro.ru Backpatch-through: 13	2 months ago
Tom Lane	f46d77377b	Use SOCK_ERRNO[_SET] in fe-secure-gssapi.c. ... On Windows, this code did not handle error conditions correctly at all, since it looked at "errno" which is not used for socket-related errors on that platform. This resulted, for example, in failure to connect to a PostgreSQL server with GSSAPI enabled. We have a convention for dealing with this within libpq, which is to use SOCK_ERRNO and SOCK_ERRNO_SET rather than touching errno directly; but the GSSAPI code is a relative latecomer and did not get that memo. (The equivalent backend code continues to use errno, because the backend does this differently. Maybe libpq's approach should be rethought someday.) Apparently nobody tries to build libpq with GSSAPI support on Windows, or we'd have heard about this before, because it's been broken all along. Back-patch to all supported branches. Author: Ning Wu <ning94803@gmail.com> Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFGqpvg-pRw=cdsUpKYfwY6D3d-m9tw8WMcAEE7HHWfm-oYWvw@mail.gmail.com Backpatch-through: 13	2 months ago
Michael Paquier	9b3beb7c6f	pgbench: Fail cleanly when finding a COPY result state ... Currently, pgbench aborts when a COPY response is received in readCommandResponse(). However, as PQgetResult() returns an empty result when there is no asynchronous result, through getCopyResult(), the logic done at the end of readCommandResponse() for the error path leads to an infinite loop. This commit forcefully exits the COPY state with PQendcopy() before moving to the error handler when fiding a COPY state, avoiding the infinite loop. The COPY protocol is not supported by pgbench anyway, as an error is assumed in this case, so giving up is better than having the tool be stuck forever. pgbench was interruptible in this state. A TAP test is added to check that an error happens if trying to use COPY. Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Discussion: https://postgr.es/m/CAO6_XqpHyF2m73ifV5a=5jhXxH2chk=XrgefY+eWWPe2Eft3=A@mail.gmail.com Backpatch-through: 13	3 months ago
Michael Paquier	ee2dbcad02	pgstattuple: Improve reports generated for indexes (hash, gist, btree) ... pgstattuple checks the state of the pages retrieved for gist and hash using some check functions from each index AM, respectively gistcheckpage() and _hash_checkpage(). When these are called, they would fail when bumping on data that is found as incorrect (like opaque area size not matching, or empty pages), contrary to btree that simply discards these cases and continues to aggregate data. Zero pages can happen after a crash, with these AMs being able to do an internal cleanup when these are seen. Also, sporadic failures are annoying when doing for example a large-scale diagnostic query based on pgstattuple with a join of pg_class, as it forces one to use tricks like quals to discard hash or gist indexes, or use a PL wrapper able to catch errors. This commit changes the reports generated for btree, gist and hash to be more user-friendly; - When seeing an empty page, report it as free space. This new rule applies to gist and hash, and already applied to btree. - For btree, a check based on the size of BTPageOpaqueData is added. - For gist indexes, gistcheckpage() is not called anymore, replaced by a check based on the size of GISTPageOpaqueData. - For hash indexes, instead of _hash_getbuf_with_strategy(), use a direct call to ReadBufferExtended(), coupled with a check based on HashPageOpaqueData. The opaque area size check was already used. - Pages that do not match these criterias are discarded from the stats reports generated. There have been a couple of bug reports over the years that complained about the current behavior for hash and gist, as being not that useful, with nothing being done about it. Hence this change is backpatched down to v13. Reported-by: Noah Misch <noah@leadboat.com> Author: Nitin Motiani <nitinmotiani@google.com> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Discussion: https://postgr.es/m/CAH5HC95gT1J3dRYK4qEnaywG8RqjbwDdt04wuj8p39R=HukayA@mail.gmail.com Backpatch-through: 13	3 months ago
Fujii Masao	cb21e1f92f	pgbench: Fix error reporting in readCommandResponse(). ... pgbench uses readCommandResponse() to process server responses. When readCommandResponse() encounters an error during a call to PQgetResult() to fetch the current result, it attempts to report it with an additional error message from PQerrorMessage(). However, previously, this extra error message could be lost or become incorrect. The cause was that after fetching the current result (and detecting an error), readCommandResponse() called PQgetResult() again to peek at the next result. This second call could overwrite the libpq connection's error message before the original error was reported, causing the error message retrieved from PQerrorMessage() to be lost or overwritten. This commit fixes the issue by updating readCommandResponse() to use PQresultErrorMessage() instead of PQerrorMessage() to retrieve the error message generated when the PQgetResult() for the current result causes an error, ensuring the correct message is reported. Backpatch to all supported versions. Author: Yugo Nagata <nagata@sraoss.co.jp> Reviewed-by: Chao Li <lic@highgo.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/20250925110940.ebacc31725758ec47d5432c6@sraoss.co.jp Backpatch-through: 13	3 months ago
Noah Misch	5816baa9bc	Fix StatisticsObjIsVisibleExt() for pg_temp. ... Neighbor get_statistics_object_oid() ignores objects in pg_temp, as has been the standard for non-relation, non-type namespace searches since CVE-2007-2138. Hence, most operations that name a statistics object correctly decline to map an unqualified name to a statistics object in pg_temp. StatisticsObjIsVisibleExt() did not. Consequently, pg_statistics_obj_is_visible() wrongly returned true for such objects, psql \dX wrongly listed them, and getObjectDescription()-based ereport() and pg_describe_object() wrongly omitted namespace qualification. Any malfunction beyond that would depend on how a human or application acts on those wrong indications. Commit d99d58cdc8 introduced this. Back-patch to v13 (all supported versions). Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/20250920162116.2e.nmisch@google.com Backpatch-through: 13	3 months ago
Tom Lane	0be39b4b1a	Fix missed copying of groupDistinct in transformPLAssignStmt. ... Because we failed to do this, DISTINCT in GROUP BY DISTINCT would be ignored in PL/pgSQL assignment statements. It's not surprising that no one noticed, since such statements will throw an error if the query produces more than one row. That eliminates most scenarios where advanced forms of GROUP BY could be useful, and indeed makes it hard even to find a simple test case. Nonetheless it's wrong. This is directly the fault of be45be9c3 which added the groupDistinct field, but I think much of the blame has to fall on c9d529848, in which I incautiously supposed that we'd manage to keep two copies of a big chunk of parse-analysis logic in sync. As a follow-up, I plan to refactor so that there's only one copy. But that seems useful only in master, so let's use this one-line fix for the back branches. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/31027.1758919078@sss.pgh.pa.us Backpatch-through: 14	3 months ago
Fujii Masao	bc476f8b8b	pg_restore: Fix security label handling with --no-publications/subscriptions. ... Previously, pg_restore did not skip security labels on publications or subscriptions even when --no-publications or --no-subscriptions was specified. As a result, it could issue SECURITY LABEL commands for objects that were never created, causing those commands to fail. This commit fixes the issue by ensuring that security labels on publications and subscriptions are also skipped when the corresponding options are used. Backpatch to all supported versions. Author: Jian He <jian.universality@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dQXxqacj0XyDhc-fA@mail.gmail.com Backpatch-through: 13	3 months ago
Tom Lane	31bf096320	Calculate agglevelsup correctly when Aggref contains a CTE. ... If an aggregate function call contains a sub-select that has an RTE referencing a CTE outside the aggregate, we must treat that reference like a Var referencing the CTE's query level for purposes of determining the aggregate's level. Otherwise we might reach the nonsensical conclusion that the aggregate should be evaluated at some query level higher than the CTE, ending in a planner error or a broken plan tree that causes executor failures. Bug: #19055 Reported-by: BugForge <dllggyx@outlook.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/19055-6970cfa8556a394d@postgresql.org Backpatch-through: 13	3 months ago
David Rowley	f78a690347	Add missing EPQ recheck for TID Range Scan ... The EvalPlanQual recheck for TID Range Scan wasn't rechecking the TID qual still passed after following update chains. This could result in tuples being updated or deleted by plans using TID Range Scans where the ctid of the new (updated) tuple no longer matches the clause of the scan. This isn't desired behavior, and isn't consistent with what would happen if the chosen plan had used an Index or Seq Scan, and that could lead to hard to predict behavior for scans that contain TID quals and other quals as the planner has freedom to choose TID Range or some other non-TID scan method for such queries, and the chosen plan could change at any moment. Here we fix this by properly implementing the recheck function for TID Range Scans. Backpatch to 14, where TID Range Scans were added Reported-by: Sophie Alpert <pg@sophiebits.com> Author: Sophie Alpert <pg@sophiebits.com> Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/4a6268ff-3340-453a-9bf5-c98d51a6f729@app.fastmail.com Backpatch-through: 14	3 months ago
David Rowley	2eb7ea97d8	Add missing EPQ recheck for TID Scan ... The EvalPlanQual recheck for TID Scan wasn't rechecking the TID qual still passed after following update chains. This could result in tuples being updated or deleted by plans using TID Scans where the ctid of the new (updated) tuple no longer matches the clause of the scan. This isn't desired behavior, and isn't consistent with what would happen if the chosen plan had used an Index or Seq Scan, and that could lead to hard to predict behavior for scans that contain TID quals and other quals as the planner has freedom to choose TID or some other scan method for such queries, and the chosen plan could change at any moment. Here we fix this by properly implementing the recheck function for TID Scans. Backpatch to 13, oldest supported version Reported-by: Sophie Alpert <pg@sophiebits.com> Author: Sophie Alpert <pg@sophiebits.com> Author: David Rowley <dgrowleyml@gmail.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Reviewed-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/4a6268ff-3340-453a-9bf5-c98d51a6f729@app.fastmail.com Backpatch-through: 13	3 months ago
Noah Misch	85d6ed31fc	Fix pg_dump COMMENT dependency for separate domain constraints. ... The COMMENT should depend on the separately-dumped constraint, not the domain. Sufficient restore parallelism might fail the COMMENT command by issuing it before the constraint exists. Back-patch to v13, like commit 0858f0f96e. Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de> Discussion: https://postgr.es/m/20250913020233.fa.nmisch@google.com Backpatch-through: 13	3 months ago
Fujii Masao	295c0a644a	pg_dump: Fix dumping of security labels on subscriptions and event triggers. ... Previously, pg_dump incorrectly queried pg_seclabel to retrieve security labels for subscriptions, which are stored in pg_shseclabel as they are global objects. This could result in security labels for subscriptions not being dumped. This commit fixes the issue by updating pg_dump to query the pg_seclabels view, which aggregates entries from both pg_seclabel and pg_shseclabel. While querying pg_shseclabel directly for subscriptions was an alternative, using pg_seclabels is simpler and sufficient. In addition, pg_dump is updated to dump security labels on event triggers, which were previously omitted. Backpatch to all supported versions. Author: Jian He <jian.universality@gmail.com> Co-authored-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dQXxqacj0XyDhc-fA@mail.gmail.com Backpatch-through: 13	3 months ago
Fujii Masao	db900ec358	pg_restore: Fix comment handling with --no-publications / --no-subscriptions. ... Previously, pg_restore did not skip comments on publications or subscriptions even when --no-publications or --no-subscriptions was specified. As a result, it could issue COMMENT commands for objects that were never created, causing those commands to fail. This commit fixes the issue by ensuring that comments on publications and subscriptions are also skipped when the corresponding options are used. Backpatch to all supported versions. Author: Jian He <jian.universality@gmail.com> Co-authored-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/CACJufxHCt00pR9h51AVu6+yPD5J7JQn=7dQXxqacj0XyDhc-fA@mail.gmail.com Backpatch-through: 13	3 months ago
Peter Eisentraut	cbd3732cbb	CREATE STATISTICS: improve misleading error message ... The previous change (commit f225473cba) was still not on target, because it talked about relation kinds, which are not what is being checked here. Provide a more accurate message. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CACJufxEZ48toGH0Em_6vdsT57Y3L8pLF=DZCQ_gCii6=C3MeXw@mail.gmail.com	3 months ago
Peter Eisentraut	fbfc36e947	jit: fix build with LLVM-21 ... LLVM-21 renamed llvm::GlobalValue::getGUID() to getGUIDAssumingExternalLinkage(), so add a version guard. Author: Holger Hoffstätte <holger@applied-asynchrony.com> Discussion: https://www.postgresql.org/message-id/flat/d25e6e4a-d1b4-84d3-2f8a-6c45b975f53d%40applied-asynchrony.com	3 months ago
Tom Lane	f75ff1b141	Amend recent fix for SIMILAR TO regex conversion. ... Commit e3ffc3e91 fixed the translation of character classes in SIMILAR TO regular expressions. Unfortunately the fix broke a corner case: if there is an escape character right after the opening bracket (for example in "[\q]"), a closing bracket right after the escape sequence would not be seen as closing the character class. There were two more oversights: a backslash or a nested opening bracket right at the beginning of a character class should remove the special meaning from any following caret or closing bracket. This bug suggests that this code needs to be more readable, so also rename the variables "charclass_depth" and "charclass_start" to something more meaningful, rewrite an "if" cascade to be more consistent, and improve the commentary. Reported-by: Dominique Devienne <ddevienne@gmail.com> Reported-by: Stephan Springl <springl-psql@bfw-online.de> Author: Laurenz Albe <laurenz.albe@cybertec.at> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAFCRh-8NwJd0jq6P=R3qhHyqU7hw0BTor3W0SvUcii24et+zAw@mail.gmail.com Backpatch-through: 13	3 months ago
Michael Paquier	5e131489fe	Fix description of WAL record blocks in hash_xlog.h ... hash_xlog.h included descriptions for the blocks used in WAL records that were was not completely consistent with how the records are generated, with one block missing for SQUEEZE_PAGE, and inconsistent descriptions used for block 0 in VACUUM_ONE_PAGE and MOVE_PAGE_CONTENTS. This information was incorrect since c11453ce0a, cross-checking the logic for the record generation. Author: Kirill Reshke <reshkekirill@gmail.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Discussion: https://postgr.es/m/CALdSSPj1j=a1d1hVA3oabRFz0hSU3KKrYtZPijw4UPUM7LY9zw@mail.gmail.com Backpatch-through: 13	3 months ago
Tom Lane	5eab9b0a47	Fix memory leakage in nodeSubplan.c. ... If the hash functions used for hashing tuples leaked any memory, we failed to clean that up, resulting in query-lifespan memory leakage in queries using hashed subplans. One way that could happen is if the values being hashed require de-toasting, since most of our hash functions don't trouble to clean up de-toasted inputs. Prior to commit bf6c614a2, this leakage was largely masked because TupleHashTableMatch would reset hashtable->tempcxt (via execTuplesMatch). But it doesn't do that anymore, and that's not really the right place for this anyway: doing it there could reset the tempcxt many times per hash lookup, or not at all. Instead put reset calls into ExecHashSubPlan and buildSubPlanHash. Along the way to that, rearrange ExecHashSubPlan so that there's just one place to call MemoryContextReset instead of several. This amounts to accepting the de-facto API spec that the caller of the TupleHashTable routines is responsible for resetting the tempcxt adequately often. Although the other callers seem to get this right, it was not documented anywhere, so add a comment about it. Bug: #19040 Reported-by: Haiyang Li <mohen.lhy@alibaba-inc.com> Author: Haiyang Li <mohen.lhy@alibaba-inc.com> Reviewed-by: Fei Changhong <feichanghong@qq.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/19040-c9b6073ef814f48c@postgresql.org Backpatch-through: 13	3 months ago
Dean Rasheed	ea65c88238	Fix compiler error introduced by 5386bfb9c1. ... Per buildfarm member wrasse, void function cannot return a value. This only affects v13-v17, where an ABI-compatible wrapper function was added. Backpatch-through: 13-17	3 months ago
Dean Rasheed	5fd569ef2d	Fix replica identity check for INSERT ON CONFLICT DO UPDATE. ... If an INSERT has an ON CONFLICT DO UPDATE clause, the executor must check that the target relation supports UPDATE as well as INSERT. In particular, it must check that the target relation has a REPLICA IDENTITY if it publishes updates. Formerly, it was not doing this check, which could lead to silently breaking replication. Fix by adding such a check to CheckValidResultRel(), which requires adding a new onConflictAction argument. In back-branches, preserve ABI compatibility by introducing a wrapper function with the original signature. Author: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Tested-by: Chao Li <li.evan.chao@gmail.com> Discussion: https://postgr.es/m/OS3PR01MB57180C87E43A679A730482DF94B62@OS3PR01MB5718.jpnprd01.prod.outlook.com Backpatch-through: 13	3 months ago
Richard Guo	160ef51c8d	Fix planner error when estimating SubPlan cost ... SubPlan nodes are typically built very early, before any RelOptInfos have been constructed for the parent query level. As a result, the simple_rel_array in the parent root has not yet been initialized. Currently, during cost estimation of a SubPlan's testexpr, we may call examine_variable() to look up statistical data about the expressions. This can lead to "no relation entry for relid" errors. To fix, pass root as NULL to cost_qual_eval() in cost_subplan(), since the root does not yet contain enough information to safely consult statistics. One exception is SubPlan nodes built for the initplans of MIN/MAX aggregates from indexes. In this case, having a NULL root is safe because testexpr will be NULL. Additionally, an initplan will by definition not consult anything from the parent plan. Backpatch to all supported branches. Although the reported call path that triggers this error is not reachable prior to v17, there's no guarantee that other code paths -- especially in extensions -- could not encounter the same issue when cost_qual_eval() is called with a root that lacks a valid simple_rel_array. The test case is not included in pre-v17 branches though. Bug: #19037 Reported-by: Alexander Lakhin <exclusion@gmail.com> Diagnosed-by: Tom Lane <tgl@sss.pgh.pa.us> Author: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/19037-3d1c7bb553c7ce84@postgresql.org Backpatch-through: 13	4 months ago
Michael Paquier	0cc540f0d0	libpq: Fix PQtrace() format for non-printable characters ... PQtrace() was generating its output for non-printable characters without casting the characters printed with unsigned char, leading to some extra "\xffffff" generated in the output due to the fact that char may be signed. Oversights introduced by commit 198b3716db, so backpatch down to v14. Author: Ran Benita <ran@unusedvar.com> Discussion: https://postgr.es/m/a3383211-4539-459b-9d51-95c736ef08e0@app.fastmail.com Backpatch-through: 14	4 months ago
Álvaro Herrera	b830f9d674	CREATE STATISTICS: improve misleading error message ... I think the error message for a different condition was inadvertently copied. This problem seems to have been introduced by commit a4d75c86bf. Author: Álvaro Herrera <alvherre@kurilemu.de> Reported-by: jian he <jian.universality@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Backpatch-through: 14 Discussion: https://postgr.es/m/CACJufxEZ48toGH0Em_6vdsT57Y3L8pLF=DZCQ_gCii6=C3MeXw@mail.gmail.com	4 months ago
Tom Lane	4a593043ee	Put "excludeOnly" GIN scan keys at the end of the scankey array. ... Commit 4b754d6c1 introduced the concept of an excludeOnly scan key, which cannot select matching index entries but can reject non-matching tuples, for example a tsquery such as '!term'. There are poorly-documented assumptions that such scan keys do not appear as the first scan key. ginNewScanKey did nothing to ensure that, however, with the result that certain GIN index searches could go into an infinite loop while apparently-equivalent queries with the clauses in a different order were fine. Fix by teaching ginNewScanKey to place all excludeOnly scan keys after all not-excludeOnly ones. So far as we know at present, it might be sufficient to avoid the case where the very first scan key is excludeOnly; but I'm not very convinced that there aren't other dependencies on the ordering. Bug: #19031 Reported-by: Tim Wood <washwithcare@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/19031-0638148643d25548@postgresql.org Backpatch-through: 13	4 months ago
Tom Lane	a7da746c11	Do CHECK_FOR_INTERRUPTS inside, not before, scanGetItem. ... The CHECK_FOR_INTERRUPTS call in gingetbitmap turns out to be inadequate to prevent a long uninterruptible loop, because we now know a case where looping occurs within scanGetItem. While the next patch will fix the bug that caused that, it seems foolish to assume that no similar patterns are possible. Let's do the CFI within scanGetItem's retry loop, instead. This demonstrably allows canceling out of the loop exhibited in bug #19031. Bug: #19031 Reported-by: Tim Wood <washwithcare@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/19031-0638148643d25548@postgresql.org Backpatch-through: 13	4 months ago
Noah Misch	22c6a44f01	Rewrite previous commit's test for TestUpgradeXversion compatibility. ... v17 introduced the MAINTAIN ON TABLES privilege. That changed the applicable "baseacls" reaching buildACLCommands(). That yielded spurious TestUpgradeXversion diffs. Change to use a TYPES privilege. Types have the same one privilege in all supported versions, so they avoid the problem. Per buildfarm. Back-patch to v13, like that commit. Discussion: https://postgr.es/m/20250823144505.88.nmisch@google.com Backpatch-through: 13	4 months ago
Noah Misch	4948bb9df4	Sort DO_DEFAULT_ACL dump objects independent of OIDs. ... Commit 0decd5e89d missed DO_DEFAULT_ACL, leading to assertion failures, potential dump order instability, and spurious schema diffs. Back-patch to v13, like that commit. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Kirill Reshke <reshkekirill@gmail.com> Discussion: https://postgr.es/m/d32aaa8d-df7c-4f94-bcb3-4c85f02bea21@gmail.com Backpatch-through: 13	4 months ago
Michael Paquier	222130edd8	Ignore temporary relations in RelidByRelfilenumber() ... Temporary relations may share the same RelFileNumber with a permanent relation, or other temporary relations associated with other sessions. Being able to uniquely identify a temporary relation would require RelidByRelfilenumber() to know about the proc number of the temporary relation it wants to identify, something it is not designed for since its introduction in f01d1ae3a1. There are currently three callers of RelidByRelfilenumber(): - autoprewarm. - Logical decoding, reorder buffer. - pg_filenode_relation(), that attempts to find a relation OID based on a tablespace OID and a RelFileNumber. This makes the situation problematic particularly for the first two cases, leading to the possibility of random ERRORs due to inconsistencies that temporary relations can create in the cache maintained by RelidByRelfilenumber(). The third case should be less of an issue, as I suspect that there are few direct callers of pg_filenode_relation(). The window where the ERRORs are happen is very narrow, requiring an OID wraparound to create a lookup conflict in RelidByRelfilenumber() with a temporary table reusing the same OID as another relation already cached. The problem is easier to reach in workloads with a high OID consumption rate, especially with a higher number of temporary relations created. We could get pg_filenode_relation() and RelidByRelfilenumber() to work with temporary relations if provided the means to identify them with an optional proc number given in input, but the years have also shown that we do not have a use case for it, yet. Note that this could not be backpatched if pg_filenode_relation() needs changes. It is simpler to ignore temporary relations. Reported-by: Shenhao Wang <wangsh.fnst@fujitsu.com> Author: Vignesh C <vignesh21@gmail.com> Reviewed-By: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Reviewed-By: Robert Haas <robertmhaas@gmail.com> Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-By: Takamichi Osumi <osumi.takamichi@fujitsu.com> Reviewed-By: Michael Paquier <michael@paquier.xyz> Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com> Reported-By: Shenhao Wang <wangsh.fnst@fujitsu.com> Discussion: https://postgr.es/m/bbaaf9f9-ebb2-645f-54bb-34d6efc7ac42@fujitsu.com Backpatch-through: 13	4 months ago
Michael Paquier	24145bbb9e	doc: Improve description of wal_compression ... The description of this GUC provides a list of the situations where full-page writes are generated. However, it is not completely exact, mentioning only the cases where full_page_writes=on or base backups. It is possible to generate full-page writes in more situations than these two, making the description confusing as it implies that no other cases exist. The description is slightly reworded to take into account that other cases are possible, without mentioning them directly to minimize the maintenance burden should FPWs be generated in more contexts in the future. Author: Jingtang Zhang <mrdrivingduck@gmail.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com> Discussion: https://postgr.es/m/CAPsk3_CtAYa_fy4p6=x7qtoutrdKvg1kGk46D5fsE=sMt2546g@mail.gmail.com Backpatch-through: 13	4 months ago