40461 Commits (eb9ee4d1818cd228d80049fd42efbfaabc187c20)

Author	SHA1	Message	Date
Amit Kapila	e411371558	Fix self-deadlock during DROP SUBSCRIPTION. ... The DROP SUBSCRIPTION command performs several operations: it stops the subscription workers, removes subscription-related entries from system catalogs, and deletes the replication slot on the publisher server. Previously, this command acquired an AccessExclusiveLock on pg_subscription before initiating these steps. However, while holding this lock, the command attempts to connect to the publisher to remove the replication slot. In cases where the connection is made to a newly created database on the same server as subscriber, the cache-building process during connection tries to acquire an AccessShareLock on pg_subscription, resulting in a self-deadlock. To resolve this issue, we reduce the lock level on pg_subscription during DROP SUBSCRIPTION from AccessExclusiveLock to RowExclusiveLock. Earlier, the higher lock level was used to prevent the launcher from starting a new worker during the drop operation, as a restarted worker could become orphaned. Now, instead of relying on a strict lock, we acquire an AccessShareLock on the specific subscription being dropped and re-validate its existence after acquiring the lock. If the subscription is no longer valid, the worker exits gracefully. This approach avoids the deadlock while still ensuring that orphan workers are not created. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Dilip Kumar <dilipbalaut@gmail.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 13 Discussion: https://postgr.es/m/18988-7312c868be2d467f@postgresql.org	4 weeks ago
Etsuro Fujita	242e5afdae	Update obsolete comments in ResultRelInfo struct. ... Commit c5b7ba4e6 changed things so that the ri_RootResultRelInfo field of this struct is set for both partitions and inheritance children and used for tuple routing and transition capture (before that commit, it was only set for partitions to route tuples into), but failed to update these comments. Author: Etsuro Fujita <etsuro.fujita@gmail.com> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://postgr.es/m/CAPmGK14NF5CcdCmTZpxrvpvBiT0y4EqKikW1r_wAu1CEHeOmUA%40mail.gmail.com Backpatch-through: 14	4 weeks ago
David Rowley	199a347d29	Fix invalid format string in HASH_DEBUG code ... This seems to have been broken back in be0a66666. Reported-by: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> Author: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/OSCPR01MB14966E11EEFB37D7857FCEDB7F535A@OSCPR01MB14966.jpnprd01.prod.outlook.com Backpatch-through: 14	4 weeks ago
Andres Freund	88f3f7fd83	ci: Per-repo configuration for manually trigger tasks ... We do not want to trigger some tasks by default, to avoid using too many compute credits. These tasks have to be manually triggered to be run. But e.g. for cfbot we do have sufficient resources, so we always want to start those tasks. With this commit, an individual repository can be configured to trigger them automatically using an environment variable defined under "Repository Settings", for example: REPO_CI_AUTOMATIC_TRIGGER_TASKS="mingw netbsd openbsd" This will enable cfbot to turn them on by default when running tests for the Commitfest app. Backpatch this back to PG 15, even though PG 15 does not have any manually triggered task. Keeping the CI infrastructure the same seems advantageous. Author: Andres Freund <andres@anarazel.de> Co-authored-by: Thomas Munro <thomas.munro@gmail.com> Co-authored-by: Nazir Bilal Yavuz <byavuz81@gmail.com> Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com> Discussion: https://postgr.es/m/20240413021221.hg53rvqlvldqh57i%40awork3.anarazel.de Backpatch-through: 16	1 month ago
Tom Lane	f4c0883448	Don't treat EINVAL from semget() as a hard failure. ... It turns out that on some platforms (at least current macOS, NetBSD, OpenBSD) semget(2) will return EINVAL if there is a pre-existing semaphore set with the same key and too few semaphores. Our code expects EEXIST in that case and treats EINVAL as a hard failure, resulting in failure during initdb or postmaster start. POSIX does document EINVAL for too-few-semaphores-in-set, and is silent on its priority relative to EEXIST, so this behavior arguably conforms to spec. Nonetheless it's quite problematic because EINVAL is also documented to mean that nsems is greater than the system's limit on the number of semaphores per set (SEMMSL). If that is where the problem lies, retrying would just become an infinite loop. To resolve this contradiction, retry after EINVAL, but also install a loop limit that will make us give up regardless of the specific errno after trying 1000 different keys. (1000 is a pretty arbitrary number, but it seems like it should be sufficient.) I like this better than the previous infinite-looping behavior, since it will also keep us out of trouble if (say) we get EACCES due to a system-level permissions problem rather than anything to do with a specific semaphore set. This problem has only been observed in the field in PG 17, which uses a higher nsems value than other branches (cf. 38da05346, 810a8b1c8). That makes it possible to get the failure if a new v17 postmaster has a key collision with an existing postmaster of another branch. In principle though, we might see such a collision against a semaphore set created by some other application, in which case all branches are vulnerable on these platforms. Hence, backpatch. Reported-by: Gavin Panella <gavinpanella@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CALL7chmzY3eXHA7zHnODUVGZLSvK3wYCSP0RmcDFHJY8f28Q3g@mail.gmail.com Backpatch-through: 13	1 month ago
Nathan Bossart	4240405068	Restrict psql meta-commands in plain-text dumps. ... A malicious server could inject psql meta-commands into plain-text dump output (i.e., scripts created with pg_dump --format=plain, pg_dumpall, or pg_restore --file) that are run at restore time on the machine running psql. To fix, introduce a new "restricted" mode in psql that blocks all meta-commands (except for \unrestrict to exit the mode), and teach pg_dump, pg_dumpall, and pg_restore to use this mode in plain-text dumps. While at it, encourage users to only restore dumps generated from trusted servers or to inspect it beforehand, since restoring causes the destination to execute arbitrary code of the source superusers' choice. However, the client running the dump and restore needn't trust the source or destination superusers. Reported-by: Martin Rakhmanov Reported-by: Matthieu Denais <litezeraw@gmail.com> Reported-by: RyotaK <ryotak.mail@gmail.com> Suggested-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Security: CVE-2025-8714 Backpatch-through: 13	1 month ago
Noah Misch	9751f934aa	Convert newlines to spaces in names written in v11+ pg_dump comments. ... Maliciously-crafted object names could achieve SQL injection during restore. CVE-2012-0868 fixed this class of problem at the time, but later work reintroduced three cases. Commit bc8cd50fef (back-patched to v11+ in 2023-05 releases) introduced the pg_dump case. Commit 6cbdbd9e8d (v12+) introduced the two pg_dumpall cases. Move sanitize_line(), unchanged, to dumputils.c so pg_dumpall has access to it in all supported versions. Back-patch to v13 (all supported versions). Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Backpatch-through: 13 Security: CVE-2025-8715	1 month ago
Peter Eisentraut	a4f513b5a8	Translation updates ... Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 4f32135f4a43dba7fa02742da9d671e73e3d7714	1 month ago
Dean Rasheed	415badc138	Fix security checks in selectivity estimation functions. ... Commit e2d4ef8de8 (the fix for CVE-2017-7484) added security checks to the selectivity estimation functions to prevent them from running user-supplied operators on data obtained from pg_statistic if the user lacks privileges to select from the underlying table. In cases involving inheritance/partitioning, those checks were originally performed against the child RTE (which for plain inheritance might actually refer to the parent table). Commit 553d2ec271 then extended that to also check the parent RTE, allowing access if the user had permissions on either the parent or the child. It turns out, however, that doing any checks using the child RTE is incorrect, since securityQuals is set to NULL when creating an RTE for an inheritance child (whether it refers to the parent table or the child table), and therefore such checks do not correctly account for any RLS policies or security barrier views. Therefore, do the security checks using only the parent RTE. This is consistent with how RLS policies are applied, and the executor's ACL checks, both of which use only the parent table's permissions/policies. Similar checks are performed in the extended stats code, so update that in the same way, centralizing all the checks in a new function. In addition, note that these checks by themselves are insufficient to ensure that the user has access to the table's data because, in a query that goes via a view, they only check that the view owner has permissions on the underlying table, not that the current user has permissions on the view itself. In the selectivity estimation functions, there is no easy way to navigate from underlying tables to views, so add permissions checks for all views mentioned in the query to the planner startup code. If the user lacks permissions on a view, a permissions error will now be reported at planner-startup, and the selectivity estimation functions will not be run. Checking view permissions at planner-startup in this way is a little ugly, since the same checks will be repeated at executor-startup. Longer-term, it might be better to move all the permissions checks from the executor to the planner so that permissions errors can be reported sooner, instead of creating a plan that won't ever be run. However, such a change seems too far-reaching to be back-patched. Back-patch to all supported versions. In v13, there is the added complication that UPDATEs and DELETEs on inherited target tables are planned using inheritance_planner(), which plans each inheritance child table separately, so that the selectivity estimation functions do not know that they are dealing with a child table accessed via its parent. Handle that by checking access permissions on the top parent table at planner-startup, in the same way as we do for views. Any securityQuals on the top parent table are moved down to the child tables by inheritance_planner(), so they continue to be checked by the selectivity estimation functions. Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Noah Misch <noah@leadboat.com> Backpatch-through: 13 Security: CVE-2025-8713	1 month ago
Noah Misch	70637d7ae0	Remove, from stable branches, the new assertion of no pg_dump OID sort. ... Commit 0decd5e89d recently added the assertion to confirm dump order remains independent of OID values. The assertion remained reachable via DO_DEFAULT_ACL. Given the release wrap tomorrow, make the assertion master-only. Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/d32aaa8d-df7c-4f94-bcb3-4c85f02bea21@gmail.com Backpatch-through: 13-18	1 month ago
Etsuro Fujita	f39a7f32ae	Fix oversight in FindTriggerIncompatibleWithInheritance. ... This function is called from ATExecAttachPartition/ATExecAddInherit, which prevent tables with row-level triggers with transition tables from becoming partitions or inheritance children, to check if there is such a trigger on the given table, but failed to check if a found trigger is row-level, causing the caller functions to needlessly prevent a table with only a statement-level trigger with transition tables from becoming a partition or inheritance child. Repair. Oversight in commit 501ed02cf. Author: Etsuro Fujita <etsuro.fujita@gmail.com> Discussion: https://postgr.es/m/CAPmGK167mXzwzzmJ_0YZ3EZrbwiCxtM1vogH_8drqsE6PtxRYw%40mail.gmail.com Backpatch-through: 13	1 month ago
Peter Eisentraut	18d2d8ae42	pg_upgrade: Add missing newline in output, another one ... This came from the backport of commit f295494d33, but older branches require the explicit newline in messages (see commit 7652353d87).	1 month ago
Etsuro Fujita	d642d23064	Disallow collecting transition tuples from child foreign tables. ... Commit 9e6104c66 disallowed transition tables on foreign tables, but failed to account for cases where a foreign table is a child table of a partitioned/inherited table on which transition tables exist, leading to incorrect transition tuples collected from such foreign tables for queries on the parent table triggering transition capture. This occurred not only for inherited UPDATE/DELETE but for partitioned INSERT later supported by commit 3d956d956, which should have handled it at least for the INSERT case, but didn't. To fix, modify ExecAR*Triggers to throw an error if the given relation is a foreign table requesting transition capture. Also, this commit fixes make_modifytable so that in case of an inherited UPDATE/DELETE triggering transition capture, FDWs choose normal operations to modify child foreign tables, not DirectModify; which is needed because they would otherwise skip the calls to ExecAR*Triggers at execution, causing unexpected behavior. Author: Etsuro Fujita <etsuro.fujita@gmail.com> Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/CAPmGK14QJYikKzBDCe3jMbpGENnQ7popFmbEgm-XTNuk55oyHg%40mail.gmail.com Backpatch-through: 13	1 month ago
Michael Paquier	769be67a3c	Add information about "generation" when dropping twice pgstats entry ... Dropping twice a pgstats entry should not happen, and the error report generated was missing the "generation" counter (tracking when an entry is reused) that has been added in 818119afcc. Like d92573adcb, backpatch down to v15 where this information is useful to have, to gather more information from instances where the problem shows up. A report has shown that this error path has been reached on a standby based on 17.3, for a relation stats entry and an OID close to wraparound. Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/CAN4RuQvYth942J2+FcLmJKgdpq6fE5eqyFvb_PuskxF2eL=Wzg@mail.gmail.com Backpatch-through: 15	1 month ago
Peter Eisentraut	a8b31b160b	pg_upgrade: Make format strings consistent ... The backport of commit f295494d33 introduced a format string using %m. This is not wrong, since those have been supported since commit d6c55de1f9, but only commit 2c8118ee5d later introduced their use in this file. This use introduces a gratuitously different translatable string and also makes it inconsistent with the rest of the file. To avoid that, switch this back to the old-style strerror() route in the appropriate backbranches	1 month ago
Peter Eisentraut	22d7833505	pg_upgrade: Add missing newline in output ... This came from the backport of commit f295494d33, but older branches require the explicit newline in messages (see commit 7652353d87).	1 month ago
Alexander Korotkov	73f897ba58	Fix checkpointer shared memory allocation ... Use Min(NBuffers, MAX_CHECKPOINT_REQUESTS) instead of NBuffers in CheckpointerShmemSize() to match the actual array size limit set in CheckpointerShmemInit(). This prevents wasting shared memory when NBuffers > MAX_CHECKPOINT_REQUESTS. Also, fix the comment. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1439188.1754506714%40sss.pgh.pa.us Author: Xuneng Zhou <xunengzhou@gmail.com> Co-authored-by: Alexander Korotkov <aekorotkov@gmail.com>	1 month ago
John Naylor	baacfb9e60	Update ICU C++ API symbols ... Recent ICU versions have added U_SHOW_CPLUSPLUS_HEADER_API, and we need to set this to zero as well to hide the ICU C++ APIs from pg_locale.h Per discussion, we want cpluspluscheck to work cleanly in backbranches, so backpatch both this and its predecessor commit ed26c4e25a to all supported versions. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1115793.1754414782%40sss.pgh.pa.us Backpatch-through: 13	1 month ago
Peter Eisentraut	63c79a6fc2	pg_upgrade: Improve message indentation ... Fix commit f295494d33 to use consistent four-space indentation for verbose messages.	1 month ago
Tom Lane	835c9374d6	Fix incorrect return value in brin_minmax_multi_distance_numeric(). ... The result of "DirectFunctionCall1(numeric_float8, d)" is already in Datum form, but the code was incorrectly applying PG_RETURN_FLOAT8() to it. On machines where float8 is pass-by-reference, this would result in complete garbage, since an unpredictable pointer value would be treated as an integer and then converted to float. It's not entirely clear how much of a problem would ensue on 64-bit hardware, but certainly interpreting a float8 bitpattern as uint64 and then converting that to float isn't the intended behavior. As luck would have it, even the complete-garbage case doesn't break BRIN indexes, since the results are only used to make choices about how to merge values into ranges: at worst, we'd make poor choices resulting in an inefficient index. Doubtless that explains the lack of field complaints. However, users with BRIN indexes that use the numeric_minmax_multi_ops opclass may wish to reindex in hopes of making their indexes more efficient. Author: Peter Eisentraut <peter@eisentraut.org> Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/2093712.1753983215@sss.pgh.pa.us Backpatch-through: 14	1 month ago
Dean Rasheed	8748148d61	Fix typo in create_index.sql. ... Introduced by 578b229718. Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/CAEZATCV_CzRSOPMf1gbHQ7xTmyrV6kE7ViCBD6B81WF7GfTAEA@mail.gmail.com Backpatch-through: 13	1 month ago
Fujii Masao	6914a330f0	Fix assertion failure in pgbench when handling multiple pipeline sync messages. ... Previously, when running pgbench in pipeline mode with a custom script that triggered retriable errors (e.g., serialization errors), an assertion failure could occur: Assertion failed: (res == ((void*)0)), function discardUntilSync, file pgbench.c, line 3515. The root cause was that pgbench incorrectly assumed only a single pipeline sync message would be received at the end. In reality, multiple pipeline sync messages can be sent and must be handled properly. This commit fixes the issue by updating pgbench to correctly process multiple pipeline sync messages, preventing the assertion failure. Back-patch to v15, where the bug was introduced. Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Tatsuo Ishii <ishii@postgresql.org> Discussion: https://postgr.es/m/CAHGQGwFAX56Tfx+1ppo431OSWiLLuW72HaGzZ39NkLkop6bMzQ@mail.gmail.com Backpatch-through: 15	1 month ago
Nathan Bossart	f79ca73d75	Allow resetting unknown custom GUCs with reserved prefixes. ... Currently, ALTER DATABASE/ROLE/SYSTEM RESET [ALL] with an unknown custom GUC with a prefix reserved by MarkGUCPrefixReserved() errors (unless a superuser runs a RESET ALL variant). This is problematic for cases such as an extension library upgrade that removes a GUC. To fix, simply make sure the relevant code paths explicitly allow it. Note that we require superuser or privileges on the parameter to reset it. This is perhaps a bit more restrictive than is necessary, but it's not clear whether further relaxing the requirements is safe. Oversight in commit 88103567cb. The ALTER SYSTEM fix is dependent on commit 2d870b4aef, which first appeared in v17. Unfortunately, back-patching that commit would introduce ABI breakage, and while that breakage seems unlikely to bother anyone, it doesn't seem worth the risk. Hence, the ALTER SYSTEM part of this commit is omitted on v15 and v16. Reported-by: Mert Alev <mert@futo.org> Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Discussion: https://postgr.es/m/18964-ba09dea8c98fccd6%40postgresql.org Backpatch-through: 15	1 month ago
Amit Kapila	434d2d147b	Fix a deadlock during ALTER SUBSCRIPTION ... DROP PUBLICATION. ... A deadlock can occur when the DDL command and the apply worker acquire catalog locks in different orders while dropping replication origins. The issue is rare in PG16 and higher branches because, in most cases, the tablesync worker performs the origin drop in those branches, and its locking sequence does not conflict with DDL operations. This patch ensures consistent lock acquisition to prevent such deadlocks. As per buildfarm. Reported-by: Alexander Lakhin <exclusion@gmail.com> Author: Ajin Cherian <itsajin@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: vignesh C <vignesh21@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 14, where it was introduced Discussion: https://postgr.es/m/bab95e12-6cc5-4ebb-80a8-3e41956aa297@gmail.com	1 month ago
Noah Misch	22f126da6c	Sort dump objects independent of OIDs, for the 7 holdout object types. ... pg_dump sorts objects by their logical names, e.g. (nspname, relname, tgname), before dependency-driven reordering. That removes one source of logically-identical databases differing in their schema-only dumps. In other words, it helps with schema diffing. The logical name sort ignored essential sort keys for constraints, operators, PUBLICATION ... FOR TABLE, PUBLICATION ... FOR TABLES IN SCHEMA, operator classes, and operator families. pg_dump's sort then depended on object OID, yielding spurious schema diffs. After this change, OIDs affect dump order only in the event of catalog corruption. While pg_dump also wrongly ignored pg_collation.collencoding, CREATE COLLATION restrictions have been keeping that imperceptible in practical use. Use techniques like we use for object types already having full sort key coverage. Where the pertinent queries weren't fetching the ignored sort keys, this adds columns to those queries and stores those keys in memory for the long term. The ignorance of sort keys became more problematic when commit 172259afb5 added a schema diff test sensitive to it. Buildfarm member hippopotamus witnessed that. However, dump order stability isn't a new goal, and this might avoid other dump comparison failures. Hence, back-patch to v13 (all supported versions). Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/20250707192654.9e.nmisch@google.com Backpatch-through: 13	1 month ago
Tom Lane	e99010cbd8	pg_dump: provide a stable sort order for rules. ... Previously, we sorted rules by schema name and then rule name; if that wasn't unique, we sorted by rule OID. This can be problematic for comparing dumps from databases with different histories, especially since certain rule names like "_RETURN" are very common. Let's make the sort key schema name, rule name, table name, which should be unique. (This is the same behavior we've long used for triggers and RLS policies.) Andreas Karlsson This back-patches v18 commit 350e6b8ea8 to all supported branches. The next commit will assert that pg_dump provides a stable sort order for all object types. That assertion would fail without stabilizing DO_RULE order as this commit did. Discussion: https://postgr.es/m/b4e468d8-0cd6-42e6-ac8a-1d6afa6e0cf1@proxel.se Discussion: https://postgr.es/m/20250707192654.9e.nmisch@google.com Backpatch-through: 13-17	1 month ago
Tom Lane	19857437b0	Don't put library-supplied -L/-I switches before user-supplied ones. ... For many optional libraries, we extract the -L and -l switches needed to link the library from a helper program such as llvm-config. In some cases we put the resulting -L switches into LDFLAGS ahead of -L switches specified via --with-libraries. That risks breaking the user's intention for --with-libraries. It's not such a problem if the library's -L switch points to a directory containing only that library, but on some platforms a library helper may "helpfully" offer a switch such as -L/usr/lib that points to a directory holding all standard libraries. If the user specified --with-libraries in hopes of overriding the standard build of some library, the -L/usr/lib switch prevents that from happening since it will come before the user-specified directory. To fix, avoid inserting these switches directly into LDFLAGS during configure, instead adding them to LIBDIRS or SHLIB_LINK. They will still eventually get added to LDFLAGS, but only after the switches coming from --with-libraries. The same problem exists for -I switches: those coming from --with-includes should appear before any coming from helper programs such as llvm-config. We have not heard field complaints about this case, but it seems certain that a user attempting to override a standard library could have issues. The changes for this go well beyond configure itself, however, because many Makefiles have occasion to manipulate CPPFLAGS to insert locally-desirable -I switches, and some of them got it wrong. The correct ordering is any -I switches pointing at within-the- source-tree-or-build-tree directories, then those from the tree-wide CPPFLAGS, then those from helper programs. There were several places that risked pulling in a system-supplied copy of libpq headers, for example, instead of the in-tree files. (Commit cb36f8ec2 fixed one instance of that a few months ago, but this exercise found more.) The Meson build scripts may or may not have any comparable problems, but I'll leave it to someone else to investigate that. Reported-by: Charles Samborski <demurgos@demurgos.net> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/70f2155f-27ca-4534-b33d-7750e20633d7@demurgos.net Backpatch-through: 13	2 months ago
Tom Lane	0928e18eb8	Remove unnecessary complication around xmlParseBalancedChunkMemory. ... When I prepared 71c0921b6 et al yesterday, I was thinking that the logic involving explicitly freeing the node_list output was still needed to dodge leakage bugs in libxml2. But I was misremembering: we introduced that only because with early 2.13.x releases we could not trust xmlParseBalancedChunkMemory's result code, so we had to look to see if a node list was returned or not. There's no reason to believe that xmlParseBalancedChunkMemory will fail to clean up the node list when required, so simplify. (This essentially completes reverting all the non-cosmetic changes in 6082b3d5d.) Reported-by: Jim Jones <jim.jones@uni-muenster.de> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/997668.1753802857@sss.pgh.pa.us Backpatch-through: 13	2 months ago
Tom Lane	0ffbd345e4	Avoid regression in the size of XML input that we will accept. ... This mostly reverts commit 6082b3d5d, "Use xmlParseInNodeContext not xmlParseBalancedChunkMemory". It turns out that xmlParseInNodeContext will reject text chunks exceeding 10MB, while (in most libxml2 versions) xmlParseBalancedChunkMemory will not. The bleeding-edge libxml2 bug that we needed to work around a year ago is presumably no longer a factor, and the argument that xmlParseBalancedChunkMemory is semi-deprecated is not enough to justify a functionality regression. Hence, go back to doing it the old way. Reported-by: Michael Paquier <michael@paquier.xyz> Author: Michael Paquier <michael@paquier.xyz> Co-authored-by: Erik Wienhold <ewie@ewie.name> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/aIGknLuc8b8ega2X@paquier.xyz Backpatch-through: 13	2 months ago
Alexander Korotkov	b248a3ba4e	Limit checkpointer requests queue size ... If the number of sync requests is big enough, the palloc() call in AbsorbSyncRequests() will attempt to allocate more than 1 GB of memory, resulting in failure. This can lead to an infinite loop in the checkpointer process, as it repeatedly fails to absorb the pending requests. This commit limits the checkpointer requests queue size to 10M items. In addition to preventing the palloc() failure, this change helps to avoid long queue processing time. Also, this commit is for backpathing only. The master branch receives a more invasive yet comprehensive fix for this problem. Discussion: https://postgr.es/m/db4534f83a22a29ab5ee2566ad86ca92%40postgrespro.ru Backpatch-through: 13	2 months ago
Tom Lane	b252ce3111	Fix build breakage on Solaris-alikes with late-model GCC. ... Solaris has never bothered to add "const" to the second argument of PAM conversation procs, as all other Unixen did decades ago. This resulted in an "incompatible pointer" compiler warning when building --with-pam, but had no more serious effect than that, so we never did anything about it. However, as of GCC 14 the case is an error not warning by default. To complicate matters, recent OpenIndiana (and maybe illumos in general?) *does* supply the "const" by default, so we can't just assume that platforms using our solaris template need help. What we can do, short of building a configure-time probe, is to make solaris.h #define _PAM_LEGACY_NONCONST, which causes OpenIndiana's pam_appl.h to revert to the traditional definition, and hopefully will have no effect anywhere else. Then we can use that same symbol to control whether we include "const" in the declaration of pam_passwd_conv_proc(). Bug: #18995 Reported-by: Andrew Watkins <awatkins1966@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18995-82058da9ab4337a7@postgresql.org Backpatch-through: 13	2 months ago
Michael Paquier	0123922f82	ecpg: Fix NULL pointer dereference during connection lookup ... ECPGconnect() caches established connections to the server, supporting the case of a NULL connection name when a database name is not specified by its caller. A follow-up call to ECPGget_PGconn() to get an established connection from the cached set with a non-NULL name could cause a NULL pointer dereference if a NULL connection was listed in the cache and checked for a match. At least two connections are necessary to reproduce the issue: one with a NULL name and one with a non-NULL name. Author: Aleksander Alekseev <aleksander@tigerdata.com> Discussion: https://postgr.es/m/CAJ7c6TNvFTPUTZQuNAoqgzaSGz-iM4XR61D7vEj5PsQXwg2RyA@mail.gmail.com Backpatch-through: 13	2 months ago
Alexander Korotkov	9f270f48f3	Fix infinite wait when reading a partially written WAL record ... If a crash occurs while writing a WAL record that spans multiple pages, the recovery process marks the page with the XLP_FIRST_IS_OVERWRITE_CONTRECORD flag. However, logical decoding currently attempts to read the full WAL record based on its expected size before checking this flag, which can lead to an infinite wait if the remaining data is never written (e.g., no activity after crash). This patch updates the logic first to read the page header and check for the XLP_FIRST_IS_OVERWRITE_CONTRECORD flag before attempting to reconstruct the full WAL record. If the flag is set, decoding correctly identifies the record as incomplete and avoids waiting for WAL data that will never arrive. Discussion: https://postgr.es/m/CAAKRu_ZCOzQpEumLFgG_%2Biw3FTa%2BhJ4SRpxzaQBYxxM_ZAzWcA%40mail.gmail.com Discussion: https://postgr.es/m/CALDaNm34m36PDHzsU_GdcNXU0gLTfFY5rzh9GSQv%3Dw6B%2BQVNRQ%40mail.gmail.com Author: Vignesh C <vignesh21@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com> Backpatch-through: 13	2 months ago
Tom Lane	a372a64db7	Fix PQport to never return NULL unless the connection is NULL. ... This is the documented behavior, and it worked that way before v10. However, addition of the connhost[] array created cases where conn->connhost[conn->whichhost].port is NULL. The rest of libpq is careful to substitute DEF_PGPORT[_STR] for a null or empty port string, but we failed to do so here, leading to possibly returning NULL. As of v18 that causes psql's \conninfo command to segfault. Older psql versions avoid that, but it's pretty likely that other clients have trouble with this, so we'd better back-patch the fix. In stable branches, just revert to our historical behavior of returning an empty string when there was no user-given port specification. However, it seems substantially more useful and indeed more correct to hand back DEF_PGPORT_STR in such cases, so let's make v18 and master do that. Author: Daniele Varrazzo <daniele.varrazzo@gmail.com> Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA+mi_8YTS8WPZPO0PAb2aaGLwHuQ0DEQRF0ZMnvWss4y9FwDYQ@mail.gmail.com Backpatch-through: 13	2 months ago
Álvaro Herrera	c2720ac601	Remove assertion from PortalRunMulti ... We have an assertion to ensure that a command tag has been assigned by the time we're done executing, but if we happen to execute a command with no queries, the assertion would fail. Per discussion, rather than contort things to get a tag assigned, just remove the assertion. Oversight in 2f9661311b. That commit also retained a comment that explained logic that had been adjacent to it but diffused into various places, leaving none apt to keep part of the comment. Remove that part, and rewrite what remains for extra clarity. Bug: #18984 Backpatch-through: 13 Reported-by: Aleksander Alekseev <aleksander@tigerdata.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michaël Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/18984-0f4778a6599ac3ae@postgresql.org	2 months ago
Álvaro Herrera	5a261c135d	Fix dumping of comments on invalid constraints on domains ... We skip dumping constraints together with domains if they are invalid ('separate') so that they appear after data -- but their comments were dumped together with the domain definition, which in effect leads to the comment being dumped when the constraint does not yet exist. Delay them in the same way. Oversight in 7eca575d1c28; backpatch all the way back. Author: jian he <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxF_C2pe6J_+nPr6C5jf5rQnbYP8XOKr4HM8yHZtp2aQqQ@mail.gmail.com	2 months ago
Nathan Bossart	4349206cc3	psql: Fix note on project naming in output of \copyright. ... This adjusts the wording to match the changes in commits 5987553fde, a233a603ba, and pgweb commit 2d764dbc08. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/aHVo791guQR6uqwT%40nathan Backpatch-through: 13	2 months ago
Tom Lane	f32e456414	Silence uninitialized-value warnings in compareJsonbContainers(). ... Because not every path through JsonbIteratorNext() sets val->type, some compilers complain that compareJsonbContainers() is comparing possibly-uninitialized values. The paths that don't set it return WJB_DONE, WJB_END_ARRAY, or WJB_END_OBJECT, so it's clear by manual inspection that the "(ra == rb)" code path is safe, and indeed we aren't seeing warnings about that. But the (ra != rb) case is much less obviously safe. In Assert-enabled builds it seems that the asserts rejecting WJB_END_ARRAY and WJB_END_OBJECT persuade gcc 15.x not to warn, which makes little sense because it's impossible to believe that the compiler can prove of its own accord that ra/rb aren't WJB_DONE here. (In fact they never will be, so the code isn't wrong, but why is there no warning?) Without Asserts, the appearance of warnings is quite unsurprising. We discussed fixing this by converting those two Asserts into pg_assume, but that seems not very satisfactory when it's so unclear why the compiler is or isn't warning: the warning could easily reappear with some other compiler version. Let's fix it in a less magical, more future-proof way by changing JsonbIteratorNext() so that it always does set val->type. The cost of that should be pretty negligible, and it makes the function's API spec less squishy. Reported-by: Erik Rijkers <er@xs4all.nl> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/988bf1bc-3f1f-99f3-bf98-222f1cd9dc5e@xs4all.nl Discussion: https://postgr.es/m/0c623e8a204187b87b4736792398eaf1@postgrespro.ru Backpatch-through: 13	2 months ago
Tom Lane	de73cb3ed6	Fix inconsistent quoting of role names in ACLs. ... getid() and putid(), which parse and deparse role names within ACL input/output, applied isalnum() to see if a character within a role name requires quoting. They did this even for non-ASCII characters, which is problematic because the results would depend on encoding, locale, and perhaps even platform. So it's possible that putid() could elect not to quote some string that, later in some other environment, getid() will decide is not a valid identifier, causing dump/reload or similar failures. To fix this in a way that won't risk interoperability problems with unpatched versions, make getid() treat any non-ASCII as a legitimate identifier character (hence not requiring quotes), while making putid() treat any non-ASCII as requiring quoting. We could remove the resulting excess quoting once we feel that no unpatched servers remain in the wild, but that'll be years. A lesser problem is that getid() did the wrong thing with an input consisting of just two double quotes (""). That has to represent an empty string, but getid() read it as a single double quote instead. The case cannot arise in the normal course of events, since we don't allow empty-string role names. But let's fix it while we're here. Although we've not heard field reports of problems with non-ASCII role names, there's clearly a hazard there, so back-patch to all supported versions. Reported-by: Peter Eisentraut <peter@eisentraut.org> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/3792884.1751492172@sss.pgh.pa.us Backpatch-through: 13	2 months ago
Masahiko Sawada	e3584e457d	Fix tab-completion for COPY and \copy options. ... Commit c273d9d8ce reworked tab-completion of COPY and \copy in psql and added support for completing options within WITH clauses. However, the same COPY options were suggested for both COPY TO and COPY FROM commands, even though some options are only valid for one or the other. This commit separates the COPY options for COPY FROM and COPY TO commands to provide more accurate auto-completion suggestions. Back-patch to v14 where tab-completion for COPY and \copy options within WITH clauses was first supported. Author: Atsushi Torikoshi <torikoshia@oss.nttdata.com> Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp> Discussion: https://postgr.es/m/079e7a2c801f252ae8d522b772790ed7@oss.nttdata.com Backpatch-through: 14	2 months ago
Tom Lane	c65c36ab51	Restore the ability to run pl/pgsql expression queries in parallel. ... pl/pgsql's notion of an "expression" is very broad, encompassing any SQL SELECT query that returns a single column and no more than one row. So there are cases, for example evaluation of an aggregate function, where the query involves significant work and it'd be useful to run it with parallel workers. This used to be possible, but commits 3eea7a0c9 et al unintentionally disabled it. The simplest fix is to make exec_eval_expr() pass maxtuples = 0 rather than 2 to exec_run_select(). This avoids the new rule that we will never use parallelism when a nonzero "count" limit is passed to ExecutorRun(). (Note that the pre-3eea7a0c9 behavior was indeed unsafe, so reverting that rule is not in the cards.) The reason for passing 2 before was that exec_eval_expr() will throw an error if it gets more than one returned row, so we figured that as soon as we have two rows we know that will happen and we might as well stop running the query. That choice was cost-free when it was made; but disabling parallelism is far from cost-free, so now passing 2 amounts to optimizing a failure case at the expense of useful cases. An expression query that can return more than one row is certainly broken. People might now need to wait a bit longer to discover such breakage; but hopefully few will use enormously expensive cases as their first test of new pl/pgsql logic. Author: Dipesh Dhameliya <dipeshdhameliya125@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CABgZEgdfbnq9t6xXJnmXbChNTcWFjeM_6nuig41tm327gYi2ig@mail.gmail.com Backpatch-through: 13	2 months ago
Álvaro Herrera	bd48455d88	Fix new pg_upgrade query not to rely on regnamespace ... That was invented in 9.5, and pg_upgrade claims to support back to 9.0. But we don't need that with a simple query change, tested by Tom Lane. Discussion: https://postgr.es/m/202507041645.afjl5rssvrgu@alvherre.pgsql	2 months ago
Álvaro Herrera	0f49586850	pg_upgrade: check for inconsistencies in not-null constraints w/inheritance ... With tables defined like this, CREATE TABLE ip (id int PRIMARY KEY); CREATE TABLE ic (id int) INHERITS (ip); ALTER TABLE ic ALTER id DROP NOT NULL; pg_upgrade fails during the schema restore phase due to this error: ERROR: column "id" in child table must be marked NOT NULL This can only be fixed by marking the child column as NOT NULL before the upgrade, which could take an arbitrary amount of time (because ic's data must be scanned). Have pg_upgrade's check mode warn if that condition is found, so that users know what to adjust before running the upgrade for real. Author: Ali Akbar <the.apaan@gmail.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Backpatch-through: 13 Discussion: https://postgr.es/m/CACQjQLoMsE+1pyLe98pi0KvPG2jQQ94LWJ+PTiLgVRK4B=i_jg@mail.gmail.com	2 months ago
Michael Paquier	dcbbd43317	Disable commit timestamps during bootstrap ... Attempting to use commit timestamps during bootstrapping leads to an assertion failure, that can be reached for example with an initdb -c that enables track_commit_timestamp. It makes little sense to register a commit timestamp for a BootstrapTransactionId, so let's disable the activation of the module in this case. This problem has been independently reported once by each author of this commit. Each author has proposed basically the same patch, relying on IsBootstrapProcessingMode() to skip the use of commit_ts during bootstrap. The test addition is a suggestion by me, and is applied down to v16. Author: Hayato Kuroda <kuroda.hayato@fujitsu.com> Author: Andy Fan <zhihuifan1213@163.com> Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/OSCPR01MB14966FF9E4C4145F37B937E52F5102@OSCPR01MB14966.jpnprd01.prod.outlook.com Discussion: https://postgr.es/m/87plejmnpy.fsf@163.com Backpatch-through: 13	2 months ago
Tom Lane	e6dd6e6ee6	Obtain required table lock during cross-table updates, redux. ... Commits 8319e5cb5 et al missed the fact that ATPostAlterTypeCleanup contains three calls to ATPostAlterTypeParse, and the other two also need protection against passing a relid that we don't yet have lock on. Add similar logic to those code paths, and add some test cases demonstrating the need for it. In v18 and master, the test cases demonstrate that there's a behavioral discrepancy between stored generated columns and virtual generated columns: we disallow changing the expression of a stored column if it's used in any composite-type columns, but not that of a virtual column. Since the expression isn't actually relevant to either sort of composite-type usage, this prohibition seems unnecessary; but changing it is a matter for separate discussion. For now we are just documenting the existing behavior. Reported-by: jian he <jian.universality@gmail.com> Author: jian he <jian.universality@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: CACJufxGKJtGNRRSXfwMW9SqVOPEMdP17BJ7DsBf=tNsv9pWU9g@mail.gmail.com Backpatch-through: 13	2 months ago
Michael Paquier	d44efe87ea	Fix bug in archive streamer with LZ4 decompression ... When decompressing some input data, the calculation for the initial starting point and the initial size were incorrect, potentially leading to failures when decompressing contents with LZ4. These initialization points are fixed in this commit, bringing the logic closer to what exists for gzip and zstd. The contents of the compressed data is clear (for example backups taken with LZ4 can still be decompressed with a "lz4" command), only the decompression part reading the input data was impacted by this issue. This code path impacts pg_basebackup and pg_verifybackup, which can use the LZ4 decompression routines with an archive streamer, or any tools that try to use the archive streamers in src/fe_utils/. The issue is easier to reproduce with files that have a low-compression rate, like ones filled with random data, for a size of at least 512kB, but this could happen with anything as long as it is stored in a data folder. Some tests are added based on this idea, with a file filled with random bytes grabbed from the backend, written at the root of the data folder. This is proving good enough to reproduce the original problem. Author: Mikhail Gribkov <youzhick@gmail.com> Discussion: https://postgr.es/m/CAMEv5_uQS1Hg6KCaEP2JkrTBbZ-nXQhxomWrhYQvbdzR-zy-wA@mail.gmail.com Backpatch-through: 15	2 months ago
Peter Eisentraut	37b1fce858	Update comment for IndexInfo.ii_NullsNotDistinct ... Commit 7a7b3e11e6 added the ii_NullsNotDistinct field, but the comment was not updated. Author: Japin Li <japinli@hotmail.com> Reviewed-by: Richard Guo <guofenglinux@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/ME0P300MB04453E6C7EA635F0ECF41BFCB6832%40ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM	2 months ago
Tom Lane	0fb496c704	Make safeguard against incorrect flags for fsync more portable. ... The existing code assumed that O_RDONLY is defined as 0, but this is not required by POSIX and is not true on GNU Hurd. We can avoid the assumption by relying on O_ACCMODE to mask the fcntl() result. (Hopefully, all supported platforms define that.) Author: Michael Banck <mbanck@gmx.net> Co-authored-by: Samuel Thibault Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/6862e8d1.050a0220.194b8d.76fa@mx.google.com Discussion: https://postgr.es/m/68480868.5d0a0220.1e214d.68a6@mx.google.com Backpatch-through: 13	2 months ago
Tom Lane	614ffb26d1	Obtain required table lock during cross-table constraint updates. ... Sometimes a table's constraint may depend on a column of another table, so that we have to update the constraint when changing the referenced column's type. We need to have lock on the constraint's table to do that. ATPostAlterTypeCleanup believed that this case was only possible for FOREIGN KEY constraints, but it's wrong at least for CHECK and EXCLUDE constraints; and in general, we'd probably need exclusive lock to alter any sort of constraint. So just remove the contype check and acquire lock for any other table. This prevents a "you don't have lock" assertion failure, though no ill effect is observed in production builds. We'll error out later anyway because we don't presently support physically altering column types within stored composite columns. But the catalog-munging is basically all there, so we may as well make that part work. Bug: #18970 Reported-by: Alexander Lakhin <exclusion@gmail.com> Diagnosed-by: jian he <jian.universality@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18970-a7d1cfe1f8d5d8d9@postgresql.org Backpatch-through: 13	3 months ago
Nathan Bossart	4dc48ad28d	Use correct DatumGet*() function in test_shm_mq_main(). ... This is purely cosmetic, as dsm_attach() interprets its argument as a dsm_handle (i.e., an unsigned integer), but we might as well fix it. Oversight in commit 4db3744f1f. Author: Jianghua Yang <yjhjstz@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CAAZLFmRxkUD5jRs0W3K%3DUe4_ZS%2BRcAb0PCE1S0vVJBn3sWH2UQ%40mail.gmail.com Backpatch-through: 13	3 months ago