aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* llvmjit: Make llvm_types_module variable staticDaniel Gustafsson2023-09-27
| | | | | | | | | | Commit b059d2f45685a introduced llvm_types_module and accidentally exported it. As there is no usecase for accessing this variable externally, this makes it static. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/20221101055132.pjjsvlkeo4stbjkq@awork3.anarazel.de
* llvmjit: Remove unnecessary typesDaniel Gustafsson2023-09-27
| | | | | | | | | These types were added in fb46ac26fe but hasn't been used, so remove until there is a need for them. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/20221101055132.pjjsvlkeo4stbjkq@awork3.anarazel.de
* Fix the misuse of origin filter across multiple ↵Amit Kapila2023-09-27
| | | | | | | | | | | | | | | | | | | | | | pg_logical_slot_get_changes() calls. The pgoutput module uses a global variable (publish_no_origin) to cache the action for the origin filter, but we didn't reset the flag when shutting down the output plugin, so subsequent retries may access the previous publish_no_origin value. We fix this by storing the flag in the output plugin's private data. Additionally, the patch removes the currently unused origin string from the structure. For the back branch, to avoid changing the exposed structure, we eliminated the global variable and instead directly used the origin string for change filtering. Author: Hou Zhijie Reviewed-by: Amit Kapila, Michael Paquier Backpatch-through: 16 Discussion: http://postgr.es/m/OS0PR01MB571690EF24F51F51EFFCBB0E94FAA@OS0PR01MB5716.jpnprd01.prod.outlook.com
* doc: correct reference to pg_relation in commentBruce Momjian2023-09-26
| | | | | | | | Reported-by: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/87sf9apnr0.fsf@wibble.ilmari.org Backpatch-through: master
* MergeAttributes() and related variable renamingPeter Eisentraut2023-09-26
| | | | | | | Mainly, rename "schema" to "columns" and related changes. The previous naming has long been confusing. Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da%40eisentraut.org
* Clean up MergeCheckConstraint()Peter Eisentraut2023-09-26
| | | | | | | | If the constraint is not already in the list, add it ourselves, instead of making the caller do it. This makes the interface more consistent with other "merge" functions in this file. Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da%40eisentraut.org
* Fix another bug in parent page splitting during GiST index build.Heikki Linnakangas2023-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Yet another bug in the ilk of commits a7ee7c851 and 741b88435. In 741b88435, we took care to clear the memorized location of the downlink when we split the parent page, because splitting the parent page can move the downlink. But we missed that even *updating* a tuple on the parent can move it, because updating a tuple on a gist page is implemented as a delete+insert, so the updated tuple gets moved to the end of the page. This commit fixes the bug in two different ways (belt and suspenders): 1. Clear the downlink when we update a tuple on the parent page, even if it's not split. This the same approach as in commits a7ee7c851 and 741b88435. I also noticed that gistFindCorrectParent did not clear the 'downlinkoffnum' when it stepped to the right sibling. Fix that too, as it seems like a clear bug even though I haven't been able to find a test case to hit that. 2. Change gistFindCorrectParent so that it treats 'downlinkoffnum' merely as a hint. It now always first checks if the downlink is still at that location, and if not, it scans the page like before. That's more robust if there are still more cases where we fail to clear 'downlinkoffnum' that we haven't yet uncovered. With this, it's no longer necessary to meticulously clear 'downlinkoffnum', so this makes the previous fixes unnecessary, but I didn't revert them because it still seems nice to clear it when we know that the downlink has moved. Also add the test case using the same test data that Alexander posted. I tried to reduce it to a smaller test, and I also tried to reproduce this with different test data, but I was not able to, so let's just include what we have. Backpatch to v12, like the previous fixes. Reported-by: Alexander Lakhin Discussion: https://www.postgresql.org/message-id/18129-caca016eaf0c3702@postgresql.org
* Add some const qualifiersPeter Eisentraut2023-09-26
| | | | | | | | | | | There was a mismatch between the const qualifiers for excludeDirContents in src/backend/backup/basebackup.c and src/bin/pg_rewind/filemap.c, which led to a quick search for similar cases. We should make excludeDirContents match, but the rest of the changes seem like a good idea as well. Author: David Steele <david@pgmasters.net> Discussion: https://www.postgresql.org/message-id/flat/669a035c-d23d-2f38-7ff0-0cb93e01d610@pgmasters.net
* Clean up MergeAttributesIntoExisting()Peter Eisentraut2023-09-26
| | | | | | | | | | | Make variable naming clearer and more consistent. Move some variables to smaller scope. Remove some unnecessary intermediate variables. Try to save some vertical space. Apply analogous changes to nearby MergeConstraintsIntoExisting() and RemoveInheritance() for consistency. Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da%40eisentraut.org
* Remove unused includePeter Eisentraut2023-09-26
| | | | | | This was added in add5cf28d4 but was apparently never used. Discussion: https://www.postgresql.org/message-id/flat/f84640e3-00d3-5abd-3f41-e6a19d33c40b@eisentraut.org
* Fix behavior of "force" in pgstat_report_wal()Michael Paquier2023-09-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As implemented in 5891c7a8ed8f, setting "force" to true in pgstat_report_wal() causes the routine to not wait for the pgstat shmem lock if it cannot be acquired, in which case the WAL and I/O statistics finish by not being flushed. The origin of the confusion comes from pgstat_flush_wal() and pgstat_flush_io(), that use "nowait" as sole argument. The I/O stats are new in v16. This is the opposite behavior of what has been used in pgstat_report_stat(), where "force" is the opposite of "nowait". In this case, when "force" is true, the routine sets "nowait" to false, which would cause the routine to wait for the pgstat shmem lock, ensuring that the stats are always flushed. When "force" is false, "nowait" is set to true, and the stats would only not be flushed if the pgstat shmem lock can be acquired, returning immediately without flushing the stats if the lock cannot be acquired. This commit changes pgstat_report_wal() so as "force" has the same behavior as in pgstat_report_stat(). There are currently three callers of pgstat_report_wal(): - Two in the checkpointer where force=true during a shutdown and the main checkpointer loop. Now the code behaves so as the stats are always flushed. - One in the main loop of the bgwriter, where force=false. Now the code behaves so as the stats would not be flushed if the pgstat shmem lock could not be acquired. Before this commit, some stats on WAL and I/O could have been lost after a shutdown, for example. Reported-by: Ryoga Yoshida Author: Ryoga Yoshida, Michael Paquier Discussion: https://postgr.es/m/f87a4d7be70530606b864fd1df91718c@oss.nttdata.com Backpatch-through: 15
* Fix edge-case for xl_tot_len broken by bae868ca.Thomas Munro2023-09-26
| | | | | | | | | | | | | | | | | bae868ca removed a check that was still needed. If you had an xl_tot_len at the end of a page that was too small for a record header, but not big enough to span onto the next page, we'd immediately perform the CRC check using a bogus large length. Because of arbitrary coding differences between the CRC implementations on different platforms, nothing very bad happened on common modern systems. On systems using the _sb8.c fallback we could segfault. Restore that check, add a new assertion and supply a test for that case. Back-patch to 12, like bae868ca. Tested-by: Tom Lane <tgl@sss.pgh.pa.us> Tested-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGLCkTT7zYjzOxuLGahBdQ%3DMcF%3Dz5ZvrjSOnW4EDhVjT-g%40mail.gmail.com
* Add worker type to pg_stat_subscription.Nathan Bossart2023-09-25
| | | | | | | | | | | | | Thanks to commit 2a8b40e368, the logical replication worker type is easily determined. The worker type could already be deduced via other columns such as leader_pid and relid, but that is unnecessary complexity for users. Bumps catversion. Author: Peter Smith Reviewed-by: Michael Paquier, Maxim Orlov, Amit Kapila Discussion: https://postgr.es/m/CAHut%2BPtmbSMfErSk0S7xxVdZJ9XVE3xVLhqBTmT91kf57BeKDQ%40mail.gmail.com
* Collect dependency information for parsed CallStmts.Tom Lane2023-09-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Parse analysis of a CallStmt will inject mutable information, for instance the OID of the called procedure, so that subsequent DDL may create a need to re-parse the CALL. We failed to detect this for CALLs in plpgsql routines, because no dependency information was collected when putting a CallStmt into the plan cache. That could lead to misbehavior or strange errors such as "cache lookup failed". Before commit ee895a655, the issue would only manifest for CALLs appearing in atomic contexts, because we re-planned non-atomic CALLs every time through anyway. It is now apparent that extract_query_dependencies() probably needs a special case for every utility statement type for which stmt_requires_parse_analysis() returns true. I wanted to add something like Assert(!stmt_requires_parse_analysis(...)) when falling out of extract_query_dependencies_walker without doing anything, but there are API issues as well as a more fundamental point: stmt_requires_parse_analysis is supposed to be applied to raw parser output, so it'd be cheating to assume it will give the correct answer for post-parse-analysis trees. I contented myself with adding a comment. Per bug #18131 from Christian Stork. Back-patch to all supported branches. Discussion: https://postgr.es/m/18131-576854e79c5cd264@postgresql.org
* Limit to_tsvector_byid's initial array allocation to something sane.Tom Lane2023-09-25
| | | | | | | | | | | | The initial estimate of the number of distinct ParsedWords is just that: an estimate. Don't let it exceed what palloc is willing to allocate. If in fact we need more entries, we'll eventually fail trying to enlarge the array. But if we don't, this allows success on inputs that currently draw "invalid memory alloc request size". Per bug #18080 from Uwe Binder. Back-patch to all supported branches. Discussion: https://postgr.es/m/18080-d5c5e58fef8c99b7@postgresql.org
* Doc: improve cross-reference in Makefile comment.Tom Lane2023-09-25
| | | | | | Per gripe from Japin Li. Discussion: https://postgr.es/m/MEYP282MB16692171F13B5DF40DB768EEB6FCA@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* Fix typo in numutils.c commentsDaniel Gustafsson2023-09-25
| | | | s/messges/messages/
* Add GUC for temporarily disabling event triggersDaniel Gustafsson2023-09-25
| | | | | | | | | | | | | | | | | | | In order to troubleshoot misbehaving or buggy event triggers, the documented advice is to enter single-user mode. In an attempt to reduce the number of situations where single-user mode is required (or even recommended) for non-extraordinary maintenance, this GUC allows to temporarily suspend event triggers. This was originally extracted from a larger patchset which aimed at supporting event triggers on login events. Reviewed-by: Ted Yu <yuzhihong@gmail.com> Reviewed-by: Mikhail Gribkov <youzhick@gmail.com> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Michael Paquier <michael@paquier.xyz Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9@yesql.se Discussion: https://postgr.es/m/0d46d29f-4558-3af9-9c85-7774e14a7709@postgrespro.ru
* Don't trust unvalidated xl_tot_len.Thomas Munro2023-09-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | xl_tot_len comes first in a WAL record. Usually we don't trust it to be the true length until we've validated the record header. If the record header was split across two pages, previously we wouldn't do the validation until after we'd already tried to allocate enough memory to hold the record, which was bad because it might actually be garbage bytes from a recycled WAL file, so we could try to allocate a lot of memory. Release 15 made it worse. Since 70b4f82a4b5, we'd at least generate an end-of-WAL condition if the garbage 4 byte value happened to be > 1GB, but we'd still try to allocate up to 1GB of memory bogusly otherwise. That was an improvement, but unfortunately release 15 tries to allocate another object before that, so you could get a FATAL error and recovery could fail. We can fix both variants of the problem more fundamentally using pre-existing page-level validation, if we just re-order some logic. The new order of operations in the split-header case defers all memory allocation based on xl_tot_len until we've read the following page. At that point we know that its first few bytes are not recycled data, by checking its xlp_pageaddr, and that its xlp_rem_len agrees with xl_tot_len on the preceding page. That is strong evidence that xl_tot_len was truly the start of a record that was logged. This problem was most likely to occur on a standby, because walreceiver.c recycles WAL files without zeroing out trailing regions of each page. We could fix that too, but it wouldn't protect us from rare crash scenarios where the trailing zeroes don't make it to disk. With reliable xl_tot_len validation in place, the ancient policy of considering malloc failure to indicate corruption at end-of-WAL seems quite surprising, but changing that is left for later work. Also included is a new TAP test to exercise various cases of end-of-WAL detection by writing contrived data into the WAL from Perl. Back-patch to 12. We decided not to put this change into the final release of 11. Author: Thomas Munro <thomas.munro@gmail.com> Author: Michael Paquier <michael@paquier.xyz> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Noah Misch <noah@leadboat.com> (the idea, not the code) Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Sergei Kornilov <sk@zsrv.org> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/17928-aa92416a70ff44a2%40postgresql.org
* Avoid potential pfree on NULL on OpenSSL errorsDaniel Gustafsson2023-09-22
| | | | | | | | | | | | Guard against the pointer being NULL before pfreeing upon an error returned from OpenSSL. Also handle errors from X509_NAME_print_ex which can return -1 on memory allocation errors. Backpatch down to v15 where the code was added. Author: Sergey Shinderuk <s.shinderuk@postgrespro.ru> Discussion: https://postgr.es/m/8db5374d-32e0-6abb-d402-40762511eff2@postgrespro.ru Backpatch-through: v15
* Simplify information schema check constraint deparsingPeter Eisentraut2023-09-22
| | | | | | | | | | | | The computation of the column information_schema.check_constraints.check_clause used pg_get_constraintdef() plus some string manipulation to get the check clause back out. This ended up with an extra pair of parentheses, which is only an aesthetic problem, but also with suffixes like "NOT VALID", which don't belong into that column. We can fix both of these problems and simplify the code by just using pg_get_expr() instead. Discussion: https://www.postgresql.org/message-id/799b59ef-3330-f0d2-ee23-8cdfa1740987@eisentraut.org
* Fix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions.Tom Lane2023-09-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In older branches, COMMIT/ROLLBACK AND CHAIN failed to propagate the current transaction's properties to the new transaction if there was any open subtransaction (unreleased savepoint). Instead, some previous transaction's properties would be restored. This is because the "if (s->chain)" check in CommitTransactionCommand examined the wrong instance of the "chain" flag and falsely concluded that it didn't need to save transaction properties. Our regression tests would have noticed this, except they used identical transaction properties for multiple tests in a row, so that the faulty behavior was not distinguishable from correct behavior. Commit 12d768e70 fixed the problem in v15 and later, but only rather accidentally, because I removed the "if (s->chain)" test to avoid a compiler warning, while not realizing that the warning was flagging a real bug. In v14 and before, remove the if-test and save transaction properties unconditionally; just as in the newer branches, that's not expensive enough to justify thinking harder. Add the comment and extra regression test to v15 and later to forestall any future recurrence, but there's no live bug in those branches. Patch by me, per bug #18118 from Liu Xiang. Back-patch to v12 where the AND CHAIN feature was added. Discussion: https://postgr.es/m/18118-4b72fcbb903aace6@postgresql.org
* Update comment about set_join_pathlist_hook().Etsuro Fujita2023-09-21
| | | | | | | | | | | | | | | | The comment introduced by commit e7cb7ee14 was a bit too terse, which could lead to extensions doing different things within the hook function than we intend to allow. Extend the comment to explain what they can do within the hook function. Back-patch to all supported branches. In passing, I rephrased a nearby comment that I recently added to the back branches. Reviewed by David Rowley and Andrei Lepikhov. Discussion: https://postgr.es/m/CAPmGK15SBPA1nr3Aqsdm%2BYyS-ay0Ayo2BRYQ8_A2To9eLqwopQ%40mail.gmail.com
* Fix typos in pgoutput.cMichael Paquier2023-09-20
| | | | | | | | | RelationSyncCache was mentioned in two comments under a different name. Issue noticed while reviewing a different patch touching the same area. Introduced by 665d1fad99e7. Discussion: https://postgr.es/m/ZQk1Ca_eFDTmBiZy@paquier.xyz
* Replace more MemSet calls with struct initializationPeter Eisentraut2023-09-19
| | | | | | | This fixes up 10ea0f924a2 to use the style introduced by 9fd45870c1. Author: Richard Guo <guofenglinux@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAMbWs490gJf5A=ydqyjh+Z8mVQa_foTGtcmBtHGLra0aOwLWHQ@mail.gmail.com
* Fix GiST README's explanation of the NSN cross-check.Heikki Linnakangas2023-09-19
| | | | | | | | The text got the condition backwards, it's "NSN > LSN", not "NSN < LSN". While we're at it, expand it a little for clarity. Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/4cb46e18-e688-524a-0f73-b1f03ed5d6ee@iki.fi
* Standardize type of extend_by counterPeter Eisentraut2023-09-19
| | | | | | | | | | | | The counter of extend_by loops is mixed int and uint32. Fix by standardizing from int to uint32, to match the extend_by variable. Fixup for 31966b151e. Author: Ranier Vilela <ranier.vf@gmail.com> Reviewed-by: Gurjeet Singh <gurjeet@singh.im> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAEudQAqHG-JP-YnG54ftL_b7v6-57rMKwET_MSvEoen0UHuPig@mail.gmail.com
* Improve error message for snapshot import in snapmgr.c, take twoMichael Paquier2023-09-19
| | | | | | | | | | | | | | | | | | | | | | | | | When a snapshot file fails to be read in ImportSnapshot(), it would issue an ERROR as "invalid snapshot identifier" when opening a stream for it in read-only mode. The error handling is improved to be more talkative in failure cases: - If a snapshot identifier uses incorrect characters, complain with the same error as before this commit. - If the snapshot file cannot be found in pg_snapshots/, complain with a "snapshot \"foo\" does not exist" instead. This maps to the case where AllocateFile() fails on ENOENT. Based on a suggestion from Andres Freund. - If AllocateFile() throws something else than ENOENT as errno, report it with more details in %m instead, as these failures are never expected. b29504eeb489 was the first improvement take. The older error message exists since bb446b689b66 that introduced snapshot imports. Two test cases are added to cover the cases of an identifier with an incorrect format and of a missing snapshot. Author: Bharath Rupireddy Reviewed-by: Andres Freund, Daniel Gustafsson, Michael Paquier Discussion: https://postgr.es/m/CALj2ACWmr=3KdxDkm8h7Zn1XxBoF6hdzq8WQyMn2y1OL5RYFrg@mail.gmail.com
* Make binaryheap available to frontend code.Nathan Bossart2023-09-18
| | | | | | | | | | | | | | | | There are a couple of places in frontend code that could make use of this simple binary heap implementation. This commit makes binaryheap usable in frontend code, much like commit 26aaf97b68 did for StringInfo. Like StringInfo, the header file is left in lib/ to reduce the likelihood of unnecessary breakage. The frontend version of binaryheap exposes a void *-based API since frontend code does not have access to the Datum definitions. This seemed like a better approach than switching all existing uses to void * or making the Datum definitions available to frontend code. Reviewed-by: Tom Lane, Alvaro Herrera Discussion: https://postgr.es/m/3612876.1689443232%40sss.pgh.pa.us
* Don't crash if cursor_to_xmlschema is used on a non-data-returning Portal.Tom Lane2023-09-18
| | | | | | | | | | | | | | | | cursor_to_xmlschema() assumed that any Portal must have a tupDesc, which is not so. Add a defensive check. It's plausible that this mistake occurred because of the rather poorly chosen name of the lookup function SPI_cursor_find(), which in such cases is returning something that isn't very much like a cursor. Add some documentation to try to forestall future errors of the same ilk. Report and patch by Boyu Yang (docs changes by me). Back-patch to all supported branches. Discussion: https://postgr.es/m/dd343010-c637-434c-a8cb-418f53bda3b8.yangboyu.yby@alibaba-inc.com
* Fix information schema for catalogued not-null constraintsPeter Eisentraut2023-09-18
| | | | | | | | | | The column check_constraints.check_clause should be like col IS NOT NULL without a surrounding CHECK (...). Discussion: https://www.postgresql.org/message-id/09489196-0bc1-e796-c43e-63425f7c5910@eisentraut.org
* Track nesting depth correctly when drilling down into RECORD Vars.Tom Lane2023-09-15
| | | | | | | | | | | | | | | | | | expandRecordVariable() failed to adjust the parse nesting structure correctly when recursing to inspect an outer-level Var. This could result in assertion failures or core dumps in corner cases. Likewise, get_name_for_var_field() failed to adjust the deparse namespace stack correctly when recursing to inspect an outer-level Var. In this case the likely result was a "bogus varno" error while deparsing a view. Per bug #18077 from Jingzhou Fu. Back-patch to all supported branches. Richard Guo, with some adjustments by me Discussion: https://postgr.es/m/18077-b9db97c6e0ab45d8@postgresql.org
* Rename variable for code clarityDaniel Gustafsson2023-09-15
| | | | | | | | | | | When tracking IO timing for WAL, the duration is what we calculate based on the start and end timestamps, it's not what the variable contains. Rename the timestamp variable to end to better communicate what it contains. Original patch by Krishnakumar with additional hacking to fix another occurrence by me. Author: Krishnakumar R <kksrcv001@gmail.com> Discussion: https://postgr.es/m/CAPMWgZ9f9o8awrQpjo8oxnNQ=bMDVPx00NE0QcDzvHD_ZrdLPw@mail.gmail.com
* Remove unnecessary smgrimmedsync() when creating unlogged table.Heikki Linnakangas2023-09-15
| | | | | | | | | | | | | | This became safe after commit 4b4798e138. The smgrcreate() call will now register the segment for syncing at the next checkpoint, so we don't need to sync it here. If a checkpoint happens before the creation is WAL-logged, the records will be replayed when starting recovery from the checkpoint. If a checkpoint happens after the WAL logging, the checkpoint will fsync() it. In the passing, clarify a comment in smgrDoPendingSyncs(). Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9%40iki.fi Reviewed-by: Robert Haas
* Quote filenames in error messagesDaniel Gustafsson2023-09-14
| | | | | | | | | | | | | | The majority of all filenames are quoted in user facing error and log messages, but a few were still printed without quotes. While these filenames do not risk causing any ambiguity as their format is strict, quote them anyways to be consistent across all logs. Also concatenate a message to keep it one line to make it easier to grep for in the code. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/080EEABE-6645-4A46-AB20-6285ADAC44FE@yesql.se
* Fix indentation in SQL filePeter Eisentraut2023-09-14
|
* Revert "Improve error message on snapshot import in snapmgr.c"Michael Paquier2023-09-14
| | | | | | | | | | This reverts commit a0d87bcd9b57, following a remark from Andres Frend that the new error can be triggered with an incorrect SET TRANSACTION SNAPSHOT command without being really helpful for the user as it uses the internal file name. Discussion: https://postgr.es/m/20230914020724.hlks7vunitvtbbz4@awork3.anarazel.de Backpatch-through: 11
* Flush logical slots to disk during a shutdown checkpoint if required.Amit Kapila2023-09-14
| | | | | | | | | | | | | | | | | | | | | It's entirely possible for a logical slot to have a confirmed_flush LSN higher than the last value saved on disk while not being marked as dirty. Currently, it is not a major problem but a later patch adding support for the upgrade of slots relies on that value being properly flushed to disk. It can also help avoid processing the same transactions again in some boundary cases after the clean shutdown and restart. Say, we process some transactions for which we didn't send anything downstream (the changes got filtered) but the confirm_flush LSN is updated due to keepalives. As we don't flush the latest value of confirm_flush LSN, it may lead to processing the same changes again without this patch. The approach taken by this patch has been suggested by Ashutosh Bapat. Author: Vignesh C, Julien Rouhaud, Kuroda Hayato Reviewed-by: Amit Kapila, Dilip Kumar, Michael Paquier, Ashutosh Bapat, Peter Smith, Hou Zhijie Discussion: http://postgr.es/m/CAA4eK1JzJagMmb_E8D4au=GYQkxox0AfNBm1FbP7sy7t4YWXPQ@mail.gmail.com Discussion: http://postgr.es/m/TYAPR01MB58664C81887B3AF2EB6B16E3F5939@TYAPR01MB5866.jpnprd01.prod.outlook.com
* Fix tracking of temp table relation extensions as writesAndres Freund2023-09-13
| | | | | | | | | | | | | | | Karina figured out that I (Andres) confused BufferUsage.temp_blks_written with BufferUsage.local_blks_written in fcdda1e4b5. Tests in core PG can't easily test this, as BufferUsage is just used for EXPLAIN (ANALYZE, BUFFERS) and pg_stat_statements. Thus this commit adds tests for this to pg_stat_statements. Reported-by: Karina Litskevich <litskevichkarina@gmail.com> Author: Karina Litskevich <litskevichkarina@gmail.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CACiT8ibxXA6+0amGikbeFhm8B84XdQVo6D0Qfd1pQ1s8zpsnxQ@mail.gmail.com Backpatch: 16-, where fcdda1e4b5 was merged
* Improve error message on snapshot import in snapmgr.cMichael Paquier2023-09-14
| | | | | | | | | | | | | | | | | When a snapshot file fails to be read in ImportSnapshot(), it would issue an ERROR as "invalid snapshot identifier" when opening a stream for it in read-only mode. This error message is reworded to be the same as all the other messages used in this case on failure, which is useful when debugging this area. Thinko introduced by bb446b689b66 where snapshot imports have been added. A backpatch down to 11 is done as this can improve any work related to snapshot imports in older branches. Author: Bharath Rupireddy Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CALj2ACWmr=3KdxDkm8h7Zn1XxBoF6hdzq8WQyMn2y1OL5RYFrg@mail.gmail.com Backpatch-through: 11
* Refactor error messages for unsupported providers in pg_locale.cMichael Paquier2023-09-14
| | | | | | | | | | | | | | | | | | These code paths should not be reached normally, but if they are an error with "(null)" as information for the collation provider would show up if no locale is set, while we can assume that we are referring to libc. This refactors the code so as the provider is always reported even if no locale is set. The name of the function where the error happens is added, while on it, as it can be helpful for debugging. Issue introduced by d87d548cd030, so backpatch down to 16. Author: Michael Paquier, Ranier Vilela Reviewed-by: Jeff Davis, Kyotaro Horiguchi Discussion: https://postgr.es/m/7073610042fcf97e1bea2ce08b7e0214b5e11094.camel@j-davis.com Backpatch-through: 16
* Fix incorrect logic in plan dependency recordingDavid Rowley2023-09-14
| | | | | | | | | | | | | | | Both 50e17ad28 and 29f45e299 mistakenly tried to record a plan dependency on a function but mistakenly inverted the OidIsValid test. This meant that we'd record a dependency only when the function's Oid was InvalidOid. Clearly this was meant to *not* record the dependency in that case. 50e17ad28 made this mistake first, then in v15 29f45e299 copied the same mistake. Reported-by: Tom Lane Backpatch-through: 14, where 50e17ad28 first made this mistake Discussion: https://postgr.es/m/2277537.1694301772@sss.pgh.pa.us
* Fix the ALTER SUBSCRIPTION to reflect the change in run_as_owner option.Amit Kapila2023-09-13
| | | | | | | | Reported-by: Jeff Davis Author: Hou Zhijie Reviewed-by: Amit Kapila Backpatch-through: 16 Discussion: http://postgr.es/m/17b62714fd115bd1899afd922954540a5c6a0467.camel@j-davis.com
* Fix exception safety bug in typcache.c.Thomas Munro2023-09-13
| | | | | | | | | | | | | | | | If an out-of-memory error was thrown at an unfortunate time, ensure_record_cache_typmod_slot_exists() could leak memory and leave behind a global state that produced an infinite loop on the next call. Fix by merging RecordCacheArray and RecordIdentifierArray into a single array. With only one allocation or re-allocation, there is no intermediate state. Back-patch to all supported releases. Reported-by: "James Pang (chaolpan)" <chaolpan@cisco.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/PH0PR11MB519113E738814BDDA702EDADD6EFA%40PH0PR11MB5191.namprd11.prod.outlook.com
* Remove redundant assignments in copyfrom.cMichael Paquier2023-09-09
| | | | | | | | | The tuple descriptor and the number of attributes are assigned twice to the same values in BeginCopyFrom(), for what looks like a small thinko coming from the refactoring done in c532d15dddff1. Author: Jingtang Zhang Discussion: https://postgr.es/m/CAPsk3_CrYeXUVHEiaWAYxY9BKiGvGT3AoXo_+Jm0xP_s_VmXCA@mail.gmail.com
* Add JIT deform_counterDaniel Gustafsson2023-09-08
| | | | | | | | | | | | | | | | | | generation_counter includes time spent on both JIT:ing expressions and tuple deforming which are configured independently via options jit_expressions and jit_tuple_deforming. As they are combined in the same counter it's not apparent what fraction of time the tuple deforming takes. This adds deform_counter dedicated to tuple deforming, which allows seeing more directly the influence jit_tuple_deforming is having on the query. The counter is exposed in EXPLAIN and pg_stat_statements bumpin pg_stat_statements to 1.11. Author: Dmitry Dolgov <9erthalion6@gmail.com> Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/20220612091253.eegstkufdsu4kfls@erthalion.local
* Teach WaitEventSetWait() to report multiple events on Windows.Thomas Munro2023-09-08
| | | | | | | | | | | | | | | | | | | | | | The WAIT_USE_WIN32 implementation of WaitEventSetWait() previously reported at most one event per call, because that's what the underlying WaitForMultipleObjects() call does. We can make the behavior match the three Unix implementations by looping until our output buffer is full, or there are no more events available now. This makes no difference to most callers including the regular FEBE socket code, since they ask for at most one event anyway. A difference in socket accept priority might be perceived by end users after commit 7389aad6 started using WaitEventSet in the postmaster. With this commit, the accept order now matches Unix systems, servicing listening sockets in round-robin order. We decided it wasn't really a bug or worth back-patching, but it seems good to align the behavior across platforms. Reviewed-by: Andres Freund <andres@anarazel.de> (earlier version) Tested-by: "Wei Wang (Fujitsu)" <wangw.fnst@fujitsu.com> Discussion: https://postgr.es/m/CA%2BhUKG%2BA2dk29hr5zRP3HVJQ-_PncNJM6HVQ7aaYLXLRBZU-xw%40mail.gmail.com
* Remove some more "snapshot too old" vestiges.Thomas Munro2023-09-08
| | | | | | | | | Commit f691f5b8 removed the logic, but left behind some now-useless Snapshot arguments to various AM-internal functions, and missed a couple of comments. Reported-by: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wznj9qSNXZ1P1uWTUD_FeaTezbUazb416EPwi4Qr_jR_6A%40mail.gmail.com
* Improve BackendXidGetPid() to only access allProcs on matching XIDMichael Paquier2023-09-08
| | | | | | | | | Compilers are able to optimize that, but it makes the code slightly more readable this way. Author: Zhao Junwang Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/CAEG8a3+i9gtqF65B+g_puVaCQuf0rZC-EMqMyEjGFJYOqUUWfA@mail.gmail.com
* Reorder tests in get_cheapest_path_for_pathkeys().Robert Haas2023-09-07
| | | | | | | | | | | Checking parallel safety should be even cheaper than cost comparison, so do that first. Also make some minor, related comment improvements. Richard Guo, reviewed by Aleksander Alekseev, Andy Fan, and me. Discussion: http://postgr.es/m/CAMbWs4-KE2wf4QPj_Sr5mX4QFtBNNKGmxK=+e=KZEGUjdG33=g@mail.gmail.com