- 15 Jun, 2021 10 commits
-
-
Andrew Dunstan authored
TestLib::perl2host can take a file argument as well as a directory argument, so that code becomes substantially simpler. Also add comments on why we're using forward slashes, and why we're setting PERL_BADLANG=0. Discussion: https://postgr.es/m/e9947bcd-20ee-027c-f0fe-01f736b7e345@dunslane.net
-
Alexander Korotkov authored
Reported-by: Tom Lane Discussion: https://postgr.es/m/CAPpHfdvcnw3x7jdV3r52p4%3D5S4WUxBCzcQKB3JukQHoicv1LSQ%40mail.gmail.com
-
Peter Geoghegan authored
Bugfix commit 5fc89376 effectively made the lock_waiter_detected field from vacuumlazy.c's global state struct into private state owned by lazy_truncate_heap(). Finish this off by replacing the struct field with a local variable.
-
Alexander Korotkov authored
Discussion: https://postgr.es/m/CAFj8pRDioOxiJgmgw9TqQqZ3CxnJC4P5B2Oospf2eMgAjJuewA%40mail.gmail.com Author: Pavel Stehule, Alexander Korotkov Reviewed-by: Justin Pryzby, Tom Lane
-
Alexander Korotkov authored
It has been spotted that multiranges lack of ability to decompose them into individual ranges. Subscription and proper expanded object representation require substantial work, and it's too late for v14. This commit provides the implementation of unnest(multirange) and cast multirange as an array of ranges, which is quite trivial. unnest(multirange) is defined as a polymorphic procedure. The catalog description of the cast underlying procedure is duplicated for each multirange type because we don't have anyrangearray polymorphic type to use here. Catversion is bumped. Reported-by: Jonathan S. Katz Discussion: https://postgr.es/m/flat/60258efe-bd7e-4886-82e1-196e0cac5433%40postgresql.org Author: Alexander Korotkov Reviewed-by: Justin Pryzby, Jonathan S. Katz, Zhihong Yu
-
Amit Kapila authored
During decoding for speculative inserts, we were relying for cleaning toast hash on confirmation records or next change records. But that could lead to multiple problems (a) memory leak if there is neither a confirmation record nor any other record after toast insertion for a speculative insert in the transaction, (b) error and assertion failures if the next operation is not an insert/update on the same table. The fix is to start queuing spec abort change and clean up toast hash and change record during its processing. Currently, we are queuing the spec aborts for both toast and main table even though we perform cleanup while processing the main table's spec abort record. Later, if we have a way to distinguish between the spec abort record of toast and the main table, we can avoid queuing the change for spec aborts of toast tables. Reported-by: Ashutosh Bapat Author: Dilip Kumar Reviewed-by: Amit Kapila Backpatch-through: 9.6, where it was introduced Discussion: https://postgr.es/m/CAExHW5sPKF-Oovx_qZe4p5oM6Dvof7_P+XgsNAViug15Fm99jA@mail.gmail.com
-
Tom Lane authored
This should have been updated in d2d8a229, but it was overlooked. According to 31a877f1 which added it, this file is meant to show the results you get under default_transaction_isolation = serializable. We've largely lost track of that goal in other isolation tests, but as long as we've got this one, it should be right. Noted while fooling about with the isolationtester.
-
Noah Misch authored
It was unable to wait on a backend that had already left the procarray. Users tolerant of that limitation can poll pg_stat_activity. Other users can employ the "timeout" argument of pg_terminate_backend(). Reviewed by Bharath Rupireddy. Discussion: https://postgr.es/m/20210605013236.GA208701@rfd.leadboat.com
-
Noah Misch authored
Revert the pg_description entry to its v13 form, since those messages usually remain shorter and don't discuss individual parameters. No catversion bump, since pg_description content does not impair backend compatibility or application compatibility. Justin Pryzby Discussion: https://postgr.es/m/20210612182743.GY16435@telsasoft.com
-
- 14 Jun, 2021 6 commits
-
-
Alvaro Herrera authored
I overlooked that one condition was logically inverted. The fix is a little bit more involved than simply negating the condition, to make the code easier to read. Fix some outdated comments left by the same commit, while at it. Author: Masahiko Sawada <sawada.mshk@gmail.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://postgr.es/m/YMRlmB3/lZw8YBH+@paquier.xyz
-
Bruce Momjian authored
Items related to logical replication attribution and BRIN indexes Reported-by: Tomas Vondra, John Naylor Discussion: https://postgr.es/m/0db66294-a668-2caa-2b5e-a8db60b30662@enterprisedb.com, CAFBsxsH21KnteYdk33F1oZu2O726NSD6_XBq51Tn0jytsA1AnA@mail.gmail.com
-
Bruce Momjian authored
Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20210612034551.GU16435@telsasoft.com
-
Bruce Momjian authored
User-defined objects that reference some built-in array functions will need to be recreated in PG 14. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20210608225618.GR16435@telsasoft.com
-
Michael Paquier authored
An object found as dropped when digging into the list of objects returned by pg_event_trigger_ddl_commands() could cause a cache lookup error, as the calls grabbing for the object address and the type name would fail if the object was missing. Those lookup errors could be seen with combinations of ALTER TABLE sub-commands involving identity columns. The lookup logic is changed in this code path to get a behavior similar to any other SQL-callable function by ignoring objects that are not found, taking advantage of 2a10fdc4. The back-branches are not changed, as they require this commit that is too invasive for stable branches. While on it, add test cases to exercise event triggers with identity columns, and stress more cases with the event ddl_command_end for relations. Author: Sven Klemm, Aleksander Alekseev, Michael Paquier Discussion: https://postgr.es/m/CAMCrgp2R1cEXU53iYKtW6yVEp2_yKUz+z=3-CTrYpPP+xryRtg@mail.gmail.com
-
Michael Paquier authored
The extra checks added by the recompression of toast data introduced in bbe0a81d is proving to have a performance impact on VACUUM or CLUSTER even if no recompression is done. This is more noticeable with more toastable columns that contain non-NULL values. Improvements could be done to make those extra checks less expensive, but that's not material for 14 at this stage, and we are not sure either if the code path of VACUUM FULL/CLUSTER is adapted for this job. Per discussion with several people, including Andres Freund, Robert Haas, Álvaro Herrera, Tom Lane and myself. Discussion: https://postgr.es/m/20210527003144.xxqppojoiwurc2iz@alap3.anarazel.de
-
- 13 Jun, 2021 3 commits
-
-
Tom Lane authored
Recent glibc versions have made mktime() fail if tm_isdst is inconsistent with the prevailing timezone; in particular it fails for tm_isdst = 1 when the zone is UTC. (This seems wildly inconsistent with the POSIX-mandated treatment of "incorrect" values for the other fields of struct tm, so if you ask me it's a bug, but I bet they'll say it's intentional.) This has been observed to cause cosmetic problems when pg_restore'ing an archive created in a different timezone. To fix, do mktime() using the field values from the archive, and if that fails try again with tm_isdst = -1. This will give a result that's off by the UTC-offset difference from the original zone, but that was true before, too. It's not terribly critical since we don't do anything with the result except possibly print it. (Someday we should flush this entire bit of logic and record a standard-format timestamp in the archive instead. That's not okay for a back-patched bug fix, though.) Also, guard our only other use of mktime() by having initdb's build_time_t() set tm_isdst = -1 not 0. This case could only have an issue in zones that are DST year-round; but I think some do exist, or could in future. Per report from Wells Oliver. Back-patch to all supported versions, since any of them might need to run with a newer glibc. Discussion: https://postgr.es/m/CAOC+FBWDhDHO7G-i1_n_hjRzCnUeFO+H-Czi1y10mFhRWpBrew@mail.gmail.com
-
Andrew Dunstan authored
Translate path slashes on target directory path. This was confusing old branches, but is applied to all branches for the sake of uniformity. Perl is perfectly able to understand paths with forward slashes. Along the way, restore the previous archive_wait query, for the sake of uniformity with other tests, per gripe from Tom Lane.
-
Michael Paquier authored
This is similar to the work done in 8279f68a for TestLib.pm, where environment variables set may cause unwanted failures if using a temporary installation with pg_regress. The list of variables reset is adjusted in each stable branch depending on what is supported. Comments are added to remember that the lists in TestLib.pm and pg_regress.c had better be kept in sync. Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/YMNR9GYDn+fHlMta@paquier.xyz Backpatch-through: 9.6
-
- 12 Jun, 2021 8 commits
-
-
Tom Lane authored
Several TAP tests use poll_query_until() to wait for the postmaster to restart. They were checking to see if a trivial query (e.g. "SELECT 1") succeeds. However, that's problematic in the wake of commit 11e9caff, because now that we feed said query to psql via stdin, we risk IPC::Run whining about a SIGPIPE failure if psql quits before reading the query. Hence, we can't use a nonempty query in cases where we need to wait for connection failures to stop happening. Per the precedent of commits c757a3da and 6d41dd04, we can pass "undef" as the query in such cases to ensure that IPC::Run has nothing to write. However, then we have to say that the expected output is empty, and this exposes a deficiency in poll_query_until: if psql fails altogether and returns empty stdout, poll_query_until will treat that as a success! That's because, contrary to its documentation, it makes no actual check for psql failure, looking neither at the exit status nor at stderr. To fix that, adjust poll_query_until to insist on empty stderr as well as a stdout match. (I experimented with checking exit status instead, but it seems that psql often does exit(1) in cases that we need to consider successes. That might be something to fix someday, but it would be a non-back-patchable behavior change.) Back-patch to v10. The test cases needing this exist only as far back as v11, but it seems wise to keep poll_query_until's behavior the same in v10, in case we back-patch another such test case in future. (9.6 does not currently need this change, because in that branch poll_query_until can't be told to accept empty stdout as a success case.) Per assorted buildfarm failures, mostly on hoverfly. Discussion: https://postgr.es/m/CAA4eK1+zM6L4QSA1XMvXY_qqWwdUmqkOS1+hWvL8QcYEBGA1Uw@mail.gmail.com
-
Tom Lane authored
Previously, a zero value for the relfilenode resulted in a confusing error message about "unexpected duplicate". This function returns NULL for other invalid relfilenode values, so zero should be treated likewise. It's been like this all along, so back-patch to all supported branches. Justin Pryzby Discussion: https://postgr.es/m/20210612023324.GT16435@telsasoft.com
-
Tom Lane authored
Using an Assert to check the validity of incoming messages is an extremely poor decision. In a debug build, it should not be that easy for a broken or malicious remote client to crash the logrep worker. The consequences could be even worse in non-debug builds, which will fail to make such checks at all, leading to who-knows-what misbehavior. Hence, promote every Assert that could possibly be triggered by wrong or out-of-order replication messages to a full test-and-ereport. To avoid bloating the set of messages the translation team has to cope with, establish a policy that replication protocol violation error reports don't need to be translated. Hence, all the new messages here use errmsg_internal(). A couple of old messages are changed likewise for consistency. Along the way, fix some non-idiomatic or outright wrong uses of hash_search(). Most of these mistakes are new with the "streaming replication" patch (commit 46482432), but a couple go back a long way. Back-patch as appropriate. Discussion: https://postgr.es/m/1719083.1623351052@sss.pgh.pa.us
-
Andrew Dunstan authored
Commit caba8f0d wasn't quite right for msys, as demonstrated by several buildfarm animals, including jacana and fairywren. We need to use the msys perl in the archive command, but call it in such a way that Windows will understand the path. Furthermore, inside the copy script we need to convert a Windows path to an msys path.
-
Michael Paquier authored
This routine is designed to never return an empty description or NULL, providing description fallbacks even if missing objects are accepted, but it included a code path where this was considered possible. All the callers of this routine already consider NULL as not possible, so change a bit the code to map with the assumptions of the callers, and add more comments close to the callers of this routine to outline the behavior expected. This code is new as of 2a10fdc4, so no backpatch is needed. Discussion: https://postgr.es/m/YMNY6RGPBRCeLmFb@paquier.xyz
-
Michael Paquier authored
ab55d742 has introduced some tests with rows found as missing in logical replication subscriptions for partitioned tables, relying on a logic with a lookup of the logs generated, scanning the whole file. This commit makes the logic more precise, by scanning the logs only from the position before the key queries are run to the position where we check for the logs. This will reduce the risk of issues with log patterns overlapping with each other if those tests get more complicated in the future. Per discussion with Tom Lane. Discussion: https://postgr.es/m/YMP+Gx2S8meYYHW4@paquier.xyz Backpatch-through: 13
-
Andres Freund authored
941697c3 changed ProcArrayAdd()/Remove() substantially. As reported by zhanyi, I missed that due to the introduction of the PGPROC->pgxactoff ProcArrayRemove() does not need to search for the position in procArray->pgprocnos anymore - that's pgxactoff. Remove the search loop. The removal of the search loop reduces assertion coverage a bit - but we can easily do better than before by adding assertions to other loops over pgprocnos elements. Also do a bit of cleanup, mostly by reducing line lengths by introducing local helper variables and adding newlines. Author: zhanyi <w@hidva.com> Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/tencent_5624AA3B116B3D1C31CA9744@qq.com
-
Bruce Momjian authored
Reported-by: Justin Pryzby
-
- 11 Jun, 2021 11 commits
-
-
Bruce Momjian authored
-
Alvaro Herrera authored
We were already reporting it, but only after the parallel workers were finished, which is visibly much later than what happens in a serial build. With this change we report it when the leader starts its own sort phase when participating in the build (the normal case). Now this might happen a little later than when the workers start their sorting phases, but a) communicating the actual phase start from workers is likely to be a hassle, and b) the sort phase start is pretty fuzzy anyway, since sorting per se is actually initiated by tuplesort.c internally earlier than tuplesort_performsort() is called. Backpatch to pg12, where the progress reporting code for CREATE INDEX went in. Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Author: Matthias van de Meent <boekewurm+postgres@gmail.com> Reviewed-by: Greg Nancarrow <gregn4422@gmail.com> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/1128176d-1eee-55d4-37ca-e63644422adb
-
Tom Lane authored
apply_handle_tuple_routing(), having detected and reported that the tuple it needed to update didn't exist, tried to update that tuple anyway, leading to a null-pointer dereference. logicalrep_partition_open() failed to ensure that the LogicalRepPartMapEntry it built for a partition was fully independent of that for the partition root, leading to trouble if the root entry was later freed or rebuilt. Meanwhile, on the publisher's side, pgoutput_change() sometimes attempted to apply execute_attr_map_tuple() to a NULL tuple. The first of these was reported by Sergey Bernikov in bug #17055; I found the other two while developing some test cases for this sadly under-tested code. Diagnosis and patch for the first issue by Amit Langote; patches for the others by me; new test cases by me. Back-patch to v13 where this logic came in. Discussion: https://postgr.es/m/17055-9ba800ec8522668b@postgresql.org
-
Alvaro Herrera authored
Commit acb7e4eb added a new implementation for PQsendQuery so that it works in pipeline mode (by using extended query protocol), but it behaves differently from the 'Q' message (in simple query protocol) used by regular implementation: the new one doesn't close the unnamed portal. Change the new code to have identical behavior to the old. Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202106072107.d4i55hdscxqj@alvherre.pgsql
-
Alvaro Herrera authored
Per 96540f80; the awkward API introduced by c6550776 is no longer needed. Author: Andres Freund <andres@anarazel.de> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20210408020913.zzprrlvqyvlt5cyy@alap3.anarazel.de
-
Tomas Vondra authored
Commit b663a413 introduced bulk inserts for FDW, but the handling of tuple slots turned out to be problematic for two reasons. Firstly, the slots were re-created for each individual batch. Secondly, all slots referenced the same tuple descriptor - with reasonably small batches this is not an issue, but with large batches this triggers O(N^2) behavior in the resource owner code. These two issues work against each other - to reduce the number of times a slot has to be created/dropped, larger batches are needed. However, the larger the batch, the more expensive the resource owner gets. For practical batch sizes (100 - 1000) this would not be a big problem, as the benefits (latency savings) greatly exceed the resource owner costs. But for extremely large batches it might be much worse, possibly even losing with non-batching mode. Fixed by initializing tuple slots only once (and reusing them across batches) and by using a new tuple descriptor copy for each slot. Discussion: https://postgr.es/m/ebbbcc7d-4286-8c28-0272-61b4753af761%40enterprisedb.com
-
Alvaro Herrera authored
The code added to mark replication slots invalid in commit c6550776 had the race condition that a slot can be dropped or advanced concurrently with checkpointer trying to invalidate it. Rewrite the code to close those races. The changes to ReplicationSlotAcquire's API added with c6550776 are not necessary anymore. To avoid an ABI break in released branches, this commit leaves that unchanged; it'll be changed in a master-only commit separately. Backpatch to 13, where this code first appeared. Reported-by: Andres Freund <andres@anarazel.de> Author: Andres Freund <andres@anarazel.de> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/20210408001037.wfmk6jud36auhfqm@alap3.anarazel.de
-
Michael Paquier authored
The list of options provided by the tab completion was outdated for the following commands: - ALTER SUBSCRIPTION - CREATE SUBSCRIPTION - ALTER PUBLICATION - CREATE PUBLICATION Author: Vignesh C Reviewed-by: Bharath Rupireddy Discussion: https://postgr.es/m/CALDaNm18oHDFu6SFCHE=ZbiO153Fx7E-L1MG0YyScbaDV--U+A@mail.gmail.com
-
Noah Misch authored
Resolve the disagreement with nodes/*funcs.c field order in favor of the latter, which is better-aligned with the IndexStmt field order. This field is new in v14. Discussion: https://postgr.es/m/20210611045546.GA573364@rfd.leadboat.com
-
Noah Misch authored
We have a dozen PQset*() functions. PQresultSetInstanceData() and this were the libpq setter functions having a different word order. Adopt the majority word order. Reviewed by Alvaro Herrera and Robert Haas, though this choice of name was not unanimous. Discussion: https://postgr.es/m/20210605060555.GA216695@rfd.leadboat.com
-
David Rowley authored
We've accumulated quite a mix of instances of "an SQL" and "a SQL" in the documents. It would be good to be a bit more consistent with these. The most recent version of the SQL standard I looked at seems to prefer "an SQL". That seems like a good lead to follow, so here we change all instances of "a SQL" to become "an SQL". Most instances correctly use "an SQL" already, so it also makes sense to use the dominant variation in order to minimise churn. Additionally, there were some other abbreviations that needed to be adjusted. FSM, SSPI, SRF and a few others. Also fix some pronounceable, abbreviations to use "a" instead of "an". For example, "a SASL" instead of "an SASL". Here I've only adjusted the documents and error messages. Many others still exist in source code comments. Translator hint comments seem to be the biggest culprit. It currently does not seem worth the churn to change these. Discussion: https://postgr.es/m/CAApHDvpML27UqFXnrYO1MJddsKVMQoiZisPvsAGhKE_tsKXquw%40mail.gmail.com
-
- 10 Jun, 2021 2 commits
-
-
Tom Lane authored
Commit 2453ea14 redefined pg_proc.proargtypes to include the types of OUT parameters, for procedures only. While that had some advantages for implementing the SQL-spec behavior of DROP PROCEDURE, it was pretty disastrous from a number of other perspectives. Notably, since the primary key of pg_proc is name + proargtypes, this made it possible to have multiple procedures with identical names + input arguments and differing output argument types. That would make it impossible to call any one of the procedures by writing just NULL (or "?", or any other data-type-free notation) for the output argument(s). The change also seems likely to cause grave confusion for client applications that examine pg_proc and expect the traditional definition of proargtypes. Hence, revert the definition of proargtypes to what it was, and undo a number of complications that had been added to support that. To support the SQL-spec behavior of DROP PROCEDURE, when there are no argmode markers in the command's parameter list, we perform the lookup both ways (that is, matching against both proargtypes and proallargtypes), succeeding if we get just one unique match. In principle this could result in ambiguous-function failures that would not happen when using only one of the two rules. However, overloading of procedure names is thought to be a pretty rare usage, so this shouldn't cause many problems in practice. Postgres-specific code such as pg_dump can defend against any possibility of such failures by being careful to specify argmodes for all procedure arguments. This also fixes a few other bugs in the area of CALL statements with named parameters, and improves the documentation a little. catversion bump forced because the representation of procedures with OUT arguments changes. Discussion: https://postgr.es/m/3742981.1621533210@sss.pgh.pa.us
-
Tom Lane authored
It turns out that worker.c's code path for TRUNCATE was also careless about establishing a snapshot while executing user-defined code, allowing the checks added by commit 84f5c290 to fail when a trigger is fired in that context. We could just wrap Push/PopActiveSnapshot around the truncate call, but it seems better to establish a policy of holding a snapshot throughout execution of a replication step. To help with that and possible future requirements, replace the previous ensure_transaction calls with pairs of begin/end_replication_step calls. Per report from Mark Dilger. Back-patch to v11, like the previous changes. Discussion: https://postgr.es/m/B4A3AF82-79ED-4F4C-A4E5-CD2622098972@enterprisedb.com
-