aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* 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
* Use Test::Builder::todo_start(), replacing $::TODO.Noah Misch2022-02-09
| | | | | | | | | | Some pre-2017 Test::More versions need perfect $Test::Builder::Level maintenance to find the variable. Buildfarm member snapper reported an overall failure that the file intended to hide via the TODO construct. That trouble was reachable in v11 and v10. For later branches, this serves as defense in depth. Back-patch to v10 (all supported versions). Discussion: https://postgr.es/m/20220202055556.GB2745933@rfd.leadboat.com
* Fix typo in multixact.cMichael Paquier2022-02-10
| | | | | | | Introduced in aa64f23. Author: Nathan Bossart Discussion: https://postgr.es/m/20220209175338.GB1627503@nathanxps13
* Reduce more the number of calls to GetMaxBackends()Michael Paquier2022-02-10
| | | | | | | | | | | | | Some of the code paths changed by aa64f23 can reduce the number of times GetMaxBackends() is called. The performance gain is marginal, but most of the code changed by this commit already did that. Hence, let's be clean and apply the same rule everywhere, for consistency. Some of the code paths, like in deadlock.c, involve only assertions. These are left unchanged. Reviewed-by: Nathan Bossart, Robert Haas Discussion: https://postgr.es/m/YgMpGZhPOjNfS7er@paquier.xyz
* Further tweaks for psql's new tab-completion logic.Tom Lane2022-02-09
| | | | | | | | | | | | | | | | | | | | The behavior I proposed, of matching case only when only keywords are available to complete, turns out to be too cute. It adds about as many problems as it removes. Simplify down to ilmari's original proposal of just always matching case when completing a keyword. Also, I noticed while testing this that we've pessimized the behavior for qualified GUC names: the code is insisting that they be double-quoted, which was not the case before. Fix that by treating GUC names as verbatim matches instead of possibly-schema-qualified names. (While it's tempting to try to split qualified GUC names so that we *could* treat them with the schema-qualified-name code path, that really isn't going to work in light of guc.c's willingness to allow more than two name components.) Dagfinn Ilmari Mannsåker and Tom Lane Discussion: https://postgr.es/m/445692.1644018081@sss.pgh.pa.us
* Test honestly for <sys/signalfd.h>.Tom Lane2022-02-09
| | | | | | | | | | | | | | | Commit 6a2a70a02 supposed that any platform having <sys/epoll.h> would also have <sys/signalfd.h>. It turns out there are still a few people using platforms where that's not so, so we'd better make a separate configure probe for it. But since it took this long to notice, I'm content with the decision to not have a separate code path for epoll-only machines; we'll just fall back to using poll() for these stragglers. Per gripe from Gabriela Serventi. Back-patch to v14 where this code came in. Discussion: https://postgr.es/m/CAHOHWE-JjJDfcYuLAAEO7Jk07atFAU47z8TzHzg71gbC0aMy=g@mail.gmail.com
* Free temporary memory when reading TOCDaniel Gustafsson2022-02-09
| | | | | | | | | | | | | ReadStr returns allocated memory which the caller is responsible for freeing when done with the string. This commit ensures that memory is freed in one case which used ReadStr in a conditional. While the leak might not be too concerning, this makes the code consistent across all ReadStr callsites in ReadToc. Due to the lack of complaints of issues in production from this, no backpatch is performed at this point. Author: Bharath Rupireddy, Georgios Kokolatos Reviewed-by: Kyotaro Horiguchi Discussion: https://postgr.es/m/oZwKiUxFsVaetG2xOJp7Hwao8F1AKIdfFDQLNJrnwoaxmjyB-45r_aYmhgXHKLcMI3GT24m9L6HafSi2ns7WFxXe0mw2_tIJpD-Z3vb_eyI=@pm.me
* Retire src/backend/utils/misc/check_gucMichael Paquier2022-02-09
| | | | | | | | | | This script has existed for a long time, and attempting to run it today causes a lot of false positives as an effect of GUCs added in the last couple of years. An equivalent, automatically-run and cross-platform solution is available in the TAP test introduced in b0a55f4. So, let it go. Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz
* Add TAP test to automate the equivalent of check_gucMichael Paquier2022-02-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | src/backend/utils/misc/check_guc is a script that cross-checks the consistency of the GUCs with postgresql.conf.sample, making sure that its format is in line with what guc.c has. It has never been run automatically, and has rotten over the years, creating a lot of false positives as per a report from Justin Pryzby. d10e41d has introduced a SQL function to publish the most relevant flags associated to a GUC, with tests added in the main regression test suite to make sure that we avoid most of the inconsistencies in the GUC settings, based on recent reports, but there was nothing able to cross-check postgresql.conf.sample with the contents of guc.c. This commit adds a TAP test that covers the remaining gap. It emulates the most relevant checks that check_guc does, so as any format mistakes are detected in postgresql.conf.sample at development stage, with the following checks: - Check that parameters marked as NOT_IN_SAMPLE are not in the sample file. - Check that there are no dead entries in postgresql.conf.sample for parameters not marked as NOT_IN_SAMPLE. - Check that no parameters are missing from the sample file if listed in guc.c without NOT_IN_SAMPLE. The idea of building a list of the GUCs by parsing the sample file comes from Justin, and he wrote the regex used in the patch to find all the GUCs (this same formatting rule basically applies for the last 20~ years or so). In order to test this patch, I have played with manual modifications of postgresql.conf.sample and guc.c, making sure that we detect problems with the GUC rules and the sample file format. The test is located in src/test/modules/test_misc, which is the best location I could think about for such sanity checks. Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/Yf9YGSwPiMu0c7fP@paquier.xyz
* Remove ppport.h's broken re-implementation of eval_pv().Tom Lane2022-02-08
| | | | | | | | | | | | | | | | | | Recent versions of Devel::PPPort try to redefine eval_pv() to dodge a bug in pre-5.31 Perl versions. Unfortunately the redefinition fails on compilers that don't support statements nested within expressions. However, we aren't actually interested in this bug fix, since we always call eval_pv() with croak_on_error = FALSE. So, until there's an upstream fix for this breakage, just comment out the macro to revert to the older behavior. Per report from Wei Sun, as well as previous buildfarm failure on pademelon (which I'd unfortunately not looked at carefully enough to understand the cause). Back-patch to all supported versions, since we're using the same ppport.h in all. Discussion: https://postgr.es/m/tencent_2EFCC8BA0107B6EC0F97179E019A8A43C806@qq.com Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pademelon&dt=2022-02-02%2001%3A22%3A58
* Remove MaxBackends variable in favor of GetMaxBackends() function.Robert Haas2022-02-08
| | | | | | | | | | | | | | Previously, it was really easy to write code that accessed MaxBackends before we'd actually initialized it, especially when coding up an extension. To make this less error-prune, introduce a new function GetMaxBackends() which should be used to obtain the correct value. This will ERROR if called too early. Demote the global variable to a file-level static, so that nobody can peak at it directly. Nathan Bossart. Idea by Andres Freund. Review by Greg Sabino Mullane, by Michael Paquier (who had doubts about the approach), and by me. Discussion: http://postgr.es/m/20210802224204.bckcikl45uezv5e4@alap3.anarazel.de
* Rename create_function_N test scripts for clarity.Tom Lane2022-02-08
| | | | | | | | | | | | Rename create_function_0 to create_function_c, and create_function_3 to create_function_sql, to establish their charters more clearly. This should also reduce confusion versus our underscore-digit convention for naming variant expected-files. I separated this from the previous commit on the premise that keeping the renaming distinct might make "git blame" tracking easier. Discussion: https://postgr.es/m/1114748.1640383217@sss.pgh.pa.us
* Rearrange core regression tests to reduce cross-script dependencies.Tom Lane2022-02-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The idea behind this patch is to make it possible to run individual test scripts without running the entire core test suite. Making all the scripts completely independent would involve a massive rewrite, and would probably be worse for coverage of things like concurrent DDL. So this patch just does what seems practical with limited changes. The net effect is that any test script can be run after running limited earlier dependencies: * all scripts depend on test_setup * many scripts depend on create_index * other dependencies are few in number, and are documented in the parallel_schedule file. To accomplish this, I chose a small number of commonly-used tables and moved their creation and filling into test_setup. Later scripts are expected not to modify these tables' data contents, for fear of affecting other scripts' results. Also, our former habit of declaring all C functions in one place is now gone in favor of declaring them where they're used, if that's just one script, or in test_setup if necessary. There's more that could be done to remove some of the remaining inter-script dependencies, but significantly more-invasive changes would be needed, and at least for now it doesn't seem worth it. Discussion: https://postgr.es/m/1114748.1640383217@sss.pgh.pa.us
* Add PostgreSQL::Test::Cluster::config_data()Michael Paquier2022-02-08
| | | | | | | | | | This is useful to grab some configuration information from a node already set up, and I personally found two cases for it: pg_upgrade and a test to emulate check_guc. Author: Michael Paquier Discussion: https://postgr.es/m/20211129030833.GJ17618@telsasoft.com Discussion: https://postgr.es/m/YJ8xTmLQkotVLpN5@paquier.xyz
* Reduce non-leaf keys overlap in GiST indexes produced by a sorted buildAlexander Korotkov2022-02-07
| | | | | | | | | | | | | | | | | | The GiST sorted build currently chooses split points according to the only page space utilization. That may lead to higher non-leaf keys overlap and, in turn, slower search query answers. This commit makes the sorted build use the opclass's picksplit method. Once four pages at the level are accumulated, the picksplit method is applied until each split partition fits the page. Some of our split algorithms could show significant performance degradation while processing 4-times more data at once. But those opclasses haven't received the sorted build support and shouldn't receive it before their split algorithms are improved. Discussion: https://postgr.es/m/CAHqSB9jqtS94e9%3D0vxqQX5dxQA89N95UKyz-%3DA7Y%2B_YJt%2BVW5A%40mail.gmail.com Author: Aliaksandr Kalenik, Sergei Shoulbakov, Andrey Borodin Reviewed-by: Björn Harrtell, Darafei Praliaskouski, Andres Freund Reviewed-by: Alexander Korotkov
* Add (void) cast in front of rmtree() call at the end of pg_upgradeMichael Paquier2022-02-07
| | | | | | | | | | Most calls of rmtree() report an error, and the code coming from 38bfae3 has introduced one caller where this is not done. The previous behavior was to not fail hard if any log file generated is not properly unlinked when cleaning up the contents generated once the upgrade has completed, so add a cast to (void) to indicate the intention behind this new code. Per gripe from Coverity.
* pg_upgrade: Move all the files generated internally to a subdirectoryMichael Paquier2022-02-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, the location of any files generated by pg_upgrade, as of the per-database logs and internal dumps, has been the current working directory, leaving all those files behind when using --retain or on a failure. Putting all those contents in a targeted subdirectory makes the whole easier to debug, and simplifies the code in charge of cleaning up the logs. Note that another reason is that this facilitates the move of pg_upgrade to TAP with a fixed location for all the logs to grab if the test fails repeatedly. Initially, we thought about being able to specify the output directory with a new option, but we have settled on using a subdirectory located at the root of the new cluster's data folder, "pg_upgrade_output.d", instead, as at the end the new data directory is the location of all the data generated by pg_upgrade. There is a take with group permissions here though: if the new data folder has been initialized with this option, we need to create all the files and paths with the correct permissions or a base backup taken after a pg_upgrade --retain would fail, meaning that GetDataDirectoryCreatePerm() has to be called before creating the log paths, before a couple of sanity checks on the clusters and before getting the socket directory for the cluster's host settings. The idea of the new location is based on a suggestion from Peter Eisentraut. Also thanks to Andrew Dunstan, Peter Eisentraut, Daniel Gustafsson, Tom Lane and Bruce Momjian for the discussion (in alphabetical order). Author: Justin Pryzby Discussion: https://postgr.es/m/20211212025017.GN17618@telsasoft.com
* Test, don't just Assert, that mergejoin's inputs are in order.Tom Lane2022-02-05
| | | | | | | | | | | | | | | | | There are two Asserts in nodeMergejoin.c that are reachable if the input data is not in the expected order. This seems way too fragile. Alexander Lakhin reported a case where the assertions could be triggered with misconfigured foreign-table partitions, and bitter experience with unstable operating system collation definitions suggests another easy route to hitting them. Neither Assert is in a place where we can't afford one more test-and-branch, so replace 'em with plain test-and-elog logic. Per bug #17395. While the reported symptom is relatively recent, collation changes could happen anytime, so back-patch to all supported branches. Discussion: https://postgr.es/m/17395-8c326292078d1a57@postgresql.org
* Improve worst-case performance of text_position_get_match_pos()John Naylor2022-02-04
| | | | | | | | | | | | | | | | | This function converts a byte position to a character position after a successful string match. Rather than calling pg_mblen() in a loop, use pg_mbstrlen_with_len() since the latter can inline its own call to pg_mblen(). When the string match is at the end of the haystack text, this change results in 10-20% performance improvement, depending on platform and typical character length in bytes. This also simplifies the code a little. Specializing for UTF-8 could result in further improvement, but the performance gain was not found to be reliable between platforms. The modest gain in this commit is stable between platforms and usable by all server encodings. Discussion: https://www.postgresql.org/message-id/CAFBsxsH1Yutrmu+6LLHKK8iXY+vG--Do6zN+2900spHXQNNQKQ@mail.gmail.com
* Track LLVM 14 API changes, up to 2022-01-30.Thomas Munro2022-02-04
| | | | | | | | | | | Tested with LLVM 11, LLVM 13 and LLVM's main branch at commit 8d8fce87bbd5. There are still some deprecation warnings that will need to be sorted out, but this may be enough to turn "seawasp" green again. Like commit e6a76002, done on master only for now. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CA%2BhUKG%2B3Ac3He9_SpJcxeiiVknbcES1tbZEkH9sRBdJFGj8K5Q%40mail.gmail.com
* Improve invalidation handling in pgoutput.c.Amit Kapila2022-02-04
| | | | | | | | | | | | | | | | | | | | | | Fix the following issues in pgoutput.c: * rel_sync_cache_relation_cb does the wrong thing when called for a cache flush (i.e., relid == 0). Instead of invalidating all RelationSyncCache entries as it should, it does nothing. * When rel_sync_cache_relation_cb does invalidate an entry, it immediately zaps the entry->map structure, even though that might still be in use. We instead just mark the entry as invalid and rebuild it at a later safe point. * Similarly, rel_sync_cache_publication_cb is way too eager to reset the pubactions flags, which would likely lead to failing to transmit changes that we should transmit. In this case also, we just mark the entry as invalid and rebuild it at a later safe point. Author: Tom Lane Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/885288.1641420714@sss.pgh.pa.us
* Allow archiving via loadable modules.Robert Haas2022-02-03
| | | | | | | | | | | | | | Running a shell command for each file to be archived has a lot of overhead and may not offer as much error checking as you want, or the exact semantics that you want. So, offer the option to call a loadable module for each file to be archived, rather than running a shell command. Also, add a 'basic_archive' contrib module as an example implementation that archives to a local directory. Nathan Bossart, with a little bit of kibitzing by me. Discussion: http://postgr.es/m/20220202224433.GA1036711@nathanxps13
* Fix compiler warning in non-assert builds, introduced in f862d57057f.Andres Freund2022-02-03
| | | | | Discussion: https://postgr.es/m/20220203183655.ralgkh54sdcgysmn@alap3.anarazel.de Backpatch: 14-, like f862d57057f
* Authorize new user in pg_basebackup testsAndrew Dunstan2022-02-03
| | | | | | Commit 8e2b6d45a0 added a new unprivileged user for testing pg_basebackup, but omitted to add them to the cluster's authorized logins, breaking Windows tests run without using Unix sockets.
* Add UNIQUE null treatment optionPeter Eisentraut2022-02-03
| | | | | | | | | | | | | | | | | | | | | | | | | The SQL standard has been ambiguous about whether null values in unique constraints should be considered equal or not. Different implementations have different behaviors. In the SQL:202x draft, this has been formalized by making this implementation-defined and adding an option on unique constraint definitions UNIQUE [ NULLS [NOT] DISTINCT ] to choose a behavior explicitly. This patch adds this option to PostgreSQL. The default behavior remains UNIQUE NULLS DISTINCT. Making this happen in the btree code is pretty easy; most of the patch is just to carry the flag around to all the places that need it. The CREATE UNIQUE INDEX syntax extension is not from the standard, it's my own invention. I named all the internal flags, catalog columns, etc. in the negative ("nulls not distinct") so that the default PostgreSQL behavior is the default if the flag is false. Reviewed-by: Maxim Orlov <orlovmg@gmail.com> Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/84e5ee1b-387e-9a54-c326-9082674bde78@enterprisedb.com
* Further fix for EvalPlanQual with mix of local and foreign partitions.Etsuro Fujita2022-02-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | We assume that direct-modify ForeignScan nodes cannot be re-evaluated during EvalPlanQual processing, but the rework for inherited UPDATE/DELETE in commit 86dc90056 changed things, without considering that, so that such ForeignScan nodes get called as part of the EvalPlanQual subtree during EvalPlanQual processing in the case of an inherited UPDATE/DELETE where the inheritance set contains foreign target relations. To avoid re-evaluating such ForeignScan nodes during EvalPlanQual processing, commit c3928b467 modified nodeForeignscan.c, but the assumption made there that ExecForeignScan() should never be called for such ForeignScan nodes during EvalPlanQual processing turned out to be wrong in some cases, leading to a segmentation fault or a "cannot re-evaluate a Foreign Update or Delete during EvalPlanQual" error. Fix by modifying nodeForeignscan.c further to avoid re-evaluating such ForeignScan nodes even in ExecForeignScan()/ExecReScanForeignScan() during EvalPlanQual processing. Since this makes non-reachable the test-and-elog added to ForeignNext() by commit c3928b467 that produced the aforesaid error, convert the test-and-elog to an Assert. Per bug #17355 from Alexander Lakhin. Back-patch to v14 where both commits came in. Patch by me, reviewed and tested by Alexander Lakhin and Amit Langote. Discussion: https://postgr.es/m/17355-de8e362eb7001a96@postgresql.org
* Remove configure's check for rl_completion_append_character.Tom Lane2022-02-02
| | | | | | | | | | | | | | | The comment for PGAC_READLINE_VARIABLES says "Readline versions < 2.1 don't have rl_completion_append_character". It seems certain that such versions are extinct in the wild, though; for sure there are none in the buildfarm. Libedit has had this variable for at least twenty years too. Also, tab-complete.c's behavior without it is quite unfriendly, since we'll emit a space even when completion fails; but we've had no complaints about that. Therefore, let's assume this variable is always there, and drop the configure check to save a few build cycles. Discussion: https://postgr.es/m/147685.1643858911@sss.pgh.pa.us
* windows: Improve crash / assert / exception handling.Andres Freund2022-02-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | startup_hacks() called SetErrorMode() with the SEM_NOGPFAULTERRORBOX argument to prevent GUI popups on error. While that likely was sufficient at some point, there are other sources of error popups. At the same time SEM_NOGPFAULTERRORBOX unfortunately also prevents "just-in-time debuggers" from working reliably, i.e. the ability to attach to a process on crash. This prevents collecting crash dumps as part of CI. The error popups are particularly problematic when they occur during automated testing, as they can cause the tests to hang, waiting for a button to be clicked. This commit improves the error handling setup in startup_hacks() to address those problems. SEM_NOGPFAULTERRORBOX is not used anymore, instead various other APIs are used to disable popups and to redirect output to stderr where possible. While this improves the situation for postgres.exe, it doesn't address similar issues in all the other executables. There currently is no codepath that's called early on for all frontend programs. I've tested that this prevents GUI popups and allows JIT debugging in case of crashes due to: - abort() - assert() - C runtime errors - unhandled exceptions both in debug and non-debug mode, on Win10 with MSVC 2019 and with MinGW. Now that crash reports are generated on windows, collect them in windows CI. Discussion: https://postgr.es/m/20211005193033.tg4pqswgvu3hcolm@alap3.anarazel.de
* Improve psql tab-completion tests.Tom Lane2022-02-02
| | | | | | | | | | | | | | Fix up recently-added test cases in 010_tab_completion.pl so that they pass with the rather seriously broken libedit found in Debian 10 (Buster). Also, add a few more test cases to improve code coverage. The total line coverage still looks pretty awful, because we exercise only a few paths of the giant if-else chain in psql_completion(). However, this now covers almost all of the code that isn't in one of those if-blocks. Discussion: https://postgr.es/m/960764.1643751011@sss.pgh.pa.us
* Fix server crash bug in 'server' backup target.Robert Haas2022-02-02
| | | | | | | | | | When this code executed as superuser it appeared to work because no system catalog lookups happened, but otherwise it crashes because there is no transaction environment. Fix that. Report and code change by me. Test case by Dagfinn Ilmari Mannsåker. Discussion: http://postgr.es/m/CA+TgmobiKLXne-2AVzYyWRiO8=rChBQ=7ywoxp=2SmcFw=oDDw@mail.gmail.com
* Some cleanup for change of collate and ctype fields to type textPeter Eisentraut2022-02-02
| | | | | | | Some cleanup for commit 54637508f87bd5f07fb9406bac6b08240283be3b: Reformat pg_database.dat to reflect the new field order. Also update the corresponding example in bki.sgml. Reorder the way the fields are filled in dbcommands.c to correspond to the new order.
* Fix recovery conflict in 027_stream_regress.pl.Thomas Munro2022-02-02
| | | | | | | | | | | | | To avoid "ERROR: canceling statement due to conflict with recovery", as seen on a couple of slower build farm animals, crank max_standby_streaming_delay right up. In passing, adjust a configuration option that accidentally used a non-standard format (not a problem, but needlessly inconsistent). Reviewed-by: Andres Freund <andres@anarazel.de> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CA%2BhUKGK65xVqNgsSPyrr2LEwtfUN%3DGfEuQ868hTC-mu0bFG42A%40mail.gmail.com
* Treat case of tab-completion keywords a bit more carefully.Tom Lane2022-02-01
| | | | | | | | | | | | | | | | | | | When completing keywords that are offered alongside names obtained from a query, preserve the user's choice of keyword case. This would have been messy to do before 02b8048ba, but now it's fairly simple. A complication is that we want keywords to be shown in upper case in any tab-completion menus that include both keywords and non-keywords, so we can't switch their case until enough has been typed that only keyword(s) remain to be chosen. Also, adjust some places where 02b8048ba thoughtlessly held over a previous choice to display keywords in lower case. (I think I got confused as to whether those words were keywords or variable names, but they're the former.) Dagfinn Ilmari Mannsåker and Tom Lane Discussion: https://postgr.es/m/8735l41ynm.fsf@wibble.ilmari.org
* Fix missing undefine in sort_template.hJohn Naylor2022-01-31
| | | | | | | | | | | All parameter macros are supposed to be undefined at the end of the header. ST_CHECK_FOR_INTERRUPTS was forgotten, so could affect later inclusions. Thomas Munro The patch set of which this is a part is discussed in https://www.postgresql.org/message-id/CA%2BhUKGLPommgNw-SVwUGkw1YmTDwmJ5vSKO0kFnZfbRHtNFW5w%40mail.gmail.com
* Simplify coding around path_contains_parent_reference().Tom Lane2022-01-31
| | | | | | | | | | | | | | | | | | | | | | Given the existing stipulation that path_contains_parent_reference() must only be invoked on canonicalized paths, we can simplify things in the wake of commit c10f830c5. It is now only possible to see ".." at the start of a relative path. That means we can simplify path_contains_parent_reference() itself quite a bit, and it makes the two existing outside call sites dead code, since they'd already checked that the path is absolute. We could now fold path_contains_parent_reference() into its only remaining caller path_is_relative_and_below_cwd(). But it seems better to leave it as a separately callable function, in case any extensions are using it. Also document the pre-existing requirement for path_is_relative_and_below_cwd's input to be likewise canonicalized. Shenhao Wang and Tom Lane Discussion: https://postgr.es/m/OSBPR01MB4214FA221FFE046F11F2AD74F2D49@OSBPR01MB4214.jpnprd01.prod.outlook.com
* Make canonicalize_path() more canonical.Tom Lane2022-01-31
| | | | | | | | | | | | | | | | | | | | | | | | | | Teach canonicalize_path() how to strip all unnecessary uses of "." and "..", replacing the previous ad-hoc code that got rid of only some such cases. In particular, we can always remove all such uses from absolute paths. The proximate reason to do this is that Windows rejects paths involving ".." in some cases (in particular, you can't put one in a symlink), so we ought to be sure we don't use ".." unnecessarily. Moreover, it seems like good cleanup on general principles. There is other path-munging code that could be simplified now, but we'll leave that for followup work. It is tempting to call this a bug fix and back-patch it. On the other hand, the misbehavior can only be reached if a highly privileged user does something dubious, so it's not unreasonable to say "so don't do that". And this patch could result in unexpected behavioral changes, in case anybody was expecting uses of ".." to stay put. So at least for now, just put it in HEAD. Shenhao Wang, editorialized a bit by me Discussion: https://postgr.es/m/OSBPR01MB4214FA221FFE046F11F2AD74F2D49@OSBPR01MB4214.jpnprd01.prod.outlook.com