aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Run pgindent on heapam.cDavid Rowley2023-01-23
| | | | | | | | | | | An upcoming patch by Melanie Plageman does some refactoring work in this area. Run pgindent on that file now before making any changes so that it's easier to maintain/evolve each of the individual patches doing the refactor work. Additionally, add a few new required typedefs to the list to make it easier to do future pgindent runs on this file during the refactor work. Discussion: https://postgr.es/m/CAAKRu_YSOnhKsDyFcqJsKtBSrd32DP-jjXmv7hL0BPD-z0TGXQ@mail.gmail.com
* Fix and clarify function comment on LogicalTapeSetCreate.Heikki Linnakangas2023-01-23
| | | | | | | | | | | | | | | | | Commit c4649cce39 removed the "shared" and "ntapes" arguments, but the comment still talked about "shared". It also talked about "a shared file handle", which was technically correct because even before commit c4649cce39, the "shared file handle" referred to the "fileset" argument, not "shared". But it was very confusing. Improve the comment. Also add a comment on what the "preallocate" argument does. Backpatch to v15, just to make backpatching other patches easier in the future. Discussion: https://www.postgresql.org/message-id/af989685-91d5-aad4-8f60-1d066b5ec309@enterprisedb.com Reviewed-by: Peter Eisentraut
* Harden new parallel string_agg/array_agg regression testDavid Rowley2023-01-23
| | | | | | | | Per buildfarm member mandrill, it seems that max_parallel_workers_per_gather may not always be set to the default value of 2 when the new test added in 16fd03e95 is executed. Here let's just explicitly set that to 2 so that the planner never opts to use more than that many parallel workers.
* Allow parallel aggregate on string_agg and array_aggDavid Rowley2023-01-23
| | | | | | | | | | | | | | This adds combine, serial and deserial functions for the array_agg() and string_agg() aggregate functions, thus allowing these aggregates to partake in partial aggregations. This allows both parallel aggregation to take place when these aggregates are present and also allows additional partition-wise aggregation plan shapes to include plans that require additional aggregation once the partially aggregated results from the partitions have been combined. Author: David Rowley Reviewed-by: Andres Freund, Tomas Vondra, Stephen Frost, Tom Lane Discussion: https://postgr.es/m/CAKJS1f9sx_6GTcvd6TMuZnNtCh0VhBzhX6FZqw17TgVFH-ga_A@mail.gmail.com
* Track logrep apply workers' last start times to avoid useless waits.Tom Lane2023-01-22
| | | | | | | | | | | | | | Enforce wal_retrieve_retry_interval on a per-subscription basis, rather than globally, and arrange to skip that delay in case of an intentional worker exit. This probably makes little difference in the field, where apply workers wouldn't be restarted often; but it has a significant impact on the runtime of our logical replication regression tests (even though those tests use artificially-small wal_retrieve_retry_interval settings already). Nathan Bossart, with mostly-cosmetic editorialization by me Discussion: https://postgr.es/m/20221122004119.GA132961@nathanxps13
* Allow REPLICA IDENTITY to be set on an index that's not (yet) valid.Tom Lane2023-01-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The motivation for this change is that when pg_dump dumps a partitioned index that's marked REPLICA IDENTITY, it generates a command sequence that applies REPLICA IDENTITY before the partitioned index has been marked valid, causing restore to fail. We could perhaps change pg_dump to not do it like that, but that would be difficult and would not fix existing dump files with the problem. There seems to be very little reason for the backend to disallow this anyway --- the code ignores indisreplident when the index isn't valid --- so instead let's fix it by allowing the case. Commit 9511fb37a previously expressed a concern that allowing indisreplident to be set on invalid indexes might allow us to wind up in a situation where a table could have indisreplident set on multiple indexes. I'm not sure I follow that concern exactly, but in any case the only way that could happen is because relation_mark_replica_identity is too trusting about the existing set of markings being valid. Let's just rip out its early-exit code path (which sure looks like premature optimization anyway; what are we doing expending code to make redundant ALTER TABLE ... REPLICA IDENTITY commands marginally faster and not-redundant ones marginally slower?) and fix it to positively guarantee that no more than one index is marked indisreplident. The pg_dump failure can be demonstrated in all supported branches, so back-patch all the way. I chose to back-patch 9511fb37a as well, just to keep indisreplident handling the same in all branches. Per bug #17756 from Sergey Belyashov. Discussion: https://postgr.es/m/17756-dd50e8e0c8dd4a40@postgresql.org
* Reject CancelRequestPacket having unexpected length.Noah Misch2023-01-21
| | | | | | | | | | | | | When the length was too short, the server read outside the allocation. That yielded the same log noise as sending the correct length with (backendPID,cancelAuthCode) matching nothing. Change to a message about the unexpected length. Given the attacker's lack of control over the memory layout and the general lack of diversity in memory layouts at the code in question, we doubt a would-be attacker could cause a segfault. Hence, while the report arrived via security@postgresql.org, this is not a vulnerability. Back-patch to v11 (all supported versions). Andrey Borodin, reviewed by Tom Lane. Reported by Andrey Borodin.
* instr_time: Represent time as an int64 on all platformsAndres Freund2023-01-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now we used struct timespec for instr_time on all platforms but windows. Using struct timespec causes a fair bit of memory (struct timeval is 16 bytes) and runtime overhead (much more complicated additions). Instead we can convert the time to nanoseconds in INSTR_TIME_SET_CURRENT(), making the remaining operations cheaper. Representing time as int64 nanoseconds provides sufficient range, ~292 years relative to a starting point (depending on clock source, relative to the unix epoch or the system's boot time). That'd not be sufficient for calendar time stored on disk, but is plenty for runtime interval time measurement. On windows instr_time already is represented as cycles. It might make sense to represent time as cycles on other platforms as well, as using cycle acquisition instructions like rdtsc directly can reduce the overhead of time acquisition substantially. This could be done in a fairly localized manner as the code stands after this commit. Because the windows and non-windows paths are now more similar, use a common set of macros. To make that possible, most of the use of LARGE_INTEGER had to be removed, which looks nicer anyway. To avoid users of the API relying on the integer representation, we wrap the 64bit integer inside struct struct instr_time. Author: Andres Freund <andres@anarazel.de> Author: Lukas Fittl <lukas@fittl.com> Author: David Geier <geidav.pg@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20230113195547.k4nlrmawpijqwlsa@awork3.anarazel.de
* Zero initialize uses of instr_time about to trigger compiler warningsAndres Freund2023-01-20
| | | | | | | | | These are all not necessary from a correctness POV. However, in the near future instr_time will be simplified to an int64, at which point gcc would otherwise start to warn about the changed places. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/20230116023639.rn36vf6ajqmfciua@awork3.anarazel.de
* Rework format of comments in headers for nodesMichael Paquier2023-01-21
| | | | | | | | | This is similar to 835d476, except that this one is to add node attributes related to query jumbling and avoid long lines in the headers and in the node structures changed by this commit. Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/Y5BHOUhX3zTH/ig6@paquier.xyz
* Move queryjumble.c code to src/backend/nodes/Michael Paquier2023-01-21
| | | | | | | | | | | | | This will ease a follow-up move that will generate automatically this code. The C file is renamed, for consistency with the node-related files whose code are generated by gen_node_support.pl: - queryjumble.c -> queryjumblefuncs.c - utils/queryjumble.h -> nodes/queryjumble.h Per a suggestion from Peter Eisentraut. Reviewed-by: Peter Eisentraut Discussion: https://postgr.es/m/Y5BHOUhX3zTH/ig6@paquier.xyz
* Bump catversion for 6e2775e4d4e47775f0d933e4a93c148024a3bc63.Robert Haas2023-01-20
| | | | It creates a new predefined role.
* Add new GUC reserved_connections.Robert Haas2023-01-20
| | | | | | | | | | | | This provides a way to reserve connection slots for non-superusers. The slots reserved via the new GUC are available only to users who have the new predefined role pg_use_reserved_connections. superuser_reserved_connections remains as a final reserve in case reserved_connections has been exhausted. Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13
* Rename ReservedBackends variable to SuperuserReservedConnections.Robert Haas2023-01-20
| | | | | | | | | | This is in preparation for adding a new reserved_connections GUC, but aligning the GUC name with the variable name is also a good idea on general principle. Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13
* Update docs and error message for superuser_reserved_connections.Robert Haas2023-01-20
| | | | | | | | | | | | | | Commit ea92368cd1da1e290f9ab8efb7f60cb7598fc310 made max_wal_senders a separate pool of backends from max_connections, but the documentation and error message for superuser_reserved_connections weren't updated at the time, and as a result are somewhat misleading. Update. This is arguably a back-patchable bug fix, but because it seems quite minor, no back-patch. Patch by Nathan Bossart. Reviewed by Tushar Ahuja and by me. Discussion: http://postgr.es/m/20230119194601.GA4105788@nathanxps13
* Remove SHM_QUEUEAndres Freund2023-01-19
| | | | | | | | | | Prior patches got rid of all the uses of SHM_QUEUE. ilist.h style lists are more widely used and have an easier to use interface. As there are no users left, remove SHM_QUEUE. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> (in an older version) Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Use dlists instead of SHM_QUEUE for predicate lockingAndres Freund2023-01-19
| | | | | | | | | Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used and have an easier to use interface. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> (in an older version) Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Support the same patterns for pg-user in pg_ident.conf as in pg_hba.confMichael Paquier2023-01-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While pg_hba.conf has support for non-literal username matches, and this commit extends the capabilities that are supported for the PostgreSQL user listed in an ident entry part of pg_ident.conf, with support for: 1. The "all" keyword, where all the requested users are allowed. 2. Membership checks using the + prefix. 3. Using a regex to match against multiple roles. 1. is a feature that has been requested by Jelte Fennema, 2. something that has been mentioned independently by Andrew Dunstan, and 3. is something I came up with while discussing how to extend the first one, whose implementation is facilitated by 8fea868. This allows matching certain system users against many different postgres users with a single line in pg_ident.conf. Without this, one would need one line for each of the postgres users that a system user can log in as, which can be cumbersome to maintain. Tests are added to the TAP test of peer authentication to provide coverage for all that. Note that this introduces a set of backward-incompatible changes to be able to detect the new patterns, for the following cases: - A role named "all". - A role prefixed with '+' characters, which is something that would not have worked in HBA entries anyway. - A role prefixed by a slash character, similarly to 8fea868. Any of these can be still be handled by using quotes in the Postgres role defined in an ident entry. A huge advantage of this change is that the code applies the same checks for the Postgres roles in HBA and ident entries, via the common routine check_role(). **This compatibility change should be mentioned in the release notes.** Author: Jelte Fennema Discussion: https://postgr.es/m/DBBPR83MB0507FEC2E8965012990A80D0F7FC9@DBBPR83MB0507.EURPRD83.prod.outlook.com
* Avoid harmless warning from pg_dump --if-exists mode.Tom Lane2023-01-19
| | | | | | | | | | | | | | | | | | | If the public schema has a non-default owner (perhaps due to dropping and recreating it) then use of pg_dump's "--if-exists" option results in a warning message: warning: could not find where to insert IF EXISTS in statement "-- *not* dropping schema, since initdb creates it" This is harmless since the dump output is the same either way, but nonetheless it's undesirable. It's the fault of commit a7a7be1f2, which created situations where a TOC entry's "defn" or "dropStmt" fields could be just comments. Although that commit fixed up the kluges in pg_backup_archiver.c that munge defn strings, it missed doing so for the one that munges dropStmts. Per bug# 17753 from Justin Zhang. Discussion: https://postgr.es/m/17753-9c8773631747ee1c@postgresql.org
* Use appendStringInfoSpaces in more placesDavid Rowley2023-01-20
| | | | | | | | | | | | | | | | | | | | | | | This adjusts a few places which were appending a string constant containing spaces onto a StringInfo. We have appendStringInfoSpaces for that job, so let's use that instead. For the change to jsonb.c's add_indent() function, appendStringInfoString was being called inside a loop to append 4 spaces on each loop. This meant that enlargeStringInfo would get called once per loop. Here it should be much more efficient to get rid of the loop and just calculate the number of spaces with "level * 4" and just append all the spaces in one go. Here we additionally adjust the appendStringInfoSpaces function so it makes use of memset rather than a while loop to apply the required spaces to the StringInfo. One of the problems with the while loop was that it was incrementing one variable and decrementing another variable once per loop. That's more work than what's required to get the job done. We may as well use memset for this rather than trying to optimize the existing loop. Some testing has shown memset is faster even for very small sizes. Discussion: https://postgr.es/m/CAApHDvp_rKkvwudBKgBHniNRg67bzXVjyvVKfX0G2zS967K43A@mail.gmail.com
* Improve comment about GetWALAvailability's WALAVAIL_REMOVED code.Tom Lane2023-01-19
| | | | | | Sirisha Chamarthi and Kyotaro Horiguchi Discussion: https://postgr.es/m/CAKrAKeXt-=bgm=d+EDmcC9kWoikp8kbVb3LH0K3K+AGGsykpHQ@mail.gmail.com
* Fix ts_headline() to handle ORs and phrase queries more honestly.Tom Lane2023-01-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch largely reverts what I did in commits c9b0c678d and 78e73e875. The maximum cover length limit that I added in 78e73e875 (to band-aid over c9b0c678d's performance issues) creates too many user-visible behavior discrepancies, as complained of for example in bug #17691. The real problem with hlCover() is not what I thought at the time, but more that it seems to have been designed with only AND tsquery semantics in mind. It doesn't work quite right for OR, and even less so for NOT or phrase queries. However, we can improve that situation by building a variant of TS_execute() that returns a list of match locations. We already get an ExecPhraseData struct representing match locations for the primitive case of a simple match, as well as one for a phrase match; we just need to add some logic to combine these for AND and OR operators. The result is a list of ExecPhraseDatas, which hlCover can regard as having simple AND semantics, so that its old algorithm works correctly. There's still a lot not to like about ts_headline's behavior, but I think the remaining issues have to do with the heuristics used in mark_hl_words and mark_hl_fragments (which, likewise, were not revisited when phrase search was added). Improving those is a task for another day. Patch by me; thanks to Alvaro Herrera for review. Discussion: https://postgr.es/m/840.1669405935@sss.pgh.pa.us
* Log the correct ending timestamp in recovery_target_xid mode.Tom Lane2023-01-19
| | | | | | | | | | | | | | | | When ending recovery based on recovery_target_xid matching with recovery_target_inclusive = off, we printed an incorrect timestamp (always 2000-01-01) in the "recovery stopping before ... transaction" log message. This is a consequence of sloppy refactoring in c945af80c: the code to fetch recordXtime out of the commit/abort record used to be executed unconditionally, but it was changed to get called only in the RECOVERY_TARGET_TIME case. We need only flip the order of operations to restore the intended behavior. Per report from Torsten Förtsch. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAKkG4_kUevPqbmyOfLajx7opAQk6Cvwkvx0HRcFjSPfRPTXanA@mail.gmail.com
* Remove some dead code in selfuncs.cAlvaro Herrera2023-01-19
| | | | | | | | | | | RelOptInfo.userid is the same for all relations in a given inheritance tree, so the code in examine_variable() and example_simple_variable() that repeats the ACL checks on the root parent rel instead of a given leaf child relations need not recompute userid too. Author: Amit Langote <amitlangote09@gmail.com> Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20221210201753.GA27893@telsasoft.com
* Constify proclist.hPeter Eisentraut2023-01-19
| | | | | | | This is a follow-up to c8ad4d81. Author: Aleksander Alekseev Discussion: https://www.postgresql.org/message-id/flat/CAJ7c6TM084Ai_8%3DfZaWtULJBLtT1bgzL%3Dk9vHMYom3eyZsekAA%40mail.gmail.com
* Add missing assign hook for GUC checkpoint_completion_targetMichael Paquier2023-01-19
| | | | | | | | | | | | | | | | | | This is wrong since 88e9823, that has switched the WAL sizing configuration from checkpoint_segments to min_wal_size and max_wal_size. This missed the recalculation of the internal value of the internal "CheckPointSegments", that works as a mapping of the old GUC checkpoint_segments, on reload, for example, and it controls the timing of checkpoints depending on the volume of WAL generated. Most users tend to leave checkpoint_completion_target at 0.9 to smooth the I/O workload, which is why I guess this has gone unnoticed for so long, still it can be useful to tweak and reload the value dynamically in some cases to control the timing of checkpoints. Author: Bharath Rupireddy Discussion: https://postgr.es/m/CALj2ACXgPPAm28mruojSBno+F_=9cTOOxHAywu_dfZPeBdybQw@mail.gmail.com Backpatch-through: 11
* Fix failure with perlcritic in psql's create_help.plMichael Paquier2023-01-19
| | | | | | | | | No buildfarm members have reported that yet, but a recently-refreshed Debian host did. Reviewed-by: Andrew Dunstan Discussion: https://postgr.es/m/Y8ey5z4Nav62g4/K@paquier.xyz Backpatch-through: 11
* Fix AdjustUpgrade.pm's view conversion list for --with-lz4.Tom Lane2023-01-18
| | | | | | | | | Turns out the compression.sql test creates a view that needs to be adjusted in the wake of 47bb9db75 --- except that without --with-lz4, it fails to create the view at all, so I'd not noticed this in testing. Per buildfarm member crake.
* Update expected/collate.windows.win1252.out for 47bb9db75.Tom Lane2023-01-18
| | | | | | | This delta in expected output wasn't spotted in any previous testing of the patch. Reported by Andres Freund. Discussion: https://postgr.es/m/20230118195855.7yjc4mminmv7iyy5@awork3.anarazel.de
* Use dlists instead of SHM_QUEUE for syncrep queueAndres Freund2023-01-18
| | | | | | | | | Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used and have an easier to use interface. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> (in an older version) Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Use dlist/dclist instead of PROC_QUEUE / SHM_QUEUE for heavyweight locksAndres Freund2023-01-18
| | | | | | | | | | | Part of a series to remove SHM_QUEUE. ilist.h style lists are more widely used and have an easier to use interface. As PROC_QUEUE is now unused, remove it. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> (in an older version) Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Add detached node functions to ilistAndres Freund2023-01-18
| | | | | | | | | | These allow to test whether an element is in a list by checking whether prev/next are NULL. Needed to replace SHMQueueIsDetached() when converting from SHM_QUEUE to ilist.h style lists. Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20221120055930.t6kl3tyivzhlrzu2@awork3.anarazel.de Discussion: https://postgr.es/m/20200211042229.msv23badgqljrdg2@alap3.anarazel.de
* Fix ILIST_DEBUG buildAndres Freund2023-01-18
| | | | | | | | | | | In c8ad4d8166a dlist_member_check()'s arguments were made const. Unfortunately the implementation of dlist_member_check() used dlist_foreach(), which currently doesn't work for const lists. As a workaround, open-code the list iteration. The other check functions already do so. Discussion: https://postgr.es/m/20230118182214.co7dp4oahiunwg57@awork3.anarazel.de
* Get rid of the "new" and "old" entries in a view's rangetable.Tom Lane2023-01-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The rule system needs "old" and/or "new" pseudo-RTEs in rule actions that are ON INSERT/UPDATE/DELETE. Historically it's put such entries into the ON SELECT rules of views as well, but those are really quite vestigial. The only thing we've used them for is to carry the view's relid forward to AcquireExecutorLocks (so that we can re-lock the view to verify it hasn't changed before re-using a plan) and to carry its relid and permissions data forward to execution-time permissions checks. What we can do instead of that is to retain these fields of the RTE_RELATION RTE for the view even after we convert it to an RTE_SUBQUERY RTE. This requires a tiny amount of extra complication in the planner and AcquireExecutorLocks, but on the other hand we can get rid of the logic that moves that data from one place to another. The principal immediate benefit of doing this, aside from a small saving in the pg_rewrite data for views, is that these pseudo-RTEs no longer trigger ruleutils.c's heuristic about qualifying variable names when the rangetable's length is more than 1. That results in quite a number of small simplifications in regression test outputs, which are all to the good IMO. Bump catversion because we need to dump a few more fields of RTE_SUBQUERY RTEs. While those will always be zeroes anyway in stored rules (because we'd never populate them until query rewrite) they are useful for debugging, and it seems like we'd better make sure to transmit such RTEs accurately in plans sent to parallel workers. I don't think the executor actually examines these fields after startup, but someday it might. This is a second attempt at committing 1b4d280ea. The difference from the first time is that now we can add some filtering rules to AdjustUpgrade.pm to allow cross-version upgrade testing to pass despite all the cosmetic changes in CREATE VIEW outputs. Amit Langote (filtering rules by me) Discussion: https://postgr.es/m/CA+HiwqEf7gPN4Hn+LoZ4tP2q_Qt7n3vw7-6fJKOf92tSEnX6Gg@mail.gmail.com Discussion: https://postgr.es/m/891521.1673657296@sss.pgh.pa.us
* Remove redundant grouping and DISTINCT columns.Tom Lane2023-01-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Avoid explicitly grouping by columns that we know are redundant for sorting, for example we need group by only one of x and y in SELECT ... WHERE x = y GROUP BY x, y This comes up more often than you might think, as shown by the changes in the regression tests. It's nearly free to detect too, since we are just piggybacking on the existing logic that detects redundant pathkeys. (In some of the existing plans that change, it's visible that a sort step preceding the grouping step already didn't bother to sort by the redundant column, making the old plan a bit silly-looking.) To do this, build processed_groupClause and processed_distinctClause lists that omit any provably-redundant sort items, and consult those not the originals where relevant. This means that within the planner, one should usually consult root->processed_groupClause or root->processed_distinctClause if one wants to know which columns are to be grouped on; but to check whether grouping or distinct-ing is happening at all, check non-NIL-ness of parse->groupClause or parse->distinctClause. This is comparable to longstanding rules about handling the HAVING clause, so I don't think it'll be a huge maintenance problem. nodeAgg.c also needs minor mods, because it's now possible to generate AGG_PLAIN and AGG_SORTED Agg nodes with zero grouping columns. Patch by me; thanks to Richard Guo and David Rowley for review. Discussion: https://postgr.es/m/185315.1672179489@sss.pgh.pa.us
* Display the leader apply worker's PID for parallel apply workers.Amit Kapila2023-01-18
| | | | | | | | | | | | | | | | | Add leader_pid to pg_stat_subscription. leader_pid is the process ID of the leader apply worker if this process is a parallel apply worker. If this field is NULL, it indicates that the process is a leader apply worker or a synchronization worker. The new column makes it easier to distinguish parallel apply workers from other kinds of workers and helps to identify the leader for the parallel workers corresponding to a particular subscription. Additionally, update the leader_pid column in pg_stat_activity as well to display the PID of the leader apply worker for parallel apply workers. Author: Hou Zhijie Reviewed-by: Peter Smith, Sawada Masahiko, Amit Kapila, Shveta Mallik Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com
* Refactor code for restoring files via shell commandsMichael Paquier2023-01-18
| | | | | | | | | | | | | | | | | | | Presently, restore_command uses a different code path than archive_cleanup_command and recovery_end_command. These code paths are similar and can be easily combined, as long as it is possible to identify if a command should: - Issue a FATAL on signal. - Exit immediately on SIGTERM. While on it, this removes src/common/archive.c and its associated header. Since the introduction of c96de2c, BuildRestoreCommand() has become a simple wrapper of replace_percent_placeholders() able to call make_native_path(). This simplifies shell_restore.c as long as RestoreArchivedFile() includes a call to make_native_path(). Author: Nathan Bossart Reviewed-by: Andres Freund, Michael Paquier Discussion: https://postgr.es/m/20221227192449.GA3672473@nathanxps13
* Constify the arguments of copydir.h functionsMichael Paquier2023-01-18
| | | | | | | | | | This makes sure that the internal logic of these functions does not attempt to change the value of the arguments constified, and it removes one unconstify() in basic_archive.c. Author: Nathan Bossart Reviewed-by: Andrew Dunstan, Peter Eisentraut Discussion: https://postgr.es/m/20230114231126.GA2580330@nathanxps13
* meson: Add two missing regress testsAndres Freund2023-01-17
| | | | | | | | It's likely worth adding some automated way of preventing further omissions. We're discussing how to best do that. Reported-by: Justin Pryzby <pryzby@telsasoft.com> Discussion: https://postgr.es/m/20230117173509.GV9837@telsasoft.com
* AdjustUpgrade.pm should zap test_ext_cine, too.Tom Lane2023-01-17
| | | | | | | | | | | | | | test_extensions' test_ext_cine extension has the same upgrade hazard as test_ext7: the regression test leaves it in an updated state from which no downgrade path to default is provided. This causes the update_extensions.sql script helpfully provided by pg_upgrade to fail. So drop it in cross-version-upgrade testing. Not entirely sure how come I didn't hit this in testing yesterday; possibly I'd built the upgrade reference databases with testmodules-install-check disabled. Backpatch to v10 where this module was introduced.
* Refactor recordExtObjInitPriv()Peter Eisentraut2023-01-17
| | | | | | | | | | Instead of half a dozen of mostly-duplicate conditional branches, write one common one that can handle most catalogs. We already have all the information we need, such as which system catalog corresponds to which catalog table and which column is the ACL column. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/504bc485-6bd6-dd1b-fe10-e7351aeb310d@enterprisedb.com
* Remove AggregateRelationId from recordExtObjInitPriv()Peter Eisentraut2023-01-17
| | | | | | | | | This was erroneous, because AggregateRelationId has no OID, so it cannot be part of an extension directly. (Aggregates are registered via pg_proc.) No harm in practice, but better to make it correct. Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/504bc485-6bd6-dd1b-fe10-e7351aeb310d@enterprisedb.com
* Remove redundant relkind checkJohn Naylor2023-01-17
| | | | | | | Ranier Vilela Reviewed by Justin Pryzby Discussion: https://www.postgresql.org/message-id/CAEudQAp2R2fbbi0OHHhv_n4%3DCh0t1VtjObR9YMqtGKHJ%2BfaUFQ%40mail.gmail.com
* Remove dead code in formatting.cJohn Naylor2023-01-17
| | | | | | | | | | | | | | | Remove some code guarded by IS_MINUS() or IS_PLUS(), where the entire stanza is inside an else-block where both of these are false. This should slightly improve test coverage. While at it, remove coding that apparently assumes that unsetting a bit is so expensive that we have to first check if it's already set in the first place. Per Coverity report from Ranier Vilela Analysis and review by Justin Pryzby Discussion: https://www.postgresql.org/message-id/20221223010818.GP1153%40telsasoft.com
* Improve the code to decide and process the apply action.Amit Kapila2023-01-17
| | | | | | | | | | | | | | | | | | | | | The code that decides the apply action missed to handle non-transactional messages and we didn't catch it in our testing as currently such messages are simply ignored by the apply worker. This was introduced by changes in commit 216a784829. While testing this, I noticed that we forgot to reset stream_xid after processing the stream stop message which could also result in the wrong apply action after the fix for non-transactional messages. In passing, change assert to elog for unexpected apply action in some of the routines so as to catch the problems in the production environment, if any. Reported-by: Tomas Vondra Author: Amit Kapila Reviewed-by: Tomas Vondra, Sawada Masahiko, Hou Zhijie Discussion: https://postgr.es/m/984ff689-adde-9977-affe-cd6029e850be@enterprisedb.com Discussion: https://postgr.es/m/CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com
* Fix typo in comment.Amit Kapila2023-01-17
| | | | | Author: Osumi Takamichi Discussion: https://postgr.es/m/TYCPR01MB83737EA140C79B7D099F65E8EDC69@TYCPR01MB8373.jpnprd01.prod.outlook.com
* Track behavior of \1 in pg_ident.conf when quotedMichael Paquier2023-01-17
| | | | | | | | | | | | | | Entries of pg-user in pg_ident.conf that are quoted and include '\1' allow a replacement from a subexpression in a system user regexp. This commit adds a test to track this behavior and a note in the documentation, as it could be affected by the use of an AuthToken for the pg-user in the IdentLines parsed. This subject has come up in the discussion aimed at extending the support of pg-user in ident entries for more patterns. Author: Jelte Fennema Discussion: https://postgr.es/m/CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com
* Don't presort ORDER BY/DISTINCT Aggrefs with volatile functionsDavid Rowley2023-01-17
| | | | | | | | | | | | | | | | | | | | | | In 1349d2790, we gave the planner the ability to provide ORDER BY/DISTINCT Aggrefs with presorted input so that nodeAgg would not have to perform sorts during execution. That commit failed to properly consider the implications of if the Aggref had a volatile function in its ORDER BY/DISTINCT clause. As it happened, this resulted in an ERROR about the volatile function being missing from the targetlist. Here, instead of adding the volatile function to the targetlist, we just never consider an Aggref with a volatile function in its ORDER BY/DISTINCT clause when choosing which Aggrefs we should sort by. We do this as if we were to choose a plan which provided these aggregates with presorted input, then if there were many such aggregates which could all share the same sort order, then it may be surprising if they all shared the same sort sometimes and didn't at other times when some other set of aggregates were given presorted results. We can avoid this inconsistency by just never providing these volatile function aggregates with presorted input. Reported-by: Dean Rasheed Discussion: https://postgr.es/m/CAEZATCWETioXs5kY8vT6BVguY41_wD962VDk=u_Nvd7S1UXzuQ@mail.gmail.com
* Create common infrastructure for cross-version upgrade testing.Tom Lane2023-01-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To test pg_upgrade across major PG versions, we have to be able to modify or drop any old objects with no-longer-supported properties, and we have to be able to deal with cosmetic changes in pg_dump output. Up to now, the buildfarm and pg_upgrade's own test infrastructure had separate implementations of the former, and we had nothing but very ad-hoc rules for the latter (including an arbitrary threshold on how many lines of unchecked diff were okay!). This patch creates a Perl module that can be shared by both those use-cases, and adds logic that deals with pg_dump output diffs in a much more tightly defined fashion. This largely supersedes previous efforts in commits 0df9641d3, 9814ff550, and 62be9e4cd, which developed a SQL-script-based solution for the task of dropping old objects. There was nothing fundamentally wrong with that work in itself, but it had no basis for solving the output-formatting problem. The most plausible way to deal with formatting is to build a Perl module that can perform editing on the dump files; and once we commit to that, it makes more sense for the same module to also embed the knowledge of what has to be done for dropping old objects. Back-patch versions of the helper module as far as 9.2, to support buildfarm animals that still test that far back. It's also necessary to back-patch PostgreSQL/Version.pm, because the new code depends on that. I fixed up pg_upgrade's 002_pg_upgrade.pl in v15, but did not look into back-patching it further than that. Tom Lane and Andrew Dunstan Discussion: https://postgr.es/m/891521.1673657296@sss.pgh.pa.us
* Tighten up VACUUM's approach to setting VM bits.Peter Geoghegan2023-01-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Tighten up the way that visibilitymap_set() is called: request that both the all-visible and all-frozen bits get set whenever the all-frozen bit is set, regardless of what we think we know about the present state of the all-visible bit. Also make sure that the page level PD_ALL_VISIBLE flag is set in the same code path. In practice there doesn't seem to be a concrete scenario in which the previous approach could lead to inconsistencies. It was almost possible in scenarios involving concurrent HOT updates from transactions that abort, but (unlike pruning) freezing can never remove XIDs > VACUUM's OldestXmin, even those from transactions that are known to have aborted. That was protective here. These issues have been around since commit a892234f83, which added the all-frozen bit to the VM fork. There is no known live bug here, so no backpatch. In passing, add some defensive assertions to catch the issue, and stop reading the existing state of the VM when setting the VM in VACUUM's final heap pass. We already know that affected pages must have had at least one LP_DEAD item before we set it LP_UNUSED, so there is no point in reading the VM when it is set like this. Author: Peter Geoghegan <pg@bowt.ie> Reviewed-By: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/CAH2-WznuNGSzF8v6OsgjaC5aYsb3cZ6HW6MLm30X0d65cmSH6A@mail.gmail.com