summaryrefslogtreecommitdiff
path: root/contrib
Commit message (Collapse)AuthorAgeFilesLines
* Ensure Soundex difference() function handles empty input sanely.Tom Lane2023-05-163-7/+15
| | | | | | | | | | | | | | | | fuzzystrmatch's difference() function assumes that _soundex() always initializes its output buffer fully. This was not so for the case of a string containing no alphabetic characters, resulting in unstable output and Valgrind complaints. Fix by using memset() to fill the whole buffer in the early-exit case. Also make some cosmetic improvements (I didn't care for the random switches between "instr[0]" and "*instr" notation). Report and diagnosis by Alexander Lakhin (bug #17935). Back-patch to all supported branches. Discussion: https://postgr.es/m/17935-b99316aa79c18513@postgresql.org
* Adjust sepgsql expected output for 681d9e462 et al.Tom Lane2023-05-081-1/+0
| | | | Security: CVE-2023-2454
* Replace last PushOverrideSearchPath() call with set_config_option().Noah Misch2023-05-083-1/+65
| | | | | | | | | | | | | | | | | | | | | The two methods don't cooperate, so set_config_option("search_path", ...) has been ineffective under non-empty overrideStack. This defect enabled an attacker having database-level CREATE privilege to execute arbitrary code as the bootstrap superuser. While that particular attack requires v13+ for the trusted extension attribute, other attacks are feasible in all supported versions. Standardize on the combination of NewGUCNestLevel() and set_config_option("search_path", ...). It is newer than PushOverrideSearchPath(), more-prevalent, and has no known disadvantages. The "override" mechanism remains for now, for compatibility with out-of-tree code. Users should update such code, which likely suffers from the same sort of vulnerability closed here. Back-patch to v11 (all supported versions). Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2023-2454
* Fix sepgsql expected test outputAlvaro Herrera2023-05-051-2/+2
| | | | | | | | Commit f75cec4fff87 changed the order in which the relations are permission-checked in RI_Initial_Check, which the sepgsql test is sensitive to. Adapt. Discussion: https://postgr.es/m/3468125.1683238309@sss.pgh.pa.us
* Fix typos in commentsMichael Paquier2023-05-022-3/+3
| | | | | | | | | The changes done in this commit impact comments with no direct user-visible changes, with fixes for incorrect function, variable or structure names. Author: Alexander Lakhin Discussion: https://postgr.es/m/e8c38840-596a-83d6-bd8d-cebc51111572@gmail.com
* In hstore_plpython, avoid crashing when return value isn't a mapping.Tom Lane2023-04-273-1/+29
| | | | | | | | | | | | | | | | | | Python 3 changed the behavior of PyMapping_Check(), breaking the test in plpython_to_hstore() that verifies whether a function result to be transformed is acceptable. A backwards-compatible fix is to first verify that the object doesn't pass PySequence_Check(). Perhaps accidentally, our other uses of PyMapping_Check() already follow uses of PySequence_Check(), so that no other bugs were created by this change. Per bug #17908 from Alexander Lakhin. Back-patch to all supported branches. Dmitry Dolgov and Tom Lane Discussion: https://postgr.es/m/17908-3f19a125d56a11d6@postgresql.org
* Fix buffer refcount leak with FDW bulk insertsMichael Paquier2023-04-252-0/+40
| | | | | | | | | | | | | | | | | | | | | The leak would show up when using batch inserts with foreign tables included in a partition tree, as the slots used in the batch were not reset once processed. In order to fix this problem, some ExecClearTuple() are added to clean up the slots used once a batch is filled and processed, mapping with the number of slots currently in use as tracked by the counter ri_NumSlots. This buffer refcount leak has been introduced in b676ac4 with the addition of the executor facility to improve bulk inserts for FDWs, so backpatch down to 14. Alexander has provided the patch (slightly modified by me). The test for postgres_fdw comes from me, based on the test case that the author has sent in the report. Author: Alexander Pyhalov Discussion: https://postgr.es/m/b035780a740efd38dc30790c76927255@postgrespro.ru Backpatch-through: 14
* Validate ltree siglen GiST option to be int-alignedAlexander Korotkov2023-04-233-3/+25
| | | | | | | | | | | | Unaligned siglen could lead to an unaligned access to subsequent key fields. Backpatch to 13, where opclass options were introduced. Reported-by: Alexander Lakhin Bug: 17847 Discussion: https://postgr.es/m/17847-171232970bea406b%40postgresql.org Reviewed-by: Tom Lane, Pavel Borisov, Alexander Lakhin Backpatch-through: 13
* Remove io prefix from pg_stat_io columnsMichael Paquier2023-04-212-4/+4
| | | | | | | | | | | | | | | a9c70b46 added the statistics view pg_stat_io which contained columns "io_context" and "io_object". Given that the columns are in the pg_stat_io view, the "io" prefix is somewhat redundant, so remove it. The code variables referring to these fields are kept unchanged so as they can keep their context about I/O. Bump catalog version. Author: Melanie Plageman Reviewed-by: Kyotaro Horiguchi, Fabrízio de Royes Mello Discussion: https://postgr.es/m/CAAKRu_aAQoJWrvT2BYYQvJChFKra_O-5ra3jhzKJZqWsTR1CPQ@mail.gmail.com
* Fix various typos and incorrect/outdated name referencesDavid Rowley2023-04-192-2/+2
| | | | | Author: Alexander Lakhin Discussion: https://postgr.es/m/699beab4-a6ca-92c9-f152-f559caf6dc25@gmail.com
* Fix some typos and some incorrectly duplicated wordsDavid Rowley2023-04-182-2/+2
| | | | | | Author: Justin Pryzby Reviewed-by: David Rowley Discussion: https://postgr.es/m/ZD3D1QxoccnN8A1V@telsasoft.com
* Fix various typosDavid Rowley2023-04-184-4/+4
| | | | | | | | | | | | This fixes many spelling mistakes in comments, but a few references to invalid parameter names, function names and option names too in comments and also some in string constants Also, fix an #undef that was undefining the incorrect definition Author: Alexander Lakhin Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/d5f68d19-c0fc-91a9-118d-7c6a5a3f5fad@gmail.com
* Avoid using non-ASCII commentary in daitch_mokotoff.c.Tom Lane2023-04-161-15/+9
| | | | | | | | | | | | | Commit d6b5dee42 failed to suppress that warning from MSVC, so let's just get rid of all non-ASCII glyphs in the comments. The comment for iso8859_1_to_ascii_upper[] is not essential, and the other cases can be handled by spelling out the Unicode character names. (I'm now really in the dark as to why MSVC doesn't complain about predicate.c's comment. But whatever.) Discussion: https://postgr.es/m/1546512.1681495035@sss.pgh.pa.us
* Remove some non-ASCII symbols from a comment.Tom Lane2023-04-151-1/+1
| | | | | | | | | | | | | | | | | | | | Buildfarm member hamerkop is unhappy that daitch_mokotoff.c "contains a character that cannot be represented in the current code page (932). Save the file in Unicode format to prevent data loss". There are a lot of non-ASCII characters in comments, but I do not see any outside a comment, so this is just cosmetic. Nonetheless the warning is fairly scary and people might waste time trying to understand the cause, so we ought to silence it. We have only one other occurrence of a non-ASCII character in .c or .h files in the tree: predicate.c mentions "Uwe Röhm". It's far from clear why hamerkop isn't complaining about that too; but it isn't, so maybe there is some special provision for accented Roman letters. We can remove the non-letter symbols from the comment for iso8859_1_to_ascii_upper without any real loss of usefulness, so let's try that first to see if it silences the warning. Discussion: https://postgr.es/m/1546512.1681495035@sss.pgh.pa.us
* De-Revert "Add support for Kerberos credential delegation"Stephen Frost2023-04-136-73/+158
| | | | | | | | | | | | | | | | | | This reverts commit 3d03b24c3 (Revert Add support for Kerberos credential delegation) which was committed on the grounds of concern about portability, but on further review and discussion, it's clear that we are better off explicitly requiring MIT Kerberos as that appears to be the only GSSAPI library currently that's under proper maintenance and ongoing development. The API used for storing credentials was added to MIT Kerberos over a decade ago while for the other libraries which appear to be mainly based on Heimdal, which exists explicitly to be a re-implementation of MIT Kerberos, the API never made it to a released version (even though it was added to the Heimdal git repo over 5 years ago..). This post-feature-freeze change was approved by the RMT. Discussion: https://postgr.es/m/ZDDO6jaESKaBgej0%40tamriel.snowman.net
* Revert "Adjust contrib/sepgsql regression test expected outputs."Alvaro Herrera2023-04-122-2/+12
| | | | | | | This reverts commit 76c111a7f166; should have been included in 9ce04b50e120. Noted by Joe Conway
* basebackup_to_shell: Check for a NULL return from OpenPipeStream.Robert Haas2023-04-121-0/+5
| | | | | | Per complaint from Peter Eisentraut. Discussion: http://postgr.es/m/4f1707cc-2432-da35-64a2-5c2a8d92a388@enterprisedb.com
* Revert "Add support for Kerberos credential delegation"Stephen Frost2023-04-086-158/+73
| | | | | | | | | | | This reverts commit 3d4fa227bce4294ce1cc214b4a9d3b7caa3f0454. Per discussion and buildfarm, this depends on APIs that seem to not be available on at least one platform (NetBSD). Should be certainly possible to rework to be optional on that platform if necessary but bit late for that at this point. Discussion: https://postgr.es/m/3286097.1680922218@sss.pgh.pa.us
* Update contrib/trgm_regexp's memory management.Thomas Munro2023-04-081-15/+2
| | | | | | | | | While no code change was necessary for this code to keep working, we don't need to use PG_TRY()/PG_FINALLY() with explicit clean-up while working with regexes anymore. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGK3PGKwcKqzoosamn36YW-fsuTdOPPF1i_rtEO%3DnEYKSg%40mail.gmail.com
* Introduce PG_IO_ALIGN_SIZE and align all I/O buffers.Thomas Munro2023-04-082-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In order to have the option to use O_DIRECT/FILE_FLAG_NO_BUFFERING in a later commit, we need the addresses of user space buffers to be well aligned. The exact requirements vary by OS and file system (typically sectors and/or memory pages). The address alignment size is set to 4096, which is enough for currently known systems: it matches modern sectors and common memory page size. There is no standard governing O_DIRECT's requirements so we might eventually have to reconsider this with more information from the field or future systems. Aligning I/O buffers on memory pages is also known to improve regular buffered I/O performance. Three classes of I/O buffers for regular data pages are adjusted: (1) Heap buffers are now allocated with the new palloc_aligned() or MemoryContextAllocAligned() functions introduced by commit 439f6175. (2) Stack buffers now use a new struct PGIOAlignedBlock to respect PG_IO_ALIGN_SIZE, if possible with this compiler. (3) The buffer pool is also aligned in shared memory. WAL buffers were already aligned on XLOG_BLCKSZ. It's possible for XLOG_BLCKSZ to be configured smaller than PG_IO_ALIGNED_SIZE and thus for O_DIRECT WAL writes to fail to be well aligned, but that's a pre-existing condition and will be addressed by a later commit. BufFiles are not yet addressed (there's no current plan to use O_DIRECT for those, but they could potentially get some incidental speedup even in plain buffered I/O operations through better alignment). If we can't align stack objects suitably using the compiler extensions we know about, we disable the use of O_DIRECT by setting PG_O_DIRECT to 0. This avoids the need to consider systems that have O_DIRECT but can't align stack objects the way we want; such systems could in theory be supported with more work but we don't currently know of any such machines, so it's easier to pretend there is no O_DIRECT support instead. That's an existing and tested class of system. Add assertions that all buffers passed into smgrread(), smgrwrite() and smgrextend() are correctly aligned, unless PG_O_DIRECT is 0 (= stack alignment tricks may be unavailable) or the block size has been set too small to allow arrays of buffers to be all aligned. Author: Thomas Munro <thomas.munro@gmail.com> Author: Andres Freund <andres@anarazel.de> Reviewed-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/CA+hUKGK1X532hYqJ_MzFWt0n1zt8trz980D79WbjwnT-yYLZpg@mail.gmail.com
* Remove useless dependencies in daitch_mokotoff_header.pl.Tom Lane2023-04-071-4/+0
| | | | | | | | Actually, the correct fix for this is "we don't need this at all", because this program isn't dealing in any non-ASCII data. The dependency on Data::Dumper seems to be a leftover too. Discussion: https://postgr.es/m/3287943.1680922997@sss.pgh.pa.us
* Add support for Kerberos credential delegationStephen Frost2023-04-076-73/+158
| | | | | | | | | | | | | | | | | | | Support GSSAPI/Kerberos credentials being delegated to the server by a client. With this, a user authenticating to PostgreSQL using Kerberos (GSSAPI) credentials can choose to delegate their credentials to the PostgreSQL server (which can choose to accept them, or not), allowing the server to then use those delegated credentials to connect to another service, such as with postgres_fdw or dblink or theoretically any other service which is able to be authenticated using Kerberos. Both postgres_fdw and dblink are changed to allow non-superuser password-less connections but only when GSSAPI credentials have been delegated to the server by the client and GSSAPI is used to authenticate to the remote system. Authors: Stephen Frost, Peifeng Qiu Reviewed-By: David Christensen Discussion: https://postgr.es/m/CO1PR05MB8023CC2CB575E0FAAD7DF4F8A8E29@CO1PR05MB8023.namprd05.prod.outlook.com
* Pacify perlcritic.Tom Lane2023-04-071-1/+1
| | | | Discussion: https://postgr.es/m/3271512.1680916423@sss.pgh.pa.us
* Adjust contrib/sepgsql regression test expected outputs.Tom Lane2023-04-072-12/+2
| | | | | | | | Per buildfarm, the log output has changed as a consequence of commit e056c557a changing the catalog accesses performed in some commands. Discussion: https://postgr.es/m/20230407211950.aojbdirwdrxjeyzb@awork3.anarazel.de
* Add support for Daitch-Mokotoff Soundex in contrib/fuzzystrmatch.Tom Lane2023-04-0712-4/+1153
| | | | | | | | | This modernized version of Soundex works significantly better than the original, particularly for non-English names. Dag Lem, reviewed by quite a few people along the way Discussion: https://postgr.es/m/yger1atbgfy.fsf@sid.nimrod.no
* Refactor background psql TAP functionsDaniel Gustafsson2023-04-071-47/+23
| | | | | | | | | | | | | | | | | | | | | | This breaks out the background and interactive psql functionality into a new class, PostgreSQL::Test::BackgroundPsql. Sessions are still initiated via PostgreSQL::Test::Cluster, but once started they can be manipulated by the new helper functions which intend to make querying easier. A sample session for a command which can be expected to finish at a later time can be seen below. my $session = $node->background_psql('postgres'); $bsession->query_until(qr/start/, q( \echo start CREATE INDEX CONCURRENTLY idx ON t(a); )); $bsession->quit; Patch by Andres Freund with some additional hacking by me. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20230130194350.zj5v467x4jgqt3d6@awork3.anarazel.de
* Add pg_buffercache_usage_counts() to contrib/pg_buffercache.Tom Lane2023-04-074-0/+72
| | | | | | | | | | | | | | It was pointed out that pg_buffercache_summary()'s report of the overall average usage count isn't that useful, and what would be more helpful in many cases is to report totals for each possible usage count. Add a new function to do it like that. Since pg_buffercache 1.4 is already new for v16, we don't need to create a new extension version; we'll just define this as part of 1.4. Nathan Bossart Discussion: https://postgr.es/m/20230130233040.GA2800702@nathanxps13
* postgres_fdw: Add support for parallel abort.Etsuro Fujita2023-04-064-27/+458
| | | | | | | | | | | | postgres_fdw aborts remote (sub)transactions opened on remote server(s) in a local (sub)transaction one by one when the local (sub)transaction aborts. This patch allows it to abort the remote (sub)transactions in parallel to improve performance. This is enabled by the server option "parallel_abort". The default is false. Etsuro Fujita, reviewed by David Zhang. Discussion: http://postgr.es/m/CAPmGK15FuPVGx3TGHKShsbPKKtF1y58-ZLcKoxfN-nqLj1dZ%3Dg%40mail.gmail.com
* Convert many uses of ReadBuffer[Extended](P_NEW) to ExtendBufferedRel()Andres Freund2023-04-051-10/+2
| | | | | | | | | A few places are not converted. Some because they are tackled in later commits (e.g. hio.c, xlogutils.c), some because they are more complicated (e.g. brin_pageops.c). Having a few users of ReadBuffer(P_NEW) is good anyway, to ensure the backward compat path stays working. Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
* Fix row tracking in pg_stat_statements with extended query protocolMichael Paquier2023-04-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pg_stat_statements relies on EState->es_processed to count the number of rows processed by ExecutorRun(). This proves to be a problem under the extended query protocol when the result of a query is fetched through more than one call of ExecutorRun(), as es_processed is reset each time ExecutorRun() is called. This causes pg_stat_statements to report the number of rows calculated in the last execute fetch, rather than the global sum of all the rows processed. As pquery.c tells, this is a problem when a portal does not use holdStore. For example, DMLs with RETURNING would report a correct tuple count as these do one execution cycle when the query is first executed to fill in the portal's store with one ExecutorRun(), feeding on the portal's store for each follow-up execute fetch depending on the fetch size requested by the client. The fix proposed for this issue is simple with the addition of an extra counter in EState that's preserved across multiple ExecutorRun() calls, incremented with the value calculated in es_processed. This approach is not back-patchable, unfortunately. Note that libpq does not currently give any way to control the fetch size when using the extended v3 protocol, meaning that in-core testing is not possible yet. This issue can be easily verified with the JDBC driver, though, with *autocommit disabled*. Hence, having in-core tests requires more features, left for future discussion: - At least two new libpq routines splitting PQsendQueryGuts(), one for the bind/describe and a second for a series of execute fetches with a custom fetch size, likely in a fashion similar to what JDBC does. - A psql meta-command for the execute phase. This part is not strictly mandatory, still it could be handy. Reported-by: Andrew Dunstan (original discovery by Simon Siggs) Author: Sami Imseih Reviewed-by: Tom Lane, Michael Paquier Discussion: https://postgr.es/m/EBE6C507-9EB6-4142-9E4D-38B1673363A7@amazon.com Discussion: https://postgr.es/m/c90890e7-9c89-c34f-d3c5-d5c763a34bd8@dunslane.net
* Fix function reference in commentDaniel Gustafsson2023-04-051-1/+1
| | | | | | | | | | Commit a61b1f748 renamed ExecCheckRTEPerms to ExecCheckPermissions as part of a larger body of work, but missed this comment. Fix by updating the referenced function name to make the comment the same as other occurrences. Author: Koshi Shibagaki <shibagaki.koshi@fujitsu.com> Discussion: https://postgr.es/m/OS3PR01MB653359ACBE8DBBE29EE2BC71FA909@OS3PR01MB6533.jpnprd01.prod.outlook.com
* Fix copy-pasto in contrib/auth_delay/meson.build variable name.Noah Misch2023-04-021-2/+2
|
* Pass down table relation into more index relation functionsAndres Freund2023-04-011-7/+8
| | | | | | | | | | | | This is done in preparation for logical decoding on standby, which needs to include whether visibility affecting WAL records are about a (user) catalog table. Which is only known for the table, not the indexes. It's also nice to be able to pass the heap relation to GlobalVisTestFor() in vacuumRedirectAndPlaceholder(). Author: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com> Discussion: https://postgr.es/m/21b700c3-eecf-2e05-a699-f8c78dd31ec7@gmail.com
* Add show_data option to pg_get_wal_block_info.Peter Geoghegan2023-03-315-21/+25
| | | | | | | | | | | | | | | | | | | | | | | Allow users to opt out of returning FPI data and block data from pg_get_wal_block_info as an optimization. Testing has shown that this can make function execution over twice as fast in some cases. When pg_get_wal_block_info is called with "show_data := false", it always returns NULL values for its block_data and block_fpi_data bytea output parameters. Nothing else changes. In particular, the function will still return the usual per-block summary of block data/FPI space overhead. Use of "show_data := false" is therefore feasible with all queries that don't specifically require these raw binary strings. Follow-up to recent work in commit 122376f0. There still hasn't been a stable release with the pg_get_wal_block_info function, so no bump in the pg_walinspect extension version. Per suggestion from Melanie Plageman. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAAKRu_bJvbcYBRj2cN6G2xV7B7-Ja+pjTO1nEnEhRR8OXYiABA@mail.gmail.com Discussion: https://postgr.es/m/CAH2-Wzm9shOkEDM10_+qOZkRSQhKVxwBFiehH6EHWQQRd_rDPw@mail.gmail.com
* Show record information in pg_get_wal_block_info.Peter Geoghegan2023-03-304-88/+140
| | | | | | | | | | | | | | | | | | | | | | | | | | | Expand the output parameters in pg_walinspect's pg_get_wal_block_info function to return additional information that was previously only available from pg_walinspect's pg_get_wal_records_info function. Some of the details are attributed to individual block references, rather than aggregated into whole-record values, since the function returns one row per block reference per WAL record (unlike pg_get_wal_records_info, which always returns one row per WAL record). This structure is much easier to work with when writing queries that track how individual blocks changed over time, or when attributing costs to individual blocks (not WAL records) is useful. This is the second time that pg_get_wal_block_info has been enhanced in recent weeks. Commit 9ecb134a expanded on the original version of the function added in commit c31cf1c0 (where it first appeared under the name pg_get_wal_fpi_info). There still hasn't been a stable release since commit c31cf1c0, so no bump in the pg_walinspect extension version. Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Kyotaro HORIGUCHI <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CALj2ACVRK5=Z+2ZVsjgTTSkfEnQzCuwny7iigpG7g1btk4Ws2A@mail.gmail.com
* amcheck: In verify_heapam, allows tuples with xmin 0.Robert Haas2023-03-281-2/+1
| | | | | | | | | | | Commit e88754a1965c0f40a723e6e46d670cacda9e19bd caused that case to be reported as corruption, but Peter Geoghegan pointed out that it can legitimately happen in the case of a speculative insertion that aborts, so we'd better not flag it as corruption after all. Back-patch to v14, like the commit that introduced the issue. Discussion: http://postgr.es/m/CAH2-WzmEabzcPTxSY-NXKH6Qt3FkAPYHGQSe2PtvGgj17ZQkCw@mail.gmail.com
* Fix recent pg_walinspect fpi_length bug.Peter Geoghegan2023-03-281-8/+7
| | | | | | | | | | | | | | Commit 0276ae42dd taught pg_walinspect's pg_get_wal_record_info() function to output NULLs rather than empty strings for its record description and block_ref output parameters. However, it inadvertently moved the function call that sets fpi_length until after it was already set. As a result, pg_get_wal_record_info() always output spurious fpi_length values of 0. Fix by switching the order back (but keep the behavioral change). Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Discussion: https://postgr.es/m/CAH2-WzkJmgSYkt6-smQ+57SxSmov+EKqFZdSimFewosoL_JKoA@mail.gmail.com
* pg_walinspect: Adjust memory context name.Peter Geoghegan2023-03-271-1/+1
| | | | | | | Correct the name of the memory context used by the pg_get_wal_block_info() SQL-callable function. Oversight in commit 9ecb134a93.
* amcheck: Generalize one of the recently-added update chain checks.Robert Haas2023-03-271-11/+9
| | | | | | | | | | | | | | | | | | | | Commit bbc1376b39627c6bddd8a0dc0a7dda24c91a97a0 checked that if a redirected line pointer pointed to a tuple, the tuple should be marked both HEAP_ONLY_TUPLE and HEAP_UPDATED. But Andres Freund pointed out that *any* tuple that is marked HEAP_ONLY_TUPLE should be marked HEAP_UPDATED, not just one that is the target of a redirected line pointer. Do that instead. To see why this is better, consider a redirect line pointer A which points to a heap-only tuple B which points (via CTID) to another heap-only tuple C. With the old code, we'd complain if B was not marked HEAP_UPDATED, but with this change, we'll complain if either B or C is not marked HEAP_UPDATED. (Note that, with or without this commit, if either B or C were not marked HEAP_ONLY_TUPLE, we would also complain about that.) Discussion: http://postgr.es/m/CA%2BTgmobLypZx%3DcOH%2ByY1GZmCruaoucHm77A6y_-Bo%3Dh-_3H28g%40mail.gmail.com
* amcheck: Tighten up validation of redirect line pointers.Robert Haas2023-03-271-9/+31
| | | | | | | | | | | | | | | | | | | | | | | | Commit bbc1376b39627c6bddd8a0dc0a7dda24c91a97a0 added a new lp_valid[] array which records whether or not a line pointer was thought to be valid, but entries could sometimes get set to true in cases where that wasn't actually safe. Fix that. Suppose A is a redirect line pointer and B is the other line pointer to which it points. The old code could mishandle this situation in a couple of different ways. First, if B was unused, we'd complain about corruption but still set lp_valid[A] = true, causing later code to try to access the B as if it were pointing to a tuple. Second, if B was dead, we wouldn't complain about corruption at all, which is an oversight, and would also set lp_valid[A] = true, which would again confuse later code. Fix all that. In the case where B is a redirect, the old code was correct, but refactor things a bit anyway so that all of these cases are handled more symmetrically. Also add an Assert() and some comments. Andres Freund and Robert Haas Discussion: http://postgr.es/m/20230323172607.y3lejpntjnuis5vv%40awork3.anarazel.de
* Improve a few things in pg_walinspectMichael Paquier2023-03-271-15/+27
| | | | | | | | | | | | | | | | This improves a few things in pg_walinspect: - Return NULL rather than empty strings in pg_get_wal_records_info() for the block references and the record description if there is no information provided by the fallback. This point has been raised by Peter Geoghegan. - Add a check on XLogRecHasAnyBlockRefs() for pg_get_wal_block_info(), to directly skip records that have no block references. This speeds up the function a bit, depending on the number of records that have no block references. Author: Bharath Rupireddy Reviewed-by: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/CALj2ACWL9RG8sGJHinggRNBTxgRWJTSxCkB+cE6=t3Phh=Ey+A@mail.gmail.com
* amcheck: Fix verify_heapam for tuples where xmin or xmax is 0.Robert Haas2023-03-241-2/+6
| | | | | | | | | | | | | | | | | | | | | | | In such cases, get_xid_status() doesn't set its output parameter (the third argument), so we shouldn't fall through to code which will test the value of that parameter. There are five existing calls to get_xid_status(), three of which seem to already handle this case properly. This commit tries to fix the other two. If we're checking xmin and find that it is invalid (i.e. 0) just report that as corruption, similar to what's already done in the three cases that seem correct. If we're checking xmax and find that's invalid, that's fine: it just means that the tuple hasn't been updated or deleted. Thanks to Andres Freund and valgrind for finding this problem, and also to Andres for having a look at the patch. This bug seems to go all the way back to where verify_heapam was first introduced, but wasn't detected until recently, possibly because of the new test cases added for update chain verification. Back-patch to v14, where this code showed up. Discussion: http://postgr.es/m/CA+TgmoZAYzQZqyUparXy_ks3OEOfLD9-bEXt8N-2tS1qghX9gQ@mail.gmail.com
* amcheck: Fix a few bugs in new update chain validation.Robert Haas2023-03-231-3/+7
| | | | | | | | | | | | | | | | | | | | We shouldn't set successor[whatever] to an offset number that is less than FirstOffsetNumber or more than maxoff. We already avoided that for redirects, but not for CTID links. Allowing bad offset numbers into the successor[] array causes core dumps. We shouldn't use HeapTupleHeaderIsHotUpdated() because it checks stuff other than the status of the infomask2 bit HEAP_HOT_UPDATED. We only care about the status of that bit, not the other stuff that HeapTupleHeaderIsHotUpdated() checks. This mistake can cause verify_heapam() to report corruption when none is present. The first hunk of this patch was written by me. The other two were written by Andres Freund. This could probably do with more review before commit, but I'd like to try to get the buildfarm green again sooner rather than later. Discussion: http://postgr.es/m/20230322204552.s6cv3ybqkklhhybb@awork3.anarazel.de
* Improve a bit the tests of pg_walinspectMichael Paquier2023-03-234-45/+47
| | | | | | | | | | | | | This commit improves the tests of pg_walinspect on a few things: - Remove aggregates for queries that should fail. If the code is reworked in such a way that the behavior of these queries is changed, we would get more input from them, written this way. - Expect at least one record reported in the valid queries doing scans across ranges, rather than zero records. - Adjust a few comments, for consistency. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACVaoXW3nJD9zq8E66BEf-phgJfFcKRVJq9GXkuX0b3ULQ@mail.gmail.com
* Teach verify_heapam() to validate update chains within a page.Robert Haas2023-03-221-6/+285
| | | | | | | | | | | Prior to this commit, we only consider each tuple or line pointer on the page in isolation, but now we can do some validation of a line pointer against its successor. For example, a redirect line pointer shouldn't point to another redirect line pointer, and if a tuple is HOT-updated, the result should be a heap-only tuple. Himanshu Upadhyaya and Robert Haas, reviewed by Aleksander Alekseev, Andres Freund, and Peter Geoghegan.
* Fix t_isspace(), etc., when datlocprovider=i and datctype=C.Jeff Davis2023-03-173-28/+0
| | | | | | | | | | | | | Check whether the datctype is C to determine whether t_isspace() and related functions use isspace() or iswspace(). Previously, t_isspace() checked whether the database default collation was C; which is incorrect when the default collation uses the ICU provider. Discussion: https://postgr.es/m/79e4354d9eccfdb00483146a6b9f6295202e7890.camel@j-davis.com Reviewed-by: Peter Eisentraut Backpatch-through: 15
* Improve several permission-related error messages.Peter Eisentraut2023-03-173-7/+18
| | | | | | | | | Mainly move some detail from errmsg to errdetail, remove explicit mention of superuser where appropriate, since that is implied in most permission checks, and make messages more uniform. Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/20230316234701.GA903298@nathanxps13
* postgres_fdw: Remove useless if-test in GetConnection().Etsuro Fujita2023-03-171-2/+1
| | | | | | | | | | Checking whether entry->conn is NULL after doing disconnect_pg_server() for that entry is pointless, as that function ensures that it is NULL. Thinko in commit 7fc1a81e4; this would be harmless, so patch HEAD only. Reviewed-by: Richard Guo and Daniel Gustafsson Discussion: https://postgr.es/m/CAPmGK169vQ83PQwQkoxO-AK2EeK1EsgsxixedM%2BBLWEAhZ_AqQ%40mail.gmail.com
* Add macros for ReorderBufferTXN toptxn.Amit Kapila2023-03-171-2/+2
| | | | | | | | | | Currently, there are quite a few places in reorderbuffer.c that tries to access top-transaction for a subtransaction. This makes the code to access top-transaction consistent and easier to follow. Author: Peter Smith Reviewed-by: Vignesh C, Sawada Masahiko Discussion: https://postgr.es/m/CAHut+PuCznOyTqBQwjRUu-ibG-=KHyCv-0FTcWQtZUdR88umfg@mail.gmail.com
* Use "data directory" not "current directory" in error messages.Tom Lane2023-03-162-2/+2
| | | | | | | | | | | The user receiving the message might not understand where the server's "current directory" is. "Data directory" seems clearer. (This would not be good for frontend code, but both of these messages are only issued in the backend.) Kyotaro Horiguchi Discussion: https://postgr.es/m/20230316.111646.1564684434328830712.horikyota.ntt@gmail.com