aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Prevent long-term memory leakage in autovacuum launcher.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | get_database_list() failed to restore the caller's memory context, instead leaving current context set to TopMemoryContext which is how CommitTransactionCommand() leaves it. The callers both think they are using short-lived contexts, for the express purpose of not having to worry about cleaning up individual allocations. The net effect therefore is that supposedly short-lived allocations could accumulate indefinitely in the launcher's TopMemoryContext. Although this has been broken for a long time, it seems we didn't have any obvious memory leak here until v15's rearrangement of the stats logic. I (tgl) am not entirely convinced that there's no other leak at all, though, and we're surely at risk of adding one in future back-patched fixes. So back-patch to all supported branches, even though this may be only a latent bug in pre-v15. Reid Thompson Discussion: https://postgr.es/m/972a4e12b68b0f96db514777a150ceef7dcd2e0f.camel@crunchydata.com
* In the Snowball dictionary, don't try to stem excessively-long words.Tom Lane2022-08-31
| | | | | | | | | | | | | | | | | | | | | If the input word exceeds 1000 bytes, don't pass it to the stemmer; just return it as-is after case folding. Such an input is surely not a word in any human language, so whatever the stemmer might do to it would be pretty dubious in the first place. Adding this restriction protects us against a known recursion-to-stack-overflow problem in the Turkish stemmer, and it seems like good insurance against any other safety or performance issues that may exist in the Snowball stemmers. (I note, for example, that they contain no CHECK_FOR_INTERRUPTS calls, so we really don't want them running for a long time.) The threshold of 1000 bytes is arbitrary. An alternative definition could have been to treat such words as stopwords, but that seems like a bigger break from the old behavior. Per report from Egor Chindyaskin and Alexander Lakhin. Thanks to Olly Betts for the recommendation to fix it this way. Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* plpython: Don't create pgxsdir subdirectory in installdir targetPeter Eisentraut2022-08-31
| | | | | | As of db23464715f4792298c639153dda7bfd9ad9d602, we don't install anything there anymore from plpython, so we don't need to create the installation directory anymore.
* On NetBSD, force dynamic symbol resolution at postmaster start.Tom Lane2022-08-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The default of lazy symbol resolution means that when the postmaster first reaches the select() call in ServerLoop, it'll need to resolve the link to that libc entry point. NetBSD's dynamic loader takes an internal lock while doing that, and if a signal interrupts the operation then there is a risk of self-deadlock should the signal handler do anything that requires that lock, as several of the postmaster signal handlers do. The window for this is pretty narrow, and timing considerations make it unlikely that a signal would arrive right then anyway. But it's semi-repeatable on slow single-CPU machines, and in principle the race could happen with any hardware. The least messy solution to this is to force binding of dynamic symbols at postmaster start, using the "-z now" linker option. While we're at it, also use "-z relro" so as to provide a small security gain. It's not entirely clear whether any other platforms share this issue, but for now we'll assume it's NetBSD-specific. (We might later try to use "-z now" on more platforms for performance reasons, but that would not likely be something to back-patch.) Report and patch by me; the idea to fix it this way is from Andres Freund. Discussion: https://postgr.es/m/3384826.1661802235@sss.pgh.pa.us
* Prevent WAL corruption after a standby promotion.Robert Haas2022-08-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | When a PostgreSQL instance performing archive recovery but not using standby mode is promoted, and the last WAL segment that it attempted to read ended in a partial record, the previous code would create invalid WAL on the new timeline. The WAL from the previously timeline would be copied to the new timeline up until the end of the last valid record, but instead of beginning to write WAL at immediately afterwards, the promoted server would write an overwrite contrecord at the beginning of the next segment. The end of the previous segment would be left as all-zeroes, resulting in failures if anything tried to read WAL from that file. The root of the issue is that ReadRecord() decides whether to set abortedRecPtr and missingContrecPtr based on the value of StandbyMode, but ReadRecord() switches to a new timeline based on the value of ArchiveRecoveryRequested. We shouldn't try to write an overwrite contrecord if we're switching to a new timeline, so change the test in ReadRecod() to check ArchiveRecoveryRequested instead. Code fix by Dilip Kumar. Comments by me incorporating suggested language from Álvaro Herrera. Further review from Kyotaro Horiguchi and Sami Imseih. Discussion: http://postgr.es/m/CAFiTN-t7umki=PK8dT1tcPV=mOUe2vNhHML6b3T7W7qqvvajjg@mail.gmail.com Discussion: http://postgr.es/m/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com
* Switch format specifier for replication origins to %dJohn Naylor2022-08-28
| | | | | | | | | Using %u with uint16 causes warnings with -Wformat-signedness. There are many other warnings, but for now change only these since c920fe4818 already changed the message string for most of them. Per report from Peter Eisentraut Discussion: https://www.postgresql.org/message-id/31e63649-0355-7088-831e-b07d5f908a8c%40enterprisedb.com
* Use correct connection for cancellation in frontend's parallel slotsMichael Paquier2022-08-27
| | | | | | | | | | | | | | While waiting for slots to become available in wait_on_slots() in parallel_slot.c, the cancellation always relied on the first connection in the set to do the job. This could cause problems when this slot's socket is gone as PQgetCancel() would return NULL in this case. Rather than always using the first connection, this changes the logic to use the first valid connection for the cancellation. Author: Ranier Vilela Reviewed-by: Justin Pryzby Discussion: https://postgr.es/m/CAEudQAokk1h_pUwGXsYS4oVOuf35s1O2o3TXGHpV8=AWikvgHA@mail.gmail.com Backpatch-through: 14
* Remove obsolete commentPeter Eisentraut2022-08-26
| | | | | | | | The comment in basebackup.c updated by 33bd4698c11 was actually obsolete to begin with, since the symbols it was referring to haven't existed in that header file for quite some time. The header file is still needed for other reasons, though, so keep the #include, just drop the comment.
* Fix typo in comment.Etsuro Fujita2022-08-26
|
* libpq code should use libpq_gettext(), not _()Peter Eisentraut2022-08-25
| | | | Fix some wrong use and install a safeguard against future mistakes.
* Update another comment still referring to pg_start/stop_backup()Peter Eisentraut2022-08-25
|
* pg_dump: Fix new ICU testsPeter Eisentraut2022-08-25
| | | | | ICU doesn't support some server encodings, so we need to exclude them if a non-supported encoding was set up.
* Fix code comments still referring to pg_start/stop_backup()Michael Paquier2022-08-24
| | | | | | | | | pg_start_backup() and pg_stop_backup() have been respectively renamed to pg_backup_start() and pg_backup_stop() as of 39969e2, but a few comments did not get the call. Reviewed-by: Kyotaro Horiguchi, David Steele Discussion: https://postgr.es/m/YrqGlj1+4DF3dbZ/@paquier.xyz
* pg_dump: Dump colliculocalePeter Eisentraut2022-08-24
| | | | | | | | This was forgotten when the new column was introduced. Author: Marina Polyakova <m.polyakova@postgrespro.ru> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://www.postgresql.org/message-id/7ad26354e75259f59c4a6c6997b8ee32%40postgrespro.ru
* Defend against stack overrun in a few more places.Tom Lane2022-08-24
| | | | | | | | | | | | | | | | | | | SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c, and regex_selectivity_sub() in selectivity estimation could recurse until stack overflow; fix by adding check_stack_depth() calls. So could next() in the regex compiler, but that case is better fixed by converting its tail recursion to a loop. (We probably get better code that way too, since next() can now be inlined into its sole caller.) There remains a reachable stack overrun in the Turkish stemmer, but we'll need some advice from the Snowball people about how to fix that. Per report from Egor Chindyaskin and Alexander Lakhin. These mistakes are old, so back-patch to all supported branches. Richard Guo and Tom Lane Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
* Fix ICU locale option handling in CREATE DATABASEPeter Eisentraut2022-08-24
| | | | | | | | | The code took the LOCALE option as the default/fallback for ICU_LOCALE, but this was neither documented nor intended, so remove it. (It was probably left in from an earlier patch version.) Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru> Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e
* Message style adjustmentPeter Eisentraut2022-08-23
|
* Doc: prefer sysctl to /proc/sys in docs and comments.Tom Lane2022-08-23
| | | | | | | | | sysctl is more portable than Linux's /proc/sys file tree, and often easier to use too. That's why most of our docs refer to sysctl when talking about how to adjust kernel parameters. Bring the few stragglers into line. Discussion: https://postgr.es/m/361175.1661187463@sss.pgh.pa.us
* Add CHECK_FOR_INTERRUPTS while decoding changes.Amit Kapila2022-08-23
| | | | | | | | | | | | | While decoding changes in a loop, if we skip all the changes there is no CFI making the loop uninterruptible. Reported-by: Whale Song and Andrey Borodin Bug: 17580 Author: Masahiko Sawada Reviwed-by: Amit Kapila Backpatch-through: 10 Discussion: https://postgr.es/m/17580-849c1d5b6d7eb422@postgresql.org Discussion: https://postgr.es/m/B319ECD6-9A28-4CDF-A8F4-3591E0BF2369@yandex-team.ru
* pgstat: Acquire lock when reading variable-numbered statsAndres Freund2022-08-22
| | | | | | | | | | | | | | Somewhere during the development of the patch acquiring a lock during read access to variable-numbered stats got lost. The missing lock acquisition won't cause corruption, but can lead to reading torn values when accessing stats. Add the missing lock acquisitions. Reported-by: Greg Stark <stark@mit.edu> Reviewed-by: "Drouvot, Bertrand" <bdrouvot@amazon.com> Reviewed-by: Andres Freund <andres@anarazel.de> Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com Backpatch: 15-
* Fix assertion failure in CREATE DATABASEPeter Eisentraut2022-08-22
| | | | | | | | | An assertion would fail when creating a database with libc locale provider from a template database with icu locale provider. Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e
* pg_upgrade: Fix thinko in database info acquisition routinePeter Eisentraut2022-08-22
| | | | | | | | | | | When checking whether the major version supports per-database locale providers, it was always looking at the version of the old cluster instead of the cluster that was passed in. This would lead to failures to detect locale provider mismatches. Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru> Reviewed-by: Julien Rouhaud <rjuju123@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e
* Use logical operator && instead of & in vacuumparallel.c.Amit Kapila2022-08-22
| | | | | | | | | | | As such the current usage of & won't produce incorrect results but it would be better to use && to short-circuit the evaluation of second condition when the same is not required. Author: Ranier Vilela Reviewed-by: Tom Lane, Bharath Rupireddy Backpatch-through: 15, where it was introduced Discussion: https://postgr.es/m/CAEudQApL8QcoYwQuutkWKY_h7gBY8F0Xs34YKfc7-G0i83K_pw@mail.gmail.com
* Remove shadowed local variables that are new in v15David Rowley2022-08-20
| | | | | | | | | | | | | | | | | | | | | | | | | Compiling with -Wshadow=compatible-local yields quite a few warnings about local variables being shadowed by compatible local variables in an inner scope. Of course, this is perfectly valid in C, but we have had bugs in the past as a result of developers failing to notice this. af7d270dd is a recent example. Here we do a cleanup of warnings we receive from -Wshadow=compatible-local for code which is new to PostgreSQL 15. We've yet to have the discussion about if we actually ever want to run that as a standard compilation flag. We'll need to at least get the number of warnings down to something easier to manage before we can realistically consider if we want this or not. This commit is the first step towards reducing the warnings. The changes being made here are all fairly trivial. Because of that, and the fact that v15 is still in beta, this is being back-patched into 15. It seems more risky not to do this as the risk of future bugs is increased by the additional conflicts that this commit could cause for any future bug fixes touching the same areas as this commit. Author: Justin Pryzby Discussion: https://postgr.es/m/20220817145434.GC26426%40telsasoft.com Backpatch-through: 15
* Avoid reltuples distortion in very small tables.Peter Geoghegan2022-08-19
| | | | | | | | | | | | | | | | | | | | | | | Consistently avoid trusting a sample of only one page at the point that VACUUM determines a new reltuples for the target table (though only when the table is larger than a single page). This is follow-up work to commit 74388a1a, which added a heuristic to prevent reltuples from becoming distorted by successive VACUUM operations that each scan only a single heap page (which was itself more or less a bugfix for an issue in commit 44fa8488, which simplified VACUUM's handling of scanned pages). The original bugfix commit did not account for certain remaining cases that where not affected by its "2% of total relpages" heuristic. This happened with relations that are small enough that just one of its pages exceeded the 2% threshold, yet still big enough for VACUUM to deem skipping most of its pages via the visibility map worthwhile. reltuples could still become distorted over time with such a table, at least in scenarios where the VACUUM command is run repeatedly and without the table itself ever changing. Author: Peter Geoghegan <pg@bowt.ie> Discussion: https://postgr.es/m/CAH2-Wzk7d4m3oEbEWkWQKd+gz-eD_peBvdXVk1a_KBygXadFeg@mail.gmail.com Backpatch: 15-, where the rules for scanned pages changed.
* Initialize index stats during parallel VACUUM.Peter Geoghegan2022-08-18
| | | | | | | | | | | | | Initialize shared memory allocated for index stats to avoid a hard crash. This was possible when parallel VACUUM became confused about the current phase of index processing. Oversight in commit 8e1fae1938, which refactored parallel VACUUM. Author: Masahiko Sawada <sawada.mshk@gmail.com> Reported-By: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20220818133406.GL26426@telsasoft.com Backpatch: 15-, the first version with the refactoring commit.
* Fix subtly-incorrect matching of parent and child partitioned indexes.Tom Lane2022-08-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When creating a partitioned index, DefineIndex tries to identify any existing indexes on the partitions that match the partitioned index, so that it can absorb those as child indexes instead of building new ones. Part of the matching is to compare IndexInfo structs --- but that wasn't done quite right. We're comparing the IndexInfo built within DefineIndex itself to one made from existing catalog contents by BuildIndexInfo. Notably, while BuildIndexInfo will run index expressions and predicates through expression preprocessing, that has not happened to DefineIndex's struct. The result is failure to match and subsequent creation of duplicate indexes. The easiest and most bulletproof fix is to build a new IndexInfo using BuildIndexInfo, thereby guaranteeing that the processing done is identical. While here, let's also extract the opfamily and collation data from the new partitioned index, removing ad-hoc logic that duplicated knowledge about how those are constructed. Per report from Christophe Pettus. Back-patch to v11 where we invented partitioned indexes. Richard Guo and Tom Lane Discussion: https://postgr.es/m/8864BFAA-81FD-4BF9-8E06-7DEB8D4164ED@thebuild.com
* When using the WAL-logged CREATE DATABASE strategy, bulk extend.Robert Haas2022-08-18
| | | | | | | | | | This should improve performance, and was suggested by Andres Freund. Back-patch to v15 to keep the code consistent across branches. Dilip Kumar Discussion: http://postgr.es/m/C3458199-FEDD-4356-865A-08DFAA5D4065@anarazel.de Discussion: http://postgr.es/m/CAFiTN-sJ0vVpJrZ=R5M+g7Tr8=NN4wKOtrqOcDEsfFfnZgivVA@mail.gmail.com
* Simplify and clarify an error messagePeter Eisentraut2022-08-18
|
* Refer to replication origin roident as "ID" in user facing messages and docsJohn Naylor2022-08-18
| | | | | | | | | | | | | | The table column that stores this is of type oid, but is actually limited to uint16 and has a different path for creating new values. Some of the documentation already referred to it as an ID, so let's standardize on that. While at it, most format strings already use %u, so for consintency change the remaining stragglers using %d. Per suggestions from Tom Lane and Justin Pryzby Discussion: https://www.postgresql.org/message-id/3437166.1659620465%40sss.pgh.pa.us Backpatch to v15
* Allow event trigger table_rewrite for ALTER MATERIALIZED VIEWMichael Paquier2022-08-17
| | | | | | | | | | | This event can happen when using SET ACCESS METHOD, as the data files of the materialized need a full refresh but this command tag was not updated to reflect that. The documentation is updated to track this behavior. Author: Onder Kalaci Discussion: https://postgr.es/m/CACawEhXwHN3X34FiwoYG8vXR-oyUdrp7qcfRWSzS+NPahS5gSw@mail.gmail.com Backpatch-through: 15
* Fix assert in logicalmsg_descTomas Vondra2022-08-17
| | | | | | | | | | | The assert, introduced by 9f1cf97bb5, is intended to check if the prefix is terminated by a \0 byte, but it has two flaws. Firstly, prefix_size includes the \0 byte, so prefix[prefix_size] points to the byte after the null byte. Secondly, the check ensures the byte is not equal \0, while it should be checking the opposite. Backpatch-through: 14 Discussion: https://postgr.es/m/b99b6101-2f14-3796-3dfa-4a6cd7d4326d@enterprisedb.com
* Fix replica identity check for a partitioned table.Amit Kapila2022-08-16
| | | | | | | | | | | | | | The current publisher code checks if UPDATE or DELETE can be executed with the replica identity of the table even if it's a partitioned table. We can skip checking the replica identity for partitioned tables because the operations are actually performed on the leaf partitions (not the partitioned table). Reported-by: Brad Nicholson Author: Hou Zhijie Reviewed-by: Peter Smith, Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAMMnM%3D8i5DohH%3DYKzV0_wYuYSYvuOJoL9F5nzXTc%2ByzsG1f6rg%40mail.gmail.com
* Add missing bad-PGconn guards in libpq entry points.Tom Lane2022-08-15
| | | | | | | | | | | | | There's a convention that externally-visible libpq functions should check for a NULL PGconn pointer, and fail gracefully instead of crashing. PQflush() and PQisnonblocking() didn't get that memo though. Also add a similar check to PQdefaultSSLKeyPassHook_OpenSSL; while it's not clear that ordinary usage could reach that with a null conn pointer, it's cheap enough to check, so let's be consistent. Daniele Varrazzo and Tom Lane Discussion: https://postgr.es/m/CA+mi_8Zm_mVVyW1iNFgyMd9Oh0Nv8-F+7Y3-BqwMgTMHuo_h2Q@mail.gmail.com
* Fix outdated --help message for postgres -fMichael Paquier2022-08-15
| | | | | | | | | | This option switch supports a total of 8 values, as told by set_plan_disabling_options() and the documentation, but this was not reflected in the output generated by --help. Author: Junwang Zhao Discussion: https://postgr.es/m/CAEG8a3+pT3cWzyjzKs184L1XMNm8NDnoJLiSjAYSO7XqpRh_vA@mail.gmail.com Backpatch-through: 10
* Preserve memory context of VarStringSortSupport buffers.Tom Lane2022-08-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When enlarging the work buffers of a VarStringSortSupport object, varstrfastcmp_locale was careful to keep them in the ssup_cxt memory context; but varstr_abbrev_convert just used palloc(). The latter creates a hazard that the buffers could be freed out from under the VarStringSortSupport object, resulting in stomping on whatever gets allocated in that memory later. In practice, because we only use this code for ICU collations (cf. 3df9c374e), the problem is confined to use of ICU collations. I believe it may have been unreachable before the introduction of incremental sort, too, as traditional sorting usually just uses one context for the duration of the sort. We could fix this by making the broken stanzas in varstr_abbrev_convert match the non-broken ones in varstrfastcmp_locale. However, it seems like a better idea to dodge the issue altogether by replacing the pfree-and-allocate-anew coding with repalloc, which automatically preserves the chunk's memory context. This fix does add a few cycles because repalloc will copy the chunk's content, which the existing coding assumes is useless. However, we don't expect that these buffer enlargement operations are performance-critical. Besides that, it's far from obvious that copying the buffer contents isn't required, since these stanzas make no effort to mark the buffers invalid by resetting last_returned, cache_blob, etc. That seems to be safe upon examination, but it's fragile and could easily get broken in future, which wouldn't get revealed in testing with short-to-moderate-size strings. Per bug #17584 from James Inform. Whether or not the issue is reachable in the older branches, this code has been broken on its own terms from its introduction, so patch all the way back. Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
* Avoid misbehavior when hash_table_bytes < bucket_size.Tom Lane2022-08-13
| | | | | | | | | | | | | | | It's possible to reach this case when work_mem is very small and tupsize is (relatively) very large. In that case ExecChooseHashTableSize would get an assertion failure, or with asserts off it'd compute nbuckets = 0, which'd likely cause misbehavior later (I've not checked). To fix, clamp the number of buckets to be at least 1. This is due to faulty conversion of old my_log2() coding in 28d936031. Back-patch to v13, as that was. Zhang Mingli Discussion: https://postgr.es/m/beb64ca0-91e2-44ac-bf4a-7ea36275ec02@Spark
* Catch stack overflow when recursing in transformFromClauseItem().Tom Lane2022-08-13
| | | | | | | | | | | | | | | Most parts of the parser can expect that the stack overflow check in transformExprRecurse() will trigger before things get desperate. However, transformFromClauseItem() can recurse directly to self without having analyzed any expressions, so it's possible to drive it to a stack-overrun crash. Add a check to prevent that. Per bug #17583 from Egor Chindyaskin. Back-patch to all supported branches. Richard Guo Discussion: https://postgr.es/m/17583-33be55b9f981f75c@postgresql.org
* Add missing fields to _outConstraint()Peter Eisentraut2022-08-13
| | | | | | | | As of 897795240cfaaed724af2f53ed2c50c9862f951f, check constraints can be declared invalid. But that patch didn't update _outConstraint() to also show the relevant struct fields (which were only applicable to foreign keys before that). This currently only affects debugging output, so no impact in practice.
* pg_upgrade: Fix some minor code issuesPeter Eisentraut2022-08-13
| | | | | | | | | | | | 96ef3b8ff1cf1950e897fd2f766d4bd9ef0d5d56 accidentally copied a not applicable comment from the float8_pass_by_value code to the data_checksums code. Remove that. 87d3b35a1ca31a9d947a8f919a6006679216dff0 changed pg_upgrade to checking the checksum version rather than just the Boolean presence of checksums, but didn't change the field type in its ControlData struct from bool. So this would not work correctly if there ever is a checksum version larger than 1.
* Avoid using a fake relcache entry to own an SmgrRelation.Robert Haas2022-08-12
| | | | | | | | | | | | | | | | | | | | If an error occurs before we close the fake relcache entry, the the fake relcache entry will be destroyed by the SmgrRelation will survive until end of transaction. Its smgr_owner pointer ends up pointing to already-freed memory. The original reason for using a fake relcache entry here was to try to avoid reusing an SMgrRelation across a relevant invalidation. To avoid that problem, just call smgropen() again each time we need a reference to it. Hopefully someday we will come up with a more elegant approach, but accessing uninitialized memory is bad so let's do this for now. Dilip Kumar, reviewed by Andres Freund and Tom Lane. Report by Justin Pryzby. Discussion: http://postgr.es/m/20220802175043.GA13682@telsasoft.com Discussion: http://postgr.es/m/CAFiTN-vSFeE6_W9z698XNtFROOA_nSqUXWqLcG0emob_kJ+dEQ@mail.gmail.com
* Reject MERGE in CTEs and COPYAlvaro Herrera2022-08-12
| | | | | | | | | | | | The grammar added for MERGE inadvertently made it accepted syntax in places that were not prepared to deal with it -- namely COPY and inside CTEs, but invoking these things with MERGE currently causes assertion failures or weird misbehavior in non-assertion builds. Protect those places by checking for it explicitly until somebody decides to implement it. Reported-by: Alexey Borzov <borz_off@cs.msu.su> Discussion: https://postgr.es/m/17579-82482cd7b267b862@postgresql.org
* Fix _outConstraint() for "identity" constraintsPeter Eisentraut2022-08-12
| | | | | | | | | The set of fields printed by _outConstraint() in the CONSTR_IDENTITY case didn't match the set of fields actually used in that case. (The code was probably uncarefully copied from the CONSTR_DEFAULT case.) Fix that by using the right set of fields. Since there is no read support for this node type, this is really just for debugging output right now, so it doesn't affect anything important.
* Back-Patch "Add wait_for_subscription_sync for TAP tests."Amit Kapila2022-08-12
| | | | | | | | | | | | | | | This was originally done in commit 0c20dd33db for 16 only, to eliminate duplicate code and as an infrastructure that makes it easier to write future tests. However, it has been suggested that it would be good to back-patch this testing infrastructure to aid future tests in back-branches. Backpatch to all supported versions. Author: Masahiko Sawada Reviewed by: Amit Kapila, Shi yu Discussion: https://postgr.es/m/CAD21AoC-fvAkaKHa4t1urupwL8xbAcWRePeETvshvy80f6WV1A@mail.gmail.com Discussion: https://postgr.es/m/E1oJBIf-0006sw-SA@gemulon.postgresql.org
* Add missing space in _outA_Const() outputPeter Eisentraut2022-08-11
| | | | Mistake introduced by 639a86e36aaecb84faaf941dcd0b183ba0aba9e9.
* Fix catalog lookup with the wrong snapshot during logical decoding.Amit Kapila2022-08-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we relied on HEAP2_NEW_CID records and XACT_INVALIDATION records to know if the transaction has modified the catalog, and that information is not serialized to snapshot. Therefore, after the restart, if the logical decoding decodes only the commit record of the transaction that has actually modified a catalog, we will miss adding its XID to the snapshot. Thus, we will end up looking at catalogs with the wrong snapshot. To fix this problem, this changes the snapshot builder so that it remembers the last-running-xacts list of the decoded RUNNING_XACTS record after restoring the previously serialized snapshot. Then, we mark the transaction as containing catalog changes if it's in the list of initial running transactions and its commit record has XACT_XINFO_HAS_INVALS. To avoid ABI breakage, we store the array of the initial running transactions in the static variables InitialRunningXacts and NInitialRunningXacts, instead of storing those in SnapBuild or ReorderBuffer. This approach has a false positive; we could end up adding the transaction that didn't change catalog to the snapshot since we cannot distinguish whether the transaction has catalog changes only by checking the COMMIT record. It doesn't have the information on which (sub) transaction has catalog changes, and XACT_XINFO_HAS_INVALS doesn't necessarily indicate that the transaction has catalog change. But that won't be a problem since we use snapshot built during decoding only to read system catalogs. On the master branch, we took a more future-proof approach by writing catalog modifying transactions to the serialized snapshot which avoids the above false positive. But we cannot backpatch it because of a change in the SnapBuild. Reported-by: Mike Oh Author: Masahiko Sawada Reviewed-by: Amit Kapila, Shi yu, Takamichi Osumi, Kyotaro Horiguchi, Bertrand Drouvot, Ahsan Hadi Backpatch-through: 10 Discussion: https://postgr.es/m/81D0D8B0-E7C4-4999-B616-1E5004DBDCD2%40amazon.com
* Move basebackup code to new directory src/backend/backupRobert Haas2022-08-10
| | | | | | Reviewed by David Steele and Justin Pryzby Discussion: http://postgr.es/m/CA+TgmoafqboATDSoXHz8VLrSwK_MDhjthK4hEpYjqf9_1Fmczw%40mail.gmail.com
* Fix handling of R/W expanded datums that are passed to SQL functions.Tom Lane2022-08-10
| | | | | | | | | | | | | | | | | | | fmgr_sql must make expanded-datum arguments read-only, because it's possible that the function body will pass the argument to more than one callee function. If one of those functions takes the datum's R/W property as license to scribble on it, then later callees will see an unexpected value, leading to wrong answers. From a performance standpoint, it'd be nice to skip this in the common case that the argument value is passed to only one callee. However, detecting that seems fairly hard, and certainly not something that I care to attempt in a back-patched bug fix. Per report from Adam Mackler. This has been broken since we invented expanded datums, so back-patch to all supported branches. Discussion: https://postgr.es/m/WScDU5qfoZ7PB2gXwNqwGGgDPmWzz08VdydcPFLhOwUKZcdWbblbo-0Lku-qhuEiZoXJ82jpiQU4hOjOcrevYEDeoAvz6nR0IU4IHhXnaCA=@mackler.email Discussion: https://postgr.es/m/187436.1660143060@sss.pgh.pa.us
* Fix typo in test_oat_hooks READMEDaniel Gustafsson2022-08-10
| | | | Discussion: https://postgr.es/m/3F066AFE-19F9-4DF5-A498-B09643857A39@yesql.se
* Stabilize output of new regression test.Tom Lane2022-08-08
| | | | | | | | | | Per buildfarm, the output order of \dx+ isn't consistent across locales. Apply NO_LOCALE to force C locale. There might be a more localized way, but I'm not seeing it offhand, and anyway there is nothing in this test module that particularly cares about locales. Security: CVE-2022-2625