aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Add missing source files to nls.mkPeter Eisentraut2022-05-11
|
* Improve setup of environment values for commands in MSVC's vcregress.plMichael Paquier2022-05-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The current setup assumes that commands for lz4, zstd and gzip always exist by default if not enforced by a user's environment. However, vcpkg, as one example, installs libraries but no binaries, so this default setup to assume that a command should always be present would cause failures. This commit improves the detection of such external commands as follows: * If a ENV value is available, trust the environment/user and use it. * If a ENV value is not available, check its execution by looking in the current PATH, by launching a simple "$command --version" (that should be portable enough). ** On execution failure, ignore ENV{command}. ** On execution success, set ENV{command} = "$command". Note that this new rule applies to gzip, lz4 and zstd but not tar that we assume will always exist. Those commands are set up in the environment only when using bincheck and taptest. The CI includes all those commands and I have checked that their setup is correct there. I have also tested this change in a MSVC environment where we have none of those commands. While on it, remove the references to lz4 from the documentation and vcregress.pl in ~v13. --with-lz4 has been added in v14~ so there is no point to have this information in these older branches. Reported-by: Andrew Dunstan Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/14402151-376b-a57a-6d0c-10ad12608e12@dunslane.net Backpatch-through: 10
* Fix some incorrect preprocessor tests in tuplesort specializationsDavid Rowley2022-05-11
| | | | | | | | | | | | | | | | | | | | | | | | | 697492434 added 3 new quicksort specialization functions for common datatypes. That commit was not very consistent in how it would determine if we're compiling for 32-bit or 64-bit machines. It would sometimes use USE_FLOAT8_BYVAL and at other times check if SIZEOF_DATUM == 8. This could cause theoretical problems due to the way USE_FLOAT8_BYVAL is now defined based on SIZEOF_VOID_P >= 8. If pointers for some reason were ever larger than 8-bytes then we'd end up doing 32-bit comparisons mistakenly. Let's just always check SIZEOF_DATUM >= 8. It also seems that ssup_datum_signed_cmp is just never used on 32-bit builds, so let's just ifdef that out to make sure we never accidentally use that comparison function on such machines. This also allows us to ifdef out 1 of the 3 new specialization quicksort functions in 32-bit builds which seems to shrink down the binary by over 4KB on my machine. In passing, also add the missing DatumGetInt32() / DatumGetInt64() macros in the comparison functions. Discussion: https://postgr.es/m/CAApHDvqcQExRhtRa9hJrJB_5egs3SUfOcutP3m+3HO8A+fZTPA@mail.gmail.com Reviewed-by: John Naylor
* Formatting and punctuation improvements in sample configuration filesPeter Eisentraut2022-05-10
|
* Remove some tabs in SQL code in C string literalsPeter Eisentraut2022-05-10
| | | | | This is not handled uniformly throughout the code, but at least nearby code can be consistent.
* Fix several issues with the TAP tests of pg_upgradeMichael Paquier2022-05-10
| | | | | | | | | | | | | | | | | | | | | | | This commit addresses the following issues in the TAP tests of pg_upgrade, introduced in 322becb: - Remove --port and --host for commands that already rely on a node's environment PGHOST and PGPORT. - Switch from run_log() to command_ok(), as all the commands executed in the tests should succeed. - Change EXTRA_REGRESS_OPTS to make it count as a shell fragment (fixing s/OPT/OPTS on a way), to be compatible with the various Makefiles using it as well as 027_stream_regress.pl in the recovery tests. The command built for the execution the pg_regress command is reformatted, while on it, to map with the recovery test doing the same thing (we should refactor and consolidate that in the future, perhaps). - Re-add the test for database names stressing the behavior of backslashes with double quotes, mostly here for Windows. Tests doable with the upgrade across different major versions still work the same way. Reported-by: Noah Misch Discussion: https://postgr.es/m/20220502042718.GB1565149@rfd.leadboat.com
* Fix core dump in transformValuesClause when there are no columns.Tom Lane2022-05-09
| | | | | | | | | | | | The parser code that transformed VALUES from row-oriented to column-oriented lists failed if there were zero columns. You can't write that straightforwardly (though probably you should be able to), but the case can be reached by expanding a "tab.*" reference to a zero-column table. Per bug #17477 from Wang Ke. Back-patch to all supported branches. Discussion: https://postgr.es/m/17477-0af3c6ac6b0a6ae0@postgresql.org
* Revert "Disallow infinite endpoints in generate_series() for timestamps."Tom Lane2022-05-09
| | | | | | | | | | | | | | | | | | | This reverts commit eafdf9de06e9b60168f5e47cedcfceecdc6d4b5f and its back-branch counterparts. Corey Huinker pointed out that we'd discussed this exact change back in 2016 and rejected it, on the grounds that there's at least one usage pattern with LIMIT where an infinite endpoint can usefully be used. Perhaps that argument needs to be re-litigated, but there's no time left before our back-branch releases. To keep our options open, restore the status quo ante; if we do end up deciding to change things, waiting one more quarter won't hurt anything. Rather than just doing a straight revert, I added a new test case demonstrating the usage with LIMIT. That'll at least remind us of the issue if we forget again. Discussion: https://postgr.es/m/3603504.1652068977@sss.pgh.pa.us Discussion: https://postgr.es/m/CADkLM=dzw0Pvdqp5yWKxMd+VmNkAMhG=4ku7GnCZxebWnzmz3Q@mail.gmail.com
* In REFRESH MATERIALIZED VIEW, set user ID before running user code.Noah Misch2022-05-09
| | | | | | | | | | It intended to, but did not, achieve this. Adopt the new standard of setting user ID just after locking the relation. Back-patch to v10 (all supported versions). Reviewed by Simon Riggs. Reported by Alvaro Herrera. Security: CVE-2022-1552
* Make relation-enumerating operations be security-restricted operations.Noah Misch2022-05-09
| | | | | | | | | | | | | | | | | | When a feature enumerates relations and runs functions associated with all found relations, the feature's user shall not need to trust every user having permission to create objects. BRIN-specific functionality in autovacuum neglected to account for this, as did pg_amcheck and CLUSTER. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. CREATE INDEX (not a relation-enumerating operation) and REINDEX protected themselves too late. This change extends to the non-enumerating amcheck interface. Back-patch to v10 (all supported versions). Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2022-1552
* Add missing source files to nls.mkPeter Eisentraut2022-05-09
|
* Fix control file update done in restartpoints still running after promotionMichael Paquier2022-05-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If a cluster is promoted (aka the control file shows a state different than DB_IN_ARCHIVE_RECOVERY) while CreateRestartPoint() is still processing, this function could miss an update of the control file for "checkPoint" and "checkPointCopy" but still do the recycling and/or removal of the past WAL segments, assuming that the to-be-updated LSN values should be used as reference points for the cleanup. This causes a follow-up restart attempting crash recovery to fail with a PANIC on a missing checkpoint record if the end-of-recovery checkpoint triggered by the promotion did not complete while the cluster abruptly stopped or crashed before the completion of this checkpoint. The PANIC would be caused by the redo LSN referred in the control file as located in a segment already gone, recycled by the previous restartpoint with "checkPoint" out-of-sync in the control file. This commit fixes the update of the control file during restartpoints so as "checkPoint" and "checkPointCopy" are updated even if the cluster has been promoted while a restartpoint is running, to be on par with the set of WAL segments actually recycled in the end of CreateRestartPoint(). This problem exists in all the stable branches. However, commit 7ff23c6, by removing the last call of CreateCheckPoint() from the startup process, has made this bug much easier to reason about as concurrent checkpoints are not possible anymore. No backpatch is done yet, mostly out of caution from me as a point release is close by, but we need to think harder about the case of concurrent checkpoints at promotion if the bgwriter is not considered as running by the startup process in ~v14, so this change is done only on HEAD for the moment. Reported-by: Fujii Masao, Rui Zhao Author: Kyotaro Horiguchi Reviewed-by: Nathan Bossart, Michael Paquier Discussion: https://postgr.es/m/20220316.102444.2193181487576617583.horikyota.ntt@gmail.com
* Fix race in 032_relfilenode_reuse.pl.Thomas Munro2022-05-08
| | | | | | | | | Add wait_for_catchup() call to the test added by commit e2f65f42. Per slow build farm animal grison. Also fix a comment. Discussion: https://postgr.es/m/CA%2BhUKGLJ2Vy8hVQmnYotmTaEKZK0%3D-GcXgNAgcHzArZvtS4L_g%40mail.gmail.com
* Fix old-fd issues using global barriers everywhere.Thomas Munro2022-05-07
| | | | | | | | | | | | | | | | | | | | | Commits 4eb21763 and b74e94dc introduced a way to force every backend to close all relation files, to fix an ancient Windows-only bug. This commit extends that behavior to all operating systems and adds a couple of extra barrier points, to fix a totally different class of bug: the reuse of relfilenodes in scenarios that have no other kind of cache invalidation to prevent file descriptor mix-ups. In all releases, data corruption could occur when you moved a database to another tablespace and then back again. Despite that, no back-patch for now as the infrastructure required is too new and invasive. In master only, since commit aa010514, it could also happen when using CREATE DATABASE with a user-supplied OID or via pg_upgrade. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20220209220004.kb3dgtn2x2k2gtdm%40alap3.anarazel.de
* Rethink PROCSIGNAL_BARRIER_SMGRRELEASE.Thomas Munro2022-05-07
| | | | | | | | | | | | | | | | With sufficiently bad luck, it was possible for IssuePendingWritebacks() to reopen a file after we'd processed PROCSIGNAL_BARRIER_SMGRRELEASE and before the file was unlinked by some other backend. That left a small hole in commit 4eb21763's plan to fix all spurious errors from DROP TABLESPACE and similar on Windows. Fix by closing md.c's segments, instead of just closing fd.c's descriptors, and then teaching smgrwriteback() not to open files that aren't already open. Reported-by: Andres Freund <andres@anarazel.de> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/20220209220004.kb3dgtn2x2k2gtdm%40alap3.anarazel.de
* Fix misleading comments about background worker registration.Robert Haas2022-05-06
| | | | | | | | | | | | | | | | Since 6bc8ef0b7f1f1df3998745a66e1790e27424aa0c, the maximum number of backends can't change as background workers are registered, but these comments still reflect the way things worked prior to that. Also, per recent discussion, some modules call SetConfigOption() from _PG_init(). It's not entirely clear to me whether we want to regard that as a fully supported operation, but since we know it's a thing that happens, it at least deserves a mention in the comments, so add that. Nathan Bossart, reviewed by Anton A. Melnikov Discussion: http://postgr.es/m/20220419154658.GA2487941@nathanxps13
* Clear the OpenSSL error queue before cryptohash operationsDaniel Gustafsson2022-05-06
| | | | | | | | | | | | | | | | Setting up an EVP context for ciphers banned under FIPS generate two OpenSSL errors in the queue, and as we only consume one from the queue the other is at the head for the next invocation: postgres=# select md5('foo'); ERROR: could not compute MD5 hash: unsupported postgres=# select md5('foo'); ERROR: could not compute MD5 hash: initialization error Clearing the error queue when creating the context ensures that we don't pull in an error from an earlier operation. Discussion: https://postgr.es/m/C89D932C-501E-4473-9750-638CFCD9095E@yesql.se
* Fix typo in origin.cMichael Paquier2022-05-06
| | | | | | | Introduced in 5aa2350. Author: Peter Smith Discussion: https://postgr.es/m/CAHut+PsuWz6_7aCmivNU5FahgQxDUTQtc3+__XnWkBzQcfn43w@mail.gmail.com
* Update SQL featuresPeter Eisentraut2022-05-06
| | | | | Update a few items that have become supported or mostly supported but weren't updated at the time.
* Update time zone data files to tzdata release 2022a.Tom Lane2022-05-05
| | | | | DST law changes in Palestine. Historical corrections for Chile and Ukraine.
* Fix timing issue in deadlock recovery conflict test.Andres Freund2022-05-04
| | | | | | | Per buildfarm members longfin and skink. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-
* Fix rowcount estimate for SubqueryScan that's under a Gather.Tom Lane2022-05-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | SubqueryScan was always getting labeled with a rowcount estimate appropriate for non-parallel cases. However, nodes that are underneath a Gather should be treated as processing only one worker's share of the rows, whether the particular node is explicitly parallel-aware or not. Most non-scan-level node types get this right automatically because they base their rowcount estimate on that of their input sub-Path(s). But SubqueryScan didn't do that, instead using the whole-relation rowcount estimate as if it were a non-parallel-aware scan node. If there is a parallel-aware node below the SubqueryScan, this is wrong, and it results in inflating the cost estimates for nodes above the SubqueryScan, which can cause us to not choose a parallel plan, or choose a silly one --- as indeed is visible in the one regression test whose results change with this patch. (Although that plan tree appears to contain no SubqueryScans, there were some in it before setrefs.c deleted them.) To fix, use path->subpath->rows not baserel->tuples as the number of input tuples we'll process. This requires estimating the quals' selectivity afresh, which is slightly annoying; but it shouldn't really add much cost thanks to the caching done in RestrictInfo. This is pretty clearly a bug fix, but I'll refrain from back-patching as people might not appreciate plan choices changing in stable branches. The fact that it took us this long to identify the bug suggests that it's not a major problem. Per report from bucoo, though this is not his proposed patch. Discussion: https://postgr.es/m/202204121457159307248@sohu.com
* Remove JsonPathSpec typedefPeter Eisentraut2022-05-04
| | | | | | | It doesn't seem very useful, and it's a bit in the way of the planned node support automation. Discussion: https://www.postgresql.org/message-id/202204191140.3wsbevfhqmu3@alvherre.pgsql
* Add missing enum tag in enum used in nodesPeter Eisentraut2022-05-04
| | | | | | | Similar to 983bdc4fac492a99bb8ab5a471ca7437139e5cf6. Author: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/202204191140.3wsbevfhqmu3@alvherre.pgsql
* Simplify configure testPeter Eisentraut2022-05-04
| | | | | | | The test for lz4.h used AC_CHECK_HEADERS, but nothing was using the resulting symbol HAVE_LZ4_H. Change this to use AC_CHECK_HEADER instead. This was probably an oversight, seeing that the nearby similar tests do this correctly.
* Rename libpq test programs with libpq_ prefixDaniel Gustafsson2022-05-04
| | | | | | | | | | | | The testclient and uri-regress programs in the libpq test suite had quite generic names which didn't convey much meaning. Since they are installed as part of the MSVC test runs, ensure that their purpose is a little bit clearer by renaming with a libpq_ prefix. While at it rename uri-regress to uri_regress to avoid mixing dash and under- score in the same filename. Reported-by: Noah Misch <noah@leadboat.com> Discussion: https://postgr.es/m/20220501080706.GA1542365@rfd.leadboat.com
* Fix incorrect format placeholdersPeter Eisentraut2022-05-04
|
* Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().Andres Freund2022-05-02
| | | | | | | | | | | | | | | | | | | | | The tests added in 9f8a050f68d failed nearly reliably on FreeBSD in CI, and occasionally on the buildfarm. That turns out to be caused not by a bug in the test, but by a longstanding bug in recovery conflict handling. The standby timeout handler, used by ResolveRecoveryConflictWithBufferPin(), executed SendRecoveryConflictWithBufferPin() inside a signal handler. A bad idea, because the deadlock timeout handler (or a spurious latch set) could have interrupted ProcWaitForSignal(). If unlucky that could cause a self-deadlock on ProcArrayLock, if the deadlock check is in SendRecoveryConflictWithBufferPin()->CancelDBBackends(). To fix, set a flag in StandbyTimeoutHandler(), and check the flag in ResolveRecoveryConflictWithBufferPin(). Subsequently the recovery conflict tests will be backpatched. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-
* Add tests for recovery deadlock conflicts.Andres Freund2022-05-02
| | | | | | | | | | | The recovery conflict tests added in 9f8a050f68d surfaced a bug in the interaction between buffer pin and deadlock recovery conflicts. To make sure that the bugfix won't break deadlock conflict detection, add a test for that scenario. 031_recovery_conflict.pl will later be backpatched, with this included. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de
* Fix typo in comment.Etsuro Fujita2022-05-02
|
* pg_walinspect: fix case where flush LSN is in the middle of a record.Jeff Davis2022-04-30
| | | | | | | | | | | | | | | | | | | | | Instability in the test for pg_walinspect revealed that pg_get_wal_records_info_till_end_of_wal(x) would try to decode all the records with a start LSN earlier than the flush LSN, even though that might include a partial record at the end of the range. In that case, read_local_xlog_page_no_wait() would return NULL when it tried to read past the flush LSN, which would be interpreted as an error by the caller. That caused a test failure only on a BF animal that had been restarted recently, but could be expected to happen in the wild quite easily depending on the alignment of various parameters. Fix by using private data in read_local_xlog_page_no_wait() to signal end-of-wal to the caller, so that it can be properly distinguished from a real error. Discussion: https://postgr.es/m/Ymd/e5eeZMNAkrXo%40paquier.xyz Discussion: https://postgr.es/m/111657.1650910309@sss.pgh.pa.us Authors: Thomas Munro, Bharath Rupireddy.
* Tighten enforcement of variable CONSTANT markings in plpgsql.Tom Lane2022-04-30
| | | | | | | | | | | | | | | | | | | I noticed that plpgsql would allow assignment of a new value to a variable even when that variable is marked CONSTANT, if the variable is used as an output parameter in CALL or is a refcursor variable that OPEN assigns a new value to. Fix these oversights. In the CALL case, the check has to be done at runtime because we cannot know at parse time which parameters are OUT parameters. For OPEN, it seems best to likewise enforce at runtime because then we needn't throw error if the variable has a nonnull value (since OPEN will only try to overwrite a null value). Although this is surely a bug fix, no back-patch: it seems unlikely that anyone would thank us for breaking formerly-working code in minor releases. Discussion: https://postgr.es/m/214453.1651182729@sss.pgh.pa.us
* Claim SQL standard compliance for SQL/JSON featuresAndrew Dunstan2022-04-29
| | | | Discussion: https://postgr.es/m/d03d809c-d0fb-fd6a-1476-d6dc18ec940e@dunslane.net
* Fix JSON_OBJECTAGG uniquefying bugAndrew Dunstan2022-04-28
| | | | | | | | Commit f4fb45d15c contained a bug in removing items with null values when unique keys are required, where the leading items that are sorted contained such values. Fix that and add a test for it. Discussion: https://postgr.es/m/CAJA4AWQ_XbSmsNbW226UqNyRLJ+wb=iQkQMj77cQyoNkqtf=2Q@mail.gmail.com
* Disable asynchronous execution if using gating Result nodes.Etsuro Fujita2022-04-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | mark_async_capable_plan(), which is called from create_append_plan() to determine whether subplans are async-capable, failed to take into account that the given subplan created from a given subpath might include a gating Result node if the subpath is a SubqueryScanPath or ForeignPath, causing a segmentation fault there when the subplan created from a SubqueryScanPath includes the Result node, or causing ExecAsyncRequest() to throw an error about an unrecognized node type when the subplan created from a ForeignPath includes the Result node, because in the latter case the Result node was unintentionally considered as async-capable, but we don't currently support executing Result nodes asynchronously. Fix by modifying mark_async_capable_plan() to disable asynchronous execution in such cases. Also, adjust code in the ProjectionPath case in mark_async_capable_plan(), for consistency with other cases, and adjust/improve comments there. is_async_capable_path() added in commit 27e1f1456, which was rewritten to mark_async_capable_plan() in a later commit, has the same issue, causing the error at execution mentioned above, so back-patch to v14 where the aforesaid commit went in. Per report from Justin Pryzby. Etsuro Fujita, reviewed by Zhihong Yu and Justin Pryzby. Discussion: https://postgr.es/m/20220408124338.GK24419%40telsasoft.com
* Revert recent changes with durable_rename_excl()Michael Paquier2022-04-28
| | | | | | | | | | | | | | | This reverts commits 2c902bb and ccfbd92. Per buildfarm members kestrel, rorqual and calliphoridae, the assertions checking that a TLI history file should not exist when created by a WAL receiver have been failing, and switching to durable_rename() over durable_rename_excl() would cause the newest TLI history file to overwrite the existing one. We need to think harder about such cases, so revert the new logic for now. Note that all the failures have been reported in the test 025_stuck_on_old_timeline. Discussion: https://postgr.es/m/511362.1651116498@sss.pgh.pa.us
* Fix SQL syntax in comment in logical/worker.cJohn Naylor2022-04-28
| | | | | | Euler Taveira Disussion: https://www.postgresql.org/message-id/25f95189-eef8-43c4-9d7b-419b651963c8%40www.fastmail.com
* Remove durable_rename_excl()Michael Paquier2022-04-28
| | | | | | | | | | ccfbd92 has replaced all existing in-core callers of this function in favor of durable_rename(). durable_rename_excl() is by nature unsafe on crashes happening at the wrong time, so just remove it. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13
* Replace existing durable_rename_excl() calls with durable_rename()Michael Paquier2022-04-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | durable_rename_excl() attempts to avoid overwriting any existing files by using link() and unlink(), falling back to rename() on some platforms (e.g., Windows where link() followed by unlink() is not concurrent-safe, see 909b449). Most callers of durable_rename_excl() use it just in case there is an existing file, but it happens that for all of them we never expect a target file to exist (WAL segment recycling, creation of timeline history file and basic_archive). basic_archive used durable_rename_excl() to avoid overwriting an archive concurrently created by another server. Now, there is a stat() call to avoid overwriting an existing archive a couple of lines above, so note that this change opens a small TOCTOU window in this module between the stat() call and durable_rename(). Furthermore, as mentioned in the top comment of durable_rename_excl(), this routine can result in multiple hard links to the same file and data corruption, with two or more links to the same file in pg_wal/ if a crash happens before the unlink() call during WAL recycling. Specifically, this would produce links to the same file for the current WAL file and the next one because the half-recycled WAL file was re-recycled during crash recovery of a follow-up cluster restart. This change replaces all calls to durable_rename_excl() with durable_rename(). This removes the protection against accidentally overwriting an existing file, but some platforms are already living without it, and all those code paths never expect an existing file (a couple of assertions are added to check after that, in case). This is a bug fix, but knowing the unlikeliness of the problem involving one of more crashes at an exceptionally bad moment, no backpatch is done. This could be revisited in the future. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13
* Fix incorrect format placeholdersPeter Eisentraut2022-04-27
|
* Handle NULL fields in WRITE_INDEX_ARRAYPeter Eisentraut2022-04-27
| | | | | | | | | | | | Unlike existing WRITE_*_ARRAY macros, WRITE_INDEX_ARRAY needs to handle the case that the field is NULL. We already have the convention to print NULL fields as "<>", so we do that here as well. There is currently no corresponding read function for this, so reading this back in is not implemented, but it could be if needed. Reported-by: Richard Guo <guofenglinux@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/CAMbWs4-LN%3DbF8f9eU2R94dJtF54DfDvBq%2BovqHnOQqbinYDrUw%40mail.gmail.com
* Add some isolation tests for CLUSTERMichael Paquier2022-04-26
| | | | | | | | | | | | | | | This commit adds two isolation tests for CLUSTER, using: - A normal table, making sure that CLUSTER blocks and completes if the table is locked by a concurrent session. - A partitioned table with a partition owned by a different user. If the partitioned table is locked by a concurrent session, CLUSTER on the partitioned table should block. If the partition owned by a different user is locked, CLUSTER on its partitioned table should complete and skip the partition. 3f19e17 has added an early check to ignore such a partition with a SQL regression test, but this was not checking that CLUSTER should not block. Discussion: https://postgr.es/m/YlqveniXn9AI6RFZ@paquier.xyz
* Inhibit mingw CRT's auto-globbing of command line argumentsAndrew Dunstan2022-04-25
| | | | | | | | | | | | | | | | For some reason by default the mingw C Runtime takes it upon itself to expand program arguments that look like shell globbing characters. That has caused much scratching of heads and mis-attribution of the causes of some TAP test failures, so stop doing that. This removes an inconsistency with Windows binaries built with MSVC, which have no such behaviour. Per suggestion from Noah Misch. Backpatch to all live branches. Discussion: https://postgr.es/m/20220423025927.GA1274057@rfd.leadboat.com
* Drop unlogged table after test is doneAlvaro Herrera2022-04-25
| | | | | | | | Another test is constructed on top of regression tests, which does not work correctly with unlogged tables. For now, cope with that by making sure no unlogged table is left behind. Per buildfarm pink after 4fb5c794e586.
* Cover brin/gin/gist/spgist ambuildempty routines in regression testsAlvaro Herrera2022-04-25
| | | | | | | | | Changing some TEMP or permanent tables to UNLOGGED is sufficient to invoke these ambuildempty routines, which were all not uncovered by any tests. These changes do not otherwise affect the test suite. Author: Amul Sul <sulamul@gmail.com> Discussion: https://postgr.es/m/CAAJ_b95nneRCLM-=qELEdgCYSk6W_++-C+Q_t+wH3SW-hF50iw@mail.gmail.com
* Always pfree strings returned by GetDatabasePathAlvaro Herrera2022-04-25
| | | | | | | | | | | | | | Several places didn't do it, and in many cases it didn't matter because it would be a small allocation in a short-lived context; but other places may accumulate a few (for example, in CreateDatabaseUsingFileCopy, one per tablespace). In most databases this is highly unlikely to be very serious either, but it seems better to make the code consistent in case there's future copy-and-paste. The only case of actual concern seems to be the aforementioned routine, which is new with commit 9c08aea6a309, so there's no need to backpatch. As pointed out by Coverity.
* Fix incautious CTE matching in rewriteSearchAndCycle().Tom Lane2022-04-23
| | | | | | | | | | | | | | | | This function looks for a reference to the recursive WITH CTE, but it checked only the CTE name not ctelevelsup, so that it could seize on a lower CTE that happened to have the same name. This would result in planner failures later, either weird errors such as "could not find attribute 2 in subquery targetlist", or crashes or assertion failures. The code also merely Assert'ed that it found a matching entry, which is not guaranteed at all by the parser. Per bugs #17320 and #17318 from Zhiyong Wu. Thanks to Kyotaro Horiguchi for investigation. Discussion: https://postgr.es/m/17320-70e37868182512ab@postgresql.org Discussion: https://postgr.es/m/17318-2eb65a3a611d2368@postgresql.org
* Test ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.Noah Misch2022-04-22
| | | | | | | | | Today's test case detected alignment problems only when executing on AIX. This change lets popular platforms detect the same problems. Reviewed by Masahiko Sawada. Discussion: https://postgr.es/m/20220415072601.GG862547@rfd.leadboat.com
* Remove some recently-added pg_dump test cases.Robert Haas2022-04-22
| | | | | | | | | | | | | Commit d2d35479796c3510e249d6fc72adbd5df918efbf included a pretty extensive set of test cases, and some of them don't work on all of our Windows machines. This happens because IPC::Run expands its arguments as shell globs on a few machines, but doesn't on most of the buildfarm. It might be good to fix that problem systematically somehow, but in the meantime, there are enough test cases for this commit that it seems OK to just remove the ones that are failing. Discussion: http://postgr.es/m/3a190754-b2b0-d02b-dcfd-4ec1610ffbcb@dunslane.net Discussion: http://postgr.es/m/CA+TgmoYRGUcFBy6VgN0+Pn4f6Wv=2H0HZLuPHqSy6VC8Ba7vdg@mail.gmail.com
* Fix performance regression in tuplesort specializationsDavid Rowley2022-04-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 697492434 added 3 new qsort specialization functions aimed to improve the performance of sorting many of the common pass-by-value data types when they're the leading or only sort key. Unfortunately, that has caused a performance regression when sorting datasets where many of the values being compared were equal. What was happening here was that we were falling back to the standard sort comparison function to handle tiebreaks. When the two given Datums compared equally we would incur both the overhead of an indirect function call to the standard comparer to perform the tiebreak and also the standard comparer function would go and compare the leading key needlessly all over again. Here improve the situation in the 3 new comparison functions. We now return 0 directly when the two Datums compare equally and we're performing a 1-key sort. Here we don't do anything to help the multi-key sort case where the leading key uses one of the sort specializations functions. On testing this case, even when the leading key's values are all equal, there appeared to be no performance regression. Let's leave it up to future work to optimize that case so that the tiebreak function no longer re-compares the leading key over again. Another possible fix for this would have been to add 3 additional sort specialization functions to handle single-key sorts for these pass-by-value types. The reason we didn't do that here is that we may deem some other sort specialization to be more useful than single-key sorts. It may be impractical to have sort specialization functions for every single combination of what may be useful and it was already decided that further analysis into which ones are the most useful would be delayed until the v16 cycle. Let's not let this regression force our hand into trying to make that decision for v15. Author: David Rowley Reviewed-by: John Naylor Discussion: https://postgr.es/m/CA+hUKGJRbzaAOUtBUcjF5hLtaSHnJUqXmtiaLEoi53zeWSizeA@mail.gmail.com