aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* pgindent worker.c.Amit Kapila2021-01-19
| | | | | | | | | This is a leftover from commit 0926e96c49. Changing this separately because this file is being modified for upcoming patch logical replication of 2PC. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Ps+EgG8KzcmAyAgBUi_vuTps6o9ZA8DG6SdnO0-YuOhPQ@mail.gmail.com
* Avoid crash with WHERE CURRENT OF and a custom scan plan.Tom Lane2021-01-18
| | | | | | | | | | | | | | | | | | | | | | | execCurrent.c's search_plan_tree() assumed that ForeignScanStates and CustomScanStates necessarily have a valid ss_currentRelation. This is demonstrably untrue for postgres_fdw's remote join and remote aggregation plans, and non-leaf custom scans might not have an identifiable scan relation either. Avoid crashing by ignoring such nodes when the field is null. This solution will lead to errors like 'cursor "foo" is not a simply updatable scan of table "bar"' in cases where maybe we could have allowed WHERE CURRENT OF to work. That's not an issue for postgres_fdw's usages, since joins or aggregations would render WHERE CURRENT OF invalid anyway. But an otherwise-transparent upper level custom scan node might find this annoying. When and if someone cares to expend work on such a scenario, we could invent a custom-scan-provider callback to determine what's safe. Report and patch by David Geier, commentary by me. It's been like this for awhile, so back-patch to all supported branches. Discussion: https://postgr.es/m/0253344d-9bdd-11c4-7f0d-d88c02cd7991@swarm64.com
* Narrow the scope of a local variable.Tom Lane2021-01-18
| | | | | | | | | | This is better style and more symmetrical with the other if-branch. This likely should have been included in 9de77b545 (which created the opportunity), but it was overlooked. Japin Li Discussion: https://postgr.es/m/MEYP282MB16699FA4A7CD57EB250E871FB6A40@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Add bytea equivalents of ltrim() and rtrim().Tom Lane2021-01-18
| | | | | | | | We had bytea btrim() already, but for some reason not the other two. Joel Jacobson Discussion: https://postgr.es/m/d10cd5cd-a901-42f1-b832-763ac6f7ff3a@www.fastmail.com
* Allow for error or refusal while absorbing a ProcSignalBarrier.Robert Haas2021-01-18
| | | | | | | | | | | | | | | | | | | | Previously, the per-barrier-type functions tasked with absorbing them were expected to always succeed and never throw an error. However, that's a bit inconvenient. Further study has revealed that there are realistic cases where it might not be possible to absorb a ProcSignalBarrier without terminating the transaction, or even the whole backend. Similarly, for some barrier types, there might be other reasons where it's not reasonably possible to absorb the barrier at certain points in the code, so provide a way for a per-barrier-type function to reject absorbing the barrier. Unfortunately, there's still no committed code making use of this infrastructure; hopefully, we'll get there. :-( Patch by me, reviewed by Andres Freund and Amul Sul. Discussion: http://postgr.es/m/20200908182005.xya7wetdh3pndzim@alap3.anarazel.de Discussion: http://postgr.es/m/CA+Tgmob56Pk1-5aTJdVPCWFHon7me4M96ENpGe9n_R4JUjjhZA@mail.gmail.com
* Bump PGSTAT_FILE_FORMAT_IDMagnus Hagander2021-01-18
| | | | | | | This was missed in 960869da08 Reported-By: Laurenz Albe Discussion: https://postgr.es/m/4f0aacc5fe1b4bfafa32b36ecd97469fae526a75.camel@cybertec.at
* Pause recovery for insufficient parameter settingsPeter Eisentraut2021-01-18
| | | | | | | | | | | | | | | | | | When certain parameters are changed on a physical replication primary, this is communicated to standbys using the XLOG_PARAMETER_CHANGE WAL record. The standby then checks whether its own settings are at least as big as the ones on the primary. If not, the standby shuts down with a fatal error. This patch changes this behavior for hot standbys to pause recovery at that point instead. That allows read traffic on the standby to continue while database administrators figure out next steps. When recovery is unpaused, the server shuts down (as before). The idea is to fix the parameters while recovery is paused and then restart when there is a maintenance window. Reviewed-by: Sergei Kornilov <sk@zsrv.org> Discussion: https://www.postgresql.org/message-id/flat/4ad69a4c-cc9b-0dfe-0352-8b1b0cd36c7b@2ndquadrant.com
* Refactor option handling of CLUSTER, REINDEX and VACUUMMichael Paquier2021-01-18
| | | | | | | | | | | | | | | | | | | | | | This continues the work done in b5913f6. All the options of those commands are changed to use hex values rather than enums to reduce the risk of compatibility bugs when introducing new options. Each option set is moved into a new structure that can be extended with more non-boolean options (this was already the case of VACUUM). The code of REINDEX is restructured so as manual REINDEX commands go through a single routine from utility.c, like VACUUM, to ease the allocation handling of option parameters when a command needs to go through multiple transactions. This can be used as a base infrastructure for future patches related to those commands, including reindex filtering and tablespace support. Per discussion with people mentioned below, as well as Alvaro Herrera and Peter Eisentraut. Author: Michael Paquier, Justin Pryzby Reviewed-by: Alexey Kondratov, Justin Pryzby Discussion: https://postgr.es/m/X8riynBLwxAD9uKk@paquier.xyz
* Set PD_ALL_VISIBLE and visibility map bits in COPY FREEZETomas Vondra2021-01-17
| | | | | | | | | | | | | | | | | | Make sure COPY FREEZE marks the pages as PD_ALL_VISIBLE and updates the visibility map. Until now we only marked individual tuples as frozen, but page-level flags were not updated, so the first VACUUM after the COPY FREEZE had to rewrite the whole table. This is a fairly old patch, and multiple people worked on it. The first version was written by Jeff Janes, and then reworked by Pavan Deolasee and Anastasia Lubennikova. Author: Anastasia Lubennikova, Pavan Deolasee, Jeff Janes Reviewed-by: Kuntal Ghosh, Jeff Janes, Tomas Vondra, Masahiko Sawada, Andres Freund, Ibrar Ahmed, Robert Haas, Tatsuro Ishii, Darafei Praliaskouski Discussion: https://postgr.es/m/CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com Discussion: https://postgr.es/m/CAMkU%3D1w3osJJ2FneELhhNRLxfZitDgp9FPHee08NT2FQFmz_pQ%40mail.gmail.com
* Add missing array-enlargement logic to test_regex.c.Tom Lane2021-01-17
| | | | | | | | | | | The stanza to report a "partial" match could overrun the initially allocated output array, so it needs its own copy of the array-resizing logic that's in the main loop. I overlooked the need for this in ca8217c10. Per report from Alexander Lakhin. Discussion: https://postgr.es/m/3206aace-50db-e02a-bbea-76d5cdaa2cb6@gmail.com
* Revert "psql \dX: list extended statistics objects"Tomas Vondra2021-01-17
| | | | | | | | | Reverts 891a1d0bca, because the new psql command \dX only worked for users users who can read pg_statistic_ext_data catalog, and most regular users lack that privilege (the catalog may contain sensitive user data). Reported-by: Noriyoshi Shinoda Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
* Add --no-instructions parameter to initdbMagnus Hagander2021-01-17
| | | | | | | | | | | | Specifying this parameter removes the informational messages about how to start the server. This is intended for use by wrappers in different packaging systems, where those instructions would most likely be wrong anyway, but the other output from initdb would still be useful (and thus just redirecting everything to /dev/null would be bad). Author: Magnus Hagander Reviewed-By: Peter Eisentraut Discusion: https://postgr.es/m/CABUevEzo4t5bmTXF0_B9WzmuWpVbMpkNZZiGvzV8NZa-=fPqeQ@mail.gmail.com
* Add pg_stat_database counters for sessions and session timeMagnus Hagander2021-01-17
| | | | | | | | | | | | | | | This add counters for number of sessions, the different kind of session termination types, and timers for how much time is spent in active vs idle in a database to pg_stat_database. Internally this also renames the parameter "force" to disconnect. This was the only use-case for the parameter before, so repurposing it to this mroe narrow usecase makes things cleaner than inventing something new. Author: Laurenz Albe Reviewed-By: Magnus Hagander, Soumyadeep Chakraborty, Masahiro Ikeda Discussion: https://postgr.es/m/b07e1f9953701b90c66ed368656f2aef40cac4fb.camel@cybertec.at
* psql \dX: list extended statistics objectsTomas Vondra2021-01-17
| | | | | | | | | The new command lists extended statistics objects, possibly with their sizes. All past releases with extended statistics are supported. Author: Tatsuro Yamada Reviewed-by: Julien Rouhaud, Alvaro Herrera, Tomas Vondra Discussion: https://postgr.es/m/c027a541-5856-75a5-0868-341301e1624b%40nttcom.co.jp_1
* Fix pg_dump for GRANT OPTION among initial privileges.Noah Misch2021-01-16
| | | | | | | | | | | | | | | | The context is an object that no longer bears some aclitem that it bore initially. (A user issued REVOKE or GRANT statements upon the object.) pg_dump is forming SQL to reproduce the object ACL. Since initdb creates no ACL bearing GRANT OPTION, reaching this bug requires an extension where the creation script establishes such an ACL. No PGXN extension does that. If an installation did reach the bug, pg_dump would have omitted a semicolon, causing a REVOKE and the next SQL statement to fail. Separately, since the affected code exists to eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT OPTION FOR. Back-patch to 9.6, where commit 23f34fa4ba358671adab16773e79c17c92cbc870 first appeared. Discussion: https://postgr.es/m/20210109102423.GA160022@rfd.leadboat.com
* Prevent excess SimpleLruTruncate() deletion.Noah Misch2021-01-16
| | | | | | | | | | | | | | | | | Every core SLRU wraps around. With the exception of pg_notify, the wrap point can fall in the middle of a page. Account for this in the PagePrecedes callback specification and in SimpleLruTruncate()'s use of said callback. Update each callback implementation to fit the new specification. This changes SerialPagePrecedesLogically() from the style of asyncQueuePagePrecedes() to the style of CLOGPagePrecedes(). (Whereas pg_clog and pg_serial share a key space, pg_serial is nothing like pg_notify.) The bug fixed here has the same symptoms and user followup steps as 592a589a04bd456410b853d86bd05faa9432cbbb. Back-patch to 9.5 (all supported versions). Reviewed by Andrey Borodin and (in earlier versions) by Tom Lane. Discussion: https://postgr.es/m/20190202083822.GC32531@gust.leadboat.com
* Remove unnecessary pstrdup in fetch_table_list.Amit Kapila2021-01-16
| | | | | | | | | | The result of TextDatumGetCString is already palloc'ed so we don't need to allocate memory for it again. We decide not to backpatch it as there doesn't seem to be any case where it can create a meaningful leak. Author: Zhijie Hou Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/229fed2eb8c54c71a96ccb99e516eb12@G08CNEXMBPEKD05.g08.fujitsu.local
* Disallow CREATE STATISTICS on system catalogsTomas Vondra2021-01-15
| | | | | | | | | | | | | | Add a check that CREATE STATISTICS does not add extended statistics on system catalogs, similarly to indexes etc. It can be overriden using the allow_system_table_mods GUC. This bug exists since 7b504eb282c, adding the extended statistics, so backpatch all the way back to PostgreSQL 10. Author: Tomas Vondra Reported-by: Dean Rasheed Backpatch-through: 10 Discussion: https://postgr.es/m/CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb%2BrQ%40mail.gmail.com
* Improve our heuristic for selecting PG_SYSROOT on macOS.Tom Lane2021-01-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In cases where Xcode is newer than the underlying macOS version, asking xcodebuild for the SDK path will produce a pointer to the SDK shipped with Xcode, which may end up building code that does not work on the underlying macOS version. It appears that in such cases, xcodebuild's answer also fails to match the default behavior of Apple's compiler: assuming one has installed Xcode's "command line tools", there will be an SDK for the OS's own version in /Library/Developer/CommandLineTools, and the compiler will default to using that. This is all pretty poorly documented, but experimentation suggests that "xcrun --show-sdk-path" gives the sysroot path that the compiler is actually using, at least in some cases. Hence, try that first, but revert to xcodebuild if xcrun fails (in very old Xcode, it is missing or lacks the --show-sdk-path switch). Also, "xcrun --show-sdk-path" may give a path that is valid but lacks any OS version identifier. We don't really want that, since most of the motivation for wiring -isysroot into the build flags at all is to ensure that all parts of a PG installation are built against the same SDK, even when considering extensions built later and/or on a different machine. Insist on finding "N.N" in the directory name before accepting the result. (Adding "--sdk macosx" to the xcrun call seems to produce the same answer as xcodebuild, but usually more quickly because it's cached, so we also try that as a fallback.) The core reason why we don't want to use Xcode's default SDK in cases like this is that Apple's technology for introducing new syscalls does not play nice with Autoconf: for example, configure will think that preadv/pwritev exist when using a Big Sur SDK, even when building on an older macOS version where they don't exist. It'd be nice to have a better solution to that problem, but this patch doesn't attempt to fix that. Per report from Sergey Shinderuk. Back-patch to all supported versions. Discussion: https://postgr.es/m/ed3b8e5d-0da8-6ebd-fd1c-e0ac80a4b204@postgrespro.ru
* Avoid spurious wait in concurrent reindexAlvaro Herrera2021-01-15
| | | | | | | | | | | | | | This is like commit c98763bf51bf, but for REINDEX CONCURRENTLY. To wit: this flags indicates that the current process is safe to ignore for the purposes of waiting for other snapshots, when doing CREATE INDEX CONCURRENTLY and REINDEX CONCURRENTLY. This helps two processes doing either of those things not deadlock, and also avoids spurious waits. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Hamid Akhtar <hamid.akhtar@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/20201130195439.GA24598@alvherre.pgsql
* Fix calculation of how much shared memory is required to store a TOC.Fujii Masao2021-01-15
| | | | | | | | | | | Commit ac883ac453 refactored shm_toc_estimate() but changed its calculation of shared memory size for TOC incorrectly. Previously this could cause too large memory to be allocated. Back-patch to v11 where the bug was introduced. Author: Takayuki Tsunakawa Discussion: https://postgr.es/m/TYAPR01MB2990BFB73170E2C4921E2C4DFEA80@TYAPR01MB2990.jpnprd01.prod.outlook.com
* Remove PG_SHA*_DIGEST_STRING_LENGTH from sha2.hMichael Paquier2021-01-15
| | | | | | | The last reference to those variables has been removed in aef8948, so this cleans up a bit the code. Discussion: https://postgr.es/m/X//ggAqmTtt+3t7X@paquier.xyz
* Fix O(N^2) stat() calls when recycling WAL segmentsMichael Paquier2021-01-15
| | | | | | | | | | | | | | | | | | The counter tracking the last segment number recycled was getting initialized when recycling one single segment, while it should be used across a full cycle of segments recycled to prevent useless checks related to entries already recycled. This performance issue has been introduced by b2a5545, and it was first implemented in 61b86142. No backpatch is done per the lack of field complaints. Reported-by: Andres Freund, Thomas Munro Author: Michael Paquier Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20170621211016.eln6cxxp3jrv7m4m@alap3.anarazel.de Discussion: https://postgr.es/m/CA+hUKG+DRiF9z1_MU4fWq+RfJMxP7zjoptfcmuCFPeO4JM2iVg@mail.gmail.com
* pg_dump: label PUBLICATION TABLE ArchiveEntries with an owner.Tom Lane2021-01-14
| | | | | | | | | | | | | | | | | | | | | | This is the same fix as commit 9eabfe300 applied to INDEX ATTACH entries, but for table-to-publication attachments. As in that case, even though the backend doesn't record "ownership" of the attachment, we still ought to label it in the dump archive with the role name that should run the ALTER PUBLICATION command. The existing behavior causes the ALTER to be done by the original role that started the restore; that will usually work fine, but there may be corner cases where it fails. The bulk of the patch is concerned with changing struct PublicationRelInfo to include a pointer to the associated PublicationInfo object, so that we can get the owner's name out of that when the time comes. While at it, I rewrote getPublicationTables() to do just one query of pg_publication_rel, not one per table. Back-patch to v10 where this code was introduced. Discussion: https://postgr.es/m/1165710.1610473242@sss.pgh.pa.us
* Prevent drop of tablespaces used by partitioned relationsAlvaro Herrera2021-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a tablespace is used in a partitioned relation (per commits ca4103025dfe in pg12 for tables and 33e6c34c3267 in pg11 for indexes), it is possible to drop the tablespace, potentially causing various problems. One such was reported in bug #16577, where a rewriting ALTER TABLE causes a server crash. Protect against this by using pg_shdepend to keep track of tablespaces when used for relations that don't keep physical files; we now abort a tablespace if we see that the tablespace is referenced from any partitioned relations. Backpatch this to 11, where this problem has been latent all along. We don't try to create pg_shdepend entries for existing partitioned indexes/tables, but any ones that are modified going forward will be protected. Note slight behavior change: when trying to drop a tablespace that contains both regular tables as well as partitioned ones, you'd previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST. Arguably, the latter is more correct. It is possible to add protecting pg_shdepend entries for existing tables/indexes, by doing ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default; ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace; for each partitioned table/index that is not in the database default tablespace. Because these partitioned objects do not have storage, no file needs to be actually moved, so it shouldn't take more time than what's required to acquire locks. This query can be used to search for such relations: SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0 Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/16577-881633a9f9894fd5@postgresql.org Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Stabilize timeline switch regression test.Fujii Masao2021-01-14
| | | | | | | | | | | | | | | | | | Commit fef5b47f6b added the regression test to check whether a standby is able to follow a primary on a newer timeline when WAL archiving is enabled. But the buildfarm member florican reported that this test failed because the requested WAL segment was removed and replication failed. This is a timing issue. Since neither replication slot is used nor wal_keep_size is set in the test, checkpoint could remove the WAL segment that's still necessary for replication. This commit stabilizes the test by setting wal_keep_size. Back-patch to v13 where the regression test that this commit stabilizes was added. Author: Fujii Masao Discussion: https://postgr.es/m/X//PsenxcC50jDzX@paquier.xyz
* Improve tab-completion for CLOSE, DECLARE, FETCH and MOVE.Fujii Masao2021-01-14
| | | | | | | | | This commit makes CLOSE, FETCH and MOVE commands tab-complete the list of cursors. Also this commit makes DECLARE command tab-complete the options. Author: Shinya Kato, Sawada Masahiko, tweaked by Fujii Masao Reviewed-by: Shinya Kato, Sawada Masahiko, Fujii Masao Discussion: https://postgr.es/m/b0e4c5c53ef84c5395524f5056fc71f0@MP-MSGSS-MBX001.msg.nttdata.co.jp
* Minor header cleanup for the new iovec code.Thomas Munro2021-01-14
| | | | | | Remove redundant function declaration and improve header comment in pg_iovec.h. Move the new declaration in fd.h next to a group of more similar functions.
* Ensure that a standby is able to follow a primary on a newer timeline.Fujii Masao2021-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 709d003fbd refactored WAL-reading code, but accidentally caused WalSndSegmentOpen() to fail to follow a timeline switch while reading from a historic timeline. This issue caused a standby to fail to follow a primary on a newer timeline when WAL archiving is enabled. If there is a timeline switch within the segment, WalSndSegmentOpen() should read from the WAL segment belonging to the new timeline. But previously since it failed to follow a timeline switch, it tried to read the WAL segment with old timeline. When WAL archiving is enabled, that WAL segment with old timeline doesn't exist because it's renamed to .partial. This leads a primary to have tried to read non-existent WAL segment, and which caused replication to faill with the error "ERROR: requested WAL segment ... has already been removed". This commit fixes WalSndSegmentOpen() so that it's able to follow a timeline switch, to ensure that a standby is able to follow a primary on a newer timeline even when WAL archiving is enabled. This commit also adds the regression test to check whether a standby is able to follow a primary on a newer timeline when WAL archiving is enabled. Back-patch to v13 where the bug was introduced. Reported-by: Kyotaro Horiguchi Author: Kyotaro Horiguchi, tweaked by Fujii Masao Reviewed-by: Alvaro Herrera, Fujii Masao Discussion: https://postgr.es/m/20201209.174314.282492377848029776.horikyota.ntt@gmail.com
* Rework refactoring of hex and encoding routinesMichael Paquier2021-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit addresses some issues with c3826f83 that moved the hex decoding routine to src/common/: - The decoding function lacked overflow checks, so when used for security-related features it was an open door to out-of-bound writes if not carefully used that could remain undetected. Like the base64 routines already in src/common/ used by SCRAM, this routine is reworked to check for overflows by having the size of the destination buffer passed as argument, with overflows checked before doing any writes. - The encoding routine was missing. This is moved to src/common/ and it gains the same overflow checks as the decoding part. On failure, the hex routines of src/common/ issue an error as per the discussion done to make them usable by frontend tools, but not by shared libraries. Note that this is why ECPG is left out of this commit, and it still includes a duplicated logic doing hex encoding and decoding. While on it, this commit uses better variable names for the source and destination buffers in the existing escape and base64 routines in encode.c and it makes them more robust to overflow detection. The previous core code issued a FATAL after doing out-of-bound writes if going through the SQL functions, which would be enough to detect problems when working on changes that impacted this area of the code. Instead, an error is issued before doing an out-of-bound write. The hex routines were being directly called for bytea conversions and backup manifests without such sanity checks. The current calls happen to not have any problems, but careless uses of such APIs could easily lead to CVE-class bugs. Author: Bruce Momjian, Michael Paquier Reviewed-by: Sehrope Sarkuni Discussion: https://postgr.es/m/20201231003557.GB22199@momjian.us
* Move our p{read,write}v replacements into their own files.Thomas Munro2021-01-14
| | | | | | | | | | | | | macOS's ranlib issued a warning about an empty pread.o file with the previous arrangement, on systems new enough to require no replacement functions. Let's go back to using configure's AC_REPLACE_FUNCS system to build and include each .o in the library only if it's needed, which requires moving the *v() functions to their own files. Also move the _with_retry() wrapper to a more permanent home. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/1283127.1610554395%40sss.pgh.pa.us
* Mark inet_server_addr() and inet_server_port() as parallel-restricted.Tom Lane2021-01-13
| | | | | | | | | | | | | | | | | These need to be PR because they access the MyProcPort data structure, which doesn't get copied to parallel workers. The very similar functions inet_client_addr() and inet_client_port() are already marked PR, but somebody missed these. Although this is a pre-existing bug, we can't readily fix it in the back branches since we can't force initdb. Given the small usage of these two functions, and the even smaller likelihood that they'd get pushed to a parallel worker anyway, it doesn't seem worth the trouble to suggest that DBAs should fix it manually. Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoAT4aHP0Uxq91qpD7NL009tnUYQe-b14R3MnSVOjtE71g@mail.gmail.com
* Run reformat-dat-files to declutter the catalog data files.Tom Lane2021-01-13
| | | | | Things had gotten pretty messy here, apparently mostly but not entirely the fault of the multirange patch. No functional changes.
* Doc, more or less: uncomment tutorial example that was fixed long ago.Tom Lane2021-01-13
| | | | | | | | | | Reverts a portion of commit 344190b7e. Apparently, back in the twentieth century we had some issues with multi-statement SQL functions, but they've worked fine for a long time. Daniel Westermann Discussion: https://postgr.es/m/GVAP278MB04242DCBF5E31F528D53FA18D2A90@GVAP278MB0424.CHEP278.PROD.OUTLOOK.COM
* Disallow a digit as the first character of a variable name in pgbench.Tom Lane2021-01-13
| | | | | | | | | | | | | | | | | | The point of this restriction is to avoid trying to substitute variables into timestamp literal values, which may contain strings like '12:34'. There is a good deal more that should be done to reduce pgbench's tendency to substitute where it shouldn't. But this is sufficient to solve the case complained of by Jaime Soler, and it's simple enough to back-patch. Back-patch to v11; before commit 9d36a3866, pgbench had a slightly different definition of what a variable name is, and anyway it seems unwise to change long-stable branches for this. Fabien Coelho Discussion: https://postgr.es/m/alpine.DEB.2.22.394.2006291740420.805678@pseudo
* Enhance nbtree index tuple deletion.Peter Geoghegan2021-01-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach nbtree and heapam to cooperate in order to eagerly remove duplicate tuples representing dead MVCC versions. This is "bottom-up deletion". Each bottom-up deletion pass is triggered lazily in response to a flood of versions on an nbtree leaf page. This usually involves a "logically unchanged index" hint (these are produced by the executor mechanism added by commit 9dc718bd). The immediate goal of bottom-up index deletion is to avoid "unnecessary" page splits caused entirely by version duplicates. It naturally has an even more useful effect, though: it acts as a backstop against accumulating an excessive number of index tuple versions for any given _logical row_. Bottom-up index deletion complements what we might now call "top-down index deletion": index vacuuming performed by VACUUM. Bottom-up index deletion responds to the immediate local needs of queries, while leaving it up to autovacuum to perform infrequent clean sweeps of the index. The overall effect is to avoid certain pathological performance issues related to "version churn" from UPDATEs. The previous tableam interface used by index AMs to perform tuple deletion (the table_compute_xid_horizon_for_tuples() function) has been replaced with a new interface that supports certain new requirements. Many (perhaps all) of the capabilities added to nbtree by this commit could also be extended to other index AMs. That is left as work for a later commit. Extend deletion of LP_DEAD-marked index tuples in nbtree by adding logic to consider extra index tuples (that are not LP_DEAD-marked) for deletion in passing. This increases the number of index tuples deleted significantly in many cases. The LP_DEAD deletion process (which is now called "simple deletion" to clearly distinguish it from bottom-up deletion) won't usually need to visit any extra table blocks to check these extra tuples. We have to visit the same table blocks anyway to generate a latestRemovedXid value (at least in the common case where the index deletion operation's WAL record needs such a value). Testing has shown that the "extra tuples" simple deletion enhancement increases the number of index tuples deleted with almost any workload that has LP_DEAD bits set in leaf pages. That is, it almost never fails to delete at least a few extra index tuples. It helps most of all in cases that happen to naturally have a lot of delete-safe tuples. It's not uncommon for an individual deletion operation to end up deleting an order of magnitude more index tuples compared to the old naive approach (e.g., custom instrumentation of the patch shows that this happens fairly often when the regression tests are run). Add a further enhancement that augments simple deletion and bottom-up deletion in indexes that make use of deduplication: Teach nbtree's _bt_delitems_delete() function to support granular TID deletion in posting list tuples. It is now possible to delete individual TIDs from posting list tuples provided the TIDs have a tableam block number of a table block that gets visited as part of the deletion process (visiting the table block can be triggered directly or indirectly). Setting the LP_DEAD bit of a posting list tuple is still an all-or-nothing thing, but that matters much less now that deletion only needs to start out with the right _general_ idea about which index tuples are deletable. Bump XLOG_PAGE_MAGIC because xl_btree_delete changed. No bump in BTREE_VERSION, since there are no changes to the on-disk representation of nbtree indexes. Indexes built on PostgreSQL 12 or PostgreSQL 13 will automatically benefit from bottom-up index deletion (i.e. no reindexing required) following a pg_upgrade. The enhancement to simple deletion is available with all B-Tree indexes following a pg_upgrade, no matter what PostgreSQL version the user upgrades from. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Heikki Linnakangas <hlinnaka@iki.fi> Reviewed-By: Victor Yegorov <vyegorov@gmail.com> Discussion: https://postgr.es/m/CAH2-Wzm+maE3apHB8NOtmM=p-DO65j2V5GzAWCOEEuy3JZgb2g@mail.gmail.com
* Pass down "logically unchanged index" hint.Peter Geoghegan2021-01-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add an executor aminsert() hint mechanism that informs index AMs that the incoming index tuple (the tuple that accompanies the hint) is not being inserted by execution of an SQL statement that logically modifies any of the index's key columns. The hint is received by indexes when an UPDATE takes place that does not apply an optimization like heapam's HOT (though only for indexes where all key columns are logically unchanged). Any index tuple that receives the hint on insert is expected to be a duplicate of at least one existing older version that is needed for the same logical row. Related versions will typically be stored on the same index page, at least within index AMs that apply the hint. Recognizing the difference between MVCC version churn duplicates and true logical row duplicates at the index AM level can help with cleanup of garbage index tuples. Cleanup can intelligently target tuples that are likely to be garbage, without wasting too many cycles on less promising tuples/pages (index pages with little or no version churn). This is infrastructure for an upcoming commit that will teach nbtree to perform bottom-up index deletion. No index AM actually applies the hint just yet. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Victor Yegorov <vyegorov@gmail.com> Discussion: https://postgr.es/m/CAH2-Wz=CEKFa74EScx_hFVshCOn6AA5T-ajFASTdzipdkLTNQQ@mail.gmail.com
* Log long wait time on recovery conflict when it's resolved.Fujii Masao2021-01-13
| | | | | | | | | | | | | This is a follow-up of the work done in commit 0650ff2303. This commit extends log_recovery_conflict_waits so that a log message is produced also when recovery conflict has already been resolved after deadlock_timeout passes, i.e., when the startup process finishes waiting for recovery conflict after deadlock_timeout. This is useful in investigating how long recovery conflicts prevented the recovery from applying WAL. Author: Fujii Masao Reviewed-by: Kyotaro Horiguchi, Bertrand Drouvot Discussion: https://postgr.es/m/9a60178c-a853-1440-2cdc-c3af916cff59@amazon.com
* Don't use elog() in src/port/pwrite.c.Thomas Munro2021-01-13
| | | | | | | | Nothing broke because of this oversight yet, but it would fail to link if we tried to use pg_pwrite() in frontend code on a system that lacks pwrite(). Use an assertion instead. Also pgindent while here. Discussion: https://postgr.es/m/CA%2BhUKGL57RvoQsS35TVPnQoPYqbtBixsdRhynB8NpcUKpHTTtg%40mail.gmail.com
* Fix memory leak in SnapBuildSerialize.Amit Kapila2021-01-13
| | | | | | | | | | | | | | The memory for the snapshot was leaked while serializing it to disk during logical decoding. This memory will be freed only once walsender stops streaming the changes. This can lead to a huge memory increase when master logs Standby Snapshot too frequently say when the user is trying to create many replication slots. Reported-by: funnyxj.fxj@alibaba-inc.com Diagnosed-by: funnyxj.fxj@alibaba-inc.com Author: Amit Kapila Backpatch-through: 9.5 Discussion: https://postgr.es/m/033ab54c-6393-42ee-8ec9-2b399b5d8cde.funnyxj.fxj@alibaba-inc.com
* Optimize DropRelFileNodesAllBuffers() for recovery.Amit Kapila2021-01-13
| | | | | | | | | | | | | | | Similar to commit d6ad34f341, this patch optimizes DropRelFileNodesAllBuffers() by avoiding the complete buffer pool scan and instead find the buffers to be invalidated by doing lookups in the BufMapping table. This optimization helps operations where the relation files need to be removed like Truncate, Drop, Abort of Create Table, etc. Author: Kirk Jamison Reviewed-by: Kyotaro Horiguchi, Takayuki Tsunakawa, and Amit Kapila Tested-By: Haiying Tang Discussion: https://postgr.es/m/OSBPR01MB3207DCA7EC725FDD661B3EDAEF660@OSBPR01MB3207.jpnprd01.prod.outlook.com
* Fix routine name in comment of catcache.cMichael Paquier2021-01-13
| | | | | Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACUDXLAkf_XxQO9tAUtnTNGi3Lmd8fANd+vBJbcHn1HoWA@mail.gmail.com
* Invent struct ReindexIndexInfoAlvaro Herrera2021-01-12
| | | | | | | | | | | This struct is used by ReindexRelationConcurrently to keep track of the relations to process. This saves having to obtain some data repeatedly, and has future uses as well. Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Hamid Akhtar <hamid.akhtar@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/20201130195439.GA24598@alvherre.pgsql
* pg_dump: label INDEX ATTACH ArchiveEntries with an owner.Tom Lane2021-01-12
| | | | | | | | | | | | | | | | Although a partitioned index's attachment to its parent doesn't have separate ownership, the ArchiveEntry for it needs to be marked with an owner anyway, to ensure that the ALTER command is run by the appropriate role when restoring with --use-set-session-authorization. Without this, the ALTER will be run by the role that started the restore session, which will usually work but it's formally the wrong thing. Back-patch to v11 where this type of ArchiveEntry was added. In HEAD, add equivalent commentary to the just-added TABLE ATTACH case, which I'd made do the right thing already. Discussion: https://postgr.es/m/1094034.1610418498@sss.pgh.pa.us
* Fix thinko in commentAlvaro Herrera2021-01-12
| | | | | | | | This comment has been wrong since its introduction in commit 2c03216d8311. Author: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoAzz6qipFJBbGEaHmyWxvvNDp8httbwLR9tUQWaTjUs2Q@mail.gmail.com
* Fix relation descriptor leak.Amit Kapila2021-01-12
| | | | | | | | | | | We missed closing the relation descriptor while sending changes via the root of partitioned relations during logical replication. Author: Amit Langote and Mark Zhao Reviewed-by: Amit Kapila and Ashutosh Bapat Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/tencent_41FEA657C206F19AB4F406BE9252A0F69C06@qq.com Discussion: https://postgr.es/m/tencent_6E296D2F7D70AFC90D83353B69187C3AA507@qq.com
* Optimize DropRelFileNodeBuffers() for recovery.Amit Kapila2021-01-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The recovery path of DropRelFileNodeBuffers() is optimized so that scanning of the whole buffer pool can be avoided when the number of blocks to be truncated in a relation is below a certain threshold. For such cases, we find the buffers by doing lookups in BufMapping table. This improves the performance by more than 100 times in many cases when several small tables (tested with 1000 relations) are truncated and where the server is configured with a large value of shared buffers (greater than equal to 100GB). This optimization helps cases (a) when vacuum or autovacuum truncated off any of the empty pages at the end of a relation, or (b) when the relation is truncated in the same transaction in which it was created. This commit introduces a new API smgrnblocks_cached which returns a cached value for the number of blocks in a relation fork. This helps us to determine the exact size of relation which is required to apply this optimization. The exact size is required to ensure that we don't leave any buffer for the relation being dropped as otherwise the background writer or checkpointer can lead to a PANIC error while flushing buffers corresponding to files that don't exist. Author: Kirk Jamison based on ideas by Amit Kapila Reviewed-by: Kyotaro Horiguchi, Takayuki Tsunakawa, and Amit Kapila Tested-By: Haiying Tang Discussion: https://postgr.es/m/OSBPR01MB3207DCA7EC725FDD661B3EDAEF660@OSBPR01MB3207.jpnprd01.prod.outlook.com
* Dump ALTER TABLE ... ATTACH PARTITION as a separate ArchiveEntry.Tom Lane2021-01-11
| | | | | | | | | | | | | | | | Previously, we emitted the ATTACH PARTITION command as part of the child table's ArchiveEntry. This was a poor choice since it complicates restoring the partition as a standalone table; you have to ignore the error from the ATTACH, which isn't even an option when restoring direct-to-database with pg_restore. (pg_restore will issue the whole ArchiveEntry as one PQexec, so that any error rolls back the table creation as well.) Hence, separate it out as its own ArchiveEntry, as indeed we already did for index ATTACH PARTITION commands. Justin Pryzby Discussion: https://postgr.es/m/20201023052940.GE9241@telsasoft.com
* Make pg_dump's table of object-type priorities more maintainable.Tom Lane2021-01-11
| | | | | | | | | | | | | | | | | Wedging a new object type into this table has historically required manually renumbering a lot of existing entries. (Although it appears that some people got lazy and re-used the priority level of an existing object type, even if it wasn't particularly related.) We can let the compiler do the counting by inventing an enum type that lists the desired priority levels in order. Now, if you want to add or remove a priority level, that's a one-liner. This patch is not purely cosmetic, because I split apart the priorities of DO_COLLATION and DO_TRANSFORM, as well as those of DO_ACCESS_METHOD and DO_OPERATOR, which look to me to have been merged out of expediency rather than because it was a good idea. Shell types continue to be sorted interchangeably with full types, and opclasses interchangeably with opfamilies.
* Fix function prototypes in dependency.h.Thomas Munro2021-01-12
| | | | | | | | Commit 257836a7 accidentally deleted a couple of redundant-but-conventional "extern" keywords on function prototypes. Put them back. Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org>