aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Ensure that the argument of shmdt(2) is declared "void *".Tom Lane2022-02-15
| | | | | | | | | Our gcc-on-Solaris buildfarm members emit "incompatible pointer type" warnings in places where it's not. This is a bit odd, since AFAICT Solaris follows the POSIX spec in declaring shmdt's argument as "const void *", and you'd think any pointer argument would satisfy that. But whatever. Part of a general push to remove off-the-beaten-track warnings where we can easily do so.
* Reject change of output-column collation in CREATE OR REPLACE VIEW.Tom Lane2022-02-15
| | | | | | | | | | | | checkViewTupleDesc() didn't get the memo that it should verify same attcollation along with same type/typmod. (A quick scan did not find other similar oversights.) Per bug #17404 from Pierre-Aurélien Georges. On another day I might've back-patched this, but today I'm feeling paranoid about unnecessary behavioral changes in back branches. Discussion: https://postgr.es/m/17404-8a4a270ef30a6709@postgresql.org
* Ensure that STDERR is empty in connect_ok testsDaniel Gustafsson2022-02-15
| | | | | | | | | | | Connections performed via connect_ok() in TAP tests should not write anything to STDERR. Author: Jacob Champion <pchampion@vmware.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/9D4FFB61-392B-4A2C-B7E4-911797B4AC14@yesql.se Discussion: https://postgr.es/m/ec146256e31afa0542f9fa970ec258c5f1a5f98.camel@vmware.com
* Add more logging to new 028_pitr_timelines.pl test.Heikki Linnakangas2022-02-15
| | | | | | | | The test has failed a couple of times on buildfarm member 'hoverfly'. It gets stuck waiting for the standby to archive 000000020000000000000003 WAL segment. I don't understand why, but with DEBUG1, we will get messages in the log whenever a segment is archived, which hopefully will give a clue the next time it happens.
* Remove IS_AF_UNIX macroPeter Eisentraut2022-02-15
| | | | | | | | | | | | The AF_UNIX macro was being used unprotected by HAVE_UNIX_SOCKETS, apparently since 2008. So the redirection through IS_AF_UNIX() is apparently no longer necessary. (More generally, all supported platforms are now HAVE_UNIX_SOCKETS, but even if there were a new platform in the future, it seems plausible that it would define the AF_UNIX symbol even without kernel support.) So remove the IS_AF_UNIX() macro and make the code a bit more consistent. Discussion: https://www.postgresql.org/message-id/flat/f2d26815-9832-e333-d52d-72fbc0ade896%40enterprisedb.com
* Add test case for trailing junk after numeric literalsPeter Eisentraut2022-02-15
| | | | | | | | | | PostgreSQL currently accepts numeric literals with trailing non-digits, such as 123abc where the abc is treated as the next token. This may be a bit surprising. This commit adds test cases for this; subsequent commits intend to change this behavior. Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Remove pg_atoi()Peter Eisentraut2022-02-15
| | | | | | | | | The last caller was int2vectorin(), and having such a general function for one user didn't seem useful, so just put the required parts inline and remove the function. Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Remove command checks in tests of pg_basebackup and pg_receivewalMichael Paquier2022-02-15
| | | | | | | | | | | | The TAP tests of those commands have been checking if commands of "gzip" and "lz4" existed by launching them with an extra --version. Based on the buildfarm, this is not required for "gzip" as the command always exists. Since 1d084fb, "lz4" has a ./configure check doing the same thing. Reported-by: Andres Freund Discussion: https://postgr.es/m/20220212220643.ozuvq2k4cjkcnr2v@alap3.anarazel.de Discussion: https://postgr.es/m/Ygm2ADakjlqGc2Ro@paquier.xyz
* Fix thinko with subdirectories generated by pg_upgrade for internal filesMichael Paquier2022-02-15
| | | | | | | | | | | | | 38bfae3 has mixed the "dump/" and "log/" subdirectories generated in "pg_upgrade_output.d/", causing the internal dump files to be generated in "log/" and the log files to be in "dump/", but the opposite should be done. This was not directly an issue for pg_upgrade runs, as the internal dump files were still picked up at the location of their creation, but the newest version of the buildfarm client would have reported the dump files instead of the log files on failures of pg_upgrade. Issue spotted while testing the TAP tests of pg_upgrade.
* Move replication slot release to before_shmem_exit().Andres Freund2022-02-14
| | | | | | | | | | | | | | | | | | | | | | Previously, replication slots were released in ProcKill() on error, resulting in reporting replication slot drop of ephemeral slots after the stats subsystem was already shut down. To fix this problem, move replication slot release to a before_shmem_exit() hook that is called before the stats collector shuts down. There wasn't really a good reason for the slot handling to be in ProcKill() anyway. Patch by Masahiko Sawada, with very minor polishing by me. I, Andres, wrote a test for dropping slots during process exit, but there may be some OS dependent issues around the number of times FATAL error messages are displayed due to a still debated libpq issue. So that test will be committed separately / later. Reviewed-By: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-By: Andres Freund <andres@anarazel.de> Author: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAD21AoDAeEpAbZEyYJsPZJUmSPaRicVSBObaL7sPaofnKz+9zg@mail.gmail.com
* Remove one use of pg_atoi()Peter Eisentraut2022-02-14
| | | | | | | There was no real need to use this here instead of a simpler API. Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Move scanint8() to numutils.cPeter Eisentraut2022-02-14
| | | | | | | | | | | Move scanint8() to numutils.c and rename to pg_strtoint64(). We already have a "16" and "32" version of that, and the code inside the functions was aligned, so this move makes all three versions consistent. The API is also changed to no longer provide the errorOK case. Users that need the error checking can use strtoi64(). Reviewed-by: John Naylor <john.naylor@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/b239564c-cad0-b23e-c57e-166d883cb97d@enterprisedb.com
* Suppress integer-overflow compiler warning for inconsistent sun_len.Tom Lane2022-02-14
| | | | | | | | | | | | On AIX 7.1, struct sockaddr_un is declared to be 1025 bytes long, but the sun_len field that should hold the length is only a byte. Clamp the value we try to store to ensure it will fit in the field. (This coding might need adjustment if there are any machines out there where sun_len is as wide as size_t; but a preliminary survey suggests there's not, so let's keep it simple.) Discussion: https://postgr.es/m/2781112.1644819528@sss.pgh.pa.us
* Add test case for an archive recovery corner case.Heikki Linnakangas2022-02-14
| | | | | | | | | | | | | | | | | While I was working on a patch to refactor things around xlog.c, I mixed up EndOfLogTLI and replayTLI at the end of recovery. As a result, if you recovered to a point with a lower-numbered timeline in a WAL segment that has a higher TLI in the filename, the end-of-recovery WAL record was created with invalid PrevTimeLineId. I noticed that while self-reviewing, but no tests failed. So add a test to cover that corner case. Thanks to Amul Sul who also submitted a test case for the same corner case, although this patch is different from that. Reviewed-by: Amul Sul, Michael Paquier Discussion: https://www.postgresql.org/message-id/52bc9ccd-8591-431b-0086-15d9acf25a3f@iki.fi Discussion: https://www.postgresql.org/message-id/CAAJ_b94Vjt5cXGza_1MkjLQWciNdEemsmiWuQj0d%3DM7JfjAa1g%40mail.gmail.com
* Add missing node support functionsPeter Eisentraut2022-02-14
| | | | forgotten in 37851a8b83d3d57ca48736093b10aa5f3bc0c177
* Database-level collation version trackingPeter Eisentraut2022-02-14
| | | | | | | | | | | | | | | | | | This adds to database objects the same version tracking that collation objects have. There is a new pg_database column datcollversion that stores the version, a new function pg_database_collation_actual_version() to get the version from the operating system, and a new subcommand ALTER DATABASE ... REFRESH COLLATION VERSION. This was not originally added together with pg_collation.collversion, since originally version tracking was only supported for ICU, and ICU on a database-level is not currently supported. But we now have version tracking for glibc (since PG13), FreeBSD (since PG14), and Windows (since PG13), so this is useful to have now. Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f0ff3190-29a3-5b39-a179-fa32eee57db6%40enterprisedb.com
* Improve correlation names in sanity testsPeter Eisentraut2022-02-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some of the queries in the "sanity" tests in the regression test suite (opr_sanity, type_sanity) are very confusing. One main stumbling block is that for some probably ancient reason many of the older queries are written with correlation names p1, p2, etc. independent of the name of the catalog. This one is a good example: SELECT p1.oid, p1.oprname, p2.oid, p2.proname FROM pg_operator AS p1, pg_proc AS p2 <-- HERE WHERE p1.oprcode = p2.oid AND p1.oprkind = 'l' AND (p2.pronargs != 1 OR NOT binary_coercible(p2.prorettype, p1.oprresult) OR NOT binary_coercible(p1.oprright, p2.proargtypes[0]) OR p1.oprleft != 0); This is better written as SELECT o1.oid, o1.oprname, p1.oid, p1.proname FROM pg_operator AS o1, pg_proc AS p1 WHERE o1.oprcode = p1.oid AND o1.oprkind = 'l' AND (p1.pronargs != 1 OR NOT binary_coercible(p1.prorettype, o1.oprresult) OR NOT binary_coercible(o1.oprright, p1.proargtypes[0]) OR o1.oprleft != 0); This patch cleans up all the queries in this manner. (As in the above case, I kept the digits like o1 and p1 even in cases where only one of each letter is used in a query. This is mainly to keep the style consistent.) Discussion: https://www.postgresql.org/message-id/flat/c538308b-319c-8784-e250-1284d12d5411%40enterprisedb.com
* Use WL_SOCKET_CLOSED for client_connection_check_interval.Thomas Munro2022-02-14
| | | | | | | | | | | | Previously we used poll() directly to check for a POLLRDHUP event. Instead, use the WaitEventSet API to poll the socket for WL_SOCKET_CLOSED, which knows how to detect this condition on many more operating systems. Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Reviewed-by: Maksim Milyutin <milyutinma@gmail.com> Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru
* Add WL_SOCKET_CLOSED for socket shutdown events.Thomas Munro2022-02-14
| | | | | | | | | | | | | | | Provide a way for WaitEventSet to report that the remote peer has shut down its socket, independently of whether there is any buffered data remaining to be read. This works only on systems where the kernel exposes that information, namely: * WAIT_USE_POLL builds using POLLRDHUP, if available * WAIT_USE_EPOLL builds using EPOLLRDHUP * WAIT_USE_KQUEUE builds using EV_EOF Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Reviewed-by: Maksim Milyutin <milyutinma@gmail.com> Discussion: https://postgr.es/m/77def86b27e41f0efcba411460e929ae%40postgrespro.ru
* WAL log unchanged toasted replica identity key attributes.Amit Kapila2022-02-14
| | | | | | | | | | | | | | | Currently, during UPDATE, the unchanged replica identity key attributes are not logged separately because they are getting logged as part of the new tuple. But if they are stored externally then the untoasted values are not getting logged as part of the new tuple and logical replication won't be able to replicate such UPDATEs. So we need to log such attributes as part of the old_key_tuple during UPDATE. Reported-by: Haiying Tang Author: Dilip Kumar and Amit Kapila Reviewed-by: Alvaro Herrera, Haiying Tang, Andres Freund Backpatch-through: 10 Discussion: https://postgr.es/m/OS0PR01MB611342D0A92D4F4BF26C0F47FB229@OS0PR01MB6113.jpnprd01.prod.outlook.com
* Track LLVM 15 changes.Thomas Munro2022-02-14
| | | | | This isn't an API change, it's just a missing #include that we got away with before. Per buildfarm animal seawasp.
* Correct Makefile dependencies for catalog scriptsJohn Naylor2022-02-14
| | | | | | At some point, Gen_fmgrtab.pl stopped needing the value of defined symbols from access/transam.h, while genbki.pl starting doing so. The Makefiles didn't get the memo, so update the relevant dependencies.
* Add ./configure check for "lz4" commandMichael Paquier2022-02-14
| | | | | | | | | | | | | | | | | | | | | | | | | Some environments may compile with --with-lz4 while the command "lz4" goes missing, causing two failures in the TAP tests of pg_verifybackup (008_untar.pl and 010_client_untar.pl) as the code assumed that the command always existed with a hardcoded value in src/Makefile.global. Rather than this method, this adds a ./configure check based on PGAC_PATH_PROGS() to find automatically the command and get an absolute path to it. Both tests need to be adjusted for the case where the command does not exist, actually, as Makefile.global would set now LZ4 to an empty value in this case. The TAP tests of pg_receivewal already do that. Per report from buildfarm member copperhead, as an effect of dab2984. The origin of the failure is actually babbbb5 that did not centralize the check for the existence of a "lz4" command at ./configure to shave a few cycles. Note that one just needs to tweak an environment to move "lz4" out of the way to reproduce the problem, which is what I did to test this change. Per discussion with Robert Haas, Tom Lane, Andres Freund and myself. Discussion: https://postgr.es/m/Ygc51WVAFGocSu4h@paquier.xyz
* Fix memory leak in IndexScan node with reorderingAlexander Korotkov2022-02-14
| | | | | | | | | | Fix ExecReScanIndexScan() to free the referenced tuples while emptying the priority queue. Backpatch to all supported versions. Discussion: https://postgr.es/m/CAHqSB9gECMENBQmpbv5rvmT3HTaORmMK3Ukg73DsX5H7EJV7jw%40mail.gmail.com Author: Aliaksandr Kalenik Reviewed-by: Tom Lane, Alexander Korotkov Backpatch-through: 10
* Make origin data initialization consistent other fields in 2PC headerMichael Paquier2022-02-14
| | | | | | | | | | | | | | | As of 1eb6d65, the origin data is optionally stored in a 2PC file header, with the data filled in EndPrepare() even in the default case where there is no origin data to add. This was inconsistent with all the other fields of TwoPhaseFileHeader which are initialized in StartPrepare(), so move the initialization of origin_lsn and origin_timestamp there instead. The effect of missing the initialization at this early stage is only cosmetic based on the current logic of the code, but could have led to issues in the long-term, and it is more consistent done this way. Reported-by: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAooECJ+gU_RZB-yhioPOV94R4ucoHAf68PiJhLpgpVpBw@mail.gmail.com
* Fix misuse of "const" qualifier.Tom Lane2022-02-13
| | | | | | | | | | | "const foo *" is quite different from "foo * const". This code was evidently trying to avoid casting away const from the arguments, but entirely failed to do so. Per study of some buildfarm warnings from anole (which unfortunately are mostly ignorable, since it seems not to understand "restrict" very well). I'm surprised though that nothing else has complained.
* Remove REGRESS_OUTPUTDIR environment variable.Thomas Munro2022-02-14
| | | | | | | | Andres Freund points out that the tmp_check path is already available as perl variable PostgreSQL::Test::Utils::tmp_check, so we can drop the new environment variable introduced by commit f47ed79cc. Discussion: https://postgr.es/m/20220213052955.dh7lheehit7bsemf%40alap3.anarazel.de
* Silence minor compiler warnings.Tom Lane2022-02-13
| | | | | | | | | | | | | | Depending on compiler version and optimization level, we might get a complaint that lazy_scan_heap's "freespace" is used uninitialized. Compilers not aware that ereport(ERROR) doesn't return complained about bbsink_lz4_new(). Assigning "-1" to a uint64 value has unportable results; fortunately, the value of xlogreadsegno is unimportant when xlogreadfd is -1. (It looks to me like there is no need for xlogreadsegno to be static in the first place, but I didn't venture to change that.)
* Move libpq's write_failed mechanism down to pqsecure_raw_write().Tom Lane2022-02-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 1f39a1c06 implemented write-failure postponement in pqSendSome, which is above SSL/GSS processing. However, we've now seen failures indicating that (some versions of?) OpenSSL have a tendency to report write failures prematurely too. Hence, move the primary responsibility for postponing write failures down to pqsecure_raw_write(), below SSL/GSS processing. pqSendSome now sets write_failed only in corner cases where we'd lost the connection already. A side-effect of this change is that errors detected in the SSL/GSS layer itself will be reported immediately (as if they were read errors) rather than being postponed like write errors. That's reverting an effect of 1f39a1c06, and I think it's fine: if there's not a socket-level error, it's hard to be sure whether an OpenSSL error ought to be considered a read or write failure anyway. Another important point is that write-failure postponement is now effective during connection setup. OpenSSL's misbehavior of this sort occurs during SSL_connect(), so that's a change we want. Per bug #17391 from Nazir Bilal Yavuz. Possibly this should be back-patched, but I think it prudent to let it age awhile in HEAD first. Discussion: https://postgr.es/m/17391-304f81bcf724b58b@postgresql.org
* Fix thinko in PQisBusy().Tom Lane2022-02-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In commit 1f39a1c06 I made PQisBusy consider conn->write_failed, but that is now looking like complete brain fade. In the first place, the logic is quite wrong: it ought to be like "and not" rather than "or". This meant that once we'd gotten into a write_failed state, PQisBusy would always return true, probably causing the calling application to iterate its loop until PQconsumeInput returns a hard failure thanks to connection loss. That's not what we want: the intended behavior is to return an error PGresult, which the application probably has much cleaner support for. But in the second place, checking write_failed here seems like the wrong thing anyway. The idea of the write_failed mechanism is to postpone handling of a write failure until we've read all we can from the server; so that flag should not interfere with input-processing behavior. (Compare 7247e243a.) What we *should* check for is status = CONNECTION_BAD, ie, socket already closed. (Most places that close the socket don't touch asyncStatus, but they do reset status.) This primarily ensures that if PQisBusy() returns true then there is an open socket, which is assumed by several call sites in our own code, and probably other applications too. While at it, fix a nearby thinko in libpq's my_sock_write: we should only consult errno for res < 0, not res == 0. This is harmless since pqsecure_raw_write would force errno to zero in such a case, but it still could confuse readers. Noted by Andres Freund. Backpatch to v12 where 1f39a1c06 came in. Discussion: https://postgr.es/m/20220211011025.ek7exh6owpzjyudn@alap3.anarazel.de
* Revert "Add TAP test to automate the equivalent of check_guc"Michael Paquier2022-02-12
| | | | | | | | | | | | | | | | | | | | | | | This reverts commit b0a55f4, to remove for now the TAP test that did the equivalent of check_guc. The test has been using pg_config --sharedir to find the location of postgresql.conf.sample. While the buildfarm and normal build environments rather liked that, this proves to be an issue for Debian where pg_config is patched to not be relocatable, causing the test to fail. Rather than relying on pg_config, we'd better find the sample file based on its location from the source directory. However, this is also an issue as a TAP test only offers the build directory as of TESTDIR in the environment context, so this would fail with VPATH builds. Perhaps the source path could be provided additionally when running the TAP tests. Or perhaps we may be able to get away by just switching to a SQL approach, by using PG_ABS_SRCDIR but this is going to require some extra loops to get the sample file from the correct path in src/backend/. In any case, this needs more thoughts, so just revert the test case until something better is done about this relocation problem. Reported-by: Christopher Berg Discussion: https://postgr.es/m/YgYw25OXV5men8Fj@msg.df7cb.de
* Consolidate VACUUM xid cutoff logic.Peter Geoghegan2022-02-11
| | | | | | | | | Push the logic for determining whether or not a VACUUM operation will be aggressive down into vacuum_set_xid_limits(). This makes the function's signature significantly simpler, and seems clearer overall. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-WzkymFbz6D_vL+jmqSn_5q1wsFvFrE+37yLgL_Rkfd6Gzg@mail.gmail.com
* Add VACUUM instrumentation for scanned pages, relfrozenxid.Peter Geoghegan2022-02-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Report on scanned pages within VACUUM VERBOSE and autovacuum logging. These are pages that were physically examined during the VACUUM operation. Note that this can include a small number of pages that were marked all-visible in the visibility map by some earlier VACUUM operation. VACUUM won't skip all-visible pages that aren't part of a range of all-visible pages that's at least 32 blocks in length (partly to avoid missing out on opportunities to advance relfrozenxid during non-aggressive VACUUMs). Commit 44fa8488 simplified the definition of scanned pages. It became the complement of the pages (of those pages from rel_pages) that were skipped using the visibility map. And so scanned pages precisely indicates how effective the visibility map was at saving work. (Before now we displayed the number of pages skipped via the visibility map when happened to be frozen pages, but not when they were merely all-visible, which was less useful to users.) Rename the user-visible OldestXmin output field to "removal cutoff", and show some supplementary information: how far behind the cutoff is (number of XIDs behind) by the time the VACUUM operation finished. This will help users to figure out what's _not_ working in extreme cases where VACUUM is fundamentally unable to remove dead tuples or freeze older tuples (e.g., due to a leaked replication slot). Also report when relfrozenxid is advanced by VACUUM in output that immediately follows "removal cutoff". This structure is intended to highlight the relationship between the new relfrozenxid value for the table, and the VACUUM operation's removal cutoff. Finally, add instrumentation of "missed dead tuples", and the number of pages that had at least one such tuple. These are fully DEAD (not just RECENTLY_DEAD) tuples with storage that could not be pruned due to failure to acquire a cleanup lock on a heap page. This is a replacement for the "skipped due to pin" instrumentation removed by commit 44fa8488. It shows more details than before for pages where failing to get a cleanup lock actually resulted in VACUUM missing out on useful work, but usually shows nothing at all instead (the mere fact that we couldn't get a cleanup lock is usually of no consequence whatsoever now). Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAH2-Wznp=c=Opj8Z7RMR3G=ec3_JfGYMN_YvmCEjoPCHzWbx0g@mail.gmail.com
* Simplify lazy_scan_heap's handling of scanned pages.Peter Geoghegan2022-02-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Redefine a scanned page as any heap page that actually gets pinned by VACUUM's first pass over the heap, regardless of whether or not the page was cleanup locked. Although it's fundamentally impossible to prune a heap page without a cleanup lock (since we cannot safely defragment the page), we can do just about everything else. The only notable further exception is freezing tuples, though even that is arguably a consequence of not being able to prune (not a separate issue). VACUUM now does as much of the same processing as possible for pages that could not be cleanup locked. Any failure to do specific required processing is treated as a special case exception, which will be rare in practice. We now collect any preexisting LP_DEAD items (left behind by earlier opportunistic pruning) in the dead_items array for these heap pages, and count their tuples in the usual way. Steps used to decide if we'll attempt relation truncation are performed in the usual way for no-cleanup-lock scanned pages, too. Although eliminating these special cases is intrinsically useful, it's even more useful as an enabler of further simplifications. The only essential difference between aggressive and non-aggressive is that only aggressive is _guaranteed_ to be able to advance relfrozenxid up to FreezeLimit. Advancing relfrozenxid is always useful, but before now non-aggressive VACUUMs threw away the opportunity to do so whenever a cleanup lock could not be acquired on any page, no matter what the details were. This was very pessimistic. It isn't actually necessary to "behave aggressively" to maintain the ability to advance relfrozenxid when a cleanup lock isn't immediately available (most of the time). The non-aggressive case will now make sure that it isn't safe to advance relfrozenxid (without waiting) using only a share lock. It will usually notice that there are no tuples that need to be frozen anyway, just like in the aggressive case -- and so it no longer wastes an opportunity to advance relfrozenxid over nothing. (The non-aggressive case still won't wait for a cleanup lock when there really are tuples on the page that need to be frozen, since that really would amount to "behaving aggressively".) VACUUM currently has a tendency to set heap pages to all-visible in the visibility map before it freezes all of the tuples on the page. Only a subsequent aggressive VACUUM will visit these pages to freeze their tuples, usually only when the tuple XIDs are much older than the vacuum_freeze_min_age GUC (FreezeLimit cutoff) is supposed to allow. And so non-aggressive VACUUMs are still far less likely to be able to advance relfrozenxid in practice, even with the enhancements from this commit. This remaining issue will be addressed by future work that overhauls the criteria for freezing tuples. Once that's in place, almost every VACUUM operation will be able to advance relfrozenxid in practice. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Andres Freund <andres@anarazel.de> Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAH2-Wznp=c=Opj8Z7RMR3G=ec3_JfGYMN_YvmCEjoPCHzWbx0g@mail.gmail.com
* Fix DROP {DATABASE,TABLESPACE} on Windows.Thomas Munro2022-02-12
| | | | | | | | | | | | | | | | | | | | | | | | | Previously, it was possible for DROP DATABASE, DROP TABLESPACE and ALTER DATABASE SET TABLESPACE to fail because other backends still had file handles open for dropped tables. Windows won't allow a directory containing unlinked-but-still-open files to be unlinked. Tackle this problem by forcing all backends to close all smgr fds. No change for Unix systems, which don't suffer from the problem, but the new code path can be tested by Unix-based developers by defining USE_BARRIER_SMGRRELEASE explicitly. It's possible that PROCSIGNAL_BARRIER_SMGRRELEASE will have more bug-fixing applications soon (under discussion). Note that this is the first user of the ProcSignalBarrier mechanism from commit 16a4e4aec. It could in principle be back-patched as far as 14, but since field complaints are rare and ProcSignalBarrier hasn't been battle-tested, that seems like a bad idea. Fix in master only, where these failures have started to show up in automated testing due to new tests. Suggested-by: Andres Freund <andres@anarazel.de> Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/CA+hUKGLdemy2gBm80kz20GTe6hNVwoErE8KwcJk6-U56oStjtg@mail.gmail.com
* Don't use_physical_tlist for an IOS with non-returnable columns.Tom Lane2022-02-11
| | | | | | | | | | | | | | | | | createplan.c tries to save a runtime projection step by specifying a scan plan node's output as being exactly the table's columns, or index's columns in the case of an index-only scan, if there is not a reason to do otherwise. This logic did not previously pay attention to whether an index's columns are returnable. That worked, sort of accidentally, until commit 9a3ddeb51 taught setrefs.c to reject plans that try to read a non-returnable column. I have no desire to loosen setrefs.c's new check, so instead adjust use_physical_tlist() to not try to optimize this way when there are non-returnable column(s). Per report from Ryan Kelly. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/CAHUie24ddN+pDNw7fkhNrjrwAX=fXXfGZZEHhRuofV_N_ftaSg@mail.gmail.com
* Replace Test::More plans with done_testingDaniel Gustafsson2022-02-11
| | | | | | | | | | | | | | | | | | | Rather than doing manual book keeping to plan the number of tests to run in each TAP suite, conclude each run with done_testing() summing up the the number of tests that ran. This removes the need for maintaning and updating the plan count at the expense of an accurate count of remaining during the test suite runtime. This patch has been discussed a number of times, often in the context of other patches which updates tests, so a larger number of discussions can be found in the archives. Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/DD399313-3D56-4666-8079-88949DAC870F@yesql.se
* pg_basebackup: Allow client-side LZ4 (de)compression.Robert Haas2022-02-11
| | | | | | | | | | | | LZ4 compression can now be performed on the client using pg_basebackup -Ft --compress client-lz4, and LZ4 decompression of a backup compressed on the server can be performed on the client using pg_basebackup -Fp --compress server-lz4. Dipesh Pandit, reviewed and tested by Jeevan Ladhe and Tushar Ahuja, with a few corrections - and some documentation - by me. Discussion: http://postgr.es/m/CAN1g5_FeDmiA9D8wdG2W6Lkq5CpubxOAqTmd2et9hsinTJtsMQ@mail.gmail.com
* Add suport for server-side LZ4 base backup compression.Robert Haas2022-02-11
| | | | | | | | | | | | | LZ4 compression can be a lot faster than gzip compression, so users may prefer it even if the compression ratio is not as good. We will want pg_basebackup to support LZ4 compression and decompression on the client side as well, and there is a pending patch for that, but it's by a different author, so I am committing this part separately for that reason. Jeevan Ladhe, reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/CANm22Cg9cArXEaYgHVZhCnzPLfqXCZLAzjwTq7Fc0quXRPfbxA@mail.gmail.com
* Make pg_ctl stop/restart/promote recheck postmaster aliveness.Tom Lane2022-02-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "pg_ctl stop/restart" checked that the postmaster PID is valid just once, as a side-effect of sending the stop signal, and then would wait-till-timeout for the postmaster.pid file to go away. This neglects the case wherein the postmaster dies uncleanly after we signal it. Similarly, once "pg_ctl promote" has sent the signal, it'd wait for the corresponding on-disk state change to occur even if the postmaster dies. I'm not sure how we've managed not to notice this problem, but it seems to explain slow execution of the 017_shm.pl test script on AIX since commit 4fdbf9af5, which added a speculative "pg_ctl stop" with the idea of making real sure that the postmaster isn't there. In the test steps that kill-9 and then restart the postmaster, it's possible to get past the initial signal attempt before kill() stops working for the doomed postmaster. If that happens, pg_ctl waited till PGCTLTIMEOUT before giving up ... and the buildfarm's AIX members have that set very high. To fix, include a "kill(pid, 0)" test (similar to what postmaster_is_alive uses) in these wait loops, so that we'll give up immediately if the postmaster PID disappears. While here, I chose to refactor those loops out of where they were. do_stop() and do_restart() can perfectly well share one copy of the wait-for-stop loop, and it seems desirable to put a similar function beside that for wait-for-promote. Back-patch to all supported versions, since pg_ctl's wait logic is substantially identical in all, and we're seeing the slow test behavior in all branches. Discussion: https://postgr.es/m/20220210023537.GA3222837@rfd.leadboat.com
* Use gendef instead of pexports for building windows .def filesAndrew Dunstan2022-02-10
| | | | | | | | | Modern msys systems lack pexports but have gendef instead, so use that. Discussion: https://postgr.es/m/3ccde7a9-e4f9-e194-30e0-0936e6ad68ba@dunslane.net Backpatch to release 9.4 to enable building with perl on older branches. Before that pexports is not used for plperl.
* Logical decoding of sequencesTomas Vondra2022-02-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This extends the logical decoding to also decode sequence increments. We differentiate between sequences created in the current (in-progress) transaction, and sequences created earlier. This mixed behavior is necessary because while sequences are not transactional (increments are not subject to ROLLBACK), relfilenode changes are. So we do this: * Changes for sequences created in the same top-level transaction are treated as transactional, i.e. just like any other change from that transaction, and discarded in case of a rollback. * Changes for sequences created earlier are applied immediately, as if performed outside any transaction. This applies also after ALTER SEQUENCE, which may create a new relfilenode. Moreover, if we ever get support for DDL replication, the sequence won't exist until the transaction gets applied. Sequences created in the current transaction are tracked in a simple hash table, identified by a relfilenode. That means a sequence may already exist, but if a transaction does ALTER SEQUENCE then the increments for the new relfilenode will be treated as transactional. For each relfilenode we track the XID of (sub)transaction that created it, which is needed for cleanup at transaction end. We don't need to check the XID to decide if an increment is transactional - if we find a match in the hash table, it has to be the same transaction. This requires two minor changes to WAL-logging. Firstly, we need to ensure the sequence record has a valid XID - until now the the increment might have XID 0 if it was the first change in a subxact. But the sequence might have been created in the same top-level transaction. So we ensure the XID is assigned when WAL-logging increments. The other change is addition of "created" flag, marking increments for newly created relfilenodes. This makes it easier to maintain the hash table of sequences that need transactional handling. Note: This is needed because of subxacts. A XID 0 might still have the sequence created in a different subxact of the same top-level xact. This does not include any changes to test_decoding and/or the built-in replication - those will be committed in separate patches. A patch adding decoding of sequences was originally submitted by Cary Huang. This commit reworks various important aspects (e.g. the WAL logging and transactional/non-transactional handling). However, the original patch and reviews were very useful. Author: Tomas Vondra, Cary Huang Reviewed-by: Peter Eisentraut, Hannu Krosing, Andres Freund Discussion: https://postgr.es/m/d045f3c2-6cfb-06d3-5540-e63c320df8bc@enterprisedb.com Discussion: https://postgr.es/m/1710ed7e13b.cd7177461430746.3372264562543607781@highgo.ca
* Remove server support for the previous base backup protocol.Robert Haas2022-02-10
| | | | | | | | | | Commit cc333f32336f5146b75190f57ef587dff225f565 added a new COPY sub-protocol for taking base backups, but retained support for the previous protocol. For the same reasons articulated in the message for commit 9cd28c2e5f11dfeef64a14035b82e70acead65fd, remove support for the previous protocol from the server. Discussion: http://postgr.es/m/CA+TgmoazKcKUWtqVa0xZqSzbKgTH+X-aw4V7GyLD68EpDLMh8A@mail.gmail.com
* Make timeout.c more robust against missed timer interrupts.Tom Lane2022-02-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 09cf1d522 taught schedule_alarm() to not do anything if the next requested event is after when we expect the next interrupt to fire. However, if somehow an interrupt gets lost, we'll continue to not do anything indefinitely, even after the "next interrupt" time is obviously in the past. Thus, one missed interrupt can break timeout scheduling for the life of the session. Michael Harris reported a scenario where a bug in a user-defined function caused this to happen, so you don't even need to assume kernel bugs exist to think this is worth fixing. We can make things more robust at little cost by detecting the case where signal_due_at is before "now" and forcing a new setitimer call to occur. This isn't a completely bulletproof fix of course; but in our typical usage pattern where we frequently set timeouts and clear them before they are reached, the interrupt will get re-enabled after at most one timeout interval, which with a little luck will be before we really need it. While here, let's mark signal_due_at as volatile, since the signal handler can both examine and set it. I'm not sure there's any actual risk given that signal_pending is already volatile, but it's surely questionable. Backpatch to v14 where this logic came in. Michael Harris and Tom Lane Discussion: https://postgr.es/m/CADofcAWbMrvgwSMqO4iG_iD3E2v8ZUrC-_crB41my=VMM02-CA@mail.gmail.com
* Remove server support for old BASE_BACKUP command syntax.Robert Haas2022-02-10
| | | | | | | | | | | | | | | | Commit 0ba281cb4bf9f5f65529dfa4c8282abb734dd454 added a new syntax for the BASE_BACKUP command, with extensible options, but maintained support for the legacy syntax. This isn't important for PostgreSQL, where pg_basebackup works with older server versions but not newer ones, but it could in theory matter for out-of-core users of the replication protocol. Discussion on pgsql-hackers, however, suggests that no one is aware of any out-of-core use of the BASE_BACKUP command, and the consensus is in favor of removing support for the old syntax to simplify the code, so do that. Discussion: http://postgr.es/m/CA+TgmoazKcKUWtqVa0xZqSzbKgTH+X-aw4V7GyLD68EpDLMh8A@mail.gmail.com
* Set SNI ClientHello extension to localhost in testsDaniel Gustafsson2022-02-10
| | | | | | | | | | | | | | | | | | | | | | The connection strings in the SSL client tests were using the host set up from Cluster.pm which is a temporary pathname. When SNI is enabled we pass the host to OpenSSL in order to set the server name indication ClientHello extension via SSL_set_tlsext_host_name. OpenSSL doesn't validate the hostname apart from checking the max length, but LibreSSL checks for RFC 5890 conformance which results in errors during testing as the pathname from Cluster.pm is not a valid hostname. Fix by setting the host explicitly to localhost, as that's closer to the intent of the test. Backpatch through 14 where SNI support came in. Reported-by: Nazir Bilal Yavuz <byavuz81@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/17391-304f81bcf724b58b@postgresql.org Backpatch-through: 14
* Remove unnecessary resetPQExpBuffer callPeter Eisentraut2022-02-10
| | | | | | | | Oversight in e2c52beecdea152ca680a22ef35c6a7da55aa30f. Author: Julien Rouhaud <rjuju123@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/20220209025007.eogz2aivcnvw46ym%40jrouhaud
* psql: Rename results to result when only a single one is meantPeter Eisentraut2022-02-10
| | | | | | | | This makes the naming more consistent with the libpq API and the rest of the code, and makes actually supporting multiple result sets in the future less confusing. Discussion: https://www.postgresql.org/message-id/flat/db72fb98-9b43-d776-7247-6ed38f28e7c6%40enterprisedb.com
* Update commentPeter Eisentraut2022-02-10
| | | | | Update a comment that assumed that libc collations don't support versioning. Also improve an adjacent error message a bit.
* Add min() and max() aggregates for xid8.Fujii Masao2022-02-10
| | | | | | | | Bump catalog version. Author: Ken Kato Reviewed-by: Kyotaro Horiguchi, Fujii Masao Discussion: https://postgr.es/m/47d77b18c44f87f8222c4c7a3e2dee6b@oss.nttdata.com