summaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2021-07-09Reject cases where a query in WITH rewrites to just NOTIFY.Tom Lane
Since the executor can't cope with a utility statement appearing as a node of a plan tree, we can't support cases where a rewrite rule inserts a NOTIFY into an INSERT/UPDATE/DELETE command appearing in a WITH clause of a larger query. (One can imagine ways around that, but it'd be a new feature not a bug fix, and so far there's been no demand for it.) RewriteQuery checked for this, but it missed the case where the DML command rewrites to *only* a NOTIFY. That'd lead to crashes later on in planning. Add the missed check, and improve the level of testing of this area. Per bug #17094 from Yaoguang Chen. It's been busted since WITH was introduced, so back-patch to all supported branches. Discussion: https://postgr.es/m/17094-bf15dff55eaf2e28@postgresql.org
2021-07-09Teach pg_size_pretty and pg_size_bytes about petabytesDavid Rowley
There was talk about adding units all the way up to yottabytes but it seems quite far-fetched that anyone would need those. Since such large units are not exactly commonplace, it seems unlikely that having pg_size_pretty outputting unit any larger than petabytes would actually be helpful to anyone. Since petabytes are on the horizon, let's just add those only. Maybe one day we'll get to add additional units, but it will likely be a while before we'll need to think beyond petabytes in regards to the size of a database. Author: David Christensen Discussion: https://postgr.es/m/CAOxo6XKmHc_WZip-x5QwaOqFEiCq_SVD0B7sbTZQk+qqcn2qaw@mail.gmail.com
2021-07-09Fix incorrect return value in pg_size_pretty(bigint)David Rowley
Due to how pg_size_pretty(bigint) was implemented, it's possible that when given a negative number of bytes that the returning value would not match the equivalent positive return value when given the equivalent positive number of bytes. This was due to two separate issues. 1. The function used bit shifting to convert the number of bytes into larger units. The rounding performed by bit shifting is not the same as dividing. For example -3 >> 1 = -2, but -3 / 2 = -1. These two operations are only equivalent with positive numbers. 2. The half_rounded() macro rounded towards positive infinity. This meant that negative numbers rounded towards zero and positive numbers rounded away from zero. Here we fix #1 by dividing the values instead of bit shifting. We fix #2 by adjusting the half_rounded macro always to round away from zero. Additionally, adjust the pg_size_pretty(numeric) function to be more explicit that it's using division rather than bit shifting. A casual observer might have believed bit shifting was used due to a static function being named numeric_shift_right. However, that function was calculating the divisor from the number of bits and performed division. Here we make that more clear. This change is just cosmetic and does not affect the return value of the numeric version of the function. Here we also add a set of regression tests both versions of pg_size_pretty() which test the values directly before and after the function switches to the next unit. This bug was introduced in 8a1fab36a. Prior to that negative values were always displayed in bytes. Author: Dean Rasheed, David Rowley Discussion: https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.com Backpatch-through: 9.6, where the bug was introduced.
2021-07-08Improve error messages about mismatching relkindPeter Eisentraut
Most error messages about a relkind that was not supported or appropriate for the command was of the pattern "relation \"%s\" is not a table, foreign table, or materialized view" This style can become verbose and tedious to maintain. Moreover, it's not very helpful: If I'm trying to create a comment on a TOAST table, which is not supported, then the information that I could have created a comment on a materialized view is pointless. Instead, write the primary error message shorter and saying more directly that what was attempted is not possible. Then, in the detail message, explain that the operation is not supported for the relkind the object was. To simplify that, add a new function errdetail_relkind_not_supported() that does this. In passing, make use of RELKIND_HAS_STORAGE() where appropriate, instead of listing out the relkinds individually. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/dc35a398-37d0-75ce-07ea-1dd71d98f8ec@2ndquadrant.com
2021-07-07Use a hash table to speed up NOT IN(values)David Rowley
Similar to 50e17ad28, which allowed hash tables to be used for IN clauses with a set of constants, here we add the same feature for NOT IN clauses. NOT IN evaluates the same as: WHERE a <> v1 AND a <> v2 AND a <> v3. Obviously, if we're using a hash table we must be exactly equivalent to that and return the same result taking into account that either side of the condition could contain a NULL. This requires a little bit of special handling to make work with the hash table version. When processing NOT IN, the ScalarArrayOpExpr's operator will be the <> operator. To be able to build and lookup a hash table we must use the <>'s negator operator. The planner checks if that exists and is hashable and sets the relevant fields in ScalarArrayOpExpr to instruct the executor to use hashing. Author: David Rowley, James Coleman Reviewed-by: James Coleman, Zhihong Yu Discussion: https://postgr.es/m/CAApHDvoF1mum_FRk6D621edcB6KSHBi2+GAgWmioj5AhOu2vwQ@mail.gmail.com
2021-07-06Improve TestLib::system_or_bail error reportingAlvaro Herrera
The original coding was not quoting the complete failing command, and it wasn't printing the reason for the failure either. Do both. This is cosmetic only, so no backpatch. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Discussion: https://postgr.es/m/202106301524.eq5pblzstapj@alvherre.pgsql
2021-07-05Rethink blocking annotations in detach-partition-concurrently-[34].Tom Lane
In 741d7f104, I tried to make the reports from canceled steps come out after the pg_cancel_backend() steps, since that was the most common ordering before. However, that doesn't ensure that a canceled step doesn't report even later, as shown in a recent failure on buildfarm member idiacanthus. Rather than complicating things even more with additional annotations, let's just force the cancel's effect to be reported first. It's not *that* unnatural-looking. Back-patch to v14 where these test cases appeared. Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus&dt=2021-07-02%2001%3A40%3A04
2021-07-05Prevent numeric overflows in parallel numeric aggregates.Dean Rasheed
Formerly various numeric aggregate functions supported parallel aggregation by having each worker convert partial aggregate values to Numeric and use numeric_send() as part of serializing their state. That's problematic, since the range of Numeric is smaller than that of NumericVar, so it's possible for it to overflow (on either side of the decimal point) in cases that would succeed in non-parallel mode. Fix by serializing NumericVars instead, to avoid the overflow risk and ensure that parallel and non-parallel modes work the same. A side benefit is that this improves the efficiency of the serialization/deserialization code, which can make a noticeable difference to performance with large numbers of parallel workers. No back-patch due to risk from changing the binary format of the aggregate serialization states, as well as lack of prior field complaints and low probability of such overflows in practice. Patch by me. Thanks to David Rowley for review and performance testing, and Ranier Vilela for an additional suggestion. Discussion: https://postgr.es/m/CAEZATCUmeFWCrq2dNzZpRj5+6LfN85jYiDoqm+ucSXhb9U2TbA@mail.gmail.com
2021-07-01Don't reset relhasindex for partitioned tables on ANALYZEAlvaro Herrera
Commit 0e69f705cc1a introduced code to analyze partitioned table; however, that code fails to preserve pg_class.relhasindex correctly. Fix by observing whether any indexes exist rather than accidentally falling through to assuming none do. Backpatch to 14. Author: Alexander Pyhalov <a.pyhalov@postgrespro.ru> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Zhihong Yu <zyu@yugabyte.com> Discussion: https://postgr.es/m/CALNJ-vS1R3Qoe5t4tbzxrkpBtzRbPq1dDcW4RmA_a+oqweF30w@mail.gmail.com
2021-07-01Fix prove_installcheck to use correct paths when used with PGXSAndrew Dunstan
The prove_installcheck recipe in src/Makefile.global.in was emitting bogus paths for a couple of elements when used with PGXS. Here we create a separate recipe for the PGXS case that does it correctly. We also take the opportunity to make the make the file more readable by breaking up the prove_installcheck and prove_check recipes across several lines, and to remove the setting for REGRESS_SHLIB to src/test/recovery/Makefile, which is the only set of tests that actually need it. Backpatch to all live branches Discussion: https://postgr.es/m/f2401388-936b-f4ef-a07c-a0bcc49b3300@dunslane.net
2021-07-01Add tests for UNBOUNDED syntax ambiguityPeter Eisentraut
There is a syntactic ambiguity in the SQL standard. Since UNBOUNDED is a non-reserved word, it could be the name of a function parameter and be used as an expression. There is a grammar hack to resolve such cases as the keyword. Add some tests to record this behavior. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://www.postgresql.org/message-id/flat/b2a09a77-3c8f-7c68-c9b7-824054f87d98%40enterprisedb.com
2021-06-29Fixes for multirange selectivity estimationAlexander Korotkov
* Fix enumeration of the multirange operators in calc_multirangesel() and calc_multirangesel() switches. * Add more regression tests for matching to empty ranges/multiranges. Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/c5269c65-f967-77c5-ff7c-15e621c47f6a%40gmail.com Author: Alexander Korotkov Backpatch-through: 14, where multiranges were introduced
2021-06-29Fix libpq state machine in pipeline modeAlvaro Herrera
The original coding required that PQpipelineSync had been called before the first call to PQgetResult, and failure to do that would result in an unexpected NULL result being returned. Fix by setting the right state when a query is sent, rather than leaving it unchanged and having PQpipelineSync apply the necessary state change. A new test case to verify the behavior is added, which relies on the new PQsendFlushRequest() function added by commit a7192326c74d. Backpatch to 14, where pipeline mode was added. Reported-by: Boris Kolpackov <boris@codesynthesis.com> Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/boris.20210616110321@codesynthesis.com
2021-06-29Fix bogus logic for reporting which hash partition conflicts.Tom Lane
Commit efbfb6424 added logic for reporting exactly which existing partition conflicts when complaining that a new hash partition's modulus isn't compatible with the existing ones. However, it misunderstood the partitioning data structure, and would select the wrong partition in some cases, or crash outright due to fetching a bogus table OID in other cases. Per bug #17076 from Alexander Lakhin. Fix by Amit Langote; some further work on the code comments by me. Discussion: https://postgr.es/m/17076-89a16ae835d329b9@postgresql.org
2021-06-28Dump public schema ownership and security labels.Noah Misch
As a side effect, this corrects dumps of public schema ACLs in databases where the DBA dropped and recreated that schema. Reviewed by Asif Rehman. Discussion: https://postgr.es/m/20201229134924.GA1431748@rfd.leadboat.com
2021-06-28Pre branch pgindent / pgperltidy runAndrew Dunstan
Along the way make a slight adjustment to src/include/utils/queryjumble.h to avoid an unused typedef.
2021-06-28Message style improvementsPeter Eisentraut
2021-06-28Add test for CREATE INDEX CONCURRENTLY with not-so-immutable predicateMichael Paquier
83158f7 has improved index_set_state_flags() so as it is possible to use transactional updates when updating pg_index state flags, but there was not really a test case which stressed directly the possibility it fixed. This commit adds such a test, using a predicate that looks valid in appearance but calls a stable function. Author: Andrey Lepikhov Discussion: https://postgr.es/m/9b905019-5297-7372-0ad2-e1a4bb66a719@postgrespro.ru Backpatch-through: 9.6
2021-06-27Remove memory leaks in isolationtester.Tom Lane
specscanner.l leaked a kilobyte of memory per token of the spec file. Apparently somebody thought that the introductory code block would be executed once; but it's once per yylex() call. A couple of functions in isolationtester.c leaked small amounts of memory due to not bothering to free one-time allocations. Might as well improve these so that valgrind gives this program a clean bill of health. Also get rid of an ugly static variable. Coverity complained about one of the one-time leaks, which led me to try valgrind'ing isolationtester, which led to discovery of the larger leak.
2021-06-25Fixes in ALTER SUBSCRIPTION DROP PUBLICATION codePeter Eisentraut
ALTER SUBSCRIPTION DROP PUBLICATION does not actually support copy_data option, so remove it from tab completion. Also, reword the error message that is thrown when all the publications from a subscription are specified to be dropped. Also, made few doc and cosmetic adjustments. Author: Vignesh C <vignesh21@gmail.com> Reviewed-by: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com> Reviewed-by: Japin Li <japinli@hotmail.com> Discussion: https://www.postgresql.org/message-id/flat/CALDaNm21RwsDzs4xj14ApteAF7auyyomHNnp+NEL-sH8m-jMvQ@mail.gmail.com
2021-06-23Allow non-quoted identifiers as isolation test session/step names.Tom Lane
For no obvious reason, isolationtester has always insisted that session and step names be written with double quotes. This is fairly tedious and does little for test readability, especially since the names that people actually choose almost always look like normal identifiers. Hence, let's tweak the lexer to allow SQL-like identifiers not only double-quoted strings. (They're SQL-like, not exactly SQL, because I didn't add any case-folding logic. Also there's no provision for U&"..." names, not that anyone's likely to care.) There is one incompatibility introduced by this change: if you write "foo""bar" with no space, that used to be taken as two identifiers, but now it's just one identifier with an embedded quote mark. I converted all the src/test/isolation/ specfiles to remove unnecessary double quotes, but stopped there because my eyes were glazing over already. Like 741d7f104, back-patch to all supported branches, so that this isn't a stumbling block for back-patching isolation test changes. Discussion: https://postgr.es/m/759113.1623861959@sss.pgh.pa.us
2021-06-23Improve display of query results in isolation tests.Tom Lane
Previously, isolationtester displayed SQL query results using some ad-hoc code that clearly hadn't had much effort expended on it. Field values longer than 14 characters weren't separated from the next field, and usually caused misalignment of the columns too. Also there was no visual separation of a query's result from subsequent isolationtester output. This made test result files confusing and hard to read. To improve matters, let's use libpq's PQprint() function. Although that's long since unused by psql, it's still plenty good enough for the purpose here. Like 741d7f104, back-patch to all supported branches, so that this isn't a stumbling block for back-patching isolation test changes. Discussion: https://postgr.es/m/582362.1623798221@sss.pgh.pa.us
2021-06-23Add test case for obsoleting slot with active walsender, take 2Alvaro Herrera
The code to signal a running walsender when its reserved WAL size grows too large is completely uncovered before this commit; this adds coverage for that case. This test involves sending SIGSTOP to walsender and walreceiver, then advancing enough WAL for a checkpoint to trigger, then sending SIGCONT. There's no precedent for STOP signalling in Perl tests, and my reading of relevant manpages says it's likely to fail on Windows. Because of this, this test is always skipped on that platform. This version fixes a couple of rarely hit race conditions in the previous attempt 09126984a263; most notably, both LOG string searches are loops, not just the second one; we acquire the start-of-log position before STOP-signalling; and reference the correct process name in the test description. All per Tom Lane. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202106102202.mjw4huiix7lo@alvherre.pgsql
2021-06-22Use annotations to reduce instability of isolation-test results.Tom Lane
We've long contended with isolation test results that aren't entirely stable. Some test scripts insert long delays to try to force stable results, which is not terribly desirable; but other erratic failure modes remain, causing unrepeatable buildfarm failures. I've spent a fair amount of time trying to solve this by improving the server-side support code, without much success: that way is fundamentally unable to cope with diffs that stem from chance ordering of arrival of messages from different server processes. We can improve matters on the client side, however, by annotating the test scripts themselves to show the desired reporting order of events that might occur in different orders. This patch adds three types of annotations to deal with (a) test steps that might or might not complete their waits before the isolationtester can see them waiting; (b) test steps in different sessions that can legitimately complete in either order; and (c) NOTIFY messages that might arrive before or after the completion of a step in another session. We might need more annotation types later, but this seems to be enough to deal with the instabilities we've seen in the buildfarm. It also lets us get rid of all the long delays that were previously used, cutting more than a minute off the runtime of the isolation tests. Back-patch to all supported branches, because the buildfarm instabilities affect all the branches, and because it seems desirable to keep isolationtester's capabilities the same across all branches to simplify possible future back-patching of tests. Discussion: https://postgr.es/m/327948.1623725828@sss.pgh.pa.us
2021-06-20Revert "Add test case for obsoleting slot with active walsender"Alvaro Herrera
This reverts commit 09126984a263; the test case added there failed once in circumstances that remain mysterious. It seems better to remove the test for now so that 14beta2 doesn't have random failures built in.
2021-06-19Handle no replica identity index case in RelationGetIdentityKeyBitmap.Amit Kapila
Commit e7eea52b2d has introduced a new function RelationGetIdentityKeyBitmap which omits to handle the case where there is no replica identity index on a relation. Author: Mark Dilger Reviewed-by: Takamichi Osumi, Amit Kapila Discussion: https://www.postgresql.org/message-id/4C99A862-69C8-431F-960A-81B1151F1B89@enterprisedb.com
2021-06-18Support disabling index bypassing by VACUUM.Peter Geoghegan
Generalize the INDEX_CLEANUP VACUUM parameter (and the corresponding reloption): make it into a ternary style boolean parameter. It now exposes a third option, "auto". The "auto" option (which is now the default) enables the "bypass index vacuuming" optimization added by commit 1e55e7d1. "VACUUM (INDEX_CLEANUP TRUE)" is redefined to once again make VACUUM simply do any required index vacuuming, regardless of how few dead tuples are encountered during the first scan of the target heap relation (unless there are exactly zero). This gives users a way of opting out of the "bypass index vacuuming" optimization, if for whatever reason that proves necessary. It is also expected to be used by PostgreSQL developers as a testing option from time to time. "VACUUM (INDEX_CLEANUP FALSE)" does the same thing as it always has: it forcibly disables both index vacuuming and index cleanup. It's not expected to be used much in PostgreSQL 14. The failsafe mechanism added by commit 1e55e7d1 addresses the same problem in a simpler way. INDEX_CLEANUP can now be thought of as a testing and compatibility option. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com> Reviewed-By: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/CAH2-WznrBoCST4_Gxh_G9hA8NzGUbeBGnOUC8FcXcrhqsv6OHQ@mail.gmail.com
2021-06-18Add test case for obsoleting slot with active walsenderAlvaro Herrera
The code to signal a running walsender when its reserved WAL size grows too large is completely uncovered before this commit; this adds coverage for that case. This test involves sending SIGSTOP to walsender and walreceiver and running a checkpoint while advancing WAL, then sending SIGCONT. There's no precedent for this coding in Perl tests, and my reading of relevant manpages says it's likely to fail on Windows. Because of this, this test is always skipped on that platform. Author: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/202106102202.mjw4huiix7lo@alvherre.pgsql
2021-06-18Fix misbehavior of DROP OWNED BY with duplicate polroles entries.Tom Lane
Ordinarily, a pg_policy.polroles array wouldn't list the same role more than once; but CREATE POLICY does not prevent that. If we perform DROP OWNED BY on a role that is listed more than once, RemoveRoleFromObjectPolicy either suffered an assertion failure or encountered a tuple-updated-by-self error. Rewrite it to cope correctly with duplicate entries, and add a CommandCounterIncrement call to prevent the other problem. Per discussion, there's other cleanup that ought to happen here, but this seems like the minimum essential fix. Per bug #17062 from Alexander Lakhin. It's been broken all along, so back-patch to all supported branches. Discussion: https://postgr.es/m/17062-11f471ae3199ca23@postgresql.org
2021-06-18Don't set a fast default for anything but a plain tableAndrew Dunstan
The fast default code added in Release 11 omitted to check that the table a fast default was being added to was a plain table. Thus one could be added to a foreign table, which predicably blows up. Here we perform that check. In addition, on the back branches, since some of these might have escaped into the wild, if we encounter a missing value for an attribute of something other than a plain table we ignore it. Fixes bug #17056 Backpatch to release 11, Reviewed by: Andres Freund, Álvaro Herrera and Tom Lane
2021-06-15Update another variant expected-result file.Tom Lane
This should have been updated in 533e9c6b0, but it was overlooked. Given the lack of complaints, I won't bother back-patching.
2021-06-15Remove another orphan expected-result file.Tom Lane
aborted-keyrevoke_2.out was apparently needed when it was added (in commit 0ac5ad513) to handle the case of serializable transaction mode. However, the output in serializable mode actually matches the regular aborted-keyrevoke.out file, and AFAICT has done so for a long time. There's no need to keep dragging this variant along.
2021-06-15Further refinement of stuck_on_old_timeline recovery testAndrew Dunstan
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
2021-06-15Revert 29854ee8d1 due to buildfarm failuresAlexander Korotkov
Reported-by: Tom Lane Discussion: https://postgr.es/m/CAPpHfdvcnw3x7jdV3r52p4%3D5S4WUxBCzcQKB3JukQHoicv1LSQ%40mail.gmail.com
2021-06-15Support for unnest(multirange) and cast multirange as an array of rangesAlexander Korotkov
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
2021-06-14Update variant expected-result file.Tom Lane
This should have been updated in d2d8a229b, but it was overlooked. According to 31a877f18 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.
2021-06-14Remove orphaned expected-result file.Tom Lane
This should have been removed in 43e084197, which removed the corresponding spec file. Noted while fooling about with the isolationtester.
2021-06-14Improve handling of dropped objects in pg_event_trigger_ddl_commands()Michael Paquier
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 2a10fdc. 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
2021-06-14Remove forced toast recompression in VACUUM FULL/CLUSTERMichael Paquier
The extra checks added by the recompression of toast data introduced in bbe0a81 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
2021-06-13Further tweaks to stuck_on_old_timeline recovery testAndrew Dunstan
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.
2021-06-13Ignore more environment variables in pg_regress.cMichael Paquier
This is similar to the work done in 8279f68 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
2021-06-12Restore robustness of TAP tests that wait for postmaster restart.Tom Lane
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 11e9caff8, 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 c757a3da0 and 6d41dd045, 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
2021-06-12Fix new recovery test for use under msysAndrew Dunstan
Commit caba8f0d43 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.
2021-06-12Improve log pattern detection in recently-added TAP testsMichael Paquier
ab55d74 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
2021-06-11Fix multiple crasher bugs in partitioned-table replication logic.Tom Lane
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
2021-06-11Add 'Portal Close' message to pipelined PQsendQuery()Alvaro Herrera
Commit acb7e4eb6b1c 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
2021-06-10Rename PQtraceSetFlags() to PQsetTraceFlags().Noah Misch
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
2021-06-10Reconsider the handling of procedure OUT parameters.Tom Lane
Commit 2453ea142 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
2021-06-10Adjust new test case to set wal_keep_size.Robert Haas
Per buildfarm member conchuela and Kyotaro Horiguchi, it's possible for the WAL segment that the cascading standby needs to be removed too quickly. Hopefully this will prevent that. Kyotaro Horiguchi Discussion: http://postgr.es/m/20210610.101240.1270925505780628275.horikyota.ntt@gmail.com
2021-06-09Fix corner case failure of new standby to follow new primary.Robert Haas
This only happens if (1) the new standby has no WAL available locally, (2) the new standby is starting from the old timeline, (3) the promotion happened in the WAL segment from which the new standby is starting, (4) the timeline history file for the new timeline is available from the archive but the WAL files for are not (i.e. this is a race), (5) the WAL files for the new timeline are available via streaming, and (6) recovery_target_timeline='latest'. Commit ee994272ca50f70b53074f0febaec97e28f83c4e introduced this logic and was an improvement over the previous code, but it mishandled this case. If recovery_target_timeline='latest' and restore_command is set, validateRecoveryParameters() can change recoveryTargetTLI to be different from receiveTLI. If streaming is then tried afterward, expectedTLEs gets initialized with the history of the wrong timeline. It's supposed to be a list of entries explaining how to get to the target timeline, but in this case it ends up with a list of entries explaining how to get to the new standby's original timeline, which isn't right. Dilip Kumar and Robert Haas, reviewed by Kyotaro Horiguchi. Discussion: http://postgr.es/m/CAFiTN-sE-jr=LB8jQuxeqikd-Ux+jHiXyh4YDiZMPedgQKup0g@mail.gmail.com