postgres

Commit Graph

Author	SHA1	Message	Date
Fujii Masao	488c35f958	doc: Fix confusing description of streaming option in START_REPLICATION. Previously, the documentation described the streaming option as a boolean, which is outdated since it's no longer a boolean as of protocol version 4. This could confuse users. This commit updates the description to remove the "boolean" reference and clearly list the valid values for the streaming option. Back-patch to v16, where the streaming option changed to a non-boolean. Author: Euler Taveira <euler@eulerto.com> Reviewed-by: Fujii Masao <masao.fujii@gmail.com> Discussion: https://postgr.es/m/8d21fb98-5c25-4dee-8387-e5a62b01ea7d@app.fastmail.com Backpatch-through: 16	6 months ago
Tom Lane	b70a2e8a1a	Doc: clarify description of regexp fields in pg_ident.conf. The grammar was a little shaky and confusing here, so word-smith it a bit. Also, adjust the comments in pg_ident.conf.sample to use the same terminology as the SGML docs, in particular "DATABASE-USERNAME" not "PG-USERNAME". Back-patch appropriate subsets. I did not risk changing pg_ident.conf.sample in released branches, but it still seems OK to change it in v18. Reported-by: Alexey Shishkin <alexey.shishkin@enterprisedb.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David G. Johnston <david.g.johnston@gmail.com> Discussion: https://postgr.es/m/175206279327.3157504.12519088928605422253@wrigleys.postgresql.org Backpatch-through: 13	6 months ago
Tom Lane	5a2139a906	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	6 months ago
Tom Lane	bdb052f53b	Doc: clarify description of current-date/time functions. Minor wordsmithing of the func.sgml paragraph describing statement_timestamp() and allied functions: don't switch between "statement" and "command" when those are being used to mean about the same thing. Also, add some text to protocol.sgml describing the perhaps-surprising behavior these functions have in a multi-statement Query message. Reported-by: P M <petermittere@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Reviewed-by: David G. Johnston <david.g.johnston@gmail.com> Discussion: https://postgr.es/m/175223006802.3157505.14764328206246105568@wrigleys.postgresql.org Backpatch-through: 13	6 months ago
Tom Lane	50959f96e7	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	6 months ago
Amit Kapila	24f6c1bd41	Fix the handling of two GUCs during upgrade. Previously, the check_hook functions for max_slot_wal_keep_size and idle_replication_slot_timeout would incorrectly raise an ERROR for values set in postgresql.conf during upgrade, even though those values were not actively used in the upgrade process. To prevent logical slot invalidation during upgrade, we used to set special values for these GUCs. Now, instead of relying on those values, we directly prevent WAL removal and logical slot invalidation caused by max_slot_wal_keep_size and idle_replication_slot_timeout. Note: PostgreSQL 17 does not include the idle_replication_slot_timeout GUC, so related changes were not backported. BUG #18979 Reported-by: jorsol <jorsol@gmail.com> Author: Dilip Kumar <dilipbalaut@gmail.com> Reviewed by: vignesh C <vignesh21@gmail.com> Reviewed by: Alvaro Herrera <alvherre@alvh.no-ip.org> Backpatch-through: 17, where it was introduced Discussion: https://postgr.es/m/219561.1751826409@sss.pgh.pa.us Discussion: https://postgr.es/m/18979-a1b7fdbb7cd181c6@postgresql.org	6 months ago
Nathan Bossart	e43fae199e	pg_dump: Fix object-type sort priority for large objects. Commit `a45c78e328` moved large object metadata from SECTION_PRE_DATA to SECTION_DATA but neglected to move PRIO_LARGE_OBJECT in dbObjectTypePriorities accordingly. While this hasn't produced any known live bugs, it causes problems for a proposed patch that optimizes upgrades with many large objects. Fixing the priority might also make the topological sort step marginally faster by reducing the number of ordering violations that have to be fixed. Reviewed-by: Nitin Motiani <nitinmotiani@google.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/aBkQLSkx1zUJ-LwJ%40nathan Discussion: https://postgr.es/m/aG_5DBCjdDX6KAoD%40nathan Backpatch-through: 17	6 months ago
Masahiko Sawada	c1c6169eb4	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	6 months ago
Amit Kapila	f2e41d9e2f	Doc: Improve logical replication failover documentation. Clarified that the failover steps apply to a specific PostgreSQL subscriber and added guidance for verifying replication slot synchronization during planned failover. Additionally, corrected the standby query to avoid false positives by checking invalidation_reason IS NULL instead of conflicting. Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Author: Shveta Malik <shveta.malik@gmail.com> Backpatch-through: 17, where it was introduced Discussion: https://www.postgresql.org/message-id/CAExHW5uiZ-fF159=jwBwPMbjZeZDtmcTbN+hd4mrURLCg2uzJg@mail.gmail.com	6 months ago
Tom Lane	95cf1a1817	Fix low-probability memory leak in XMLSERIALIZE(... INDENT). xmltotext_with_options() did not consider the possibility that pg_xml_init() could fail --- most likely due to OOM. If that happened, the already-parsed xmlDoc structure would be leaked. Oversight in commit `483bdb2af`. Bug: #18981 Author: Dmitry Kovalenko <d.kovalenko@postgrespro.ru> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18981-9bc3c80f107ae925@postgresql.org Backpatch-through: 16	6 months ago
Tom Lane	a553a2289d	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	6 months ago
Michael Paquier	c911e7802e	Fix incompatibility with libxml2 >= 2.14 libxml2 has deprecated the members of xmlBuffer, and it is recommended to access them with dedicated routines. We have only one case in the tree where this shows an impact: xml2/xpath.c where "content" was getting directly accessed. The rest of the code looked fine, checking the PostgreSQL code with libxml2 close to the top of its "2.14" branch. xmlBufferContent() exists since year 2000 based on a check of the upstream libxml2 tree, so let's switch to it. Like `400928b83b`, backpatch all the way down as this can have an impact on all the branches already released once newer versions of libxml2 get more popular. Reported-by: Walid Ibrahim <walidib@amazon.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/aGdSdcR4QTjEHX6s@paquier.xyz Backpatch-through: 13	6 months ago
Álvaro Herrera	bcb8d47cdd	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	6 months ago
Álvaro Herrera	0807a4656e	pg_upgrade: Add missing newline in error message Minor oversight in `347758b120`	6 months ago
Álvaro Herrera	b8b2e60527	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	6 months ago
Michael Paquier	ae20c105f0	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	6 months ago
Tom Lane	6d4395b402	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	6 months ago
Fujii Masao	ba9201b57b	doc: Update outdated descriptions of wal_status in pg_replication_slots. The documentation for pg_replication_slots previously mentioned only max_slot_wal_keep_size as a condition under which the wal_status column could show unreserved or lost. However, since commit `be87200`, replication slots can also be invalidated due to horizon or wal_level, and since commit `ac0e33136a`, idle_replication_slot_timeout can also trigger this state. This commit updates the description of the wal_status column to reflect that max_slot_wal_keep_size is not the only cause of the lost state. Back-patched to v16, where the additional invalidation cases were introduced. Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Reviewed-by: Nisha Moond <nisha.moond412@gmail.com> Discussion: https://postgr.es/m/78b34e84-2195-4f28-a151-5d204a382fdd@oss.nttdata.com Backpatch-through: 16	6 months ago
Fujii Masao	27224e660a	doc: Remove incorrect note about wal_status in pg_replication_slots. The documentation previously stated that the wal_status column is NULL if restart_lsn is NULL in the pg_replication_slots view. This is incorrect, and wal_status can be "lost" even when restart_lsn is NULL. This commit removes the incorrect description. Back-patched to all supported versions. Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Nisha Moond <nisha.moond412@gmail.com> Discussion: https://postgr.es/m/c9d23cdc-b5dd-455a-8ee9-f1f24d701d89@oss.nttdata.com Backpatch-through: 13	6 months ago
Tom Lane	445bd37b19	Correctly copy the target host identification in PQcancelCreate. PQcancelCreate failed to copy struct pg_conn_host's "type" field, instead leaving it zero (a/k/a CHT_HOST_NAME). This seemingly has no great ill effects if it should have been CHT_UNIX_SOCKET instead, but if it should have been CHT_HOST_ADDRESS then a null-pointer dereference will occur when the cancelConn is used. Bug: #18974 Reported-by: Maxim Boguk <maxim.boguk@gmail.com> Author: Sergei Kornilov <sk@zsrv.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18974-575f02b2168b36b3@postgresql.org Backpatch-through: 17	6 months ago
Masahiko Sawada	792238c8b4	Fix missing FSM vacuum opportunities on tables without indexes. Commit `c120550edb` optimized the vacuuming of relations without indexes (a.k.a. one-pass strategy) by directly marking dead item IDs as LP_UNUSED. However, the periodic FSM vacuum was still checking if dead item IDs had been marked as LP_DEAD when attempting to vacuum the FSM every VACUUM_FSM_EVERY_PAGES blocks. This condition was never met due to the optimization, resulting in missed FSM vacuum opportunities. This commit modifies the periodic FSM vacuum condition to use the number of tuples deleted during HOT pruning. This count includes items marked as either LP_UNUSED or LP_REDIRECT, both of which are expected to result in new free space to report. Back-patch to v17 where the vacuum optimization for tables with no indexes was introduced. Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Discussion: https://postgr.es/m/CAD21AoBL8m6B9GSzQfYxVaEgvD7-Kr3AJaS-hJPHC+avm-29zw@mail.gmail.com Backpatch-through: 17	6 months ago
Michael Paquier	0740034318	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	6 months ago
Peter Eisentraut	c6d0ef160e	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	6 months ago
Nathan Bossart	f5551538bc	Document pg_get_multixact_members(). Oversight in commit `0ac5ad5134`. Author: Sami Imseih <samimseih@gmail.com> Co-authored-by: Álvaro Herrera <alvherre@kurilemu.de> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://postgr.es/m/20150619215231.GT133018%40postgresql.org Discussion: https://postgr.es/m/CAA5RZ0sjQDDwJfMRb%3DZ13nDLuRpF13ME2L_BdGxi0op8RKjmDg%40mail.gmail.com Backpatch-through: 13	6 months ago
Peter Eisentraut	9d14d40ec5	Fix outdated comment for IndexInfo Commit `7841623571` removed the ii_OpclassOptions 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	6 months ago
Tom Lane	0991249d7a	Make sure IOV_MAX is defined. We stopped defining IOV_MAX on non-Windows systems in `75357ab94`, on the assumption that every non-Windows system defines it in <limits.h> as required by X/Open. GNU Hurd, however, doesn't follow that standard either. Put back the old logic to assume 16 if it's not defined. Author: Michael Banck <mbanck@gmx.net> Co-authored-by: Christoph Berg <myon@debian.org> 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/6846e0c3.df0a0220.39ef9b.c60e@mx.google.com Backpatch-through: 16	6 months ago
Tom Lane	29c54ea7b4	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	6 months ago
Amit Langote	98749132e8	Fix typos in comments Commit `19d8e2308b` added enum values with the prefix TU_, but a few comments still referred to TUUI_, which was used in development versions of the patches committed as `19d8e2308b`. Author: Yugo Nagata <nagata@sraoss.co.jp> Discussion: https://postgr.es/m/20250701110216.8ac8a9e4c6f607f1d954f44a@sraoss.co.jp Backpatch-through: 16	6 months ago
Daniel Gustafsson	d32933cea5	doc: Fix typo in pg_sync_replication_slots documentation Commit `1546e17f9d` accidentally misspelled additionally as additionaly. Backpatch to v17 to match where the original commit was backpatched. Author: Daniel Gustafsson <daniel@yesql.se> Backpatch-through: 17	6 months ago
Tom Lane	bbfcbc4cde	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	6 months ago
Nathan Bossart	07402e370e	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	6 months ago
Melanie Plageman	40c9b4d8da	Remove unused check in heap_xlog_insert() `8e03eb92e9` reverted the commit `39b66a91bd` which allowed freezing in the heap_insert() code path but forgot to remove the corresponding check in heap_xlog_insert(). This code is extraneous but not harmful. However, cleaning it up makes it very clear that, as of now, we do not support any freezing of pages in the heap_insert() path. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Tomas Vondra <tomas@vondra.me> Discussion: https://postgr.es/m/flat/CAAKRu_Zp4Pi-t51OFWm1YZ-cctDfBhHCMZ%3DEx6PKxv0o8y2GvA%40mail.gmail.com Backpatch-through: 14	6 months ago
Dean Rasheed	457f1a9203	doc: Fix indentation of MERGE synopsis. The convention in the documentation for other SQL commands is to indent continuation lines and sub-clauses in the "Synopsis" section by 4 spaces, so do the same for MERGE. Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/CAEZATCV+9tR9+WM-SCcdBEZ3x7WVxUpADD5jX9WeGX97z4LCGA@mail.gmail.com Backpatch-through: 15	6 months ago
Amit Kapila	a0e7e9799c	Doc: Improve documentation of stream abort. Protocol v4 introduces parallel streaming, which allows Stream Abort messages to include additional abort information such as LSN and timestamp. However, the current documentation only states, "This field is available since protocol version 4," which may misleadingly suggest that the fields are always present when using protocol v4. This patch clarifies that the abort LSN and timestamp are included only when parallel streaming is enabled, even under protocol v4. Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 16, where it was introduced Discussion: https://postgr.es/m/CAO6_XqoKteQR1AnaR8iPcegbBE+HkAc2-g12rxN04yOt4-2ORg@mail.gmail.com	6 months ago
Michael Paquier	2e0b5d252b	Avoid scribbling of VACUUM options This fixes two issues with the handling of VacuumParams in vacuum_rel(). This code path has the idea to change the passed-in pointer of VacuumParams for the "truncate" and "index_cleanup" options for the relation worked on, impacting the two following scenarios where incorrect options may be used because a VacuumParams pointer is shared across multiple relations: - Multiple relations in a single VACUUM command. - TOAST relations vacuumed with their main relation. The problem is avoided by providing to the two callers of vacuum_rel() copies of VacuumParams, before the pointer is updated for the "truncate" and "index_cleanup" options. The refactoring of the VACUUM option and parameters done in `0d83138974` did not introduce an issue, but it has encouraged the problem we are dealing with in this commit, with `b84dbc8eb8` for "truncate" and `a96c41feec` for "index_cleanup" that have been added a couple of years after the initial refactoring. HEAD will be improved with a different patch that hardens the uses of VacuumParams across the tree. This cannot be backpatched as it introduces an ABI breakage. The backend portion of the patch has been authored by Nathan, while I have implemented the tests. The tests rely on injection points to check the option values, making them faster, more reliable than the tests originally proposed by Shihao, and they also provide more coverage. This part can only be backpatched down to v17. Reported-by: Shihao Zhong <zhong950419@gmail.com> Author: Nathan Bossart <nathandbossart@gmail.com> Co-authored-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CAGRkXqTo+aK=GTy5pSc-9cy8H2F2TJvcrZ-zXEiNJj93np1UUw@mail.gmail.com Backpatch-through: 13	6 months ago
Tom Lane	9f33300e69	Prevent excessive delays before launching new logrep workers. The logical replication launcher process would sometimes sleep for as much as 3 minutes before noticing that it is supposed to launch a new worker. This could happen if (1) WaitForReplicationWorkerAttach absorbed a process latch wakeup that was meant to cause ApplyLauncherMain to do work, or (2) logicalrep_worker_launch reported failure, either because of resource limits or because the new worker terminated immediately. In case (2), the expected behavior is that we retry the launch after wal_retrieve_retry_interval, but that didn't reliably happen. It's not clear how often such conditions would occur in the field, but in our subscription test suite they are somewhat common, especially in tests that exercise cases that cause quick worker failure. That causes the tests to take substantially longer than they ought to do on typical setups. To fix (1), make WaitForReplicationWorkerAttach re-set the latch before returning if it cleared it while looping. To fix (2), ensure that we reduce wait_time to no more than wal_retrieve_retry_interval when logicalrep_worker_launch reports failure. In passing, fix a couple of perhaps-hypothetical race conditions, e.g. examining worker->in_use without a lock. Backpatch to v16. Problem (2) didn't exist before commit `5a3a95385` because the previous code always set wait_time to wal_retrieve_retry_interval when launching a worker, regardless of success or failure of the launch. That behavior also greatly mitigated problem (1), so I'm not excited about adapting the remainder of the patch to the substantially-different code in older branches. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> Discussion: https://postgr.es/m/817604.1750723007@sss.pgh.pa.us Backpatch-through: 16	7 months ago
Melanie Plageman	2c0bc47657	Test that vacuum removes tuples older than OldestXmin If vacuum fails to prune a tuple killed before OldestXmin, it will decide to freeze its xmax and later error out in pre-freeze checks. Add a test reproducing this scenario to the recovery suite which creates a table on a primary, updates the table to generate dead tuples for vacuum, and then, during the vacuum, uses a replica to force GlobalVisState->maybe_needed on the primary to move backwards and precede the value of OldestXmin set at the beginning of vacuuming the table. This test is coverage for a case fixed in `83c39a1f7f`. The test was originally committed to master in `aa607980ae` but later reverted in `efcbb76efe` due to test instability. The test requires multiple index passes. In Postgres 17+, vacuum uses a TID store for the dead TIDs that is very space efficient. With the old minimum maintenance_work_mem of 1 MB, it required a large number of dead rows to generate enough dead TIDs to force multiple index vacuuming passes. Once the source code changes were made to allow a minimum maintenance_work_mem value of 64kB, the test could be made much faster and more stable. Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: John Naylor <johncnaylorls@gmail.com> Reviewed-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAAKRu_ZJBkidusDut6i%3DbDCiXzJEp93GC1%2BNFaZt4eqanYF3Kw%40mail.gmail.com Backpatch-through: 17	7 months ago
Daniel Gustafsson	805b85027a	doc: Remove dead link to NewbieDoc Docbook Guide The link returns 404 and no replacement is available in the project on Sourceforge where the content once was. Since we already link to resources for both beginner and experienced docs hackers, remove the the dead link. Backpatch to all supported versions as the link was added in 8.1. Author: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Magnus Hagander <magnus@hagander.net> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reported-by: jian he <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxH=YzQPDOe+2WuYZ7seD-BOyjCBmP6JiErpoSiVZWDRnw@mail.gmail.com Backpatch-through: 13	7 months ago
Fujii Masao	589c5f8356	doc: Fix incorrect UUID index entry in function documentation. Previously, the UUID functions documentation defined the "UUID" index entry to link to the UUID data type page, even though that entry already exists there. Instead, the UUID functions page should define its own index entry linking to itself. This commit updates the UUID index entry in the UUID functions documentation to point to the correct section, improving navigation and avoiding duplication. Back-patch to all supported versions. Author: Fujii Masao <masao.fujii@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/f33e0493-5773-4296-87c5-7ce459054cfe@oss.nttdata.com Backpatch-through: 13	7 months ago
Peter Eisentraut	2499c3490b	meson: Fix meson warning WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in meson 2.0. Introduced by commit `bc46104fc9`. (This only happens in the msvc branch. All the other run_command calls are ok.) Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/42e13eb0-862a-441e-8d84-4f0fd5f6def0%40eisentraut.org	7 months ago
Tom Lane	9575e540f6	Doc: improve documentation about width_bucket(). Specify whether the bucket bounds are inclusive or exclusive, and improve some other vague language. Explain the behavior that occurs when the "low" bound is greater than the "high" bound. Make width_bucket_numeric's comment more like that for width_bucket_float8, in particular noting that infinite bounds are rejected (since they became possible in v14). Reported-by: Ben Peachey Higdon <bpeacheyhigdon@gmail.com> Author: Robert Treat <rob@xzilla.net> Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com> Discussion: https://postgr.es/m/2BD74F86-5B89-4AC1-8F13-23CED3546AC1@gmail.com Backpatch-through: 13	7 months ago
Tom Lane	fdd8269223	Use SnapshotDirty when checking for conflicting index names. While choosing an autogenerated name for an index, look for pre-existing relations using a SnapshotDirty snapshot, instead of the previous behavior that considered only committed-good pg_class rows. This allows us to detect and avoid conflicts against indexes that are still being built. It's still possible to fail due to a race condition, but the window is now just the amount of time that it takes DefineIndex to validate all its parameters, call smgrcreate(), and enter the index's pg_class row. Formerly the race window covered the entire time needed to create and fill an index, which could be very long if the table is large. Worse, if the conflicting index creation is part of a larger transaction, it wouldn't be visible till COMMIT. So this isn't a complete solution, but it should greatly ameliorate the problem, and the patch is simple enough to be back-patchable. It might at some point be useful to do the same for pg_constraint entries (cf. ChooseConstraintName, ConstraintNameExists, and related functions). However, in the absence of field complaints, I'll leave that alone for now. The relation-name test should be good enough for index-based constraints, while foreign-key constraints seem to be okay since they require exclusive locks to create. Bug: #18959 Reported-by: Maximilian Chrzan <maximilian.chrzan@here.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Dilip Kumar <dilipbalaut@gmail.com> Discussion: https://postgr.es/m/18959-f63b53b864bb1417@postgresql.org Backpatch-through: 13	7 months ago
Alexander Korotkov	5ed50f9386	Improve runtime and output of tests for replication slots checkpointing. The TAP tests that verify logical and physical replication slot behavior during checkpoints (046_checkpoint_logical_slot.pl and 047_checkpoint_physical_slot.pl) inserted two batches of 2 million rows each, generating approximately 520 MB of WAL. On slow machines, or when compiled with '-DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE', this caused the tests to run for 8-9 minutes and occasionally time out, as seen on the buildfarm animal prion. This commit modifies the mentioned tests to utilize the $node->advance_wal() function, thereby reducing runtime. Once we do not use the generated data, the proposed function is a good alternative, which cuts the total wall-clock run time. While here, remove superfluous '\n' characters from several note() calls; these appeared literally in the build-farm logs and looked odd. Also, remove excessive 'shared_preload_libraries' GUC from the config and add a check for 'injection_points' extension availability. Reported-by: Alexander Lakhin <exclusion@gmail.com> Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Author: Alexander Korotkov <aekorotkov@gmail.com> Author: Vitaly Davydov <v.davydov@postgrespro.ru> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/fbc5d94e-6fbd-4a64-85d4-c9e284a58eb2%40gmail.com Backpatch-through: 17	7 months ago
Jeff Davis	0f5ac7f321	Correct docs about partitions and EXCLUDE constraints. In version 17 we added support for cross-partition EXCLUDE constraints, as long as they included all partition key columns and compared them with equality (see `8c852ba9a4`). I updated the docs for exclusion constraints, but I missed that the docs for CREATE TABLE still said that they were not supported. This commit fixes that. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Co-authored-by: Jeff Davis <pgsql@j-davis.com> Discussion: https://postgr.es/m/c955d292-b92d-42d1-a2a0-1ec6715a2546@illuminatedcomputing.com Backpatch-through: 17	7 months ago
Amit Kapila	25505082f0	Improve log messages and docs for slot synchronization. Improve the clarity of LOG messages when a failover logical slot synchronization fails, making the reasons more explicit for easier debugging. Update the documentation to outline scenarios where slot synchronization can fail, especially during the initial sync, and emphasize that pg_sync_replication_slot() is primarily intended for testing and debugging purposes. We also discussed improving the functionality of pg_sync_replication_slot() so that it can be used reliably, but we would take up that work for next version after some more discussion and review. Reported-by: Suraj Kharage <suraj.kharage@enterprisedb.com> Author: shveta malik <shveta.malik@gmail.com> Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com> Reviewed-by: Peter Smith <smithpb2250@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 17, where it was introduced Discussion: https://postgr.es/m/CAF1DzPWTcg+m+x+oVVB=y4q9=PYYsL_mujVp7uJr-_oUtWNGbA@mail.gmail.com	7 months ago
Masahiko Sawada	45c357e0e8	Fix re-distributing previously distributed invalidation messages during logical decoding. Commit `4909b38af0` introduced logic to distribute invalidation messages from catalog-modifying transactions to all concurrent in-progress transactions. However, since each transaction distributes not only its original invalidation messages but also previously distributed messages to other transactions, this leads to an exponential increase in allocation request size for invalidation messages, ultimately causing memory allocation failure. This commit fixes this issue by tracking distributed invalidation messages separately per decoded transaction and not redistributing these messages to other in-progress transactions. The maximum size of distributed invalidation messages that one transaction can store is limited to MAX_DISTR_INVAL_MSG_PER_TXN (8MB). Once the size of the distributed invalidation messages exceeds this threshold, we invalidate all caches in locations where distributed invalidation messages need to be executed. Back-patch to all supported versions where we introduced the fix by commit `4909b38af0`. Note that this commit adds two new fields to ReorderBufferTXN to store the distributed transactions. This change breaks ABI compatibility in back branches, affecting third-party extensions that depend on the size of the ReorderBufferTXN struct, though this scenario seems unlikely. Additionally, it adds a new flag to the txn_flags field of ReorderBufferTXN to indicate distributed invalidation message overflow. This should not affect existing implementations, as it is unlikely that third-party extensions use unused bits in the txn_flags field. Bug: #18938 #18942 Author: vignesh C <vignesh21@gmail.com> Reported-by: Duncan Sands <duncan.sands@deepbluecap.com> Reported-by: John Hutchins <john.hutchins@wicourts.gov> Reported-by: Laurence Parry <greenreaper@hotmail.com> Reported-by: Max Madden <maxmmadden@gmail.com> Reported-by: Braulio Fdo Gonzalez <brauliofg@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com> Discussion: https://postgr.es/m/680bdaf6-f7d1-4536-b580-05c2760c67c6@deepbluecap.com Discussion: https://postgr.es/m/18942-0ab1e5ae156613ad@postgresql.org Discussion: https://postgr.es/m/18938-57c9a1c463b68ce0@postgresql.org Discussion: https://postgr.es/m/CAD1FGCT2sYrP_70RTuo56QTizyc+J3wJdtn2gtO3VttQFpdMZg@mail.gmail.com Discussion: https://postgr.es/m/CANO2=B=2BT1hSYCE=nuuTnVTnjidMg0+-FfnRnqM6kd23qoygg@mail.gmail.com Backpatch-through: 13	7 months ago
Alexander Korotkov	32ab0fd55d	Add TAP tests to check replication slot advance during the checkpoint The new tests verify that logical and physical replication slots are still valid after an immediate restart on checkpoint completion when the slot was advanced during the checkpoint. This commit introduces two new injection points to make these tests possible: * checkpoint-before-old-wal-removal - triggered in the checkpointer process just before old WAL segments cleanup; * logical-replication-slot-advance-segment - triggered in LogicalConfirmReceivedLocation() when restart_lsn was changed enough to point to the next WAL segment. Discussion: https://postgr.es/m/flat/1d12d2-67235980-35-19a406a0%4063439497 Author: Vitaly Davydov <v.davydov@postgrespro.ru> Author: Tomas Vondra <tomas@vondra.me> Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 17	7 months ago
Alexander Korotkov	2090edc6f3	Keep WAL segments by the flushed value of the slot's restart LSN The patch fixes the issue with the unexpected removal of old WAL segments after checkpoint, followed by an immediate restart. The issue occurs when a slot is advanced after the start of the checkpoint and before old WAL segments are removed at the end of the checkpoint. The idea of the patch is to get the minimal restart_lsn at the beginning of checkpoint (or restart point) creation and use this value when calculating the oldest LSN for WAL segments removal at the end of checkpoint. This idea was proposed by Tomas Vondra in the discussion. Unlike 291221c46575, this fix doesn't affect ABI and is intended for back branches. Discussion: https://postgr.es/m/flat/1d12d2-67235980-35-19a406a0%4063439497 Author: Vitaly Davydov <v.davydov@postgrespro.ru> Reviewed-by: Tomas Vondra <tomas@vondra.me> Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Backpatch-through: 13	7 months ago
Peter Geoghegan	40aa5ddea1	Make _bt_killitems drop pins it acquired itself. Teach nbtree's _bt_killitems to leave the so->currPos page that it sets LP_DEAD items on in whatever state it was in when _bt_killitems was called. In particular, make sure that so->dropPin scans don't acquire a pin whose reference is saved in so->currPos.buf. Allowing _bt_killitems to change so->currPos.buf like this is wrong. The immediate consequence of allowing it is that code in _bt_steppage (that copies so->currPos into so->markPos) will behave as if the scan is a !so->dropPin scan. so->markPos will therefore retain the buffer pin indefinitely, even though _bt_killitems only needs to acquire a pin (along with a lock) for long enough to mark known-dead items LP_DEAD. This issue came to light following a report of a failure of an assertion from recent commit `e6eed40e`. The test case in question involves the use of mark and restore. An initial call to _bt_killitems takes place that leaves so->currPos.buf in a state that is inconsistent with the scan being so->dropPin. A subsequent call to _bt_killitems for the same position (following so->currPos being saved in so->markPos, and then restored as so->currPos) resulted in the failure of an assertion that tests that so->currPos.buf is InvalidBuffer when the scan is so->dropPin (non-assert builds got a "resource was not closed" WARNING instead). The same problem exists on earlier releases, though the issue is far more subtle there. Recent commit `e6eed40e` introduced the so->dropPin field as a partial replacement for testing so->currPos.buf directly. Earlier releases won't get an assertion failure (or buffer pin leak), but they will allow the second _bt_killitems call from the test case to behave as if a buffer pin was consistently held since the original call to _bt_readpage. This is wrong; there will have been an initial window during which no pin was held on the so->currPos page, and yet the second _bt_killitems call will neglect to check if so->currPos.lsn continues to match the page's now-current LSN. As a result of all this, it's just about possible that _bt_killitems will set the wrong items LP_DEAD (on release branches). This could only happen with merge joins (the sole user of nbtree mark/restore support), when a concurrently inserted index tuple used a recently-recycled TID (and only when the new tuple was inserted onto the same page as a distinct concurrently-removed tuple with the same TID). This is exactly the scenario that _bt_killitems' check of the page's now-current LSN against the LSN stashed in currPos was supposed to prevent. A follow-up commit will make nbtree completely stop conditioning whether or not a position's pin needs to be dropped on whether the 'buf' field is set. All call sites that might need to drop a still-held pin will be taught to rely on the scan-level so->dropPin field recently introduced by commit `e6eed40e`. That will make bugs of the same general nature as this one impossible (or make them much easier to detect, at least). Author: Peter Geoghegan <pg@bowt.ie> Reported-By: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/545be1e5-3786-439a-9257-a90d30f8b849@gmail.com Backpatch-through: 13	7 months ago
Tom Lane	30e0d9ee90	Don't reduce output request size on non-Unix-socket connections. Traditionally, libpq's pqPutMsgEnd has rounded down the amount-to-send to be a multiple of 8K when it is eagerly writing some data. This still seems like a good idea when sending through a Unix socket, as pipes typically have a buffer size of 8K or some fraction/multiple of that. But there's not much argument for it on a TCP connection, since (a) standard MTU values are not commensurate with that, and (b) the kernel typically applies its own packet splitting/merging logic. Worse, our SSL and GSSAPI code paths both have API stipulations that if they fail to send all the data that was offered in the previous write attempt, we mustn't offer less data in the next attempt; else we may get "SSL error: bad length" or "GSSAPI caller failed to retransmit all data needing to be retried". The previous write attempt might've been pqFlush attempting to send everything in the buffer, so pqPutMsgEnd can't safely write less than the full buffer contents. (Well, we could add some more state to track exactly how much the previous write attempt was, but there's little value evident in such extra complication.) Hence, apply the round-down only on AF_UNIX sockets, where we never use SSL or GSSAPI. Interestingly, we had a very closely related bug report before, which I attempted to fix in commit `d053a879b`. But the test case we had then seemingly didn't trigger this pqFlush-then-pqPutMsgEnd scenario, or at least we failed to recognize this variant of the bug. Bug: #18907 Reported-by: Dorjpalam Batbaatar <htgn.dbat.95@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/18907-d41b9bcf6f29edda@postgresql.org Backpatch-through: 13	7 months ago

1 2 3 4 5 ...

59562 Commits (488c35f958e772fb446a8d90beaaefeb742e84a0) All Branches Search

59562 Commits (488c35f958e772fb446a8d90beaaefeb742e84a0)

All Branches