aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Revert "Fix psql's single transaction mode on client-side errors with -c/-f ↵Tom Lane2022-06-10
| | | | | | | | | | | | | | 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
* Un-break whole-row Vars referencing domain-over-composite types.Tom Lane2022-06-10
| | | | | | | | | | | | | | | | | 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
* Fix whitespacePeter Eisentraut2022-06-08
|
* Harden Memoization code against broken data typesDavid Rowley2022-06-08
| | | | | | | | | | | | | | | | | | | | | 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.
* Fix off-by-one loop termination condition in pg_stat_get_subscription().Tom Lane2022-06-07
| | | | | | | | | | | | | | | | 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
* Don't fail on libpq-generated error reports in pg_amcheck.Tom Lane2022-06-06
| | | | | | | | | | 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.
* Don't fail on libpq-generated error reports in ecpg_raise_backend().Tom Lane2022-06-06
| | | | | | | | | | | | | | | | 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
* Fix psql's single transaction mode on client-side errors with -c/-f switchesMichael Paquier2022-06-06
| | | | | | | | | | | | | | | | | | | | | | | 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
* Silence compiler warnings from some older compilers.Tom Lane2022-06-01
| | | | | | | | | | | | 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
* Fix pl/perl test case so it will still work under Perl 5.36.Tom Lane2022-06-01
| | | | | | | | | | | | | | | | 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
* Revert changes to CONCURRENTLY that "sped up" Xmin advanceAlvaro Herrera2022-05-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Ensure ParseTzFile() closes the input file after failing.Tom Lane2022-05-31
| | | | | | | | | | | | | | | 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
* Fix COPY FROM when database encoding is SQL_ASCII.Heikki Linnakangas2022-05-29
| | | | | | | | | | | | | | | | 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
* Handle NULL for short descriptions of custom GUC variablesMichael Paquier2022-05-28
| | | | | | | | | | | | | | | 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
* Remove misguided SSL key file ownership check in libpq.Tom Lane2022-05-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Show 'AS "?column?"' explicitly when it's important.Tom Lane2022-05-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix DDL deparse of CREATE OPERATOR CLASSAlvaro Herrera2022-05-20
| | | | | | | | | | | | | 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
* Repurpose PROC_COPYABLE_FLAGS as PROC_XMIN_FLAGSAlvaro Herrera2022-05-19
| | | | | | | | | | | | | | | 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
* Fix incorrect comments for Memoize structDavid Rowley2022-05-19
| | | | | | Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/0635f5aa-4973-8dc2-4e4e-df9fd5778a65@enterprisedb.com Backpatch-through: 14, where Memoize was added
* Update xml_1.out and xml_2.outAlvaro Herrera2022-05-18
| | | | Commit 0fbf01120023 should have updated them but didn't.
* Check column list length in XMLTABLE/JSON_TABLE aliasAlvaro Herrera2022-05-18
| | | | | | | | | | | | | | | | | 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
* Fix incorrect row estimates used for Memoize costingDavid Rowley2022-05-16
| | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix control file update done in restartpoints still running after promotionMichael Paquier2022-05-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Make pull_var_clause() handle GroupingFuncs exactly like Aggrefs.Tom Lane2022-05-12
| | | | | | | | | | | | | | | | | 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
* Fix the logical replication timeout during large transactions.Amit Kapila2022-05-11
| | | | | | | | | | | | | | | | | | | 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
* Improve setup of environment values for commands in MSVC's vcregress.plMichael Paquier2022-05-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Fix core dump in transformValuesClause when there are no columns.Tom Lane2022-05-09
| | | | | | | | | | | | 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
* Revert "Disallow infinite endpoints in generate_series() for timestamps."Tom Lane2022-05-09
| | | | | | | | | | | | | | | | | | | 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
* In REFRESH MATERIALIZED VIEW, set user ID before running user code.Noah Misch2022-05-09
| | | | | | | | | | 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
* Make relation-enumerating operations be security-restricted operations.Noah Misch2022-05-09
| | | | | | | | | | | | | | | | | | 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
* Translation updatesPeter Eisentraut2022-05-09
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: b7586f1542a8ffdfd1416e425f55e4e89c9a9505
* Disable 031_recovery_conflict.pl until after minor releases.Andres Freund2022-05-08
| | | | | | | | | | | | | 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
* Temporarily skip recovery deadlock test in back branches.Andres Freund2022-05-06
| | | | | | | | | | | | | | 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
* Backpatch addition of pump_until() more completely.Andres Freund2022-05-06
| | | | | | | | | | | | | | 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
* Update time zone data files to tzdata release 2022a.Tom Lane2022-05-05
| | | | | DST law changes in Palestine. Historical corrections for Chile and Ukraine.
* Revert "Fix timing issue in deadlock recovery conflict test."Andres Freund2022-05-04
| | | | This reverts commit 5136967f1eb194bf7598668f9893b997199935c1.
* Fix timing issue in deadlock recovery conflict test.Andres Freund2022-05-04
| | | | | | | Per buildfarm members longfin and skink. Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de Backpatch: 10-
* Backpatch 031_recovery_conflict.pl.Andres Freund2022-05-02
| | | | | | | | | | | | 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
* Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().Andres Freund2022-05-02
| | | | | | | | | | | | | | | | | | | | | 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-
* Backpatch addition of wait_for_log(), pump_until().Andres Freund2022-05-02
| | | | | | | | 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
* Fix typo in comment.Etsuro Fujita2022-05-02
|
* Disable asynchronous execution if using gating Result nodes.Etsuro Fujita2022-04-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
* Inhibit mingw CRT's auto-globbing of command line argumentsAndrew Dunstan2022-04-25
| | | | | | | | | | | | | | | | 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
* Remove some recently-added pg_dump test cases.Robert Haas2022-04-25
| | | | | | | | | | | | | 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
* Fix incautious CTE matching in rewriteSearchAndCycle().Tom Lane2022-04-23
| | | | | | | | | | | | | | | | 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
* Remove inadequate assertion check in CTE inlining.Tom Lane2022-04-21
| | | | | | | | | | | | | | | | | | | | 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
* Support new perl module namespace in stable branchesAndrew Dunstan2022-04-21
| | | | | | | | | | | | | | | | | | | 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
* Fix CLUSTER tuplesorts on abbreviated expressions.Peter Geoghegan2022-04-20
| | | | | | | | | | | | | | | | | | | | 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-
* Disallow infinite endpoints in generate_series() for timestamps.Tom Lane2022-04-20
| | | | | | | | | | 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
* Allow db.schema.table patterns, but complain about random garbage.Robert Haas2022-04-20
| | | | | | | | | | | | | | | | | | | | | | | | | | 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