aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Remove size increase in ExprEvalStep caused by hashed saopsDavid Rowley2022-07-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 50e17ad28 increased the size of ExprEvalStep from 64 bytes up to 88 bytes. Lots of effort was spent during the development of the current expression evaluation code to make an instance of this struct as small as possible. Making this struct larger than needed reduces CPU cache efficiency during expression evaluation which causes noticeable slowdowns during query execution. In order to reduce the size of the struct, here we remove the fn_addr field. The values from this field can be obtained via fcinfo, just with some extra pointer dereferencing. The extra indirection does not seem to cause any noticeable slowdowns. Various other fields have been moved into the ScalarArrayOpExprHashTable struct. These fields are only used when the ScalarArrayOpExprHashTable pointer has already been dereferenced, so no additional pointer dereferences occur for these. Here we also make hash_fcinfo_data the last field in ScalarArrayOpExprHashTable so that we can avoid a further pointer dereference to get the FunctionCallInfoBaseData. This also saves a call to palloc(). 50e17ad28 was added in 14, but it's too late to adjust the size of the ExprEvalStep in that version, so here we just backpatch to 15, which is currently in beta. Author: Andres Freund, David Rowley Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Backpatch-through: 15
* Refactor sending of DataRow messages in replication protocolPeter Eisentraut2022-07-06
| | | | | | | | | | | | Some routines open-coded the construction of DataRow messages. Use TupOutputState struct and associated functions instead, which was already done in some places. SendTimeLineHistory() is a bit more complicated and isn't converted by this. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/7e4fdbdc-699c-4cd0-115d-fb78a957fc22@enterprisedb.com
* autho_explain: Add GUC to log query parametersMichael Paquier2022-07-06
| | | | | | | | | | | | | auto_explain.log_parameter_max_length is a new GUC part of the extension, similar to the corresponding core setting, that controls the inclusion of query parameters in the logged explain output. More tests are added to check the behavior of this new parameter: when parameters logged in full (the default of -1), when disabled (value of 0) and when partially truncated (value different than the two others). Author: Dagfinn Ilmari Mannsåker Discussion: https://postgr.es/m/87ee09mohb.fsf@wibble.ilmari.org
* pgstat: reduce timer overhead by leaving timer running.Andres Freund2022-07-05
| | | | | | | | | | | | | | | | | | | | Previously the timer was enabled whenever there were any pending stats after executing a statement, just to then be disabled again when not idle anymore. That lead to an increase in GetCurrentTimestamp() calls from within timeout.c compared to 14. To avoid that increase, leave the timer enabled until stats are reported, rather than until idle. The timer is only disabled once the pending stats have been reported. For me this fixes the increase in GetCurrentTimestamp() calls, there now are fewer calls in 15 than in 14, in the previously slowed down workload. While at it, also update assertion in pgstat_report_stat() to be more precise. Author: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Backpatch: 15-
* expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size.Andres Freund2022-07-05
| | | | | | | | | | | | | | The new expression step types increased the size of ExprEvalStep by ~4 for all types of expression steps, slowing down expression evaluation noticeably. Move them out of line. There's other issues with these expression steps, but addressing them is largely independent of this aspect. Author: Andres Freund <andres@anarazel.de> Reviewed-By: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de Backpatch: 15-
* Revert 019_replslot_limit.pl related debugging aids.Andres Freund2022-07-05
| | | | | | | | | | | | | | This reverts most of 91c0570a791, f28bf667f60, fe0972ee5e6, afdeff10526. The only thing left is the retry loop in 019_replslot_limit.pl that avoids spurious failures by retrying a couple times. We haven't seen any hard evidence that this is caused by anything but slow process shutdown. We did not find any cases where walsenders did not vanish after waiting for longer. Therefore there's no reason for this debugging code to remain. Discussion: https://postgr.es/m/20220530190155.47wr3x2prdwyciah@alap3.anarazel.de Backpatch: 15-
* Rename pg_checkpointer predefined role to pg_checkpoint.Robert Haas2022-07-05
| | | | | | | | | This is more consistent with how other predefined roles that confer specific privileges are named. Nathan Bosart Discussion: http://postgr.es/m/CA+TgmoatH7+yYe+A8uJFNogg3VUDtFE6c-77yHAY8TRWR7oqyw@mail.gmail.com
* Fix errors in copyfuncs/equalfuncs support for JSON node types.Tom Lane2022-07-05
| | | | | | | | | | | Noted while comparing existing code to the output of the proposed patch to automate creation of these functions. Some of the changes are just cosmetic, but others represent real bugs. I've not attempted to analyze the user-visible impact. Back-patch to v15 where this code came in. Discussion: https://postgr.es/m/1794155.1656984188@sss.pgh.pa.us
* Fix pg_prepared_statements.result_types for DML statementsPeter Eisentraut2022-07-05
| | | | | | | | Amendment to 84ad713cf85aeffee5dd39f62d49a1b9e34632da: Not all prepared statements have a result descriptor. As currently coded, this would crash when reading pg_prepared_statements. Make those cases return null for result_types instead. Also add a test case for it.
* Add result_types column to pg_prepared_statements viewPeter Eisentraut2022-07-05
| | | | | | | | | | Containing the types of the columns returned by the prepared statement. Prompted by question from IRC user mlvzk. Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> Discussion: https://www.postgresql.org/message-id/flat/871qwpo7te.fsf@wibble.ilmari.org
* Remove durable_rename_excl()Michael Paquier2022-07-05
| | | | | | | | | | | | A previous commit replaced all the calls to this function with durable_rename() as of dac1ff3, making it used nowhere in the tree. Using it in extension code is also risky based on the issues described in this previous commit, so let's remove it. This makes possible the removal of HAVE_WORKING_LINK. Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13
* Replace durable_rename_excl() by durable_rename(), take twoMichael Paquier2022-07-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | durable_rename_excl() attempts to avoid overwriting any existing files by using link() and unlink(), and it falls back to rename() on some platforms (aka WIN32), which offers no such overwrite protection. Most callers use durable_rename_excl() just in case there is an existing file, but in practice there shouldn't be one (see below for more details). Furthermore, failures during durable_rename_excl() can result in multiple hard links to the same file. As per Nathan's tests, it is possible to end up with two links to the same file in pg_wal after a crash just before unlink() during WAL recycling. Specifically, the test produced links to the same file for the current WAL file and the next one because the half-recycled WAL file was re-recycled upon restarting, leading to WAL corruption. This change replaces all the calls of durable_rename_excl() to durable_rename(). This removes the protection against accidentally overwriting an existing file, but some platforms are already living without it and ordinarily there shouldn't be one. The function itself is left around in case any extensions are using it. It will be removed on HEAD via a follow-up commit. Here is a summary of the existing callers of durable_rename_excl() (see second discussion link at the bottom), replaced by this commit. First, basic_archive used it to avoid overwriting an archive concurrently created by another server, but as mentioned above, it will still overwrite files on some platforms. Second, xlog.c uses it to recycle past WAL segments, where an overwrite should not happen (origin of the change at f0e37a8) because there are protections about the WAL segment to select when recycling an entry. The third and last area is related to the write of timeline history files. writeTimeLineHistory() will write a new timeline history file at the end of recovery on promotion, so there should be no such files for the same timeline. What remains is writeTimeLineHistoryFile(), that can be used in parallel by a WAL receiver and the startup process, and some digging of the buildfarm shows that EEXIST from a WAL receiver can happen with an error of "could not link file \"pg_wal/xlogtemp.NN\" to \"pg_wal/MM.history\", which would cause an automatic restart of the WAL receiver as it is promoted to FATAL, hence this should improve the stability of the WAL receiver as rename() would overwrite an existing TLI history file already fetched by the startup process at recovery. This is a bug fix, but knowing the unlikeliness of the problem involving one or more crashes at an exceptionally bad moment, no backpatch is done. Also, I want to be careful with such changes (aaa3aed did the opposite of this change by removing HAVE_WORKING_LINK so as Windows would do a link() rather than a rename() but this was not concurrent-safe). A backpatch could be revisited in the future. This is the second time this change is attempted, ccfbd92 being the first one, but this time no assertions are added for the case of a TLI history file written concurrently by the WAL receiver or the startup process because we can expect one to exist (some of the TAP tests are able to trigger with a proper timing). Author: Nathan Bossart Reviewed-by: Robert Haas, Kyotaro Horiguchi, Michael Paquier Discussion: https://postgr.es/m/20220407182954.GA1231544@nathanxps13 Discussion: https://postgr.es/m/Ym6GZbqQdlalSKSG@paquier.xyz
* Refactor sending of RowDescription messages in replication protocolPeter Eisentraut2022-07-04
| | | | | | | | | Some routines open-coded the construction of RowDescription messages. Instead, we have support for doing this using tuple descriptors and DestRemoteSimple, so use that instead. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/7e4fdbdc-699c-4cd0-115d-fb78a957fc22@enterprisedb.com
* Implement List support for TransactionIdAlvaro Herrera2022-07-04
| | | | | | | | | | | | Use it for RelationSyncEntry->streamed_txns, which is currently using an integer list. The API support is not complete, not because it is hard to write but because it's unclear that it's worth the code space, there being so little use of XID lists. Discussion: https://postgr.es/m/202205130830.g5ntonhztspb@alvherre.pgsql Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
* Prevent write operations on large objects in read-only transactionsMichael Paquier2022-07-04
| | | | | | | | | | | | | | | | Attempting such an operation would already fail, but in various and confusing ways. For example, while in recovery, some elog() messages would be reported, but these should never be user-facing. This commit restricts any write operations done on large objects in a read-only context, so as the errors generated are more user-friendly. This is per the discussion done with Tom Lane and Robert Haas. Some regression tests are added to check the case of all the SQL functions working on large objects (including an update of the test's alternate output). Author: Yugo Nagata Discussion: https://postgr.es/m/20220527153028.61a4608f66abcd026fd3806f@sraoss.co.jp
* Fix for change timeline field of IDENTIFY_SYSTEM to int8Peter Eisentraut2022-07-04
| | | | | | Amendment to ec40f3422412cfdc140b5d3f67db7fd2dac0f1e2: We also need to change the way the datum is supplied to int8. Otherwise, the value is still cut off as an int4, and it will crash on 32-bit platforms.
* Change timeline field of IDENTIFY_SYSTEM to int8Peter Eisentraut2022-07-04
| | | | | | | | It was int4, but in the other replication commands, timelines are returned as int8. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/7e4fdbdc-699c-4cd0-115d-fb78a957fc22@enterprisedb.com
* Fix attlen in RowDescription of BASE_BACKUP responsePeter Eisentraut2022-07-04
| | | | | | | Should be 8 for int8, not -1. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/7e4fdbdc-699c-4cd0-115d-fb78a957fc22@enterprisedb.com
* Remove %error-verbose directive from jsonpath parserAndrew Dunstan2022-07-03
| | | | | | | | | | | None of the other bison parsers contains this directive, and it gives rise to some unfortunate and impenetrable messages, so just remove it. Backpatch to release 12, where it was introduced. Per gripe from Erik Rijkers Discussion: https://postgr.es/m/ba069ce2-a98f-dc70-dc17-2ccf2a9bf7c7@xs4all.nl
* Allow makeaclitem() to accept multiple privilege names.Tom Lane2022-07-03
| | | | | | | | | | | | Interpret its privileges argument as a comma-separated list of privilege names, as in has_table_privilege and other functions. This is actually net less code, since the support routine to parse that already exists, and we can drop convert_priv_string() which had no other use-case. Robins Tharakan Discussion: https://postgr.es/m/e5a05dc54ba64408b3dd260171c1abaf@EX13D05UWC001.ant.amazon.com
* Remove redundant null pointer checks before free()Peter Eisentraut2022-07-03
| | | | | | | | | | Per applicable standards, free() with a null pointer is a no-op. Systems that don't observe that are ancient and no longer relevant. Some PostgreSQL code already required this behavior, so this change does not introduce any new requirements, just makes the code more consistent. Discussion: https://www.postgresql.org/message-id/flat/dac5d2d0-98f5-94d9-8e69-46da2413593d%40enterprisedb.com
* Emit debug message when executing extension script.Jeff Davis2022-07-02
| | | | | | | | Allows extension authors to more easily debug problems related to the sequence of update scripts that are executed. Discussion: https://postgr.es/m/5636a7534a4833884172fe4369d825b26170b3cc.camel%40j-davis.com Reviewed-by: Peter Eisentraut, Nathan Bossart
* Default to dynamic_shared_memory_type=sysv on Solaris.Thomas Munro2022-07-02
| | | | | | | | | | | | | | | | | POSIX shm_open() can sleep for a long time and fail spuriously because of contention on an internal lock file on Solaris (and presumably illumos). Commit 389869af fixed the main problem with this, namely that we could crash, but it's now clear that "posix" is not a good default. Therefore, choose "sysv" at initdb time on Solaris and illumos. Other choices are still available by editing the postgresql.conf file. Back-patch only to 15, because contention is much less likely further back, and it doesn't seem like a good idea to change this in released branches. This should clear up the failures on build farm animal margay. Discussion: https://postgr.es/m/CA%2BhUKGKqKrCV5xKWfh9rnm%3Do%3DDwZLTLtnsj_XpUi9g5%3DV%2B9oyg%40mail.gmail.com
* Remove no-longer-used parameter for create_groupingsets_path().Tom Lane2022-07-01
| | | | | | | | numGroups is unused since commit b5635948a; let's get rid of it. XueJing Zhao, reviewed by Richard Guo Discussion: https://postgr.es/m/DM6PR05MB64923CC8B63A2CAF3B2E5D47B7AD9@DM6PR05MB6492.namprd05.prod.outlook.com
* Add construct_array_builtin, deconstruct_array_builtinPeter Eisentraut2022-07-01
| | | | | | | | | | | | | | | There were many calls to construct_array() and deconstruct_array() for built-in types, for example, when dealing with system catalog columns. These all hardcoded the type attributes necessary to pass to these functions. To simplify this a bit, add construct_array_builtin(), deconstruct_array_builtin() as wrappers that centralize this hardcoded knowledge. This simplifies many call sites and reduces the amount of hardcoded stuff that is spread around. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://www.postgresql.org/message-id/flat/2914356f-9e5f-8c59-2995-5997fc48bcba%40enterprisedb.com
* Harden dsm_impl.c against unexpected EEXIST.Thomas Munro2022-07-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, we trusted the OS not to report EEXIST unless we'd passed in IPC_CREAT | IPC_EXCL or O_CREAT | O_EXCL, as appropriate. Solaris's shm_open() can in fact do that, causing us to crash because we didn't ereport and then we blithely assumed the mapping was successful. Let's treat EEXIST just like any other error, unless we're actually trying to create a new segment. This applies to shm_open(), where this behavior has been seen, and also to the equivalent operations for our sysv and mmap modes just on principle. Based on the underlying reason for the error, namely contention on a lock file managed by Solaris librt for each distinct name, this problem is only likely to happen on 15 and later, because the new shared memory stats system produces shm_open() calls for the same path from potentially large numbers of backends concurrently during authentication. Earlier releases only shared memory segments between a small number of parallel workers under one Gather node. You could probably hit it if you tried hard enough though, and we should have been more defensive in the first place. Therefore, back-patch to all supported releases. Per build farm animal margay. This isn't the end of the story, though, it just changes random crashes into random "File exists" errors; more work needed for a green build farm. Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGKqKrCV5xKWfh9rnm%3Do%3DDwZLTLtnsj_XpUi9g5%3DV%2B9oyg%40mail.gmail.com
* Fix code comments still referring to pg_start/stop_backup()Michael Paquier2022-07-01
| | | | | | | | | 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
* Change some unnecessary MemSet callsPeter Eisentraut2022-07-01
| | | | | | | | | | MemSet() with a value other than 0 just falls back to memset(), so the indirection is unnecessary if the value is constant and not 0. Since there is some interest in getting rid of MemSet(), this gets some easy cases out of the way. (There are a few MemSet() calls that I didn't change to maintain the consistency with their surrounding code.) Discussion: https://www.postgresql.org/message-id/flat/CAEudQApCeq4JjW1BdnwU=m=-DvG5WyUik0Yfn3p6UNphiHjj+w@mail.gmail.com
* Avoid unnecessary MemSet callPeter Eisentraut2022-06-30
| | | | | | | | | | The variable in question was changed from a struct to a pointer some time ago (77947c51c08). Using MemSet to zero it still works but is obviously unidiomatic and confusing, so change it to a straight assignment. Author: Ranier Vilela <ranier.vf@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAEudQApCeq4JjW1BdnwU=m=-DvG5WyUik0Yfn3p6UNphiHjj+w@mail.gmail.com
* pgindent run prior to branching v15.Tom Lane2022-06-30
| | | | pgperltidy and reformat-dat-files too. Not many changes.
* Translation updatesPeter Eisentraut2022-06-27
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 46c120873f1e906cc8dab74d8d756417e1b367f6
* Fix visibility check when XID is committed in CLOG but not in procarray.Heikki Linnakangas2022-06-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | TransactionIdIsInProgress had a fast path to return 'false' if the single-item CLOG cache said that the transaction was known to be committed. However, that was wrong, because a transaction is first marked as committed in the CLOG but doesn't become visible to others until it has removed its XID from the proc array. That could lead to an error: ERROR: t_xmin is uncommitted in tuple to be updated or for an UPDATE to go ahead without blocking, before the previous UPDATE on the same row was made visible. The window is usually very short, but synchronous replication makes it much wider, because the wait for synchronous replica happens in that window. Another thing that makes it hard to hit is that it's hard to get such a commit-in-progress transaction into the single item CLOG cache. Normally, if you call TransactionIdIsInProgress on such a transaction, it determines that the XID is in progress without checking the CLOG and without populating the cache. One way to prime the cache is to explicitly call pg_xact_status() on the XID. Another way is to use a lot of subtransactions, so that the subxid cache in the proc array is overflown, making TransactionIdIsInProgress rely on pg_subtrans and CLOG checks. This has been broken ever since it was introduced in 2008, but the race condition is very hard to hit, especially without synchronous replication. There were a couple of reports of the error starting from summer 2021, but no one was able to find the root cause then. TransactionIdIsKnownCompleted() is now unused. In 'master', remove it, but I left it in place in backbranches in case it's used by extensions. Also change pg_xact_status() to check TransactionIdIsInProgress(). Previously, it only checked the CLOG, and returned "committed" before the transaction was actually made visible to other queries. Note that this also means that you cannot use pg_xact_status() to reproduce the bug anymore, even if the code wasn't fixed. Report and analysis by Konstantin Knizhnik. Patch by Simon Riggs, with the pg_xact_status() change added by me. Author: Simon Riggs Reviewed-by: Andres Freund Discussion: https://www.postgresql.org/message-id/flat/4da7913d-398c-e2ad-d777-f752cf7f0bbb%40garret.ru
* Fix relptr's encoding of the base address.Thomas Munro2022-06-27
| | | | | | | | | | | | | | | | | Previously, we encoded both NULL and the first byte at the base address as 0. That confusion led to the assertion in commit e07d4ddc, which failed when min_dynamic_shared_memory was used. Give them distinct encodings, by switching to 1-based offsets for non-NULL pointers. Also improve macro hygiene in passing (missing/misplaced parentheses), and remove open-coded access to the raw offset value from freepage.c/h. Although e07d4ddc was back-patched to 10, the only code that actually makes use of relptr at the base address arrived in 84b1c63a, so no need to back-patch further than 14 for now. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/20220519193839.GT19626%40telsasoft.com
* Harden range_table_mutator() against null RangeTblEntry.subquery.Tom Lane2022-06-26
| | | | | | | | | | | | | | | | | | | | Commit 64919aaab made pull_up_simple_subquery set rte->subquery = NULL after doing the deed, so that we don't waste cycles copying a now-useless subquery tree around. This turns out to create a core dump hazard in range_table_mutator, which supposes that that field is never NULL. Apparently none of our own code invokes query_tree_mutator or range_table_mutator on the top Query after subquery pullup; but it wouldn't be surprising if outside code does, and anyway I'm working on a v16 patch that will need it. We can fix this cleanly by just getting rid of the special-case handling of this field and treating it more like all the rest. I think the special case might be left over from a time when QTW_DONT_COPY_QUERY was the default behavior, but that was eons ago. Thanks to Dean Rasheed for review. Discussion: https://postgr.es/m/545569.1656107045@sss.pgh.pa.us
* Don't trust signalfd() on illumos.Thomas Munro2022-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | Since commit 6a2a70a02, we've used signalfd() to receive latch wakeups when building with WAIT_USE_EPOLL (default for Linux and illumos), and our traditional self-pipe when falling back to WAIT_USE_POLL (default for other Unixes with neither epoll() nor kqueue()). Unexplained hangs and kernel panics have been reported on illumos systems, apparently linked to this use of signalfd(), leading illumos users and build farm members to have to define WAIT_USE_POLL explicitly as a work-around. A bug report exists at https://www.illumos.org/issues/13700 but no fix is available yet. Let's provide a way for illumos users to go back to self-pipes with epoll(), like releases before 14, and choose that by default. No change for Linux users. To help with development/debugging, macros WAIT_USE_{EPOLL,POLL} and WAIT_USE_{SIGNALFD,SELF_PIPE} can be defined explicitly to override the defaults. Back-patch to 14, where we started using signalfd(). Reported-by: Japin Li <japinli@hotmail.com> Reported-by: Olaf Bohlen <olbohlen@eenfach.de> (off-list) Reviewed-by: Japin Li <japinli@hotmail.com> Discussion: https://postgr.es/m/MEYP282MB1669C8D88F0997354C2313C1B6CA9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
* CREATE INDEX: use the original userid for more ACL checks.Noah Misch2022-06-25
| | | | | | | | | | | | | Commit a117cebd638dd02e5c2e791c25e43745f233111b used the original userid for ACL checks located directly in DefineIndex(), but it still adopted the table owner userid for more ACL checks than intended. That broke dump/reload of indexes that refer to an operator class, collation, or exclusion operator in a schema other than "public" or "pg_catalog". Back-patch to v10 (all supported versions), like the earlier commit. Nathan Bossart and Noah Misch Discussion: https://postgr.es/m/f8a4105f076544c180a87ef0c4822352@stmuk.bayern.de
* Fix typo in pg_publication.cMichael Paquier2022-06-23
| | | | | Author: Peter Smith Discussion: https://postgr.es/m/CAHut+PuV2XXjC4spHXy_EOhpD6MDrmmDMWnVJLYpd1_P=2+mJw@mail.gmail.com
* Fix memory leak due to LogicalRepRelMapEntry.attrmap.Amit Kapila2022-06-23
| | | | | | | | | | | | | | When rebuilding the relation mapping on subscribers, we were not releasing the attribute mapping's memory which was no longer required. The attribute mapping used in logical tuple conversion was refactored in PG13 (by commit e1551f96e6) but we forgot to update the related code that frees the attribute map. Author: Hou Zhijie Reviewed-by: Amit Langote, Amit Kapila, Shi yu Backpatch-through: 10, where it was introduced Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Fix two issues with HEADER MATCH in COPYMichael Paquier2022-06-23
| | | | | | | | | | | | | | | | | | | | 072132f0 used the attnum offset to access the raw_fields array when checking that the attribute names of the header and of the relation match, leading to incorrect results or even crashes if the attribute numbers of a relation are changed, like on a dropped attribute. This fixes the logic to use the correct attribute names for the header matching requirements. Also, this commit disallows HEADER MATCH in COPY TO as there is no validation that can be done in this case. The tests are expanded for HEADER MATCH with COPY FROM and dropped columns, with cases where a relation has a dropped and re-added column, as well as a reduced set of columns. Author: Julien Rouhaud Reviewed-by: Peter Eisentraut, Michael Paquier Discussion: https://postgr.es/m/20220607154744.vvmitnqhyxrne5ms@jrouhaud
* pgstat: Mention pgstat_replslot.c in pgstat.c.Andres Freund2022-06-22
| | | | | | | Oversight, by me, in commit 5891c7a8ed8. Author: "Drouvot, Bertrand" <bdrouvot@amazon.com> Discussion: https://postgr.es/m/bd58e027-6598-57a2-679b-d576d17bfaa9@amazon.com
* Fix stale values in partition map entries on subscribers.Amit Kapila2022-06-21
| | | | | | | | | | | | | | | | | | We build the partition map entries on subscribers while applying the changes for update/delete on partitions. The component relation in each entry is closed after its use so we need to update it on successive use of cache entries. This problem was there since the original commit f1ac27bfda that introduced this code but we didn't notice it till the recent commit 26b3455afa started to use the component relation of partition map cache entry. Reported-by: Tom Lane, as per buildfarm Author: Amit Langote, Hou Zhijie Reviewed-by: Amit Kapila, Shi Yu Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Fix partition table's REPLICA IDENTITY checking on the subscriber.Amit Kapila2022-06-21
| | | | | | | | | | | | | | | | | | | | In logical replication, we will check if the target table on the subscriber is updatable by comparing the replica identity of the table on the publisher with the table on the subscriber. When the target table is a partitioned table, we only check its replica identity but not for the partition tables. This leads to assertion failure while applying changes for update/delete as we expect those to succeed only when the corresponding partition table has a primary key or has a replica identity defined. Fix it by checking the replica identity of the partition table while applying changes. Reported-by: Shi Yu Author: Shi Yu, Hou Zhijie Reviewed-by: Amit Langote, Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Revert changes in HOT handling of BRIN indexesTomas Vondra2022-06-16
| | | | | | | | | | | | | | | | | | | | | | | | This reverts commits 5753d4ee32 and fe60b67250 that modified HOT to ignore BRIN indexes. The commit message for 5753d4ee32 claims that: When determining whether an index update may be skipped by using HOT, we can ignore attributes indexed only by BRIN indexes. There are no index pointers to individual tuples in BRIN, and the page range summary will be updated anyway as it relies on visibility info. This is partially incorrect - it's true BRIN indexes don't point to individual tuples, so HOT chains are not an issue, but the visibitlity info is not sufficient to keep the index up to date. This can easily result in corrupted indexes, as demonstrated in the hackers thread. This does not mean relaxing the HOT restrictions for BRIN is a lost cause, but it needs to handle the two aspects (allowing HOT chains and updating the page range summaries) as separate. But that requires a major changes, and it's too late for that in the current dev cycle. Reported-by: Tomas Vondra Discussion: https://postgr.es/m/05ebcb44-f383-86e3-4f31-0a97a55634cf@enterprisedb.com
* Fix data inconsistency between publisher and subscriber.Amit Kapila2022-06-16
| | | | | | | | | | | | | | | | We were not updating the partition map cache in the subscriber even when the corresponding remote rel is changed. Due to this data was getting incorrectly replicated for partition tables after the publisher has changed the table schema. Fix it by resetting the required entries in the partition map cache after receiving a new relation mapping from the publisher. Reported-by: Shi Yu Author: Shi Yu, Hou Zhijie Reviewed-by: Amit Langote, Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Fix cache look-up failures while applying changes in logical replication.Amit Kapila2022-06-15
| | | | | | | | | | | | | | | | | | While building a new attrmap which maps partition attribute numbers to remoterel's, we incorrectly update the map for dropped column attributes. Later, it caused cache look-up failure when we tried to use the map to fetch the information about attributes. This also fixes the partition map cache invalidation which was using the wrong type cast to fetch the entry. We were using stale partition map entry after invalidation which leads to the assertion or cache look-up failure. Reported-by: Shi Yu Author: Hou Zhijie, Shi Yu Reviewed-by: Amit Langote, Amit Kapila Backpatch-through: 13, where it was introduced Discussion: https://postgr.es/m/OSZPR01MB6310F46CD425A967E4AEF736FDA49@OSZPR01MB6310.jpnprd01.prod.outlook.com
* 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 collation of JSON_TABLE output columnsPeter Eisentraut2022-06-10
| | | | | | | | | The output columns of JSON_TABLE should have the collations of their data type. The existing implementation sets the default collation if the type is collatable. Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://www.postgresql.org/message-id/flat/9d75ce67-0121-5050-5bec-bf5009db55ce%40enterprisedb.com
* Improve comments for trivial_subqueryscan().Etsuro Fujita2022-06-09
| | | | | | | | | | | | | | | | This function can be called from mark_async_capable_plan(), a helper function for create_append_plan(), before set_subqueryscan_references(), to determine the triviality of a SubqueryScan that is a child of an Append plan node, which is done before doing finalize_plan() on the SubqueryScan (if necessary) and set_plan_references() on the subplan, unlike when called from set_subqueryscan_references(). The reason why this is safe wouldn't be that obvious, so add comments explaining this. Follow-up for commit c2bb02bc2. Reviewed by Zhihong Yu. Discussion: https://postgr.es/m/CAPmGK17%2BGiJBthC6va7%2B9n6t75e-M1N0U18YB2G1B%2BE5OdrNTA%40mail.gmail.com
* Be more careful about GucSource for internally-driven GUC settings.Tom Lane2022-06-08
| | | | | | | | | | | | | | | | | | | | | | | The original advice for hard-wired SetConfigOption calls was to use PGC_S_OVERRIDE, particularly for PGC_INTERNAL GUCs. However, that's really overkill for PGC_INTERNAL GUCs, since there is no possibility that we need to override a user-provided setting. Instead use PGC_S_DYNAMIC_DEFAULT in most places, so that the value will appear with source = 'default' in pg_settings and thereby not be shown by psql's new \dconfig command. The one exception is that when changing in_hot_standby in a hot-standby session, we still use PGC_S_OVERRIDE, because people felt that seeing that in \dconfig would be a good thing. Similarly use PGC_S_DYNAMIC_DEFAULT for the auto-tune value of wal_buffers (if possible, that is if wal_buffers wasn't explicitly set to -1), and for the typical 2MB value of max_stack_depth. In combination these changes remove four not-very-interesting entries from the typical output of \dconfig, all of which people fingered as "why is that showing up?" in the discussion thread. Discussion: https://postgr.es/m/3118455.1649267333@sss.pgh.pa.us
* 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.