aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Fix inconsistent RestrictInfo serial numbersRichard Guo2024-11-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we generate multiple clones of the same qual condition to cope with outer join identity 3, we need to ensure that all the clones get the same serial number. To achieve this, we reset the root->last_rinfo_serial counter each time we produce RestrictInfo(s) from the qual list (see deconstruct_distribute_oj_quals). This approach works only if we ensure that we are not changing the qual list in any way that'd affect the number of RestrictInfos built from it. However, with b262ad440, an IS NULL qual on a NOT NULL column might result in an additional constant-FALSE RestrictInfo. And different versions of the same qual clause can lead to different conclusions about whether it can be reduced to constant-FALSE. This would affect the number of RestrictInfos built from the qual list for different versions, causing inconsistent RestrictInfo serial numbers across multiple clones of the same qual. This inconsistency can confuse users of these serial numbers, such as rebuild_joinclause_attr_needed, and lead to planner errors such as "ERROR: variable not found in subplan target lists". To fix, reset the root->last_rinfo_serial counter after generating the additional constant-FALSE RestrictInfo. Back-patch to v17 where the issue crept in. In v17, I failed to make a test case that would expose this bug, so no test case for v17. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs4-B6kafn+LmPuh-TYFwFyEm-vVj3Qqv7Yo-69CEv14rRg@mail.gmail.com
* Fix __attribute__((target(...))) usage.Nathan Bossart2024-11-07
| | | | | | | | | | The commonly supported way to specify multiple target options is to surround the entire list with quotes and to use a comma (with no extra spaces) as the delimiter. Oversight in commit f78667bd91. Discussion: https://postgr.es/m/Zy0jya8nF8CPpv3B%40nathan
* Use __attribute__((target(...))) for AVX-512 support.Nathan Bossart2024-11-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | Presently, we check for compiler support for the required intrinsics both with and without extra compiler flags (e.g., -mxsave), and then depending on the results of those checks, we pick which files to compile with which flags. This is tedious and complicated, and it results in unsustainable coding patterns such as separate files for each portion of code may need to be built with different compiler flags. This commit introduces support for __attribute__((target(...))) and uses it for the AVX-512 code. This simplifies both the configure-time checks and the build scripts, and it allows us to place the functions that use the intrinsics in files that we otherwise do not want to build with special CPU instructions. We are careful to avoid using __attribute__((target(...))) on compilers that do not understand it, but we still perform the configure-time checks in case the compiler allows using the intrinsics without it (e.g., MSVC). A similar change could likely be made for some of the CRC-32C code, but that is left as a future exercise. Suggested-by: Andres Freund Reviewed-by: Raghuveer Devulapalli, Andres Freund Discussion: https://postgr.es/m/20240731205254.vfpap7uxwmebqeaf%40awork3.anarazel.de
* Clarify a foreign key error messagePeter Eisentraut2024-11-07
| | | | | | | | Clarify the message about type mismatch in foreign key definition to indicate which column the referencing and which is the referenced one. Reported-by: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/CACJufxEL82ao-aXOa=d_-Xip0bix-qdSyNc9fcWxOdkEZFko8w@mail.gmail.com
* Remove an obsolete comment in gistinsert()Michael Paquier2024-11-07
| | | | | | | | | This is inconsistent since 1f7ef548ec2e where the definition of gistFormTuple() has changed. Author: Tender Wang Reviewed-by: Aleksander Alekseev Discussion: https://postgr.es/m/CAHewXNkjU95_HdioDVU=5yBq_Xt=GfBv=Od-0oKtiA006pWW7Q@mail.gmail.com
* Replicate generated columns when 'publish_generated_columns' is set.Amit Kapila2024-11-07
| | | | | | | | | | | | | | | | | | | | This patch builds on the work done in commit 745217a051 by enabling the replication of generated columns alongside regular column changes through a new publication parameter: publish_generated_columns. Example usage: CREATE PUBLICATION pub1 FOR TABLE tab_gencol WITH (publish_generated_columns = true); The column list takes precedence. If the generated columns are specified in the column list, they will be replicated even if 'publish_generated_columns' is set to false. Conversely, if generated columns are not included in the column list (assuming the user specifies a column list), they will not be replicated even if 'publish_generated_columns' is true. Author: Vignesh C, Shubham Khanna Reviewed-by: Peter Smith, Amit Kapila, Hayato Kuroda, Shlok Kyal, Ajin Cherian, Hou Zhijie, Masahiko Sawada Discussion: https://postgr.es/m/B80D17B2-2C8E-4C7D-87F2-E5B4BE3C069E@gmail.com
* Improve handling of empty query results in BackgroundPsql::query()Michael Paquier2024-11-07
| | | | | | | | | | | | | | | | | | | | | | | A newline is not added at the end of an empty query result, causing the banner of the hardcoded \echo to not be discarded. This would reflect on scripts that expect an empty result by showing the "QUERY_SEPARATOR" in the output returned back to the caller, which was confusing. This commit changes BackgroundPsql::query() so as empty results are able to work correctly, making the first newline before the banner optional, bringing more flexibility. Note that this change affects 037_invalid_database.pl, where three queries generated an empty result, with the script relying on the data from the hardcoded banner to exist in the expected output. These queries are changed to use query_safe(), leading to a simpler script. The author has also proposed a test in a different patch where empty results would exist when using BackgroundPsql. Author: Jacob Champion Reviewed-by: Andrew Dunstan, Michael Paquier Discussion: https://postgr.es/m/CAOYmi+=60deN20WDyCoHCiecgivJxr=98s7s7-C8SkXwrCfHXg@mail.gmail.com
* Find invalid databases during upgrade check stageDaniel Gustafsson2024-11-06
| | | | | | | | | Before continuing with the check start by checking that all databases allow connections to avoid a hard fail without proper error reporting. Inspired by a larger patch by Thomas Krennwallner. Discussion: https://postgr.es/m/f9315bf0-e03e-4490-9f0d-5b6f7a6d9908@postsubmeta.net
* Remove unused variableDaniel Gustafsson2024-11-06
| | | | | | | | | | | The low variable has not been used since it was added in d168b666823 and can be safely removed. The variable is present in the Sedgewick paper "Analysis of Shellsort and Related Algorithms" as a parameter to the shellsort function, but our implementation does not use it. Remove to improve readability of the code. Author: Koki Nakamura <btnakamurakoukil@oss.nttdata.com> Discussion: https://postgr.es/m/8aeb7b3eda53ca4c65fbacf8f43628fb@oss.nttdata.com
* doc: Remove event trigger firing matrixPeter Eisentraut2024-11-06
| | | | | | | | | | | | | | | | This is difficult to maintain accurately, and it was probably already somewhat incorrect, especially in the sql_drop and table_rewrite categories. The prior section already documented which DDL commands are *not* supported (which was also slightly outdated), so let's expand that a bit and just rely on that instead of listing out each command in full detail. Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CACJufxE_UAuxcM08BW5oVsg34v0cFWoEt8yBa5xSAoKLmL6LTQ%40mail.gmail.com
* Monkey-patch LLVM code to fix ARM relocation bug.Thomas Munro2024-11-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Supply a new memory manager for RuntimeDyld, to avoid crashes in generated code caused by memory placement that can overflow a 32 bit data type. This is a drop-in replacement for the llvm::SectionMemoryManager class in the LLVM library, with Michael Smith's proposed fix from https://www.github.com/llvm/llvm-project/pull/71968. We hereby slurp it into our own source tree, after moving into a new namespace llvm::backport and making some minor adjustments so that it can be compiled with older LLVM versions as far back as 12. It's harder to make it work on even older LLVM versions, but it doesn't seem likely that people are really using them so that is not investigated for now. The problem could also be addressed by switching to JITLink instead of RuntimeDyld, and that is the LLVM project's recommended solution as the latter is about to be deprecated. We'll have to do that soon enough anyway, and then when the LLVM version support window advances far enough in a few years we'll be able to delete this code. Unfortunately that wouldn't be enough for PostgreSQL today: in most relevant versions of LLVM, JITLink is missing or incomplete. Several other projects have already back-ported this fix into their fork of LLVM, which is a vote of confidence despite the lack of commit into LLVM as of today. We don't have our own copy of LLVM so we can't do exactly what they've done; instead we have a copy of the whole patched class so we can pass an instance of it to RuntimeDyld. The LLVM project hasn't chosen to commit the fix yet, and even if it did, it wouldn't be back-ported into the releases of LLVM that most of our users care about, so there is not much point in waiting any longer for that. If they make further changes and commit it to LLVM 19 or 20, we'll still need this for older versions, but we may want to resynchronize our copy and update some comments. The changes that we've had to make to our copy can be seen by diffing our SectionMemoryManager.{h,cpp} files against the ones in the tree of the pull request. Per the LLVM project's license requirements, a copy is in SectionMemoryManager.LICENSE. This should fix the spate of crash reports we've been receiving lately from users on large memory ARM systems. Back-patch to all supported releases. Co-authored-by: Thomas Munro <thomas.munro@gmail.com> Co-authored-by: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Reviewed-by: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> (license aspects) Reported-by: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com> Discussion: https://postgr.es/m/CAO6_Xqr63qj%3DSx7HY6ZiiQ6R_JbX%2B-p6sTPwDYwTWZjUmjsYBg%40mail.gmail.com
* Remove unused #include's from bin .c filesPeter Eisentraut2024-11-06
| | | | | | | | as determined by IWYU Similar to commit dbbca2cf299, but for bin and some related files. Discussion: https://www.postgresql.org/message-id/flat/0df1d5b1-8ca8-4f84-93be-121081bde049%40eisentraut.org
* Extend Cluster.pm's background_psql() to be able to start asynchronouslyMichael Paquier2024-11-06
| | | | | | | | | | | | | | This commit extends the constructor routine of BackgroundPsql.pm with a new "wait" parameter. If set to 0, the routine returns without waiting for psql to start, ready to consume input. background_psql() in Cluster.pm gains the same "wait" parameter. The default behavior is still to wait for psql to start. It becomes now possible to not wait, giving to TAP scripts the possibility to perform actions between a BackgroundPsql startup and its wait_connect() call. Author: Jacob Champion Discussion: https://postgr.es/m/CAOYmi+=60deN20WDyCoHCiecgivJxr=98s7s7-C8SkXwrCfHXg@mail.gmail.com
* Fix hypothetical bug in ExprState building for hashingDavid Rowley2024-11-06
| | | | | | | | | | | | | | | | | | | adf97c156 gave ExprStates the ability to hash expressions and return a single hash value. That commit supports seeding the hash value with an initial value to have that blended into the final hash value. Here we fix a hypothetical bug where if there are zero expressions to hash, the initial value is stored in the wrong location. The existing code stored the initial value in an intermediate location expecting that when the expressions were hashed that those steps would store the final hash value in the ExprState.resvalue field. However, that wouldn't happen when there are zero expressions to hash. The correct thing to do instead is to have a special case for zero expressions and when we hit that case, store the initial value directly in the ExprState.resvalue. The reason that this is a hypothetical bug is that no code currently calls ExecBuildHash32Expr passing a non-zero initial value. Discussion: https://postgr.es/m/CAApHDvpMAL_zxbMRr1LOex3O7Y7R7ZN2i8iUFLQhqQiJMAg3qw@mail.gmail.com
* Silence meson warning about PG_TEST_EXTRA in src/Makefile.global.inHeikki Linnakangas2024-11-05
| | | | | | | | | | | | | | Commit 99b937a44f introduced this warning when you run "meson setup": Configuring Makefile.global using configuration ../src/meson.build:31: WARNING: The variable(s) 'PG_TEST_EXTRA' in the input file 'src/Makefile.global.in' are not present in the given configuration data. To fix, add PG_TEST_EXTRA to the list of variables that are not needed in the makefiles generated by meson. In meson builds, the makefiles are only used for PGXS, not for building or testing the server itself. Reported-by: Peter Eisentraut Discussion: https://www.postgresql.org/message-id/5c380997-e270-425a-9542-e4ef36a285de@eisentraut.org
* Clear padding of PgStat_HashKey when handling pgstats entriesMichael Paquier2024-11-05
| | | | | | | | | | | | | | | | | | PgStat_HashKey is currently initialized in a way that could result in random data if the structure has any padding bytes. The structure has no padding bytes currently, fortunately, but it could become a problem should the structure change at some point in the future. The code is changed to use some memset(0) so as any padding would be handled properly, as it would be surprising to see random failures in the pgstats entry lookups. PgStat_HashKey is a structure internal to pgstats, and an ABI change could be possible in the scope of a bug fix, so backpatch down to 15 where this has been introduced. Author: Bertrand Drouvot Reviewed-by: Jelte Fennema-Nio, Michael Paquier Discussion: https://postgr.es/m/Zyb7RW1y9dVfO0UH@ip-10-97-1-34.eu-west-3.compute.internal Backpatch-through: 15
* Use portable diff options in pg_bsd_indent's regression test.Tom Lane2024-11-04
| | | | | | | | | | | | | | We had been using "diff -upd", which evidently works for most people, but Solaris's diff doesn't like it. (We'd not noticed because the Solaris buildfarm animals weren't running this test until they were upgraded to the latest buildfarm client script.) Change to "diff -U3" which is what pg_regress has used for ages. Per buildfarm (and off-list discussion with Noah Misch). Back-patch to v16 where this test was added. In v16, also back-patch the relevant part of 628c1d1f2 so that the test script looks about the same in all branches.
* Revert pg_wal_replay_wait() stored procedureAlexander Korotkov2024-11-04
| | | | | | | | | | | | | | | | This commit reverts 3c5db1d6b0, and subsequent improvements and fixes including 8036d73ae3, 867d396ccd, 3ac3ec580c, 0868d7ae70, 85b98b8d5a, 2520226c95, 014f9f34d2, e658038772, e1555645d7, 5035172e4a, 6cfebfe88b, 73da6b8d1b, and e546989a26. The reason for reverting is a set of remaining issues. Most notably, the stored procedure appears to need more effort than the utility statement to turn the backend into a "snapshot-less" state. This makes an approach to use stored procedures questionable. Catversion is bumped. Discussion: https://postgr.es/m/Zyhj2anOPRKtb0xW%40paquier.xyz
* pg_basebackup, pg_receivewal: fix failure to find password in ~/.pgpass.Tom Lane2024-11-04
| | | | | | | | | | | | | | | | | | | | | | | Sloppy refactoring in commit cca97ce6a caused these programs to pass dbname = NULL to libpq if there was no "--dbname" switch on the command line, where before "replication" would be passed. This didn't break things completely, because the source server doesn't care about the dbname specified for a physical replication connection. However, it did cause libpq to fail to match a ~/.pgpass entry that has "replication" in the dbname field. Restore the previous behavior of passing "replication". Also, closer inspection shows that if you do specify a dbname in the connection string, that is what will be matched to ~/.pgpass, not "replication". This was the pre-existing behavior so we should not change it, but the SGML docs were pretty misleading about it. Improve that. Per bug #18685 from Toshi Harada. Back-patch to v17 where the error crept in. Discussion: https://postgr.es/m/18685-fee2dd142b9688f1@postgresql.org Discussion: https://postgr.es/m/2702546.1730740456@sss.pgh.pa.us
* pg_dump: provide a stable sort order for rules.Tom Lane2024-11-04
| | | | | | | | | | | | | | Previously, we sorted rules by schema name and then rule name; if that wasn't unique, we sorted by rule OID. This can be problematic for comparing dumps from databases with different histories, especially since certain rule names like "_RETURN" are very common. Let's make the sort key schema name, rule name, table name, which should be unique. (This is the same behavior we've long used for triggers and RLS policies.) Andreas Karlsson Discussion: https://postgr.es/m/b4e468d8-0cd6-42e6-ac8a-1d6afa6e0cf1@proxel.se
* Fix typo in comment of gistdoinsert().Masahiko Sawada2024-11-04
| | | | | | Author: Tender Wang Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/CAHewXN%3D3sH2sNw4nC3QGCEVw1Lftmw9m5y1Xje0bXK6ApDrsPQ%40mail.gmail.com
* Fix obsolete _bt_first comments.Peter Geoghegan2024-11-04
| | | | | | | _bt_first doesn't necessarily hold onto a buffer pin on success exit. Fix header comments that claimed that we'll always hold onto a pin. Oversight in commit 2ed5b87f96.
* nbtree: Remove useless 'strat' local variable.Peter Geoghegan2024-11-04
| | | | | | | | | | | | | | | | | | | | | Remove a local variable that was used to avoid overwriting strat_total with the = operator strategy when a >= operator strategy key was already included in the initial positioning/insertion scan keys by _bt_first (for backwards scans it would have to be a <= key that was included). _bt_first's strat_total local variable now simply tracks the operator strategy of the final scan key that was included in the scan's insertion scan key (barring the case where the !used_all_subkeys row compare path adjusts strat_total in its own way). _bt_first already treated >= keys (or <= keys) as = keys for initial positioning purposes. There is no good reason to remember that that was what happened; no later _bt_first step cares about the distinction. Note, in particular, that the insertion scan key's 'nextkey' and 'backward' fields will be initialized the same way regardless. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Tomas Vondra <tomas@vondra.me> Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com
* Split ProcSleep function into JoinWaitQueue and ProcSleepHeikki Linnakangas2024-11-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Split ProcSleep into two functions: JoinWaitQueue and ProcSleep. JoinWaitQueue is called while holding the partition lock, and inserts the current process to the wait queue, while ProcSleep() does the actual sleeping. ProcSleep() is now called without holding the partition lock, and it no longer re-acquires the partition lock before returning. That makes the wakeup a little cheaper. Once upon a time, re-acquiring the partition lock was needed to prevent a signal handler from longjmping out at a bad time, but these days our signal handlers just set flags, and longjmping can only happen at points where we explicitly run CHECK_FOR_INTERRUPTS(). If JoinWaitQueue detects an "early deadlock" before even joining the wait queue, it returns without changing the shared lock entry, leaving the cleanup of the shared lock entry to the caller. This makes the handling of an early deadlock the same as the dontWait=true case. One small user-visible side-effect of this refactoring is that we now only set the 'ps' title to say "waiting" when we actually enter the sleep, not when the lock is skipped because dontWait=true, or when a deadlock is detected early before entering the sleep. This eliminates the 'lockAwaited' global variable in proc.c, which was largely redundant with 'awaitedLock' in lock.c Note: Updating the local lock table is now the caller's responsibility. JoinWaitQueue and ProcSleep are now only responsible for modifying the shared state. Seems a little nicer that way. Based on Thomas Munro's earlier patch and observation that ProcSleep doesn't really need to re-acquire the partition lock. Reviewed-by: Maxim Orlov Discussion: https://www.postgresql.org/message-id/7c2090cd-a72a-4e34-afaa-6dd2ef31440e@iki.fi
* pg_combinebackup: Error if incremental file exists in full backup.Robert Haas2024-11-04
| | | | | | | | | | | | | | | | | | Suppose that you run a command like "pg_combinebackup b1 b2 -o output", but both b1 and b2 contain an INCREMENTAL.$something file in a directory that is expected to contain relation files. This is an error, but the previous code would not detect the problem and instead write a garbage full file named $something to the output directory. This commit adds code to detect the error and a test case to verify the behavior. It's difficult to imagine that this will ever happen unless someone is intentionally trying to break incremental backup, but per discussion, let's consider that the lack of adequate sanity checking in this area is a bug and back-patch to v17, where incremental backup was introduced. Patch by me, reviewed by Bertrand Drouvot and Amul Sul. Discussion: http://postgr.es/m/CA+TgmoaD7dBYPqe7kMtO0dyto7rd0rUh7joh=JPUSaFszKY6Pg@mail.gmail.com
* pg_combinebackup: When reconstructing, avoid double slash in filename.Robert Haas2024-11-04
| | | | | | | | | | | | | | | | This function is always called with a relative_path that ends in a slash, so there's no need to insert a second one. So, don't. Instead, add an assertion to verify that nothing gets broken in the future, and adjust the comments. While this is not a critical bug, the duplicate slash is visible in error messages, which could create confusion, so back-patch to v17. This is also better in that it keeps the code consistent across branches. Patch by me, reviewed by Bertrand Drouvot and Amul Sul. Discussion: http://postgr.es/m/CA+TgmoaD7dBYPqe7kMtO0dyto7rd0rUh7joh=JPUSaFszKY6Pg@mail.gmail.com
* Move TRACE calls into WaitOnLock()Heikki Linnakangas2024-11-04
| | | | | | | | LockAcquire is a long and complex function. Pushing more stuff to its subroutines makes it a little more manageable. Reviewed-by: Maxim Orlov Discussion: https://www.postgresql.org/message-id/7c2090cd-a72a-4e34-afaa-6dd2ef31440e@iki.fi
* Set MyProc->heldLocks in ProcSleepHeikki Linnakangas2024-11-04
| | | | | | | | | | | | Previously, ProcSleep()'s caller was responsible for setting MyProc->heldLocks, and we had comments to remind about that. But it seems simpler to make ProcSleep() itself responsible for it. ProcSleep() already set the other info about the lock its waiting for (waitLock, waitProcLock and waitLockMode), so it is natural for it to set heldLocks too. Reviewed-by: Maxim Orlov Discussion: https://www.postgresql.org/message-id/7c2090cd-a72a-4e34-afaa-6dd2ef31440e@iki.fi
* Clarify nbtree parallel scan _bt_endpoint contract.Peter Geoghegan2024-11-04
| | | | | | | | | | | | | | | _bt_endpoint is a helper function for _bt_first that's called whenever no useful insertion scan key can be used, and we need to lock and read either the leftmost or rightmost leaf page in the index. Simplify and document its preconditions, relieving its _bt_first caller from having to end the parallel scan when it returns false. Also stop unnecessarily invalidating the current scan position in nearby code in both _bt_first and _bt_endpoint. This seems to have been copy-pasted from _bt_readnextpage, where invalidating the scan's current position really is necessary. Follow-up to the refactoring work in commit 1bd4bc85.
* Fix comment in LockReleaseAll() on when locallock->nLock can be zeroHeikki Linnakangas2024-11-04
| | | | | | | | We reach this case also e.g. when a deadlock is detected, not only when we run out of memory. Reviewed-by: Maxim Orlov Discussion: https://www.postgresql.org/message-id/7c2090cd-a72a-4e34-afaa-6dd2ef31440e@iki.fi
* Add PG_TEST_EXTRA configure option to the Make buildsHeikki Linnakangas2024-11-04
| | | | | | | | | | | | | | | | The Meson builds have PG_TEST_EXTRA as a configure-time variable, which was not available in the Make builds. To ensure both build systems are in sync, PG_TEST_EXTRA is now added as a configure-time variable. It can be set like this: ./configure PG_TEST_EXTRA="kerberos, ssl, ..." Note that to preserve the old behavior, this configure-time variable is overridden by the PG_TEST_EXTRA environment variable when you run the tests. Author: Jacob Champion Reviewed by: Ashutosh Bapat, Nazir Bilal Yavuz
* Make PG_TEST_EXTRA env var override the "meson setup" optionHeikki Linnakangas2024-11-04
| | | | | | | | | | | | | | | | | | | | | | "meson test" used to ignore the PG_TEST_EXTRA environment variable, which meant that in order to run additional tests, you had to run "meson setup -DPG_TEST_EXTRA=...". That's somewhat expensive, and not consistent with autoconf builds. Allow PG_TEST_EXTRA environment variable to override the setup-time option at run time, so that you can do "PG_TEST_EXTRA=... meson test". To implement this, the configuration time value is passed as an extra "--pg-test-extra" argument to testwrap instead of adding it to the test environment. If the environment variable is set at the time of running test, testwrap uses the value from the environment variable and ignores the --pg-test-extra option. Now that "meson test" obeys the environment variable, we can remove it from the "meson setup" steps in the CI script. It will now be picked up from the environment variable like with "make check". Author: Nazir Bilal Yavuzk, Ashutosh Bapat Reviewed-by: Ashutosh Bapat with inputs from Tom Lane and Andrew Dunstan
* Add missing newlines at the end of two SQL filesMichael Paquier2024-11-03
| | | | | | arrays.sql was already missing it before 49d6c7d8daba, and I have just noticed it thanks to this commit. The second one in test_slru has been introduced by 768a9fd5535f.
* Suppress new "may be used uninitialized" warning.Noah Misch2024-11-02
| | | | | | Buildfarm member mamba fails to deduce that the function never uses this variable without initializing it. Back-patch to v12, like commit b412f402d1e020c5dac94f3bf4a005db69519b99.
* Fix inplace update buffer self-deadlock.Noah Misch2024-11-02
| | | | | | | | | | | | | | | A CacheInvalidateHeapTuple* callee might call CatalogCacheInitializeCache(), which needs a relcache entry. Acquiring a valid relcache entry might scan pg_class. Hence, to prevent undetected LWLock self-deadlock, CacheInvalidateHeapTuple* callers must not hold BUFFER_LOCK_EXCLUSIVE on buffers of pg_class. Move the CacheInvalidateHeapTupleInplace() before the BUFFER_LOCK_EXCLUSIVE. No back-patch, since I've reverted commit 243e9b40f1b2dd09d6e5bf91ebf6e822a2cd3704 from non-master branches. Reported by Alexander Lakhin. Reviewed by Alexander Lakhin. Discussion: https://postgr.es/m/10ec0bc3-5933-1189-6bb8-5dec4114558e@gmail.com
* Move I/O before the index_update_stats() buffer lock region.Noah Misch2024-11-02
| | | | | | | | | | | Commit a07e03fd8fa7daf4d1356f7cb501ffe784ea6257 enlarged the work done here under the pg_class heap buffer lock. Two preexisting actions are best done before holding that lock. Both RelationGetNumberOfBlocks() and visibilitymap_count() do I/O, and the latter might exclusive-lock a visibility map buffer. Moving these reduces contention and risk of undetected LWLock deadlock. Back-patch to v12, like that commit. Discussion: https://postgr.es/m/20241031200139.b4@rfd.leadboat.com
* Clarify nbtree array preprocessing comment.Peter Geoghegan2024-11-01
| | | | Oversight in commit 5bf748b8.
* Rename two functions that wake up other processesHeikki Linnakangas2024-11-01
| | | | | | | | | | | Instead of talking about setting latches, which is a pretty low-level mechanism, emphasize that they wake up other processes. This is in preparation for replacing Latches with a new abstraction. That's still work in progress, but this seems a little tidier anyway, so let's get this refactoring out of the way already. Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c%40iki.fi
* Use ProcNumbers instead of direct Latch pointers to address other procsHeikki Linnakangas2024-11-01
| | | | | | | | This is in preparation for replacing Latches with a new abstraction. That's still work in progress, but this seems a little tidier anyway, so let's get this refactoring out of the way already. Discussion: https://www.postgresql.org/message-id/391abe21-413e-4d91-a650-b663af49500c%40iki.fi
* Remove use of pg_memory_is_all_zeros() in bufpage.cMichael Paquier2024-11-01
| | | | | | | | | | | | | After a closer lookup, this makes the all-zero check of the page more expensive, so let's remove the new function call in bufpage.c. The maths of the check were also incorrect, checking that the page was full of zeros only for the first 1kB. This brings back the code to the state it was at 49d6c7d8daba. Per discussion with David Rowley and Bertrand Drouvot. Discussion: https://postgr.es/m/CAApHDvrXzPAr3FxoBuB7b3D-okNoNA2jxLun1rW8Yw5wkbqusw@mail.gmail.com
* Add pg_memory_is_all_zeros() in memutils.hMichael Paquier2024-11-01
| | | | | | | | | | | | | | | | | This new function tests if a memory region starting at a given location for a defined length is made only of zeroes. This unifies in a single path the all-zero checks that were happening in a couple of places of the backend code: - For pgstats entries of relation, checkpointer and bgwriter, where some "all_zeroes" variables were previously used with memcpy(). - For all-zero buffer pages in PageIsVerifiedExtended(). This new function uses the same forward scan as the check for all-zero buffer pages, applying it to the three pgstats paths mentioned above. Author: Bertrand Drouvot Reviewed-by: Peter Eisentraut, Heikki Linnakangas, Peter Smith Discussion: https://postgr.es/m/ZupUDDyf1hHI4ibn@ip-10-97-1-34.eu-west-3.compute.internal
* Add SQL function array_reverse()Michael Paquier2024-11-01
| | | | | | | | | | | | | | | | | This function takes in input an array, and reverses the position of all its elements. This operation only affects the first dimension of the array, like array_shuffle(). The implementation structure is inspired by array_shuffle(), with a subroutine called array_reverse_n() that may come in handy in the future, should more functions able to reverse portions of arrays be introduced. Bump catalog version. Author: Aleksander Alekseev Reviewed-by: Ashutosh Bapat, Tom Lane, Vladlen Popolitov Discussion: https://postgr.es/m/CAJ7c6TMpeO_ke+QGOaAx9xdJuxa7r=49-anMh3G5476e3CX1CA@mail.gmail.com
* Make all ereport() calls within gram.y provide error locations.Tom Lane2024-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch responds to a comment that I (tgl) made in the discussion leading up to 774171c4f, that really all errors occurring during raw parsing should provide error cursors. Syntax errors reported by Bison will have one, and most of the handwritten ereport's in gram.y already provide one, but there were a few stragglers. (It is not claimed that this handles every failure reachable during raw parsing --- out-of-memory is an obvious exception. But this makes a good start on cases that are likely to occur.) While we're at it, clean up the reported positions for errors associated with LIMIT/OFFSET clauses. Previously we were relying on applying exprLocation() to the contained expressions, but that leads to slightly odd cursor placement, e.g. regression=# (select * from foo limit 10) limit 10; ERROR: multiple LIMIT clauses not allowed LINE 1: (select * from foo limit 10) limit 10; ^ We can afford to keep a little more state in the transient SelectLimit structs in order to make that better. Jian He and Tom Lane (extracted from a larger patch by Jian, with some additional work by me) Discussion: https://postgr.es/m/CACJufxEmONE3P2En=jopZy1m=cCCUs65M4+1o52MW5og9oaUPA@mail.gmail.com
* Add a parse location field to struct FunctionParameter.Tom Lane2024-10-31
| | | | | | | | | | | | | | | | This allows an error cursor to be supplied for a bunch of bad-function-definition errors that previously lacked one, or that cheated a bit by pointing at the contained type name when the error isn't really about that. Bump catversion from an abundance of caution --- I don't think this node type can actually appear in stored views/rules, but better safe than sorry. Jian He and Tom Lane (extracted from a larger patch by Jian, with some additional work by me) Discussion: https://postgr.es/m/CACJufxEmONE3P2En=jopZy1m=cCCUs65M4+1o52MW5og9oaUPA@mail.gmail.com
* Fix refreshing physical relfilenumber on shared indexHeikki Linnakangas2024-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Buildfarm member 'prion', which is configured with -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE, failed with errors like this: ERROR: could not read blocks 0..0 in file "global/2672": read only 0 of 8192 bytes while running a parallel test group that includes VACUUM FULL on some catalog tables among other things. I was not able to reproduce that just by running the tests with -DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE, even though 'prion' hit it on first run after commit 2b9b8ebbf8, so there might be something else that makes it more susceptible to the race. However, I was able to reproduce it by adding another test to the same test group that runs "vacuum full pg_database" repeatedly. The problem is that RelationReloadIndexInfo() no longer calls RelationInitPhysicalAddr() on a nailed, shared index, when an invalidation happens early during backend startup, before the critical relcaches have been built. Before commit 2b9b8ebbf8, that was done by RelationReloadNailed(), but it went missing from that path. Add it back as an explicit step. Broken by commit 2b9b8ebbf8, which refactored these functions. Discussion: https://www.postgresql.org/message-id/db876575-8f5b-4193-a538-df7e1f92d47a%40iki.fi
* Remove duplicate words in commentsDaniel Gustafsson2024-10-31
| | | | | | | | A few comments contained duplicate "the" in sentences, fix by removing one occurrence. Author: Vignesh C <vignesh21@gmail.com> Discussion: https://postgr.es/m/CALDaNm2aEEiPwGJmPdzBxROVvs8n75yCjKz4K1f1B2TdWpzxTA@mail.gmail.com
* Split RelationClearRelation into three different functionsHeikki Linnakangas2024-10-31
| | | | | | | | | | | | | | | | | | | The old RelationClearRelation function did different things depending on the arguments and circumstances. It could: a) remove the relation completely from relcache (rebuild == false), b) mark the entry as invalid (rebuild == true, but not in xact), or c) rebuild the entry (rebuild == true). Different callers used it for different purposes, and often assumed a particular behavior, which was confusing. Split it into three different functions, one for each of the above actions (one of them, RelationInvalidateRelation, was already added in commit e6cd857726). Move the responsibility of choosing the action and calling the right function to the callers. Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/9c9e8908-7b3e-4ce7-85a8-00c0e165a3d6%40iki.fi
* Simplify call to rebuild relcache entry for indexesHeikki Linnakangas2024-10-31
| | | | | | | | | | | | | | | | | | RelationClearRelation(rebuild == true) calls RelationReloadIndexInfo() for indexes. We can rely on that in RelationIdGetRelation(), instead of calling RelationReloadIndexInfo() directly. That simplifies the code a little. In the passing, add a comment in RelationBuildLocalRelation() explaining why it doesn't call RelationInitIndexAccessInfo(). It's because at index creation, it's called before the pg_index row has been created. That's also the reason that RelationClearRelation() still needs a special case to go through the full-blown rebuild if the index support information in the relcache entry hasn't been populated yet. Reviewed-by: jian he <jian.universality@gmail.com> Discussion: https://www.postgresql.org/message-id/9c9e8908-7b3e-4ce7-85a8-00c0e165a3d6%40iki.fi
* Remove unused field from SubPlanState structDavid Rowley2024-10-31
| | | | | | | | | | | | | | | | bf6c614a2 did some conversion work to use ExprState instead of manually calling equality functions to check if one set of values is not distinct from another set. That patch removed many of the fields that became redundant as a result of that change, but it forgot to remove SubPlanState.tab_eq_funcs. Fix that. In passing, fix the header comment for TupleHashEntryData to correctly spell the field name it's talking about. Author: Rafia Sabih <rafia.pghackers@gmail.com> Reviewed-by: Andrei Lepikhov <lepihov@gmail.com> Discussion: https://postgr.es/m/CA+FpmFeycdombFzrjZw7Rmc29CVm4OOzCWwu=dVBQ6q=PX8SvQ@mail.gmail.com Discussion: https://postgr.es/m/CAApHDvrWR2jYVhec=COyF2g2BE_ns91NDsCHAMFiXbyhEujKdQ@mail.gmail.com
* injection_points: Improve comment about disabled isolation permutationMichael Paquier2024-10-31
| | | | | | | | | | | 9f00edc22888 has disabled a permutation due to failures in the CI for FreeBSD environments, but this is a matter of timing. Let's document properly why this type of permutation is a bad idea if relying on a wait done in a SQL function, so as this can be avoided when implementing new tests (this spec is also a template). Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/ZyCa2qsopKaw3W3K@paquier.xyz