aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix nbtree page recycling comment.Peter Geoghegan2024-04-16
| | | | Oversight in commit e5d8a99903.
* revert: Generalize relation analyze in table AM interfaceAlexander Korotkov2024-04-16
| | | | | | This commit reverts 27bc1772fc and dd1f6b0c17. Per review by Andres Freund. Discussion: https://postgr.es/m/20240415201057.khoyxbwwxfgzomeo%40awork3.anarazel.de
* Improve test coverage in bump.cDavid Rowley2024-04-16
| | | | | | | | | | | | There were no callers of BumpAllocLarge() in the regression tests, so here we add a sort with a tuple large enough to use that path in bump.c. Also, BumpStats() wasn't being called, so add a test to sysviews.sql to call pg_backend_memory_contexts() while a bump context exists in the backend. Reported-by: Andres Freund Discussion: https://postgr.es/m/20240414223305.m3i5eju6zylabvln@awork3.anarazel.de
* Add missing PGDLLIMPORT markingsMichael Paquier2024-04-16
| | | | | | | | | | | | | All backend-side variables should be marked with PGDLLIMPORT, as per policy introduced in 8ec569479f. aafc05de1bf5 has forgotten MyClientSocket, and 05c3980e7f47 LoadedSSL. These can be spotted with a command like this one (be careful of not switching __pg_log_level): src/tools/mark_pgdllimport.pl $(git ls-files src/include/) Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/ZhzkRzrkKhbeQMRm@paquier.xyz
* Fix type-checking of RECORD-returning functions in FROM, redux.Tom Lane2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 2ed8f9a01 intended to institute a policy that if a RangeTblFunction has a coldeflist, then the function return type is certainly RECORD, and we should use the coldeflist as the source of truth about what the columns of the record type are. When the original function has been folded to a constant, inspection of the constant might give a different answer. This situation will lead to a tuple-type-mismatch error at execution, but up until that point we need to consistently believe the coldeflist, or we'll have problems from different bits of code reaching different conclusions. expandRTE didn't get that memo though, and would try to produce a tupdesc based on the constant in this situation, leading to an assertion failure. (Desultory testing suggests that non-assert builds often manage to give the expected error, although I also saw a "cache lookup failed for type 0" error, and it seems at least possible that a crash could happen.) Some other callers of get_expr_result_type and get_expr_result_tupdesc were also being incautious about this. While none of them seem to have actual bugs, they're working harder than necessary in this case, besides which it seems safest to have an explicit policy of not using those functions on an RTE with a coldeflist. Adjust the code accordingly, and add commentary to funcapi.c about this policy. Also fix an obsolete comment that claimed "get_expr_result_type() doesn't know how to extract type info from a RECORD constant". That hasn't been true since commit d57534740. Per bug #18422 from Alexander Lakhin. As with the previous commit, back-patch to all supported branches. Discussion: https://postgr.es/m/18422-89ca86c8eac5246d@postgresql.org
* ATTACH PARTITION: Don't match a PK with a UNIQUE constraintAlvaro Herrera2024-04-15
| | | | | | | | | | When matching constraints in AttachPartitionEnsureIndexes() we weren't testing the constraint type, which could make a UNIQUE key lacking a not-null constraint incorrectly satisfy a primary key requirement. Fix this by testing that the constraint types match. (Other possible mismatches are verified by comparing index properties.) Discussion: https://postgr.es/m/202402051447.wimb4xmtiiyb@alvherre.pgsql
* Grammar fixes for split/merge partitions codeAlexander Korotkov2024-04-15
| | | | | | | | | | The fixes relate to comments, error messages, and corresponding expected output of regression tests. Discussion: https://postgr.es/m/CAMbWs49DDsknxyoycBqiE72VxzL_sYHF6zqL8dSeNehKPJhkKg%40mail.gmail.com Discussion: https://postgr.es/m/86bfd241-a58c-479a-9a72-2c67a02becf8%40postgrespro.ru Discussion: https://postgr.es/m/CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw%40mail.gmail.com Author: Richard Guo, Dmitry Koval, Tender Wang
* Fix propagating attnotnull in multiple inheritanceAlvaro Herrera2024-04-15
| | | | | | | | | | | | | | | | | | | In one of the many strange corner cases of multiple inheritance being used, commit b0e96f311985 missed a CommandCounterIncrement() call after updating the attnotnull flag during ALTER TABLE ADD COLUMN, which caused a catalog tuple to be update attempted twice in the same command, giving rise to a "tuple already updated by self" error. Add the missing call to solve that, and a test case that reproduces the scenario. As a (perhaps surprising) secondary effect, this CCI addition triggers another behavior change: when a primary key is added to a parent partitioned table and the column in an existing partition does not have a not-null constraint, we no longer error out. This will probably be a welcome change by some users, and I think it's unlikely that anybody will miss the old behavior. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: http://postgr.es/m/045dec3f-9b3d-aa44-0c99-85f6992306c7@gmail.com
* psql: Make output of \dD more stablePeter Eisentraut2024-04-15
| | | | | | | \dD showed domain check constraints in arbitrary order, which can cause regression test failures, which was exposed by commit 9895b35cb8. To fix, order the constraints by conname, which matches what psql does in other queries listing constraints.
* Fix ALTER DOMAIN NOT NULL syntaxPeter Eisentraut2024-04-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This addresses a few problems with commit e5da0fe3c22 ("Catalog domain not-null constraints"). In CREATE DOMAIN, a NOT NULL constraint looks like CREATE DOMAIN d1 AS int [ CONSTRAINT conname ] NOT NULL (Before e5da0fe3c22, the constraint name was accepted but ignored.) But in ALTER DOMAIN, a NOT NULL constraint looks like ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL VALUE where VALUE is where for a table constraint the column name would be. (This works as of e5da0fe3c22. Before e5da0fe3c22, this syntax resulted in an internal error.) But for domains, this latter syntax is confusing and needlessly inconsistent between CREATE and ALTER. So this changes it to just ALTER DOMAIN d1 ADD [ CONSTRAINT conname ] NOT NULL (None of these syntaxes are per SQL standard; we are just living with the bits of inconsistency that have built up over time.) In passing, this also changes the psql \dD output to not show not-null constraints in the column "Check", since it's already shown in the column "Nullable". This has also been off since e5da0fe3c22. Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org
* Put back initialization of 'sslmode', to silence CoverityHeikki Linnakangas2024-04-14
| | | | | | | | | | | | | | | | Coverity pointed out that the function checks for conn->sslmode != NULL, which implies that it might be NULL, but later we access it without a NULL-check anyway. It doesn't know that it is in fact always initialized earlier, in conninfo_add_defaults(), and hence the NULL-check is not necessary. However, there is a lot of distance between conninfo_add_defaults() and pqConnectOptions2(), so it's not surprising that it doesn't see that. Put back the initialization code, as it existed before commit 05fd30c0e7, to silence the warning. In the long run, I'd like to refactor the libpq options handling and initalization code. It seems silly to strdup() and copy strings, for things like sslmode that have a limited set of possible values; it should be an enum. But that's for another day.
* Fix unnecessary padding in incremental backupsTomas Vondra2024-04-14
| | | | | | | | | | | | | Commit 10e3226ba13d added padding to incremental backups to ensure the block data is properly aligned. The code in sendFile() however failed to consider that the header may be a multiple of BLCKSZ and thus already aligned, adding a full BLCKSZ of unnecessary padding. Not only does this make the incremental file a bit larger, but the other places calculating the amount of padding did realize it's not needed and did not include it in the formula. This resulted in pg_basebackup getting confused while parsing the data stream, trying to access files with invalid filenames (e.g. with binary data etc.) and failing.
* Use the correct PG_DETOAST_DATUM macro in BRINTomas Vondra2024-04-14
| | | | | | | | | | | | Commit 6bcda4a721 replaced PG_DETOAST_DATUM with PG_DETOAST_DATUM_PACKED in two BRIN output functions, for minmax-multi and bloom opclasses. But this is incorrect - the code is accessing the data through structs that already include a 4B header, so the detoast needs to match that. But the PACKED macro may keep the 1B header, which means the struct fields will point to incorrect data. Backpatch-through: 16 Discussion: https://postgr.es/m/1df00a66-db5a-4e66-809a-99b386a06d86%40enterprisedb.com
* Update nbits_set in brin_bloom_unionTomas Vondra2024-04-14
| | | | | | | | | | | | | | | | | | | Properly update the number of bits set in the bitmap after merging the filters in brin_bloom_union. This is mostly harmless, as the counter is used only in the output function, which means pageinspect may show incorrect information about the BRIN summary. The counter does not affect correctness. Discovered while adding a regression test comparing indexes built with and without parallelism. The parallel index builds exercise the union procedure when merging results from workers, which is otherwise very hard to do in a test. Which is why this went unnoticed until now. Backpatch through 14, where the BRIN bloom opclasses were introduced. Backpatch-through: 14 Discussion: https://postgr.es/m/1df00a66-db5a-4e66-809a-99b386a06d86%40enterprisedb.com
* freespace: Don't return blocks past the end of the main fork.Noah Misch2024-04-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GetPageWithFreeSpace() callers assume the returned block exists in the main fork, failing with "could not read block" errors if that doesn't hold. Make that assumption reliable now. It hadn't been guaranteed, due to the weak WAL and data ordering of participating components. Most operations on the fsm fork are not WAL-logged. Relation extension is not WAL-logged. Hence, an fsm-fork block on disk can reference a main-fork block that no WAL record has initialized. That could happen after an OS crash, a replica promote, or a PITR restore. wal_log_hints makes the trouble easier to hit; a replica promote or PITR ending just after a relevant fsm-fork FPI_FOR_HINT may yield this broken state. The v16 RelationAddBlocks() mechanism also makes the trouble easier to hit, since it bulk-extends even without extension lock waiters. Commit 917dc7d2393ce680dea7a59418be9ff341df3c14 stopped trouble around truncation, but vectors involving PageIsNew() pages remained. This implementation adds a RelationGetNumberOfBlocks() call when the cached relation size doesn't confirm a block exists. We've been unable to identify a benchmark that slows materially, but this may show up as additional time in lseek(). An alternative without that overhead would be a new ReadBufferMode such that ReadBufferExtended() returns NULL after a 0-byte read, with all other errors handled normally. However, each GetFreeIndexPage() caller would then need code for the return-NULL case. Back-patch to v14, due to earlier versions not caching relation size and the absence of a pre-v16 problem report. Ronan Dunklau. Reported by Ronan Dunklau. Discussion: https://postgr.es/m/1878547.tdWV9SEqCh%40aivenlaptop
* Document PG_TEST_EXTRA=libpq_encryption and also check 'kerberos'Heikki Linnakangas2024-04-12
| | | | | | | In the libpq encryption negotiation tests, don't run the GSSAPI tests unless PG_TEST_EXTRA='kerberos' is also set. That makes it possible to still run most of the tests when GSSAPI support is compiled in, but there's no MIT Kerberos installation.
* Move libpq encryption negotiation testsHeikki Linnakangas2024-04-12
| | | | | | | | | | | | | The test targets libpq's options, so 'src/test/interfaces/libpq/t' is a more natural place for it. While doing this, I noticed that I had missed adding the libpq_encryption subdir to the Makefile. That's why this commit only needs to remove it from the meson.build file. Per Peter Eisentraut's suggestion. Discussion: https://www.postgresql.org/message-id/09d4bf5d-d0fa-4c66-a1d7-5ec757609646@eisentraut.org
* Fix compilation with --with-gssapi --without-opensslHeikki Linnakangas2024-04-12
| | | | | | | The #define is spelled ENABLE_GSS, not USE_GSS. Introduced in commit 05fd30c0e7, reported by Thomas Munro. Discussion: https://www.postgresql.org/message-id/CA%2BhUKG%2BHRTtB%2Bx%2BKKKj_cfX6sNhbeGuqmGxjGMwdVPG7YGFP8w@mail.gmail.com
* Fix libpq_encryption tests when compiled without SSL supportHeikki Linnakangas2024-04-12
| | | | | | | | | | | | | It correctly skipped tests involving SSL in the server when SSL support was not compiled in, but even when SSL is not enabled in the server and the connection is established without SSL, libpq behaves differently in many of the test scenarios when libpq is compiled without SSL support. For example, with sslmode=prefer, if libpq is compiled with SSL support it will attempt to use SSL, but without SSL support it will try authenticating in plaintext mode directly. The expected test output didn't take that into account. Discussion: https://www.postgresql.org/message-id/CA%2BhUKG%2BHRTtB%2Bx%2BKKKj_cfX6sNhbeGuqmGxjGMwdVPG7YGFP8w@mail.gmail.com
* Don't allocate large buffer on the stack in pg_verifybackupAndrew Dunstan2024-04-12
| | | | | | | | | Per complaint from Andres Freund. Follow his suggestion to allocate the buffer once in the calling routine instead. Also make a tiny indentation improvement. Discussion: https://postgr.es/m/20240411190147.a3yries632olfcgg@awork3.anarazel.de
* Assorted minor cleanups in the test_json_parser moduleAndrew Dunstan2024-04-12
| | | | | | | | | | Per gripes from Michael Paquier Discussion: https://postgr.es/m/ZhTQ6_w1vwOhqTQI@paquier.xyz Along the way, also clean up a handful of typos in 3311ea86ed and ea7b4e9a2a, found by Alexander Lakhin, and a couple of stylistic snafus noted by Daniel Westermann and Daniel Gustafsson.
* Shrink test file for test_json_parser moduleAndrew Dunstan2024-04-12
| | | | | | | | Also delete live URLs Jacob Champion Discussion: https://postgr.es/m/CAOYmi+mtH=V1wZKAOauCd5QqQWr61hnXMJbJ9h-CZXAa1JXd3w@mail.gmail.com
* Add a TAP test for test_json_parser_perfAndrew Dunstan2024-04-12
| | | | | This just makes sure the test can run with a single iteration. A real performance test would test with many more.
* Fix some memory leaks associated with parsing json and manifestsAndrew Dunstan2024-04-12
| | | | | | | | | | | Coverity complained about not freeing some memory associated with incrementally parsing backup manifests. To fix that, provide and use a new shutdown function for the JsonManifestParseIncrementalState object, in line with a suggestion from Tom Lane. While analysing the problem, I noticed a buglet in freeing memory for incremental json lexers. To fix that remove a bogus condition on freeing the memory allocated for them.
* Fix recently introduced typo in code commentDavid Rowley2024-04-12
| | | | | Reported-by: Richard Guo Discussion: https://postgr.es/m/CAMbWs49kAsZUsj7-0SBLvE9+uKz0RCqMEmM3NVytc1YvS8sTrQ@mail.gmail.com
* Fix the review comments and a bug in the slot sync code.Amit Kapila2024-04-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | Ensure that when updating the catalog_xmin of the synced slots, it is first written to disk before changing the in-memory value (effective_catalog_xmin). This is to prevent a scenario where the in-memory value change triggers a vacuum to remove catalog tuples before the catalog_xmin is written to disk. In the event of a crash before the catalog_xmin is persisted, we would not know that some required catalog tuples have been removed and the synced slot would be invalidated. Change the sanity check to ensure that remote_slot's confirmed_flush LSN can't precede the local/synced slot during slot sync. Note that the restart_lsn of the synced/local slot can be ahead of remote_slot. This can happen when slot advancing machinery finds a running xacts record after reaching the consistent state at a later point than the primary where it serializes the snapshot and updates the restart_lsn. Make the check to sync slots robust by allowing to sync only when the confirmed_lsn, restart_lsn, or catalog_xmin of the remote slot is ahead of the synced/local slot. Reported-by: Amit Kapila and Shveta Malik Author: Hou Zhijie, Shveta Malik Reviewed-by: Amit Kapila, Bertrand Drouvot Discussion: https://postgr.es/m/OS0PR01MB57162B67D3CB01B2756FBA6D94062@OS0PR01MB5716.jpnprd01.prod.outlook.com Discussion: https://postgr.es/m/CAJpy0uCSS5zmdyUXhvw41HSdTbRqX1hbYqkOfHNj7qQ+2zn0AQ@mail.gmail.com
* Fix IS [NOT] NULL qual optimization for inheritance tablesDavid Rowley2024-04-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | b262ad440 added code to have the planner remove redundant IS NOT NULL quals and eliminate needless scans for IS NULL quals on tables where the qual's column has a NOT NULL constraint. That commit failed to consider that an inheritance parent table could have differing NOT NULL constraints between the parent and the child. This caused issues as if we eliminated a qual on the parent, when applying the quals to child tables in apply_child_basequals(), the qual might not have been added to the parent's baserestrictinfo. Here we fix this by not applying the optimization to remove redundant quals to RelOptInfos belonging to inheritance parents and applying the optimization again in apply_child_basequals(). Effectively, this means that the parent and child are considered independently as the parent has both an inh=true and inh=false RTE and we still apply the optimization to the RelOptInfo corresponding to the inh=false RTE. We're able to still apply the optimization in add_base_clause_to_rel() for partitioned tables as the NULLability of partitions must match that of their parent. And, if we ever expand restriction_is_always_false() and restriction_is_always_true() to handle partition constraints then we can apply the same logic as, even in multi-level partitioned tables, there's no way to route values to a partition when the qual does not match the partition qual of the partitioned table's parent partition. The same is true for CHECK constraints as those must also match between arent partitioned tables and their partitions. Author: Richard Guo, David Rowley Discussion: https://postgr.es/m/CAMbWs4930gQSZmjR7aANzEapdy61gCg6z8dT-kAEYD0sYWKPdQ@mail.gmail.com
* Revert: Implement pg_wal_replay_wait() stored procedureAlexander Korotkov2024-04-11
| | | | | | | This commit reverts 06c418e163, e37662f221, bf1e650806, 25f42429e2, ee79928441, and 74eaf66f98 per review by Heikki Linnakangas. Discussion: https://postgr.es/m/b155606b-e744-4218-bda5-29379779da1a%40iki.fi
* Revert: Allow table AM to store complex data structures in rd_amcacheAlexander Korotkov2024-04-11
| | | | | | This commit reverts 02eb07ea89 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Revert: Allow table AM tuple_insert() method to return the different slotAlexander Korotkov2024-04-11
| | | | | | This commit reverts c35a3fb5e0 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Revert: Allow locking updated tuples in tuple_update() and tuple_delete()Alexander Korotkov2024-04-11
| | | | | | This commit reverts 87985cc925 and 818861eb57 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Revert: Let table AM insertion methods control index insertionAlexander Korotkov2024-04-11
| | | | | | This commit reverts b1484a3f19 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Revert: Custom reloptions for table AMAlexander Korotkov2024-04-11
| | | | | | This commit reverts 9bd99f4c26 and 422041542f per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
* Use correct datatype for xmin variables in slot.cMichael Paquier2024-04-11
| | | | | | | | | | | | Two variables storing a slot's effective_xmin and effective_catalog_xmin were saved as XLogRecPtr, which is incorrect as these should be TransactionIds. Oversight in 818fefd8fd44. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACVPSB74mrDTFezz-LV3Oi6F3SN71QA0oUHvndzi5dwTNg@mail.gmail.com Backpatch-through: 16
* Revert indexed and enlargable binary heap implementation.Masahiko Sawada2024-04-11
| | | | | | | | | | | This reverts commit b840508644 and bcb14f4abc. These commits were made for commit 5bec1d6bc5 (Improve eviction algorithm in ReorderBuffer using max-heap for many subtransactions). However, per discussion, commit efb8acc0d0 replaced binary heap + index with pairing heap, and made these commits unnecessary. Reported-by: Jeff Davis Discussion: https://postgr.es/m/12747c15811d94efcc5cda72d6b35c80d7bf3443.camel%40j-davis.com
* Replace binaryheap + index with pairingheap in reorderbuffer.cMasahiko Sawada2024-04-11
| | | | | | | | | | | | | | | | | | | A pairing heap can perform the same operations as the binary heap + index, with as good or better algorithmic complexity, and that's an existing data structure so that we don't need to invent anything new compared to v16. This commit makes the new binaryheap functionality that was added in commits b840508644 and bcb14f4abc unnecessary, but they will be reverted separately. Remove the optimization to only build and maintain the heap when the amount of memory used is close to the limit, becuase the bookkeeping overhead with the pairing heap seems to be small enough that it doesn't matter in practice. Reported-by: Jeff Davis Author: Heikki Linnakangas Reviewed-by: Michael Paquier, Hayato Kuroda, Masahiko Sawada Discussion: https://postgr.es/m/12747c15811d94efcc5cda72d6b35c80d7bf3443.camel%40j-davis.com
* Fix grammar.Thomas Munro2024-04-11
| | | | | Reported-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/ZhdKqj5DwoOzirFv%40paquier.xyz
* Fix potential stack overflow in incremental backup.Thomas Munro2024-04-11
| | | | | | | | | | | | The user can set RELSEG_SIZE to a high number at compile time, so we can't use it to control the size of an array on the stack: it could be many gigabytes in size. On closer inspection, we don't really need that intermediate array anyway. Let's just write directly into the output array, and then perform the absolute->relative adjustment in place. This fixes new code from commit dc212340058. Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKG%2B2hZ0sBztPW4mkLfng0qfkNtAHFUfxOMLizJ0BPmi5%2Bg%40mail.gmail.com
* Fix inconsistency with replay of hash squeeze record for clean buffersMichael Paquier2024-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | aa5edbe379d6 has tweaked _hash_freeovflpage() so as the write buffer's LSN is updated only when necessary, when REGBUF_NO_CHANGE is not used. The replay code was not consistent with that, causing the write buffer's LSN to be updated and its page to be marked as dirty even if the buffer was registered in a "clean" state. This was possible for the case of a squeeze record when there are no tuples to add to the write buffer, for (is_prim_bucket_same_wrt && !is_prev_bucket_same_wrt). I have performed some validation of this commit with wal_consistency_checking and a change in WAL that logs REGBUF_NO_CHANGE to a new BKPIMAGE_*. Thanks to that, it is possible to know at replay if a buffer was clean when it was registered, then cross-checked the LSN of the "clean" page copy coming from WAL with the LSN of the block once the record has been replayed. This eats one bit in bimg_info, which is not acceptable to be integrated as-is, but it could become handy in the future. I didn't spot other areas than the one fixed by this commit at the extent of what the main regression test suite covers. As this is an oversight in aa5edbe379d6, no backpatch is required. Reported-by: Zubeyr Eryilmaz Author: Hayato Kuroda Reviewed-by: Amit Kapila, Michael Paquier Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz
* Fix plpgsql's handling of -- comments following expressions.Tom Lane2024-04-10
| | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, read_sql_construct() has collected all the source text from the statement or expression's initial token up to the character just before the "until" token. It normally tries to strip trailing whitespace from that, largely for neatness. If there was a "-- text" comment after the expression, this resulted in removing the newline that terminates the comment, which creates a hazard if we try to paste the collected text into a larger SQL construct without inserting a newline after it. In particular this caused our handling of CASE constructs to fail if there's a comment after a WHEN expression. Commit 4adead1d2 noticed a similar problem with cursor arguments, and worked around it through the rather crude hack of suppressing the whitespace-trimming behavior for those. Rather than do that and leave the hazard open for future hackers to trip over, let's fix it properly. pl_scanner.c already has enough infrastructure to report the end location of the expression's last token, so we can copy up to that location and never collect any trailing whitespace or comment to begin with. Erik Wienhold and Tom Lane, per report from Michal Bartak. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAAVzF_FjRoi8fOVuLCZhQJx6HATQ7MKm=aFOHWZODFnLmjX-xA@mail.gmail.com
* Make GIN tests using injection points concurrent-safeMichael Paquier2024-04-10
| | | | | | | | | | | | | | | | | | | | | f587338dec87 has introduced in the test module injection_points a SQL function called injection_points_set_local(), that can be used to make all the injection points linked to the process where they are attached, discarded automatically if any remain once the process exits. e2e3b8ae9ed7 has added a NO_INSTALLCHECK to the test module to prevent the use of installcheck. Now that there is a way to make the test concurrent-safe, let's use it and remove the installcheck restriction. Concurrency issues could be easily reproduced by running in a tight loop a command like this one, in src/test/modules/gin/ (hardcoding pg_sleep() after attaching injection points enlarges the race window) and a second test suite like contrib/btree_gin/: make installcheck USE_MODULE_DB=1 Reviewed-by: Andrey Borodin Discussion: https://postgr.es/m/ZhNG4Io9uYOgwv3F@paquier.xyz
* Fix a test in failover slots regression test.Amit Kapila2024-04-10
| | | | | | | | | | | | | | | | Wait for the standby to catch up before syncing the slots with pg_sync_replication_slots(), otherwise, the logical slot could be ahead and the sync would fail. The other way to fix the test is to change it to use slotsync worker and poll for the sync to get finished but the current approach is better as this is a predictable way to write the test. Per buildfarm Author: Hou Zhijie Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/OS0PR01MB571665359F2F5DCD3ADABC9F94002@OS0PR01MB5716.jpnprd01.prod.outlook.com
* Fix illegal attribute propagation in LLVM JIT.Thomas Munro2024-04-10
| | | | | | | | | | | | | | | | | | | Commit 72559438 started copying more attributes from AttributeTemplate to the functions we generate on the fly. In the case of deform functions, which return void, this meant that "noundef", from AttributeTemplate's return value (a Datum) was copied to a void type. Older LLVM releases were OK with that, but LLVM 18 crashes. Update our llvm_copy_attributes() function to skip copying the attribute for the return value, if the target function returns void. Thanks to Dmitry Dolgov for help chasing this down. Back-patch to all supported releases, like 72559438. Reported-by: Pavel Stehule <pavel.stehule@gmail.com> Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com> Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com
* Fixup various StringInfo function usagesDavid Rowley2024-04-10
| | | | | | | | | | | | | | | This adjusts various appendStringInfo* function calls to use a more appropriate and efficient function with the same behavior. For example, use appendStringInfoChar() when appending a single character rather than appendStringInfo() and appendStringInfoString() when no formatting is required rather than using appendStringInfo(). All adjustments made here are in code that's new to v17, so it makes sense to fix these now rather than wait a few years and make backpatching harder. Discussion: https://postgr.es/m/CAApHDvojY2UvMiO+9_55ArTj10P1LBNJyyoGB+C65BLDNT0GsQ@mail.gmail.com Reviewed-by: Nathan Bossart, Tom Lane
* revert: Transform OR clauses to ANY expressionAlexander Korotkov2024-04-10
| | | | | | | This commit reverts 72bd38cc99 due to implementation and design issues. Reported-by: Tom Lane Discussion: https://postgr.es/m/3604469.1712628736%40sss.pgh.pa.us
* Remove unused BumpBlockIsValid macroDavid Rowley2024-04-10
| | | | | | | | | | | The bump allocator was recently added in 29f6a959c. Our other allocators have a similar macro to this, but seemingly the version of the macro for those allocators is only used in places where the chunk header is decoded. Since the bump allocator has no chunk header, none of those functions exist for bump therefore macro is unused. Remove it. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/5f724fb2-96e1-4f36-b65b-47b337ad432e@eisentraut.org
* Checks for ALTER TABLE ... SPLIT/MERGE PARTITIONS ... commandsAlexander Korotkov2024-04-10
| | | | | | | | Check that the target partition actually belongs to the parent table. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/cd842601-cf1a-9806-f7b7-d2509b93ba61%40gmail.com Author: Dmitry Koval
* Fix incorrect format placeholdersPeter Eisentraut2024-04-09
|
* Fix whitespacePeter Eisentraut2024-04-09
|
* Get rid of anonymous structJohn Naylor2024-04-09
| | | | | | | | | | | | | | This is a C11 feature, and we require C99. While at it, go the further step and get rid of the surrounding union (with uintptr_t) entirely, as there is currently no use case for this file to access the header of BlocktableEntry as a uintptr_t, and there are no additional alignment requirements. The least invasive way seems to be to transfer the old union name to this struct. Reported by Pavel Borisov and Andres Freund, per buildfarm member mylodon Reviewed by Pavel Borisov Discussion: https://postgr.es/m/CALT9ZEH11NYV8AOzKb1bWhCf6J0H=H31f0MgT9xX+HdqvcA1rw@mail.gmail.com