summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-09-27Add TupleDescGetDefault()Peter Eisentraut
This unifies some repetitive code. Note: I didn't push the "not found" error message into the new function, even though all existing callers would be able to make use of it. Using the existing error handling as-is would probably require exposing the Relation type via tupdesc.h, which doesn't seem desirable. (Or even if we changed it to just report the OID, it would inject the concept of a relation containing the tuple descriptor into tupdesc.h, which might be a layering violation. Perhaps some further improvements could be considered here separately.) Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da%40eisentraut.org
2023-09-27llvmjit: Use explicit LLVMContextRef for inliningDaniel Gustafsson
When performing inlining LLVM unfortunately "leaks" types (the types survive and are usable, but a new round of inlining will recreate new structurally equivalent types). This accumulation will over time amount to a memory leak which for some queries can be large enough to trigger the OOM process killer. To avoid accumulation of types, all IR related data is stored in an LLVMContextRef which is dropped and recreated in order to release all types. Dropping and recreating incurs overhead, so it will be done only after 100 queries. This is a heuristic which might be revisited, but until we can get the size of the context from LLVM we are flying a bit blind. This issue has been reported several times, there may be more references to it in the archives on top of the threads linked below. Backpatching of this fix will be handled once it has matured in master for a bit. Reported-By: Justin Pryzby <pryzby@telsasoft.com> Reported-By: Kurt Roeckx <kurt@roeckx.be> Reported-By: Jaime Casanova <jcasanov@systemguards.com.ec> Reported-By: Lauri Laanmets <pcspets@gmail.com> Author: Andres Freund and Daniel Gustafsson Discussion: https://postgr.es/m/7acc8678-df5f-4923-9cf6-e843131ae89d@www.fastmail.com Discussion: https://postgr.es/m/20201218235607.GC30237@telsasoft.com Discussion: https://postgr.es/m/CAPH-tTxLf44s3CvUUtQpkDr1D8Hxqc2NGDzGXS1ODsfiJ6WSqA@mail.gmail.com
2023-09-27llvmjit: Make llvm_types_module variable staticDaniel Gustafsson
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
2023-09-27llvmjit: Remove unnecessary typesDaniel Gustafsson
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
2023-09-27Fix the misuse of origin filter across multiple ↵Amit Kapila
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
2023-09-26Stop using "-multiply_defined suppress" on macOS.Tom Lane
We started to use this linker switch in commit 9df308697 of 2004-07-13, which was in the OS X 10.3 era. Apparently it's been a no-op since around OS X 10.9. Apple's most recent toolchain version actively complains about it, so it's time to get rid of it. Discussion: https://postgr.es/m/467042.1695766998@sss.pgh.pa.us
2023-09-26pgbench: Improve help output of -I optionPeter Eisentraut
Add a description of the step letters to the --help output. Author: Gurjeet Singh <gurjeet@singh.im> Reviewed-by: Tristen Raab <tristen.raab@highgo.ca> Discussion: https://www.postgresql.org/message-id/flat/CABwTF4Xbc=K4tFj5Znc8jx0GCufQa577GCDsWD7=71qDnUEOyQ@mail.gmail.com
2023-09-26doc: correct reference to pg_relation in commentBruce Momjian
Reported-by: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/87sf9apnr0.fsf@wibble.ilmari.org Backpatch-through: master
2023-09-26MergeAttributes() and related variable renamingPeter Eisentraut
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
2023-09-26Clean up MergeCheckConstraint()Peter Eisentraut
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
2023-09-26Fix another bug in parent page splitting during GiST index build.Heikki Linnakangas
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
2023-09-26Add some const qualifiersPeter Eisentraut
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
2023-09-26Clean up MergeAttributesIntoExisting()Peter Eisentraut
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
2023-09-26Remove unused includePeter Eisentraut
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
2023-09-26Fix behavior of "force" in pgstat_report_wal()Michael Paquier
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
2023-09-26Fix edge-case for xl_tot_len broken by bae868ca.Thomas Munro
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
2023-09-25Add worker type to pg_stat_subscription.Nathan Bossart
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
2023-09-25pg_dump: tests: Correct test condition for invalid databasesAndres Freund
For some reason I used not_like = { pg_dumpall_dbprivs => 1, } in the test condition of one of the tests added in in c66a7d75e65. That doesn't make sense for two reasons: 1) not_like isn't a valid test condition 2) the database should not be dumped in any of the tests. Due to 1), the test achieved its goal, but clearly the formulation is confusing. Instead use like => {}, with a comment explaining why. Reported-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/3ddf79f2-8b7b-a093-11d2-5c739bc64f86@eisentraut.org Backpatch: 11-, like c66a7d75e65
2023-09-25Collect dependency information for parsed CallStmts.Tom Lane
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
2023-09-25Pack struct ParsedWord more tightly.Tom Lane
In a 64-bit build there's an awful lot of useless pad space in ParsedWords. Since we may allocate large arrays of these, it's worth some effort to reduce their size. Here we reduce the alen field from uint32 to uint16, and then re-order the fields to avoid unnecessary padding. alen is only used to remember the allocated size of the apos[] array, which is not allowed to exceed MAXNUMPOS (256) elements, so uint16 is plenty of space for it. That gets us from 40 bytes to 24 on 64-bit builds, and from 20 bytes to 16 on 32-bit builds. Per discussion of bug #18080. Unfortunately this is an ABI break so we can't back-patch. Discussion: https://postgr.es/m/1146921.1695411070@sss.pgh.pa.us
2023-09-25Limit to_tsvector_byid's initial array allocation to something sane.Tom Lane
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
2023-09-25Doc: improve cross-reference in Makefile comment.Tom Lane
Per gripe from Japin Li. Discussion: https://postgr.es/m/MEYP282MB16692171F13B5DF40DB768EEB6FCA@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
2023-09-25vacuumdb: Reword --help message for clarityDaniel Gustafsson
The --help output stated that schemas were specified using PATTERN when they in fact aren't pattern matched but are required to be exact matches. This changes to SCHEMA to make that clear. Backpatch through v16 where this was introduced. Author: Kuwamura Masaki <kuwamura@db.is.i.nagoya-u.ac.jp> Discussion: https://postgr.es/m/CAMyC8qp9mXPQd5D6s6CJxvmignsbTqGZwDDB6VYJOn1A8WG38w@mail.gmail.com Backpatch-through: 16
2023-09-25vacuumdb: Fix excluding multiple schemas with -NDaniel Gustafsson
When specifying multiple schemas to exclude with -N parameters, none of the schemas are actually excluded (a single -N worked as expected). This fixes the catalog query to handle multiple exclusions and adds a test for this case. Backpatch to v16 where this was introduced. Author: Nathan Bossart <nathandbossart@gmail.com> Author: Kuwamura Masaki <kuwamura@db.is.i.nagoya-u.ac.jp> Reported-by: Kuwamura Masaki <kuwamura@db.is.i.nagoya-u.ac.jp> Discussion: https://postgr.es/m/CAMyC8qp9mXPQd5D6s6CJxvmignsbTqGZwDDB6VYJOn1A8WG38w@mail.gmail.com Backpatch-through: 16
2023-09-25pg_upgrade: check for types removed in pg12Alvaro Herrera
Commit cda6a8d01d39 removed a few datatypes, but didn't update pg_upgrade --check to throw error if these types are used. So the users find that pg_upgrade --check tells them that everything is fine, only to fail when the real upgrade is attempted. Reviewed-by: Tristan Partin <tristan@neon.tech> Reviewed-by: Suraj Kharage <suraj.kharage@enterprisedb.com> Discussion: https://postgr.es/m/202309201654.ng4ksea25mti@alvherre.pgsql
2023-09-25Fix typo in numutils.c commentsDaniel Gustafsson
s/messges/messages/
2023-09-25Add GUC for temporarily disabling event triggersDaniel Gustafsson
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
2023-09-23Fix typo in test commentDaniel Gustafsson
s/currect/correct/, accidentally introduced in 608b167f9f9.
2023-09-23Don't use Perl pack('Q') in 039_end_of_wal.pl.Thomas Munro
'Q' for 64 bit integers turns out not to work on 32 bit Perl, as revealed by the build farm. Use 'II' instead, and deal with endianness. Back-patch to 12, like bae868ca. Discussion: https://postgr.es/m/ZQ4r1vHcryBsSi_V%40paquier.xyz
2023-09-23Don't trust unvalidated xl_tot_len.Thomas Munro
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
2023-09-22Avoid using internal test methods in SSL testsDaniel Gustafsson
The SSL tests for pg_ctl restart with an incorrect key passphrase used the internal _update_pid method to set the pidfile after running pg_ctl manually instead of using the supplied ->restart method. This refactors the ->restart method to accept a fail_ok parameter like how ->start and ->stop does, and changes the SSL tests to use this instead. This removes the need to call internal test module functions. Reviewed-by: Melih Mutlu <m.melihmutlu@gmail.com> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/F81643C4-D7B8-4C6B-AF18-B73839966279@yesql.se
2023-09-22Avoid potential pfree on NULL on OpenSSL errorsDaniel Gustafsson
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
2023-09-22Simplify information schema check constraint deparsingPeter Eisentraut
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
2023-09-21Fix COMMIT/ROLLBACK AND CHAIN in the presence of subtransactions.Tom Lane
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
2023-09-21Allow dbname in pg_basebackup/pg_receivewal connstringDaniel Gustafsson
As physical replication work at the cluster level and not database level, any dbname in the connection string is ignored. Proxies and middleware used in connecting to the cluster might however need to know the dbname in order to make the correct routing decision for the connection. With this the startup packet will include the dbname parameter. Author: Jelte Fennema-Nio <me@jeltef.nl> Reviewed-by: Tristen Raab <tristen.raab@highgo.ca> Reviewed-by: Jim Jones <jim.jones@uni-muenster.de> Discussion: https://postgr.es/m/CAGECzQTw-dZkVT_RELRzfWRzY714-VaTjoBATYfZq93R8C-auA@mail.gmail.com
2023-09-21Update comment about set_join_pathlist_hook().Etsuro Fujita
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
2023-09-21Fix vacuumdb to pass buffer-usage-limit with analyze-only modeDavid Rowley
ae78cae3b added the --buffer-usage-limit to vacuumdb to allow it to include the BUFFER_USAGE_LIMIT option in the VACUUM command. Unfortunately, that commit forgot to adjust the code so the option was added to the ANALYZE command when the -Z command line argument was specified. There were no issues with the -z command as that option just adds ANALYZE to the VACUUM command. In passing adjust the code to escape the --buffer-usage-limit option before passing it to the server. It seems nothing beyond a confusing error message could become this lack of escaping as VACUUM cannot be specified in a multi-command string. Reported-by: Ryoga Yoshida Author: Ryoga Yoshida, David Rowley Discussion: https://postgr.es/m/08930c0b541700a5264e5fbf3a685f5a%40oss.nttdata.com Backpatch-through: 16, where ae78cae3b was introduced.
2023-09-19Remove open-coded binary heap in pg_dump_sort.c.Nathan Bossart
Thanks to commit 5af0263afd, binaryheap is available to frontend code. This commit replaces the open-coded heap implementation in pg_dump_sort.c with a binaryheap, saving a few lines of code. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/3612876.1689443232%40sss.pgh.pa.us
2023-09-20Fix typos in pgoutput.cMichael Paquier
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
2023-09-20psql: Reset query buffer of \e, \ef and \ev on errorMichael Paquier
If any of these commands fail during editing or pre-processing, the command stored in the query buffer would remain around without being executed immediately as PSQL_CMD_ERROR is returned as status. The next command provided by the user would run it, likely causing failures as this could include silently some of the contents generated automatically for views or functions. The problems would be different depending on the psql meta-command used: - For \ev and \ef, some errors can happen in a predictable way while doing an object lookup or while creating an object command. A failure while editing is equally problematic, but the class of failures happening in the code path of do_edit() are unlikely. The query reset is kept in exec_command_ef_ev() as a query may be unchanged. - For \e, error can happen while editing. In both cases, the query buffer is reset on error for an incorrect file number provided, whose value check is done before filling up the query buffer. This is a slight change of behavior compared to the past for some of the predictable error patterns for \ev and \ef, so for now I have made the choice to not backpatch this commit (argument particularly available for v11 that's going to be EOL'd soon). Perhaps this could be revisited later depending on the feedback of this new behavior. Author: Ryoga Yoshida, Michael Paquier Reviewed-by: Aleksander Alekseev, Kyotaro Horiguchi Discussion: https://postgr.es/m/01419622d84ef093fd4fe585520bf03c@oss.nttdata.com
2023-09-19Convert pg_restore's ready_list to a priority queue.Nathan Bossart
Presently, parallel restores spend a lot of time sorting this list so that we pick the largest items first. With many tables, this sorting can become a significant bottleneck. There are a couple of reports from the field about this, and it is easy to reproduce. This commit improves the performance of parallel pg_restore with many tables by converting its ready_list to a priority queue, i.e., a binary heap. We will first try to run the highest priority item, but if it cannot be chosen due to the lock heuristic, we'll do a sequential scan through the heap nodes until we find one that is runnable. This means that we might end up picking an item with a much lower priority. However, we expect that we will typically be able to pick one of the first few items, which should usually have a relatively high priority. Suggested-by: Tom Lane Tested-by: Pierre Ducroquet Reviewed-by: Tom Lane Discussion: https://postgr.es/m/3612876.1689443232%40sss.pgh.pa.us
2023-09-19Fix psql tab-completion for identifiers containing dollars.Heikki Linnakangas
Dollar ($) is a legit character for identifiers, except as the first character, since commit 1bd22f55cf in version 7.4. Update the tab-completion code accordingly. Author: Mikhail Gribkov Reviewed-by: Vik Fearing Discussion: https://www.postgresql.org/message-id/CAMEv5_sTAvPvhye%2Bu4jkWDe5UGDiQ1ZkQomnKCboM08zDzOe%3Dg%40mail.gmail.com
2023-09-19Replace more MemSet calls with struct initializationPeter Eisentraut
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
2023-09-19Fix GiST README's explanation of the NSN cross-check.Heikki Linnakangas
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
2023-09-19Standardize type of extend_by counterPeter Eisentraut
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
2023-09-19Improve error message for snapshot import in snapmgr.c, take twoMichael Paquier
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
2023-09-19Fix assertion failure with PL/Python exceptionsMichael Paquier
PLy_elog() was not able to handle correctly cases where a SPI called failed, which would fill in a DETAIL string able to trigger an assertion. We may want to improve this infrastructure so as it is able to provide any extra detail information provided by an error stack, but this is left as a future improvement as it could impact existing error stacks and any applications that depend on them. For now, the assertion is removed and a regression test is added to cover the case of a failure with a detail string. This problem exists since 2bd78eb8d51c, so backpatch all the way down with tweaks to the regression tests output added where required. Author: Alexander Lakhin Discussion: https://postgr.es/m/18070-ab9c171cbf4ebb0f@postgresql.org Backpatch-through: 11
2023-09-18Add function for removing arbitrary nodes in binaryheap.Nathan Bossart
This commit introduces binaryheap_remove_node(), which can be used to remove any node from a binary heap. The implementation is straightforward. The target node is replaced with the last node in the heap, and then we sift as needed to preserve the heap property. This new function is intended for use in a follow-up commit that will improve the performance of pg_restore. Reviewed-by: Tom Lane Discussion: https://postgr.es/m/3612876.1689443232%40sss.pgh.pa.us
2023-09-18Fix MSVC build for changes to binaryheap.Nathan Bossart
After 5af0263afd, binaryheap.c needs to be listed in Mkvcbuild.pm. Per buildfarm.
2023-09-18Make binaryheap available to frontend code.Nathan Bossart
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