aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* 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
* 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.
* 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
* 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
* 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
* 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
* 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
* 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
* 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
* 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 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
* 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
* 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
* Add BufFileRead variants with short read and EOF detectionPeter Eisentraut2023-01-16
| | | | | | | | | | | | | | | Most callers of BufFileRead() want to check whether they read the full specified length. Checking this at every call site is very tedious. This patch provides additional variants BufFileReadExact() and BufFileReadMaybeEOF() that include the length checks. I considered changing BufFileRead() itself, but this function is also used in extensions, and so changing the behavior like this would create a lot of problems there. The new names are analogous to the existing LogicalTapeReadExact(). Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com
* Fix some BufFileRead() error reportingPeter Eisentraut2023-01-16
| | | | | | | | | | | | Remove "%m" from error messages where errno would be bogus. Add short read byte counts where appropriate. This is equivalent to what was done in 7897e3bb902c557412645b82120f4d95f7474906, but some code was apparently developed concurrently to that and not updated accordingly. Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/f3501945-c591-8cc3-5ef0-b72a2e0eaa9c@enterprisedb.com
* Refactor code in charge of running shell-based recovery commandsMichael Paquier2023-01-16
| | | | | | | | | | | | | | | | | | | | | | | | The code specific to the execution of archive_cleanup_command, recovery_end_command and restore_command is moved to a new file named shell_restore.c. The code is split into three functions: - shell_restore(), that attempts the execution of a shell-based restore_command. - shell_archive_cleanup(), for archive_cleanup_command. - shell_recovery_end(), for recovery_end_command. This introduces no functional changes, with failure patterns and logs generated in consequence being the same as before (one case actually generates one less DEBUG2 message "could not restore" when a restore command succeeds but the follow-up stat() to check the size fails, but that only matters with a elevel high enough). This is preparatory work for allowing recovery modules, a facility similar to archive modules, with callbacks shaped similarly to the functions introduced here. Author: Nathan Bossart Reviewed-by: Andres Freund, Michael Paquier Discussion: https://postgr.es/m/20221227192449.GA3672473@nathanxps13
* Store IdentLine->pg_user as an AuthTokenMichael Paquier2023-01-16
| | | | | | | | | | | | | | | While system_user was stored as an AuthToken in IdentLine, pg_user was stored as a plain string. This commit changes the code as we start storing pg_user as an AuthToken too. This does not have any functional changes, as all the operations on pg_user only use the string from the AuthToken. There is no regexp compiled and no check based on its quoting, yet. This is in preparation of more features that intend to extend its capabilities, like support for regexps and group membership. Author: Jelte Fennema Discussion: https://postgr.es/m/CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com
* Remove arbitrary FUNC_MAX_ARGS limit in int2vectorin and oidvectorin.Tom Lane2023-01-15
| | | | | | | | | | | | | | | | | | | | | | | | | int2vectorin limited the number of array elements it'd take to FUNC_MAX_ARGS, which is probably fine for the traditional use-cases. But now that pg_publication_rel.prattrs is an int2vector, it's not fine at all: it's easy to construct cases where that can have up to about MaxTupleAttributeNumber entries. Trying to replicate such tables leads to logical-replication failures. As long as we have to touch this code anyway, let's just remove the a-priori limit altogether, and let it accept any size that'll be allowed by repalloc. (Note that since int2vector isn't toastable, we cannot store arrays longer than about BLCKSZ/2; but there is no good excuse for letting int2vectorin depend on that. Perhaps we will lift the no-toast restriction someday.) While at it, also improve the equivalent logic in oidvectorin. I don't know of any practical use-case for long oidvectors right now, but doing it right actually makes the code shorter. Per report from Erik Rijkers. Back-patch to v15 where pg_publication_rel.prattrs was added. Discussion: https://postgr.es/m/668ba539-33c5-8190-ca11-def2913cb94b@xs4all.nl
* Make new GENERATED-expressions code more bulletproof.Tom Lane2023-01-15
| | | | | | | | | | | | | | | | | In commit 8bf6ec3ba I assumed that no code path could reach ExecGetExtraUpdatedCols without having gone through ExecInitStoredGenerated. That turns out not to be the case in logical replication: if there's an ON UPDATE trigger on the target table, trigger.c will call this code before anybody has set up its generated columns. Having seen that, I don't have a lot of faith in there not being other such paths. ExecGetExtraUpdatedCols can call ExecInitStoredGenerated for itself, as long as we are willing to assume that it is only called in CMD_UPDATE operations, which on the whole seems like a safer leap of faith. Per report from Vitaly Davydov. Discussion: https://postgr.es/m/d259d69652b8c2ff50e14cda3c236c7f@postgrespro.ru
* Fix MAINTAIN privileges for toast tables and partitions.Jeff Davis2023-01-14
| | | | | | | | | | | | | | | | | | | | Commit 60684dd8 left loose ends when it came to maintaining toast tables or partitions. For toast tables, simply skip the privilege check if the toast table is an indirect target of the maintenance command, because the main table privileges have already been checked. For partitions, allow the maintenance command if the user has the MAINTAIN privilege on the partition or any parent. Also make CLUSTER emit "skipping" messages when the user doesn't have privileges, similar to VACUUM. Author: Nathan Bossart Reported-by: Pavel Luzanov Reviewed-by: Pavel Luzanov, Ted Yu Discussion: https://postgr.es/m/20230113231339.GA2422750@nathanxps13
* Manual cleanup and pgindent of pgstat and bufmgr related codeAndres Freund2023-01-13
| | | | | | This is in preparation for commiting a larger patch series in the area. Discussion: https://postgr.es/m/CAAKRu_bHwGEbzNxxy+MQDkrsgog6aO6iUvajJ4d6PD98gFU7+w@mail.gmail.com
* Clean up useless "skipping" messages for VACUUM/ANALYZE.Jeff Davis2023-01-13
| | | | | | | | | | When VACUUM/ANALYZE are run on an entire database, it warns of skipping relations for which the user doesn't have sufficient privileges. That only makes sense for tables, so skip such messages for indexes, etc. Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/c0a85c2e83158560314b576b6241c8ed0aea1745.camel%40j-davis.com
* Simplify permissions for LOCK TABLE.Jeff Davis2023-01-13
| | | | | | | | | | | | | | The prior behavior was confusing and hard to document. For instance, if you had UPDATE privileges, you could lock a table in any lock mode except ACCESS SHARE mode. Now, if granted a privilege to lock at a given mode, one also has privileges to lock at a less-conflicting mode. MAINTAIN, UPDATE, DELETE, and TRUNCATE privileges allow any lock mode. INSERT privileges allow ROW EXCLUSIVE (or below). SELECT privileges allow ACCESS SHARE. Reviewed-by: Nathan Bossart Discussion: https://postgr.es/m/9550c76535404a83156252b25a11babb4792ea1e.camel%40j-davis.com
* Ignore dropped and generated columns from the column list.Amit Kapila2023-01-13
| | | | | | | | | | | | | | | | | | | We don't allow different column lists for the same table in the different publications of the single subscription. A publication with a column list except for dropped and generated columns should be considered the same as a publication with no column list (which implicitly includes all columns as part of the columns list). However, as we were not excluding the dropped and generated columns from the column list combining such publications leads to an error "cannot use different column lists for table ...". We decided not to backpatch this fix as there is a risk of users seeing this as a behavior change and also we didn't see any field report of this case. Author: Shi yu Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/OSZPR01MB631091CCBC56F195B1B9ACB0FDFE9@OSZPR01MB6310.jpnprd01.prod.outlook.com
* Avoid creating parallel apply state hash table unless required.Amit Kapila2023-01-13
| | | | | | | | | | This hash table is used to cache the state of streaming transactions being applied by the parallel apply workers. So, this should be created only when we are successful in launching at least one worker. This avoids rare case memory leak when we are never able to launch any worker. Author: Ted Yu Discussion: https://postgr.es/m/CALte62wg0rBR3Vj2beV=HiWo2qG9L0hzKcX=yULNER0wmf4aEw@mail.gmail.com
* Fix WaitEventSetWait() buffer overrun.Thomas Munro2023-01-13
| | | | | | | | | | | | | | | | | | | | | | | | | The WAIT_USE_EPOLL and WAIT_USE_KQUEUE implementations of WaitEventSetWaitBlock() confused the size of their internal buffer with the size of the caller's output buffer, and could ask the kernel for too many events. In fact the set of events retrieved from the kernel needs to be able to fit in both buffers, so take the smaller of the two. The WAIT_USE_POLL and WAIT_USE WIN32 implementations didn't have this confusion. This probably didn't come up before because we always used the same number in both places, but commit 7389aad6 calculates a dynamic size at construction time, while using MAXLISTEN for its output event buffer on the stack. That seems like a reasonable thing to want to do, so consider this to be a pre-existing bug worth fixing. As discovered by valgrind on skink. Back-patch to all supported releases for epoll, and to release 13 for the kqueue part, which copied the incorrect epoll code. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/901504.1673504836%40sss.pgh.pa.us
* Fix jsonpath existense checking of missing variablesAlexander Korotkov2023-01-12
| | | | | | | | | | | | | | | | | The current jsonpath code assumes that the referenced variable always exists. It could only throw an error at the value valuation time. At the same time existence checking assumes variable is present without valuation, and error suppression doesn't work for missing variables. This commit makes existense checking trigger an error for missing variables. This makes the overall behavior consistent. Backpatch to 12 where jsonpath was introduced. Reported-by: David G. Johnston Discussion: https://postgr.es/m/CAKFQuwbeytffJkVnEqDyLZ%3DrQsznoTh1OgDoOF3VmOMkxcTMjA%40mail.gmail.com Author: Alexander Korotkov, David G. Johnston Backpatch-through: 12
* Constify the arguments of ilist.c/h functionsPeter Eisentraut2023-01-12
| | | | | | | | | | | | | | | | | | | | | Const qualifiers ensure that we don't do something stupid in the function implementation. Additionally they clarify the interface. As an example: void slist_delete(slist_head *head, const slist_node *node) Here one can instantly tell that node->next is not going to be set to NULL. Finally, const qualifiers potentially allow the compiler to do more optimizations. This being said, no benchmarking was done for this patch. The functions that return non-const pointers like slist_next_node(), dclist_next_node() etc. are not affected by the patch intentionally. Author: Aleksander Alekseev Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CAJ7c6TM2%3D08mNKD9aJg8vEY9hd%2BG4L7%2BNvh30UiNT3kShgRgNg%40mail.gmail.com
* Code cleanupPeter Eisentraut2023-01-12
| | | | | | | for commit c96de2ce1782116bd0489b1cd69ba88189a495e8 Author: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://www.postgresql.org/message-id/20230111185434.GA1912982@nathanxps13
* Rename some variables related to ident files in hba.{c,h}Michael Paquier2023-01-12
| | | | | | | | | | | | | | | | | | | | | | | | The code that handles authentication for user maps was pretty confusing with its choice of variable names. It involves two types of users: a system user and a Postgres user (well, role), and these were not named consistently throughout the code that processes the user maps loaded from pg_ident.conf at authentication. This commit changes the following things to improve the situation: - Rename "pg_role" to "pg_user" and "token" to "system_user" in IndetLine. These choices are more consistent with the pg_ident.conf example in the docs, as well. "token" has been introduced recently in fc579e1, and it is way worse than the choice before that, "ident_user". - Switch the order of the fields in IdentLine to map with the order of the items in the ident files, as of map name, system user and PG user. - In check_ident_usermap(), rename "regexp_pgrole" to "expanded_pg_user" when processing a regexp for the system user entry in a user map. This variable does not store a regular expression at all: it would be either a string or a substitution to \1 if the Postgres role is specified as such. Author: Jelte Fennema Discussion: https://postgr.es/m/CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-03 01:05:47 +0000