aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Simplify productions for FORMAT JSON [ ENCODING name ]Alvaro Herrera2023-12-11
| | | | | | | | | This removes the production json_encoding_clause_opt, instead merging it into json_format_clause. Also remove the auxiliary makeJsonEncoding() function. Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://postgr.es/m/202312071841.u2gueb5dsrbk%40alvherre.pgsql
* Remove trace_recovery_messagesMichael Paquier2023-12-11
| | | | | | | | | | | | This GUC was intended as a debugging help in the 9.0 area when hot standby and streaming replication were being developped, able to offer more information at LOG level rather than DEBUGn. There are more tools available these days that are able to offer rather equivalent information, like pg_waldump introduced in 9.3. It is not obvious how this facility is useful these days, so let's remove it. Author: Bharath Rupireddy Discussion: https://postgr.es/m/ZXEXEAUVFrvpquSd@paquier.xyz
* Fix an undetected deadlock due to apply worker.Amit Kapila2023-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The apply worker needs to update the state of the subscription tables to 'READY' during the synchronization phase which requires locking the corresponding subscription. The apply worker also waits for the subscription tables to reach the 'SYNCDONE' state after holding the locks on the subscription and the wait is done using WaitLatch. The 'SYNCDONE' state is changed by tablesync workers again by locking the corresponding subscription. Both the state updates use AccessShareLock mode to lock the subscription, so they can't block each other. However, a backend can simultaneously try to acquire a lock on the same subscription using AccessExclusiveLock mode to alter the subscription. Now, the backend's wait on a lock can sneak in between the apply worker and table sync worker causing deadlock. In other words, apply_worker waits for tablesync worker which waits for backend, and backend waits for apply worker. This is not detected by the deadlock detector because apply worker uses WaitLatch. The fix is to release existing locks in apply worker before it starts to wait for tablesync worker to change the state. Reported-by: Tomas Vondra Author: Shlok Kyal Reviewed-by: Amit Kapila, Peter Smith Backpatch-through: 12 Discussion: https://postgr.es/m/d291bb50-12c4-e8af-2af2-7bb9bb4d8e3e@enterprisedb.com
* Fix nbtree backward scan race condition comments.Peter Geoghegan2023-12-08
| | | | | | | | | | Remove comments that supposed that holding a pin was a useful interlock for _bt_walk_left(). There are times when _bt_walk_left() doesn't hold either a lock or a pin on any page, so clearly this can't be true. _bt_walk_left() is even prepared to deal with concurrent deletion of both the original page and any pages to its left. Oversight in commit 2ed5b87f96.
* Micro-optimize JSONTYPE_NUMERIC code path in json.c.Nathan Bossart2023-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit does the following: * In datum_to_json_internal(), the call to IsValidJsonNumber() is replaced with simplified validation code. This avoids an extra call to strlen() in this path, and it avoids validating the entire string (which is okay since we know we're dealing with a numeric data type's output). * In datum_to_json_internal(), the call to escape_json() in the JSONTYPE_NUMERIC path is replaced with code that just surrounds the string with quotes. In passing, some other nearby calls to appendStringInfo() have been replaced with similar code to avoid unnecessary calls to vsnprintf(). * In composite_to_json(), the length of the separator is now determined at compile time to avoid unnecessary calls to strlen(). On my machine, this speeds up a benchmark for the proposed COPY TO (FORMAT json) command with many integers by upwards of 20%. There are likely other code paths that could be given a similar treatment, but that is left as a future exercise. Reviewed-by: Jeff Davis, Tom Lane, David Rowley, John Naylor Discussion: https://postgr.es/m/20231207231251.GB3359478%40nathanxps13
* Cache opaque handle for GUC option to avoid repeasted lookups.Jeff Davis2023-12-08
| | | | | | | | | | When setting GUCs from proconfig, performance is important, and hash lookups in the GUC table are significant. Per suggestion from Robert Haas. Discussion: https://postgr.es/m/CA+TgmoYpKxhR3HOD9syK2XwcAUVPa0+ba0XPnwWBcYxtKLkyxA@mail.gmail.com Reviewed-by: John Naylor
* Optimize nbtree backward scan boundary cases.Peter Geoghegan2023-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach _bt_binsrch (and related helper routines like _bt_search and _bt_compare) about the initial positioning requirements of backward scans. Routines like _bt_binsrch already know all about "nextkey" searches, so it seems natural to teach them about "goback"/backward searches, too. These concepts are closely related, and are much easier to understand when discussed together. Now that certain implementation details are hidden from _bt_first, it's straightforward to add a new optimization: backward scans using the < strategy now avoid extra leaf page accesses in certain "boundary cases". Consider the following example, which uses the tenk1 table (and its tenk1_hundred index) from the standard regression tests: SELECT * FROM tenk1 WHERE hundred < 12 ORDER BY hundred DESC LIMIT 1; Before this commit, nbtree would scan two leaf pages, even though it was only really necessary to scan one leaf page. We'll now descend straight to the leaf page containing a (12, -inf) high key instead. The scan will locate matching non-pivot tuples with "hundred" values starting from the value 11. The scan won't waste a page access on the right sibling leaf page, which cannot possibly contain any matching tuples. You can think of the optimization added by this commit as disabling an optimization (the _bt_compare "!pivotsearch" behavior that was added to Postgres 12 in commit dd299df8) for a small subset of cases where it was always counterproductive. Equivalently, you can think of the new optimization as extending the "pivotsearch" behavior that page deletion by VACUUM has long required (since the aforementioned Postgres 12 commit went in) to other, similar cases. Obviously, this isn't strictly necessary for these new cases (unlike VACUUM, _bt_first is prepared to move the scan to the left once on the leaf level), but the underlying principle is the same. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com> Discussion: https://postgr.es/m/CAH2-Wz=XPzM8HzaLPq278Vms420mVSHfgs9wi5tjFKHcapZCEw@mail.gmail.com
* Allow parallel CREATE INDEX for BRIN indexesTomas Vondra2023-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Allow using multiple worker processes to build BRIN index, which until now was supported only for BTREE indexes. For large tables this often results in significant speedup when the build is CPU-bound. The work is split in a simple way - each worker builds BRIN summaries on a subset of the table, determined by the regular parallel scan used to read the data, and feeds them into a shared tuplesort which sorts them by blkno (start of the range). The leader then reads this sorted stream of ranges, merges duplicates (which may happen if the parallel scan does not align with BRIN pages_per_range), and adds the resulting ranges into the index. The number of duplicate results produced by workers (requiring merging in the leader process) should be fairly small, thanks to how parallel scans assign chunks to workers. The likelihood of duplicate results may increase for higher pages_per_range values, but then there are fewer page ranges in total. In any case, we expect the merging to be much cheaper than summarization, so this should be a win. Most of the parallelism infrastructure is a simplified copy of the code used by BTREE indexes, omitting the parts irrelevant for BRIN indexes (e.g. uniqueness checks). This also introduces a new index AM flag amcanbuildparallel, determining whether to attempt to start parallel workers for the index build. Original patch by me, with reviews and substantial reworks by Matthias van de Meent, certainly enough to make him a co-author. Author: Tomas Vondra, Matthias van de Meent Reviewed-by: Matthias van de Meent Discussion: https://postgr.es/m/c2ee7d69-ce17-43f2-d1a0-9811edbda6e6%40enterprisedb.com
* Add empty BRIN ranges during CREATE INDEXTomas Vondra2023-12-08
| | | | | | | | | | | | | | | | | | | | | | | When building BRIN indexes, the brinbuildCallback only advances to the next page range when seeing a tuple that doesn't belong to the current one. This means that the index may end up missing ranges at the end of the table, if those pages do not contain any indexable tuples. We tend not to have completely empty pages at the end of a relation, but this also applies to partial indexes, where the tuples may simply not match the index predicate. This results in inefficient scans using the affected BRIN index - without the summaries, the page ranges have to be read and processed, which consumes I/O and possibly also CPU time. The existing code already added empty ranges for earlier parts of the table, this commit makes sure we add them for the ranges at the end of the table too. Patch by Matthias van de Meent, with review/improvements by me. Author: Matthias van de Meent Reviewed-by: Tomas Vondra Discussion: https://postgr.es/m/CAEze2WiMsPZg%3DxkvSF_jt4%3D69k6K7gz5B8V2wY3gCGZ%2B1BzCbQ%40mail.gmail.com
* Remove some unnecessary #includes of postmaster/interrupt.hHeikki Linnakangas2023-12-08
| | | | | Commit 44fc6e259b removed a bunch of references to SignalHandlerForCrashExit, leaving these #includes unneeded.
* Rename ShmemVariableCache to TransamVariablesHeikki Linnakangas2023-12-08
| | | | | | | | The old name was misleading: It's not a cache, the values kept in the struct are the authoritative source. Reviewed-by: Tristan Partin, Richard Guo Discussion: https://www.postgresql.org/message-id/6537d63d-4bb5-46f8-9b5d-73a8ba4720ab@iki.fi
* Initialize ShmemVariableCache like other shmem areasHeikki Linnakangas2023-12-08
| | | | | | | For sake of consistency. Reviewed-by: Tristan Partin, Richard Guo Discussion: https://www.postgresql.org/message-id/6537d63d-4bb5-46f8-9b5d-73a8ba4720ab@iki.fi
* Don't try to open visibilitymap when analyzing a foreign tableHeikki Linnakangas2023-12-08
| | | | | | | | It's harmless, visibilitymap_count() returns 0 if the file doesn't exist. But it's also very pointless. I noticed this when I added an assertion in smgropen() that the relnumber is valid. Discussion: https://www.postgresql.org/message-id/621a52fd-3cd8-4f5d-a561-d510b853bbaf@iki.fi
* Fix potential pointer overflow in xlogreader.c.Thomas Munro2023-12-08
| | | | | | | | | | | | | | | | | | | While checking if a record could fit in the circular WAL decoding buffer, the coding from commit 3f1ce973 used arithmetic that could overflow. 64 bit systems were unaffected for various technical reasons, which probably explains the lack of problem reports. Likewise for 32 bit systems running known 32 bit kernels. The systems at risk of problems appear to be 32 bit processes running on 64 bit kernels, with unlucky placement in memory. Per complaint from GCC -fsanitize=undefined -m32, while testing variations of 039_end_of_wal.pl. Back-patch to 15. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGKH0oRPOX7DhiQ_b51sM8HqcPp2J3WA-Oen%3DdXog%2BAGGQ%40mail.gmail.com
* Verify that attribute counts match in ExecCopySlotDavid Rowley2023-12-07
| | | | | | | | | | | | | | | | | | | | | | | | tts_virtual_copyslot() contained an Assert that checked that the srcslot contained <= attributes than the dstslot. This seems to be backwards as if the srcslot contained fewer attributes then the dstslot could be left with stale Datum values from the previously stored tuple. It might make more sense to allow the source to contain additional attributes and only copy the leading ones that also exist in the destination, however, that's not what we're doing here. Here we just remove the Assert from tts_virtual_copyslot() and add an Assert to ExecCopySlot() to verify the attribute counts match. There does not seem to be any places where the destination contains fewer attributes, so instead of going to the trouble of making the code properly handle this, let's just ensure the attribute counts match. If this Assert fails then that will indicate that we do have cases that require us to handle the srcslot with more attributes than the dstslot. It seems better to only write this code if there's a genuine requirement for it rather than write it now only to leave it untested. Thanks to Andres Freund for helping with the analysis of this. Discussion: https://postgr.es/m/CAApHDvpMAvBL0T+TRORquyx1iqFQKMVTXtqNocOw0Pa2uh1heg@mail.gmail.com
* Improve some error messages with invalid indexes for REINDEX CONCURRENTLYMichael Paquier2023-12-07
| | | | | | | | | | | | | | An invalid index is skipped when doing REINDEX CONCURRENTLY at table level, with INDEX_CORRUPTED used as errcode. This is confusing, because an invalid index could exist after an interruption. The errcode is switched to OBJECT_NOT_IN_PREREQUISITE_STATE instead, as per a suggestion from Andres Freund. While on it, the error messages are reworded, and a hint is added, telling how to rebuild an invalid index in this case. This has been suggested by Noah Misch. Discussion: https://postgr.es/m/20231118230958.4fm3fhk4ypshxopa@awork3.anarazel.de
* Fix issues in binary_upgrade_logical_slot_has_caught_up().Amit Kapila2023-12-07
| | | | | | | | | | | | | | | | | The commit 29d0a77fa6 labelled binary_upgrade_logical_slot_has_caught_up() as a non-strict function to allow providing a better error message to callers in case the passed slot_name is NULL. On further discussion, it seems that it is not helpful to have a different error message for NULL input in this function, so this patch marks the function as strict. This patch also removes the explicit permission check to use replication slots as this function is invoked only by superusers and instead adds an Assert. Reported-by: Masahiko Sawada Author: Hayato Kuroda Reviewed-by: Vignesh C Discussion: https://postgr.es/m/CAD21AoDSyiBKkMXBxN_gUayZZUCOgyHnG8Ge8rcPXNP3Tf6B4g@mail.gmail.com
* Fix assertion failure with REINDEX and event triggersMichael Paquier2023-12-07
| | | | | | | | | | | | | | | | | | | | | | | A REINDEX CONCURRENTLY run on a table with no indexes would always pop the topmost snapshot from the active snapshot stack, making the snapshot handling inconsistent between the multiple-relation and single-relation cases. This commit slightly changes the snapshot stack handling so as a snapshot is popped only ReindexMultipleInternal() in this case after a relation has been reindexed, fixing a problem where an event trigger function may need a snapshot but does not have one. This also keeps the places where PopActiveSnapshot() is called closer to each other. While on it, this expands the existing tests to cover all the cases that could be faced with REINDEX commands and such event triggers, for one or more relations, with or without indexes. This behavior is inconsistent since 5dc92b844e68, but we've never had a need for an active snapshot at the end of a REINDEX until now. Thanks also to Jian He for the input. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/cb538743-484c-eb6a-a8c5-359980cd3a17@gmail.com
* Fix compilation on Windows with WAL_DEBUGMichael Paquier2023-12-06
| | | | | | | | | | This has been broken since b060dbe0001a that has reworked the callback mechanism of XLogReader, most likely unnoticed because any form of development involving WAL happens on platforms where this compiles fine. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACVF14WKQMFwcJ=3okVDhiXpuK5f7YdT+BdYXbbypMHqWA@mail.gmail.com Backpatch-through: 13
* Fix indentationDaniel Gustafsson2023-12-05
| | | | | | When preparing commit 98e675ed7af I accidentally forgot to run pgindent, which did produce a diff. Fix by adding the required whitespace per the koel buildfarm failure.
* Fix incorrect error message for IDENTIFY_SYSTEMDaniel Gustafsson2023-12-05
| | | | | | | | | | | | | | | | Commit 5a991ef8692e accidentally reversed the order of the tuples and fields parameters, making the error message incorrectly refer to 3 tuples with 1 field when IDENTIFY_SYSTEM returns 1 tuple and 3 or 4 fields. Fix by changing the order of the parameters. This also adds a comment describing why we check for < 3 when postgres since 9.4 has been sending 4 fields. Backpatch all the way since the bug is almost a decade old. Author: Tomonari Katsumata <t.katsumata1122@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Bug: #18224 Backpatch-through: v12
* Optimize SearchPathCache by saving the last entry.Jeff Davis2023-12-04
| | | | | | | Repeated lookups are common, so it's worth it to check the last entry before doing another hash lookup. Discussion: https://postgr.es/m/04c8592dbd694e4114a3ed87139a7a04e4363030.camel%40j-davis.com
* Teach convert() and friends to avoid copying when possible.Nathan Bossart2023-12-04
| | | | | | | | | | | | | Presently, pg_convert() allocates a new bytea and copies the result regardless of whether any conversion actually happened. This commit adjusts this function to return the source pointer as-is if no conversion occurred. This optimization isn't expected to make a tremendous difference, but it still seems worthwhile to avoid unnecessary memory allocations. Author: Yurii Rashkovskii Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/CA%2BRLCQyknBPSWXRBQGOi6aYEcdQ9RpH9Kch4GjoeY8dQ3D%2Bvhw%40mail.gmail.com
* Remove now-unnecessary Autovacuum[Launcher|Worker]IAm functionsHeikki Linnakangas2023-12-04
| | | | | | | After commit fd5e8b440d, InitProcess() is called later in the EXEC_BACKEND startup sequence, so it's enough to set the am_autovacuum_[launcher|worker] variables at the same place as in the !EXEC_BACKEND case.
* Add support for REINDEX in event triggersMichael Paquier2023-12-04
| | | | | | | | | | | | | | This commit adds support for REINDEX in event triggers, making this command react for the events ddl_command_start and ddl_command_end. The indexes rebuilt are collected with the ReindexStmt emitted by the caller, for the concurrent and non-concurrent paths. Thanks to that, it is possible to know a full list of the indexes that a single REINDEX command has worked on. Author: Garrett Thornburg, Jian He Reviewed-by: Jim Jones, Michael Paquier Discussion: https://postgr.es/m/CAEEqfk5bm32G7sbhzHbES9WejD8O8DCEOaLkxoBP7HNWxjPpvg@mail.gmail.com
* Refactor how InitProcess is calledHeikki Linnakangas2023-12-03
| | | | | | | | | | | The order of process initialization steps is now more consistent between !EXEC_BACKEND and EXEC_BACKEND modes. InitProcess() is called at the same place in either mode. We can now also move the AttachSharedMemoryStructs() call into InitProcess() itself. This reduces the number of "#ifdef EXEC_BACKEND" blocks. Reviewed-by: Tristan Partin, Andres Freund, Alexander Lakhin Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
* Pass BackgroundWorker entry in the parameter file in EXEC_BACKEND modeHeikki Linnakangas2023-12-03
| | | | | | | | | | | | The BackgroundWorker struct is now passed the same way as the Port struct. Seems more consistent. This makes it possible to move InitProcess later in SubPostmasterMain (in next commit), as we no longer need to access shared memory to read background worker entry. Reviewed-by: Tristan Partin, Andres Freund, Alexander Lakhin Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
* Refactor CreateSharedMemoryAndSemaphoresHeikki Linnakangas2023-12-03
| | | | | | | | | | | | For clarity, have separate functions for *creating* the shared memory and semaphores at postmaster or single-user backend startup, and for *attaching* to existing shared memory structures in EXEC_BACKEND case. CreateSharedMemoryAndSemaphores() is now called only at postmaster startup, and a new AttachSharedMemoryStructs() function is called at backend startup in EXEC_BACKEND mode. Reviewed-by: Tristan Partin, Andres Freund Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi
* Silence Valgrind complaint with EXEC_BACKENDHeikki Linnakangas2023-12-01
| | | | | | | | | | The padding bytes written to the backend params file were uninitialized. That's harmless because we don't access the padding bytes when we read the file back in, but Valgrind doesn't know that. In any case, clear the padding bytes to make Valgrind happy. Reported-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/014768ed-8b39-c44f-b07c-098c87b1644c@gmail.com
* pgindent fixPeter Eisentraut2023-12-01
| | | | for commit a11c9c42ea
* Check collation when creating partitioned indexPeter Eisentraut2023-12-01
| | | | | | | | | | | | | | | | | When creating a partitioned index, the partition key must be a subset of the index's columns. But this currently doesn't check that the collations between the partition key and the index definition match. So you can construct a unique index that fails to enforce uniqueness. (This would most likely involve a nondeterministic collation, so it would have to be crafted explicitly and is not something that would just happen by accident.) This patch adds the required collation check. As a result, any previously allowed unique index that has a collation mismatch would no longer be allowed to be created. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/3327cb54-f7f1-413b-8fdb-7a9dceebb938%40eisentraut.org
* Fix an uninitialized access in hash_xlog_squeeze_page().Amit Kapila2023-12-01
| | | | | | | | | | | Commit 861f86beea changed hash_xlog_squeeze_page() to start reading the write buffer conditionally but forgot to initialize it leading to an uninitialized access. Reported-by: Alexander Lakhin Author: Hayato Kuroda Reviewed-by: Alexander Lakhin, Amit Kapila Discussion: http://postgr.es/m/62ed1a9f-746a-8e86-904b-51b9b806a1d9@gmail.com
* Adjust obsolete comment explaining set_stack_base().Thomas Munro2023-12-01
| | | | | | | | | | | | | | | | | Commit 7389aad6 removed the notion of backends started from inside a signal handler. A stray comment still referred to them, while explaining the need for a call to set_stack_base(). That leads to the question of whether we still need the call in !EXEC_BACKEND builds. There doesn't seem to be much point in suppressing it now, as it doesn't hurt and probably helps to measure the stack base from the exact same place in EXEC_BACKEND and !EXEC_BACKEND builds. Back-patch to 16. Reported-by: Heikki Linnakangas <hlinnaka@iki.fi> Reported-by: Tristan Partin <tristan@neon.tech> Reported-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA%2BhUKG%2BEJHcevNGNOxVWxTNFbuB%3Dvjf4U68%2B85rAC_Sxvy2zEQ%40mail.gmail.com
* Print lwlock stats also for aux processes, when built with LWLOCK_STATSHeikki Linnakangas2023-12-01
| | | | | | | | | | | | | | | InitAuxiliaryProcess() closely resembles InitProcess(), but it didn't call InitLWLockAccess(). But because InitLWLockAccess() is a no-op unless compiled with LWLOCK_STATS, and everything works even if it's not called, the only consequence was that the stats were not printed for aux processes. This was an oversight in commit 1c6821be31f, in version 9.5, so it is missing in all supported branches. But since it only affects developers using LWLOCK_STATS and no one has complained, no backpatching. Discussion: https://www.postgresql.org/message-id/20231130202648.7k6agmuizdilufnv@awork3.anarazel.de
* Fix typo in 5a1dfde8334bAlexander Korotkov2023-11-30
| | | | | Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/55d8800f-4a80-5256-1e84-246fbe79acd0@gmail.com
* Fix warning due non-standard inline declaration in 4ed8f0913bfdb5f355Alexander Korotkov2023-11-30
| | | | | | Reported-by: Alexander Lakhin, Tom Lane Author: Pavel Borisov Discussion: https://postgr.es/m/55d8800f-4a80-5256-1e84-246fbe79acd0@gmail.com
* Remove redundant setting of hashkey after insertionJohn Naylor2023-11-30
| | | | | | | | | | | | | | | | It's not necessary to fill the key field in most cases, since hash_search has already done that. Some existing call sites have an assert or comment that this contract has been fulfilled, but those are quite old and that practice seems unnecessary here. While at it, remove a nearby redundant assignment that a smart compiler will elide anyway. Zhao Junwang, with some adjustments by me Reviewed by Nathan Bossart, with additional feedback from Tom Lane Discussion: http://postgr.es/m/CAEG8a3%2BUPF%3DR2QGPgJMF2mKh8xPd1H2TmfH77zPuVUFdBpiGUA%40mail.gmail.com
* Apply quotes more consistently to GUC names in logsMichael Paquier2023-11-30
| | | | | | | | | | | | | | Quotes are applied to GUCs in a very inconsistent way across the code base, with a mix of double quotes or no quotes used. This commit removes double quotes around all the GUC names that are obviously referred to as parameters with non-English words (use of underscore, mixed case, etc). This is the result of a discussion with Álvaro Herrera, Nathan Bossart, Laurenz Albe, Peter Eisentraut, Tom Lane and Daniel Gustafsson. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+Pv-kSN8SkxSdoHano_wPubqcg5789ejhCDZAcLFceBR-w@mail.gmail.com
* Improve "user mapping not found" error messagePeter Eisentraut2023-11-30
| | | | | | | | | | Display the name of the foreign server for which the user mapping was not found. Author: Ian Lawrence Barwick <barwick@gmail.com> Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://www.postgresql.org/message-id/flat/CAB8KJ=jFzNaeyFtLcTZNOc6fd1+F93pGVLFa-wyt31wn7VNxqQ@mail.gmail.com
* Make use FullTransactionId in 2PC filenamesAlexander Korotkov2023-11-29
| | | | | | | | | | | | | | Switch from using TransactionId to FullTransactionId in naming of 2PC files. Transaction state file in the pg_twophase directory now have extra 8 bytes in the name to address an epoch of a given xid. Author: Maxim Orlov, Aleksander Alekseev, Alexander Korotkov, Teodor Sigaev Author: Nikita Glukhov, Pavel Borisov, Yura Sokolov Reviewed-by: Jacob Champion, Heikki Linnakangas, Alexander Korotkov Reviewed-by: Japin Li, Pavel Borisov, Tom Lane, Peter Eisentraut, Andres Freund Reviewed-by: Andrey Borodin, Dilip Kumar, Aleksander Alekseev Discussion: https://postgr.es/m/CACG%3DezZe1NQSCnfHOr78AtAZxJZeCvxrts0ygrxYwe%3DpyyjVWA%40mail.gmail.com Discussion: https://postgr.es/m/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq%2BvfkmTF5Q%40mail.gmail.com
* Use larger segment file names for pg_notifyAlexander Korotkov2023-11-29
| | | | | | | | | | | | | | | | This avoids the wraparound in async.c and removes the corresponding code complexity. The maximum amount of allocated SLRU pages for NOTIFY / LISTEN queue is now determined by the max_notify_queue_pages GUC. The default value is 1048576. It allows to consume up to 8 GB of disk space which is exactly the limit we had previously. Author: Maxim Orlov, Aleksander Alekseev, Alexander Korotkov, Teodor Sigaev Author: Nikita Glukhov, Pavel Borisov, Yura Sokolov Reviewed-by: Jacob Champion, Heikki Linnakangas, Alexander Korotkov Reviewed-by: Japin Li, Pavel Borisov, Tom Lane, Peter Eisentraut, Andres Freund Reviewed-by: Andrey Borodin, Dilip Kumar, Aleksander Alekseev Discussion: https://postgr.es/m/CACG%3DezZe1NQSCnfHOr78AtAZxJZeCvxrts0ygrxYwe%3DpyyjVWA%40mail.gmail.com Discussion: https://postgr.es/m/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq%2BvfkmTF5Q%40mail.gmail.com
* Index SLRUs by 64-bit integers rather than by 32-bit integersAlexander Korotkov2023-11-29
| | | | | | | | | | | | | | | | | | We've had repeated bugs in the area of handling SLRU wraparound in the past, some of which have caused data loss. Switching to an indexing system for SLRUs that does not wrap around should allow us to get rid of a whole bunch of problems and improve the overall reliability of the system. This particular patch however only changes the indexing and doesn't address the wraparound per se. This is going to be done in the following patches. Author: Maxim Orlov, Aleksander Alekseev, Alexander Korotkov, Teodor Sigaev Author: Nikita Glukhov, Pavel Borisov, Yura Sokolov Reviewed-by: Jacob Champion, Heikki Linnakangas, Alexander Korotkov Reviewed-by: Japin Li, Pavel Borisov, Tom Lane, Peter Eisentraut, Andres Freund Reviewed-by: Andrey Borodin, Dilip Kumar, Aleksander Alekseev Discussion: https://postgr.es/m/CACG%3DezZe1NQSCnfHOr78AtAZxJZeCvxrts0ygrxYwe%3DpyyjVWA%40mail.gmail.com Discussion: https://postgr.es/m/CAJ7c6TPDOYBYrnCAeyndkBktO0WG2xSdYduTF0nxq%2BvfkmTF5Q%40mail.gmail.com
* Clean up usage of bison precedence for non-operator keywords.Tom Lane2023-11-28
| | | | | | | | | | | | | | | | | | | Assigning a precedence to a keyword that isn't a kind of expression operator is rather dangerous, because it might mask grammar ambiguities that we'd rather know about. It's much safer to attach explicit precedences to individual rules, which will affect the behavior of only that one rule. Moreover, when we do have to give a precedence to a non-operator keyword, we should try to give it the same precedence as IDENT, thereby reducing the risk of surprising side-effects. Apply this hard-won knowledge to SET (which I misassigned ages ago in commit 2647ad658) and some SQL/JSON-related productions (from commits 6ee30209a, 71bfd1543). Patch HEAD only, since there's no evidence of actual bugs here. Discussion: https://postgr.es/m/CADT4RqBPdbsZW7HS1jJP319TMRHs1hzUiP=iRJYR6UqgHCrgNQ@mail.gmail.com
* Use BIO_{get,set}_app_data instead of BIO_{get,set}_data.Tom Lane2023-11-28
| | | | | | | | | | | | | | | | | | | | | | We should have done it this way all along, but we accidentally got away with using the wrong BIO field up until OpenSSL 3.2. There, the library's BIO routines that we rely on use the "data" field for their own purposes, and our conflicting use causes assorted weird behaviors up to and including core dumps when SSL connections are attempted. Switch to using the approved field for the purpose, i.e. app_data. While at it, remove our configure probes for BIO_get_data as well as the fallback implementation. BIO_{get,set}_app_data have been there since long before any OpenSSL version that we still support, even in the back branches. Also, update src/test/ssl/t/001_ssltests.pl to allow for a minor change in an error message spelling that evidently came in with 3.2. Tristan Partin and Bo Andreson. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAN55FZ1eDDYsYaL7mv+oSLUij2h_u6hvD4Qmv-7PK7jkji0uyQ@mail.gmail.com
* Fix comment about ressortgrouprefs being unique in setop plans.Heikki Linnakangas2023-11-28
| | | | | Author: Richard Guo, Tom Lane Discussion: https://www.postgresql.org/message-id/CAMbWs49rAfFS-yd7=QxtDUrZDFfRBGy4rGBJNyGDH7=CLipFPg@mail.gmail.com
* Fix assertions with RI triggers in heap_update and heap_delete.Heikki Linnakangas2023-11-28
| | | | | | | | | | | | If the tuple being updated is not visible to the crosscheck snapshot, we return TM_Updated but the assertions would not hold in that case. Move them to before the cross-check. Fixes bug #17893. Backpatch to all supported versions. Author: Alexander Lakhin Backpatch-through: 12 Discussion: https://www.postgresql.org/message-id/17893-35847009eec517b5%40postgresql.org
* Don't use bms_membership() in cases where we don't need toDavid Rowley2023-11-28
| | | | | | | | | | | | | | | | | | | | | | 00b41463c adjusted Bitmapset so that an empty set is always represented as NULL. This makes checking for empty sets far cheaper than it used to be. There were various places in the code where we'd call bms_membership() to handle the 3 possible BMS_Membership values. For the BMS_SINGLETON case, we'd also call bms_singleton_member() to find the single set member. This can now be done in a more optimal way by first checking if the set is NULL and then not bothering with bms_membership() and simply call bms_get_singleton_member() instead to find the single member. This function will return false if there are multiple members in the set. Here we also tidy up some logic in examine_variable() for the single member case. There's now no need to call bms_is_member() as we've already established that we're working with a singleton Bitmapset, so we can just check if varRelid matches the singleton member. Reviewed-by: Richard Guo Discussion: https://postgr.es/m/CAApHDvqW+CxNPcY245GaWiuqkkqgTudtG2ncGvvSjGn2wdTZLA@mail.gmail.com
* Check if ii_AmCache is NULL in aminsertcleanupTomas Vondra2023-11-27
| | | | | | | | | | | | | | Fix a bug introduced by c1ec02be1d79. It may happen that the executor opens indexes on the result relation, but no rows end up being inserted. Then the index_insert_cleanup still gets executed, but passes down NULL to the AM callback. The AM callback may not expect this, as is the case of brininsertcleanup, leading to a crash. Fixed by only calling the cleanup callback if (ii_AmCache != NULL). This way the AM can simply assume to only see a valid cache. Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs4-w9qC-o9hQox9UHvdVZAYTp8OrPQOKtwbvzWaRejTT=Q@mail.gmail.com
* Reduce rate of walwriter wakeups due to async commits.Heikki Linnakangas2023-11-27
| | | | | | | | | | | | | | | | | XLogSetAsyncXactLSN(), called at asynchronous commit, would wake up walwriter every time the LSN advances, but walwriter doesn't actually do anything unless it has at least 'wal_writer_flush_after' full blocks of WAL to write. Repeatedly waking up walwriter to do nothing is a waste of CPU cycles in both walwriter and the backends doing the wakeups. To fix, apply the same logic in XLogSetAsyncXactLSN() to decide whether to wake up walwriter, as walwriter uses to determine if it has any work to do. In the passing, rename misleadingly named 'flushbytes' local variable to 'flushblocks'. Author: Andres Freund, Heikki Linnakangas Discussion: https://www.postgresql.org/message-id/20231024230929.vsc342baqs7kmbte@awork3.anarazel.de
* Avoid unconditionally filling in missing values with NULL in pgoutput.Amit Kapila2023-11-27
| | | | | | | | | | | | | | | | | | | | 52e4f0cd4 introduced a bug in pgoutput in which missing values in tuples were incorrectly filled in with NULL. The problem was the use of CreateTupleDescCopy where CreateTupleDescCopyConstr was required, as the former drops the constraints in the tuple description (specifically, the default value constraint) on the floor. The bug could result in incorrectness when a table replicated via `REPLICA IDENTITY FULL` underwent a schema change that added a column with a default value. The problem is that in such cases updates fill NULL values in old tuples for missing columns for default values. Then on the subscriber, we failed to find a matching tuple and missed updating the required row. Author: Nikhil Benesch Reviewed-by: Hou Zhijie, Amit Kapila Backpatch-through: 15 Discussion: http://postgr.es/m/CAPWqQZTEpZQamYsGMn6ZDRvVywwpVPiKH6OY4KSgA+NmeqFNzA@mail.gmail.com