summaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-06-10Revert "Fix psql's single transaction mode on client-side errors with -c/-f ↵Tom Lane
switches". This reverts commits a04ccf6df et al. in the back branches only. There was some disagreement already over whether to back-patch 157f8739a, on the grounds that it is the sort of behavioral change that we don't like to back-patch. Furthermore, it now looks like the logic needs some more work, which we don't have time for before the upcoming 14.4 release. Revert for now, and perhaps reconsider later. Discussion: https://postgr.es/m/17504-76b68018e130415e@postgresql.org
2022-06-10Un-break whole-row Vars referencing domain-over-composite types.Tom Lane
In commit ec62cb0aa, I foolishly replaced ExecEvalWholeRowVar's lookup_rowtype_tupdesc_domain call with just lookup_rowtype_tupdesc, because I didn't see how a domain could be involved there, and there were no regression test cases to jog my memory. But the existing code was correct, so revert that change and add a test case showing why it's necessary. (Note: per comment in struct DatumTupleFields, it is correct to produce an output tuple that's labeled with the base composite type, not the domain; hence just blindly looking through the domain is correct here.) Per bug #17515 from Dan Kubb. Back-patch to v11 where domains over composites became a thing. Discussion: https://postgr.es/m/17515-a24737438363aca0@postgresql.org
2022-06-08Fix whitespacePeter Eisentraut
2022-06-08Harden Memoization code against broken data typesDavid Rowley
Bug #17512 highlighted that a suitably broken data type could cause the backend to crash if either the hash function or equality function were in someway non-deterministic based on their input values. Such a data type could cause a crash of the backend due to some code which assumes that we'll always find a hash table entry corresponding to an item in the Memoize LRU list. Here we remove the assumption that we'll always find the entry corresponding to the given LRU list item and add run-time checks to verify we have found the given item in the cache. This is not a fix for bug #17512, but it will turn the crash reported by that bug report into an internal ERROR. Reported-by: Ales Zeleny Reviewed-by: Tom Lane Discussion: https://postgr.es/m/CAApHDvpxFSTwvoYWT7kmFVSZ9zLAeHb=S9vrz=RExMgSkQNWqw@mail.gmail.com Backpatch-through: 14, where Memoize was added.
2022-06-07Fix off-by-one loop termination condition in pg_stat_get_subscription().Tom Lane
pg_stat_get_subscription scanned one more LogicalRepWorker array entry than is really allocated. In the worst case this could lead to SIGSEGV, if the LogicalRepCtx data structure is near the end of shared memory. That seems quite unlikely though (thanks to the ordering of calls in CreateSharedMemoryAndSemaphores) and we've heard no field reports of it. A more likely misbehavior is one row of garbage data in the function's result, but even that is not real likely because of the check that the pid field matches some live backend. Report and fix by Kuntal Ghosh. This bug is old, so back-patch to all supported branches. Discussion: https://postgr.es/m/CAGz5QCJykEDzW6jQK6Yz7Qh_PMtD=95de_7QoocbVR2Qy8hWZA@mail.gmail.com
2022-06-06Don't fail on libpq-generated error reports in pg_amcheck.Tom Lane
An error PGresult generated by libpq itself, such as a report of connection loss, won't have broken-down error fields. should_processing_continue() blithely assumed that PG_DIAG_SEVERITY_NONLOCALIZED would always be present, and would dump core if it wasn't. Per grepping to see if 6d157e7cb's mistake was repeated elsewhere.
2022-06-06Don't fail on libpq-generated error reports in ecpg_raise_backend().Tom Lane
An error PGresult generated by libpq itself, such as a report of connection loss, won't have broken-down error fields. ecpg_raise_backend() blithely assumed that PG_DIAG_MESSAGE_PRIMARY would always be present, and would end up passing a NULL string pointer to snprintf when it isn't. That would typically crash before 3779ac62d, and it would fail to provide a useful error report in any case. Best practice is to substitute PQerrorMessage(conn) in such cases, so do that. Per bug #17421 from Masayuki Hirose. Back-patch to all supported branches. Discussion: https://postgr.es/m/17421-790ff887e3188874@postgresql.org
2022-06-06Fix psql's single transaction mode on client-side errors with -c/-f switchesMichael Paquier
psql --single-transaction is able to handle multiple -c and -f switches in a single transaction since d5563d7d, but this had the surprising behavior of forcing a transaction COMMIT even if psql failed with an error in the client (for example incorrect path given to \copy), which would generate an error, but still commit any changes that were already applied in the backend. This commit makes the behavior more consistent, by enforcing a transaction ROLLBACK if any commands fail, both client-side and backend-side, so as no changes are applied if one error happens in any of them. Some tests are added on HEAD to provide some coverage about all that. Backend-side errors are unreliable as IPC::Run can complain on SIGPIPE if psql quits before reading a query result, but that should work properly in the case where any errors come from psql itself, which is what the original report is about. Reported-by: Christoph Berg Author: Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/17504-76b68018e130415e@postgresql.org Backpatch-through: 10
2022-06-01Silence compiler warnings from some older compilers.Tom Lane
Since a117cebd6, some older gcc versions issue "variable may be used uninitialized in this function" complaints for brin_summarize_range. Silence that using the same coding pattern as in bt_index_check_internal; arguably, a117cebd6 had too narrow a view of which compilers might give trouble. Nathan Bossart and Tom Lane. Back-patch as the previous commit was. Discussion: https://postgr.es/m/20220601163537.GA2331988@nathanxps13
2022-06-01Fix pl/perl test case so it will still work under Perl 5.36.Tom Lane
Perl 5.36 has reclassified the warning condition that this test case used, so that the expected error fails to appear. Tweak the test so it instead exercises a case that's handled the same way in all Perl versions of interest. This appears to meet our standards for back-patching into out-of-support branches: it changes no user-visible behavior but enables testing of old branches with newer tools. Hence, back-patch as far as 9.2. Dagfinn Ilmari Mannsåker, per report from Jitka Plesníková. Discussion: https://postgr.es/m/564579.1654093326@sss.pgh.pa.us
2022-05-31Revert changes to CONCURRENTLY that "sped up" Xmin advanceAlvaro Herrera
This reverts commit d9d076222f5b "VACUUM: ignore indexing operations with CONCURRENTLY". These changes caused indexes created with the CONCURRENTLY option to miss heap tuples that were HOT-updated and HOT-pruned during the index creation. Before these changes, HOT pruning would have been prevented by the Xmin of the transaction creating the index, but because this change was precisely to allow the Xmin to move forward ignoring that backend, now other backends scanning the table can prune them. This is not a problem for VACUUM (which requires a lock that conflicts with a CREATE INDEX CONCURRENTLY operation), but HOT-prune can definitely occur. In other words, Xmin advancement was sped up, but at the cost of corrupting the resulting index. Regrettably, this means that the new feature in PG14 that RIC/CIC on very large tables no longer force VACUUM to retain very old tuples goes away. We might try to implement it again in a later release, but for now the risk of indexes missing tuples is too high and there's no easy fix. Backpatch to 14, where this change appeared. Reported-by: Peter Slavov <pet.slavov@gmail.com> Diagnosys-by: Andrey Borodin <x4mmm@yandex-team.ru> Diagnosys-by: Michael Paquier <michael@paquier.xyz> Diagnosys-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/17485-396609c6925b982d%40postgresql.org
2022-05-31Ensure ParseTzFile() closes the input file after failing.Tom Lane
We hadn't noticed this because (a) few people feed invalid timezone abbreviation files to the server, and (b) in typical scenarios guc.c would throw ereport(ERROR) and then transaction abort handling would silently clean up the leaked file reference. However, it was possible to observe file leakage warnings if one breaks an already-active abbreviation file, because guc.c does not throw ERROR when loading supposedly-validated settings during session start or SIGHUP processing. Report and fix by Kyotaro Horiguchi (cosmetic adjustments by me) Discussion: https://postgr.es/m/20220530.173740.748502979257582392.horikyota.ntt@gmail.com
2022-05-29Fix COPY FROM when database encoding is SQL_ASCII.Heikki Linnakangas
In the codepath when no encoding conversion is required, the check for incomplete character at the end of input incorrectly used server encoding's max character length, instead of the client's. Usually the server and client encodings are the same when we're not performing encoding conversion, but SQL_ASCII is an exception. In the passing, also fix some outdated comments that still talked about the old COPY protocol. It was removed in v14. Per bug #17501 from Vitaly Voronov. Backpatch to v14 where this was introduced. Discussion: https://www.postgresql.org/message-id/17501-128b1dd039362ae6@postgresql.org
2022-05-28Handle NULL for short descriptions of custom GUC variablesMichael Paquier
If a short description is specified as NULL in one of the various DefineCustomXXXVariable() functions available to external modules to define a custom parameter, SHOW ALL would crash. This change teaches SHOW ALL to properly handle NULL short descriptions, as well as any code paths that manipulate it, to gain in flexibility. Note that help_config.c was already able to do that, when describing a set of GUCs for postgres --describe-config. Author: Steve Chavez Reviewed by: Nathan Bossart, Andres Freund, Michael Paquier, Tom Lane Discussion: https://postgr.es/m/CAGRrpzY6hO-Kmykna_XvsTv8P2DshGiU6G3j8yGao4mk0CqjHA%40mail.gmail.com Backpatch-through: 10
2022-05-26Remove misguided SSL key file ownership check in libpq.Tom Lane
Commits a59c79564 et al. tried to sync libpq's SSL key file permissions checks with what we've used for years in the backend. We did not intend to create any new failure cases, but it turns out we did: restricting the key file's ownership breaks cases where the client is allowed to read a key file despite not having the identical UID. In particular a client running as root used to be able to read someone else's key file; and having seen that I suspect that there are other, less-dubious use cases that this restriction breaks on some platforms. We don't really need an ownership check, since if we can read the key file despite its having restricted permissions, it must have the right ownership --- under normal conditions anyway, and the point of this patch is that any additional corner cases where that works should be deemed allowable, as they have been historically. Hence, just drop the ownership check, and rearrange the permissions check to get rid of its faulty assumption that geteuid() can't be zero. (Note that the comparable backend-side code doesn't have to cater for geteuid() == 0, since the server rejects that very early on.) This does have the end result that the permissions safety check used for a root user's private key file is weaker than that used for anyone else's. While odd, root really ought to know what she's doing with file permissions, so I think this is acceptable. Per report from Yogendra Suralkar. Like the previous patch, back-patch to all supported branches. Discussion: https://postgr.es/m/MW3PR15MB3931DF96896DC36D21AFD47CA3D39@MW3PR15MB3931.namprd15.prod.outlook.com
2022-05-21Show 'AS "?column?"' explicitly when it's important.Tom Lane
ruleutils.c was coded to suppress the AS label for a SELECT output expression if the column name is "?column?", which is the parser's fallback if it can't think of something better. This is fine, and avoids ugly clutter, so long as (1) nothing further up in the parse tree relies on that column name or (2) the same fallback would be assigned when the rule or view definition is reloaded. Unfortunately (2) is far from certain, both because ruleutils.c might print the expression in a different form from how it was originally written and because FigureColname's rules might change in future releases. So we shouldn't rely on that. Detecting exactly whether there is any outer-level use of a SELECT column name would be rather expensive. This patch takes the simpler approach of just passing down a flag indicating whether there *could* be any outer use; for example, the output column names of a SubLink are not referenceable, and we also do not care about the names exposed by the right-hand side of a setop. This is sufficient to suppress unwanted clutter in all but one case in the regression tests. That seems like reasonable evidence that it won't be too much in users' faces, while still fixing the cases we need to fix. Per bug #17486 from Nicolas Lutic. This issue is ancient, so back-patch to all supported branches. Discussion: https://postgr.es/m/17486-1ad6fd786728b8af@postgresql.org
2022-05-20Fix DDL deparse of CREATE OPERATOR CLASSAlvaro Herrera
When an implicit operator family is created, it wasn't getting reported. Make it do so. This has always been missing. Backpatch to 10. Author: Masahiko Sawada <sawada.mshk@gmail.com> Reported-by: Leslie LEMAIRE <leslie.lemaire@developpement-durable.gouv.fr> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Michael Paquiër <michael@paquier.xyz> Discussion: https://postgr.es/m/f74d69e151b22171e8829551b1159e77@developpement-durable.gouv.fr
2022-05-19Repurpose PROC_COPYABLE_FLAGS as PROC_XMIN_FLAGSAlvaro Herrera
This is a slight, convenient semantics change from what commit 0f0cfb494004 ("Fix parallel operations that prevent oldest xmin from advancing") introduced that lets us simplify the coding in the one place where it is used. Backpatch to 13. This is related to commit 6fea65508a1a ("Tighten ComputeXidHorizons' handling of walsenders") rewriting the code site where this is used, which has not yet been backpatched, but it may well be in the future. Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/202204191637.eldwa2exvguw@alvherre.pgsql
2022-05-19Fix incorrect comments for Memoize structDavid Rowley
Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/0635f5aa-4973-8dc2-4e4e-df9fd5778a65@enterprisedb.com Backpatch-through: 14, where Memoize was added
2022-05-18Update xml_1.out and xml_2.outAlvaro Herrera
Commit 0fbf01120023 should have updated them but didn't.
2022-05-18Check column list length in XMLTABLE/JSON_TABLE aliasAlvaro Herrera
We weren't checking the length of the column list in the alias clause of an XMLTABLE or JSON_TABLE function (a "tablefunc" RTE), and it was possible to make the server crash by passing an overly long one. Fix it by throwing an error in that case, like the other places that deal with alias lists. In passing, modify the equivalent test used for join RTEs to look like the other ones, which was different for no apparent reason. This bug came in when XMLTABLE was born in version 10; backpatch to all stable versions. Reported-by: Wang Ke <krking@zju.edu.cn> Discussion: https://postgr.es/m/17480-1c9d73565bb28e90@postgresql.org
2022-05-16Fix incorrect row estimates used for Memoize costingDavid Rowley
In order to estimate the cache hit ratio of a Memoize node, one of the inputs we require is the estimated number of times the Memoize node will be rescanned. The higher this number, the large the cache hit ratio is likely to become. Unfortunately, the value being passed as the number of "calls" to the Memoize was incorrectly using the Nested Loop's outer_path->parent->rows instead of outer_path->rows. This failed to account for the fact that the outer_path might be parameterized by some upper-level Nested Loop. This problem could lead to Memoize plans appearing more favorable than they might actually be. It could also lead to extended executor startup times when work_mem values were large due to the planner setting overly large MemoizePath->est_entries resulting in the Memoize hash table being initially made much larger than might be required. Fix this simply by passing outer_path->rows rather than outer_path->parent->rows. Also, adjust the expected regression test output for a plan change. Reported-by: Pavel Stehule Author: David Rowley Discussion: https://postgr.es/m/CAFj8pRAMp%3DQsMi6sPQJ4W3hczoFJRvyXHJV3AZAZaMyTVM312Q%40mail.gmail.com Backpatch-through: 14, where Memoize was introduced
2022-05-16Fix control file update done in restartpoints still running after promotionMichael Paquier
If a cluster is promoted (aka the control file shows a state different than DB_IN_ARCHIVE_RECOVERY) while CreateRestartPoint() is still processing, this function could miss an update of the control file for "checkPoint" and "checkPointCopy" but still do the recycling and/or removal of the past WAL segments, assuming that the to-be-updated LSN values should be used as reference points for the cleanup. This causes a follow-up restart attempting crash recovery to fail with a PANIC on a missing checkpoint record if the end-of-recovery checkpoint triggered by the promotion did not complete while the cluster abruptly stopped or crashed before the completion of this checkpoint. The PANIC would be caused by the redo LSN referred in the control file as located in a segment already gone, recycled by the previous restartpoint with "checkPoint" out-of-sync in the control file. This commit fixes the update of the control file during restartpoints so as "checkPoint" and "checkPointCopy" are updated even if the cluster has been promoted while a restartpoint is running, to be on par with the set of WAL segments actually recycled in the end of CreateRestartPoint(). 7863ee4 has fixed this problem already on master, but the release timing of the latest point versions did not let me enough time to study and fix that on all the stable branches. Reported-by: Fujii Masao, Rui Zhao Author: Kyotaro Horiguchi Reviewed-by: Nathan Bossart, Michael Paquier Discussion: https://postgr.es/m/20220316.102444.2193181487576617583.horikyota.ntt@gmail.com Backpatch-through: 10
2022-05-12Make pull_var_clause() handle GroupingFuncs exactly like Aggrefs.Tom Lane
This follows in the footsteps of commit 2591ee8ec by removing one more ill-advised shortcut from planning of GroupingFuncs. It's true that we don't intend to execute the argument expression(s) at runtime, but we still have to process any Vars appearing within them, or we risk failure at setrefs.c time (or more fundamentally, in EXPLAIN trying to print such an expression). Vars in upper plan nodes have to have referents in the next plan level, whether we ever execute 'em or not. Per bug #17479 from Michael J. Sullivan. Back-patch to all supported branches. Richard Guo Discussion: https://postgr.es/m/17479-6260deceaf0ad304@postgresql.org
2022-05-11Fix the logical replication timeout during large transactions.Amit Kapila
The problem is that we don't send keep-alive messages for a long time while processing large transactions during logical replication where we don't send any data of such transactions. This can happen when the table modified in the transaction is not published or because all the changes got filtered. We do try to send the keep_alive if necessary at the end of the transaction (via WalSndWriteData()) but by that time the subscriber-side can timeout and exit. To fix this we try to send the keepalive message if required after processing certain threshold of changes. Reported-by: Fabrice Chapuis Author: Wang wei and Amit Kapila Reviewed By: Masahiko Sawada, Euler Taveira, Hou Zhijie, Hayato Kuroda Backpatch-through: 10 Discussion: https://postgr.es/m/CAA5-nLARN7-3SLU_QUxfy510pmrYK6JJb=bk3hcgemAM_pAv+w@mail.gmail.com
2022-05-11Improve setup of environment values for commands in MSVC's vcregress.plMichael Paquier
The current setup assumes that commands for lz4, zstd and gzip always exist by default if not enforced by a user's environment. However, vcpkg, as one example, installs libraries but no binaries, so this default setup to assume that a command should always be present would cause failures. This commit improves the detection of such external commands as follows: * If a ENV value is available, trust the environment/user and use it. * If a ENV value is not available, check its execution by looking in the current PATH, by launching a simple "$command --version" (that should be portable enough). ** On execution failure, ignore ENV{command}. ** On execution success, set ENV{command} = "$command". Note that this new rule applies to gzip, lz4 and zstd but not tar that we assume will always exist. Those commands are set up in the environment only when using bincheck and taptest. The CI includes all those commands and I have checked that their setup is correct there. I have also tested this change in a MSVC environment where we have none of those commands. While on it, remove the references to lz4 from the documentation and vcregress.pl in ~v13. --with-lz4 has been added in v14~ so there is no point to have this information in these older branches. Reported-by: Andrew Dunstan Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/14402151-376b-a57a-6d0c-10ad12608e12@dunslane.net Backpatch-through: 10
2022-05-09Fix core dump in transformValuesClause when there are no columns.Tom Lane
The parser code that transformed VALUES from row-oriented to column-oriented lists failed if there were zero columns. You can't write that straightforwardly (though probably you should be able to), but the case can be reached by expanding a "tab.*" reference to a zero-column table. Per bug #17477 from Wang Ke. Back-patch to all supported branches. Discussion: https://postgr.es/m/17477-0af3c6ac6b0a6ae0@postgresql.org
2022-05-09Revert "Disallow infinite endpoints in generate_series() for timestamps."Tom Lane
This reverts commit eafdf9de06e9b60168f5e47cedcfceecdc6d4b5f and its back-branch counterparts. Corey Huinker pointed out that we'd discussed this exact change back in 2016 and rejected it, on the grounds that there's at least one usage pattern with LIMIT where an infinite endpoint can usefully be used. Perhaps that argument needs to be re-litigated, but there's no time left before our back-branch releases. To keep our options open, restore the status quo ante; if we do end up deciding to change things, waiting one more quarter won't hurt anything. Rather than just doing a straight revert, I added a new test case demonstrating the usage with LIMIT. That'll at least remind us of the issue if we forget again. Discussion: https://postgr.es/m/3603504.1652068977@sss.pgh.pa.us Discussion: https://postgr.es/m/CADkLM=dzw0Pvdqp5yWKxMd+VmNkAMhG=4ku7GnCZxebWnzmz3Q@mail.gmail.com
2022-05-09In REFRESH MATERIALIZED VIEW, set user ID before running user code.Noah Misch
It intended to, but did not, achieve this. Adopt the new standard of setting user ID just after locking the relation. Back-patch to v10 (all supported versions). Reviewed by Simon Riggs. Reported by Alvaro Herrera. Security: CVE-2022-1552
2022-05-09Make relation-enumerating operations be security-restricted operations.Noah Misch
When a feature enumerates relations and runs functions associated with all found relations, the feature's user shall not need to trust every user having permission to create objects. BRIN-specific functionality in autovacuum neglected to account for this, as did pg_amcheck and CLUSTER. An attacker having permission to create non-temp objects in at least one schema could execute arbitrary SQL functions under the identity of the bootstrap superuser. CREATE INDEX (not a relation-enumerating operation) and REINDEX protected themselves too late. This change extends to the non-enumerating amcheck interface. Back-patch to v10 (all supported versions). Sergey Shinderuk, reviewed (in earlier versions) by Alexander Lakhin. Reported by Alexander Lakhin. Security: CVE-2022-1552
2022-05-09Translation updatesPeter Eisentraut
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: b7586f1542a8ffdfd1416e425f55e4e89c9a9505
2022-05-08Disable 031_recovery_conflict.pl until after minor releases.Andres Freund
f40d362a667 disabled part of 031_recovery_conflict.pl due to instability that's not trivial to fix in the back branches. That fixed most of the issues. But there was one more failure (on lapwing / REL_10_STABLE). That failure looks like it might be caused by a genuine problem. Disable the test until after the set of releases, to avoid packagers etc potentially having to fight with a test failure they can't do anything about. Discussion: https://postgr.es/m/3447060.1652032749@sss.pgh.pa.us Backpatch: 10-14
2022-05-06Temporarily skip recovery deadlock test in back branches.Andres Freund
The recovery deadlock test has a timing issue that was fixed in 5136967f1eb in HEAD. Unfortunately the same fix doesn't quite work in the back branches: 1) adjust_conf() doesn't exist, which is easy enough to work around 2) a restart cleares the recovery conflict stats < 15. These issues can be worked around, but given the upcoming set of minor releases, skip the problematic test for now. The buildfarm doesn't show failures in other parts of 031_recovery_conflict.pl. Discussion: https://postgr.es/m/20220506155827.dfnaheq6ufylwrqf@alap3.anarazel.de Backpatch: 10-14
2022-05-06Backpatch addition of pump_until() more completely.Andres Freund
In a2ab9c06ea1 I just backpatched the introduction of pump_until(), without changing the existing local definitions (as 6da65a3f9a9). The necessary changes seemed more verbose than desirable. However, that leads to warnings, as I failed to realize... Backpatch to all versions containing pump_until() calls before f74496dd611 (there's none in 10). Discussion: https://postgr.es/m/2808491.1651802860@sss.pgh.pa.us Discussion: https://postgr.es/m/18b37361-b482-b9d8-f30d-6115cd5ce25c@enterprisedb.com Backpatch: 11-14
2022-05-05Update time zone data files to tzdata release 2022a.Tom Lane
DST law changes in Palestine. Historical corrections for Chile and Ukraine.
2022-05-04Revert "Fix timing issue in deadlock recovery conflict test."Andres Freund
This reverts commit 5136967f1eb194bf7598668f9893b997199935c1.
2022-05-04Fix timing issue in deadlock recovery conflict test.Andres Freund
Per buildfarm members longfin and skink. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-
2022-05-02Backpatch 031_recovery_conflict.pl.Andres Freund
The prior commit showed that the introduction of recovery conflict tests was a good idea. Without these tests it's hard to know that the fix didn't break something... 031_recovery_conflict.pl was introduced in 9f8a050f68d and extended in 21e184403bf. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-14
2022-05-02Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().Andres Freund
The tests added in 9f8a050f68d failed nearly reliably on FreeBSD in CI, and occasionally on the buildfarm. That turns out to be caused not by a bug in the test, but by a longstanding bug in recovery conflict handling. The standby timeout handler, used by ResolveRecoveryConflictWithBufferPin(), executed SendRecoveryConflictWithBufferPin() inside a signal handler. A bad idea, because the deadlock timeout handler (or a spurious latch set) could have interrupted ProcWaitForSignal(). If unlucky that could cause a self-deadlock on ProcArrayLock, if the deadlock check is in SendRecoveryConflictWithBufferPin()->CancelDBBackends(). To fix, set a flag in StandbyTimeoutHandler(), and check the flag in ResolveRecoveryConflictWithBufferPin(). Subsequently the recovery conflict tests will be backpatched. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-
2022-05-02Backpatch addition of wait_for_log(), pump_until().Andres Freund
These were originally introduced in a2ab9c06ea1 and a2ab9c06ea1, as they are needed by a about-to-be-backpatched test. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-14
2022-05-02Fix typo in comment.Etsuro Fujita
2022-04-28Disable asynchronous execution if using gating Result nodes.Etsuro Fujita
mark_async_capable_plan(), which is called from create_append_plan() to determine whether subplans are async-capable, failed to take into account that the given subplan created from a given subpath might include a gating Result node if the subpath is a SubqueryScanPath or ForeignPath, causing a segmentation fault there when the subplan created from a SubqueryScanPath includes the Result node, or causing ExecAsyncRequest() to throw an error about an unrecognized node type when the subplan created from a ForeignPath includes the Result node, because in the latter case the Result node was unintentionally considered as async-capable, but we don't currently support executing Result nodes asynchronously. Fix by modifying mark_async_capable_plan() to disable asynchronous execution in such cases. Also, adjust code in the ProjectionPath case in mark_async_capable_plan(), for consistency with other cases, and adjust/improve comments there. is_async_capable_path() added in commit 27e1f1456, which was rewritten to mark_async_capable_plan() in a later commit, has the same issue, causing the error at execution mentioned above, so back-patch to v14 where the aforesaid commit went in. Per report from Justin Pryzby. Etsuro Fujita, reviewed by Zhihong Yu and Justin Pryzby. Discussion: https://postgr.es/m/20220408124338.GK24419%40telsasoft.com
2022-04-25Inhibit mingw CRT's auto-globbing of command line argumentsAndrew Dunstan
For some reason by default the mingw C Runtime takes it upon itself to expand program arguments that look like shell globbing characters. That has caused much scratching of heads and mis-attribution of the causes of some TAP test failures, so stop doing that. This removes an inconsistency with Windows binaries built with MSVC, which have no such behaviour. Per suggestion from Noah Misch. Backpatch to all live branches. Discussion: https://postgr.es/m/20220423025927.GA1274057@rfd.leadboat.com
2022-04-25Remove some recently-added pg_dump test cases.Robert Haas
Commit d2d35479796c3510e249d6fc72adbd5df918efbf included a pretty extensive set of test cases, and some of them don't work on all of our Windows machines. This happens because IPC::Run expands its arguments as shell globs on a few machines, but doesn't on most of the buildfarm. It might be good to fix that problem systematically somehow, but in the meantime, there are enough test cases for this commit that it seems OK to just remove the ones that are failing. Discussion: http://postgr.es/m/3a190754-b2b0-d02b-dcfd-4ec1610ffbcb@dunslane.net Discussion: http://postgr.es/m/CA+TgmoYRGUcFBy6VgN0+Pn4f6Wv=2H0HZLuPHqSy6VC8Ba7vdg@mail.gmail.com
2022-04-23Fix incautious CTE matching in rewriteSearchAndCycle().Tom Lane
This function looks for a reference to the recursive WITH CTE, but it checked only the CTE name not ctelevelsup, so that it could seize on a lower CTE that happened to have the same name. This would result in planner failures later, either weird errors such as "could not find attribute 2 in subquery targetlist", or crashes or assertion failures. The code also merely Assert'ed that it found a matching entry, which is not guaranteed at all by the parser. Per bugs #17320 and #17318 from Zhiyong Wu. Thanks to Kyotaro Horiguchi for investigation. Discussion: https://postgr.es/m/17320-70e37868182512ab@postgresql.org Discussion: https://postgr.es/m/17318-2eb65a3a611d2368@postgresql.org
2022-04-21Remove inadequate assertion check in CTE inlining.Tom Lane
inline_cte() expected to find exactly as many references to the target CTE as its cterefcount indicates. While that should be accurate for the tree as emitted by the parser, there are some optimizations that occur upstream of here that could falsify it, notably removal of unused subquery output expressions. Trying to make the accounting 100% accurate seems expensive and doomed to future breakage. It's not really worth it, because all this code is protecting is downstream assumptions that every referenced CTE has a plan. Let's convert those assertions to regular test-and-elog just in case there's some actual problem, and then drop the failing assertion. Per report from Tomas Vondra (thanks also to Richard Guo for analysis). Back-patch to v12 where the faulty code came in. Discussion: https://postgr.es/m/29196a1e-ed47-c7ca-9be2-b1c636816183@enterprisedb.com
2022-04-21Support new perl module namespace in stable branchesAndrew Dunstan
Commit b3b4d8e68a moved our perl test modules to a better namespace structure, but this has made life hard for people wishing to backpatch improvements in the TAP tests. Here we alleviate much of that difficulty by implementing the new module names on top of the old modules, mostly by using a little perl typeglob aliasing magic, so that we don't have a dual maintenance burden. This should work both for the case where a new test is backpatched and the case where a fix to an existing test that uses the new namespace is backpatched. Reviewed by Michael Paquier Per complaint from Andres Freund Discussion: https://postgr.es/m/20220418141530.nfxtkohefvwnzncl@alap3.anarazel.de Applied to branches 10 through 14
2022-04-20Fix CLUSTER tuplesorts on abbreviated expressions.Peter Geoghegan
CLUSTER sort won't use the datum1 SortTuple field when clustering against an index whose leading key is an expression. This makes it unsafe to use the abbreviated keys optimization, which was missed by the logic that sets up SortSupport state. Affected tuplesorts output tuples in a completely bogus order as a result (the wrong SortSupport based comparator was used for the leading attribute). This issue is similar to the bug fixed on the master branch by recent commit cc58eecc5d. But it's a far older issue, that dates back to the introduction of the abbreviated keys optimization by commit 4ea51cdfe8. Backpatch to all supported versions. Author: Peter Geoghegan <pg@bowt.ie> Author: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/CA+hUKG+bA+bmwD36_oDxAoLrCwZjVtST2fqe=b4=qZcmU7u89A@mail.gmail.com Backpatch: 10-
2022-04-20Disallow infinite endpoints in generate_series() for timestamps.Tom Lane
Such cases will lead to infinite loops, so they're of no practical value. The numeric variant of generate_series() already threw error for this, so borrow its message wording. Per report from Richard Wesley. Back-patch to all supported branches. Discussion: https://postgr.es/m/91B44E7B-68D5-448F-95C8-B4B3B0F5DEAF@duckdblabs.com
2022-04-20Allow db.schema.table patterns, but complain about random garbage.Robert Haas
psql, pg_dump, and pg_amcheck share code to process object name patterns like 'foo*.bar*' to match all tables with names starting in 'bar' that are in schemas starting with 'foo'. Before v14, any number of extra name parts were silently ignored, so a command line '\d foo.bar.baz.bletch.quux' was interpreted as '\d bletch.quux'. In v14, as a result of commit 2c8726c4b0a496608919d1f78a5abc8c9b6e0868, we instead treated this as a request for table quux in a schema named 'foo.bar.baz.bletch'. That caused problems for people like Justin Pryzby who were accustomed to copying strings of the form db.schema.table from messages generated by PostgreSQL itself and using them as arguments to \d. Accordingly, revise things so that if an object name pattern contains more parts than we're expecting, we throw an error, unless there's exactly one extra part and it matches the current database name. That way, thisdb.myschema.mytable is accepted as meaning just myschema.mytable, but otherdb.myschema.mytable is an error, and so is some.random.garbage.myschema.mytable. Mark Dilger, per report from Justin Pryzby and discussion among various people. Discussion: https://www.postgresql.org/message-id/20211013165426.GD27491%40telsasoft.com