aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
* Recalculate search_path after ALTER ROLE.Jeff Davis2023-08-09
| | | | | | | | | Renaming a role can affect the meaning of the special string $user, so must cause search_path to be recalculated. Discussion: https://postgr.es/m/186761d32c0255debbdf50b6310b581b9c973e6c.camel@j-davis.com Reviewed-by: Nathan Bossart, Michael Paquier Backpatch-through: 11
* Reject substituting extension schemas or owners matching ["$'\].Noah Misch2023-08-07
| | | | | | | | | | | | | | | | | | | Substituting such values in extension scripts facilitated SQL injection when @extowner@, @extschema@, or @extschema:...@ appeared inside a quoting construct (dollar quoting, '', or ""). No bundled extension was vulnerable. Vulnerable uses do appear in a documentation example and in non-bundled extensions. Hence, the attack prerequisite was an administrator having installed files of a vulnerable, trusted, non-bundled extension. Subject to that prerequisite, this enabled an attacker having database-level CREATE privilege to execute arbitrary code as the bootstrap superuser. By blocking this attack in the core server, there's no need to modify individual extensions. Back-patch to v11 (all supported versions). Reported by Micah Gate, Valerie Woolard, Tim Carey-Smith, and Christoph Berg. Security: CVE-2023-39417
* Translation updatesPeter Eisentraut2023-08-07
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 97398d714ace69f0c919984e160f429b6fd2300e
* Don't Memoize lateral joins with volatile join conditionsDavid Rowley2023-08-07
| | | | | | | | | | | | | | | | | | The use of Memoize was already disabled in normal joins when the join conditions had volatile functions per the code in match_opclause_to_indexcol(). Ordinarily, the parameterization for the inner side of a nested loop will be an Index Scan or at least eventually lead to an index scan (perhaps nested several joins deep). However, for lateral joins, that's not the case and seq scans can be parameterized too, so we can't rely on match_opclause_to_indexcol(). Here we explicitly check the parameterization for volatile functions and don't consider the generation of a Memoize path when such functions are present. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs49nHFnHbpepLsv_yF3qkpCS4BdB-v8HoJVv8_=Oat0u_w@mail.gmail.com Backpatch-through: 14, where Memoize was introduced
* Fix RLS policy usage in MERGE.Dean Rasheed2023-08-07
| | | | | | | | | | | | | | | | | | | | | | | | | If MERGE executes an UPDATE action on a table with row-level security, the code incorrectly applied the WITH CHECK clauses from the target table's INSERT policies to new rows, instead of the clauses from the table's UPDATE policies. In addition, it failed to check new rows against the target table's SELECT policies, if SELECT permissions were required (likely to always be the case). In addition, if MERGE executes a DO NOTHING action for matched rows, the code incorrectly applied the USING clauses from the target table's DELETE policies to existing target tuples. These policies were applied as checks that would throw an error, if they did not pass. Fix this, so that a MERGE UPDATE action applies the same RLS policies as a plain UPDATE query with a WHERE clause, and a DO NOTHING action does not apply any RLS checks (other than adding clauses from SELECT policies to the join). Back-patch to v15, where MERGE was introduced. Dean Rasheed, reviewed by Stephen Frost. Security: CVE-2023-39418
* Update comments on CustomPath struct.Etsuro Fujita2023-08-03
| | | | | | | | | Commit e7cb7ee14 allowed custom scan providers to create CustomPath paths for join relations as well, but missed updating the comments. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAPmGK15ODkN%2B%3DhkBCufj1HBW0x5OTb65Xuy7ryXchMdiCMpx_g%40mail.gmail.com
* Fix ReorderBufferCheckMemoryLimit() comment.Masahiko Sawada2023-08-02
| | | | | | | | | | Commit 7259736a6 updated the comment but it was not correct since ReorderBufferLargestStreamableTopTXN() returns only top-level transactions. Reviewed-by: Amit Kapila Discussion: https://postgr.es/m/CAD21AoA9XB7OR86BqvrCe2dMYX%2BZv3-BvVmjF%3DGY2z6jN-kqjg%40mail.gmail.com Backpatch-through: 14
* Fix pg_stat_io buffer reuse test instabilityAndres Freund2023-08-01
| | | | | | | | | | | | | | | | | | The stats regression test attempts to ensure that Buffer Access Strategy "reuses" are being counted in pg_stat_io by vacuuming a table which is larger than the size of the strategy ring. However, when shared buffers are in sufficiently high demand, another backend could evict one of the blocks in the strategy ring before the first backend has a chance to reuse the buffer. The backend using the strategy would then evict another shared buffer and add that buffer to the strategy ring. This counts as an eviction and not a reuse in pg_stat_io. Count both evictions and reuses in the test to ensure it does not fail incorrectly. Reported-by: Jeff Davis <pgsql@j-davis.com>, Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com> Discussion: https://postgr.es/m/CAAKRu_bNG27AxG9TdPtwsL6wg8AWbVckjmTL2t1HF=miDQuNtw@mail.gmail.com
* Fix performance regression in pg_strtointNN_safe functionsDavid Rowley2023-08-02
| | | | | | | | | | | | | | | | | | | | | Between 6fcda9aba and 1b6f632a3, the pg_strtoint functions became quite a bit slower in v16, despite efforts in 6b423ec67 to speed these up. Since the majority of cases for these functions will only contain base-10 digits, perhaps prefixed by a '-', it makes sense to have a special case for this and just fall back on the more complex version which processes hex, octal, binary and underscores if the fast path version fails to parse the string. While we're here, update the header comments for these functions to mention that hex, octal and binary formats along with underscore separators are now supported. Author: Andres Freund, David Rowley Reported-by: Masahiko Sawada Reviewed-by: Dean Rasheed, John Naylor Discussion: https://postgr.es/m/CAD21AoDvDmUQeJtZrau1ovnT_smN940%3DKp6mszNGK3bq9yRN6g%40mail.gmail.com Backpatch-through: 16, where 6fcda9aba and 1b6f632a3 were added
* Fix overly strict Assert in jsonpath codeDavid Rowley2023-08-02
| | | | | | | | | | | | | This was failing for queries which try to get the .type() of a jpiLikeRegex. For example: select jsonb_path_query('["string", "string"]', '($[0] like_regex ".{7}").type()'); Reported-by: Alexander Kozhemyakin Bug: #18035 Discussion: https://postgr.es/m/18035-64af5cdcb5adf2a9@postgresql.org Backpatch-through: 12, where SQL/JSON path was added.
* Disallow replacing joins with scans in problematic cases.Etsuro Fujita2023-07-28
| | | | | | | | | | | | | | | | | | | | | | | | Commit e7cb7ee14, which introduced the infrastructure for FDWs and custom scan providers to replace joins with scans, failed to add support handling of pseudoconstant quals assigned to replaced joins in createplan.c, leading to an incorrect plan without a gating Result node when postgres_fdw replaced a join with such a qual. To fix, we could add the support by 1) modifying the ForeignPath and CustomPath structs to store the list of RestrictInfo nodes to apply to the join, as in JoinPaths, if they represent foreign and custom scans replacing a join with a scan, and by 2) modifying create_scan_plan() in createplan.c to use that list in that case, instead of the baserestrictinfo list, to get pseudoconstant quals assigned to the join; but #1 would cause an ABI break. So fix by modifying the infrastructure to just disallow replacing joins with such quals. Back-patch to all supported branches. Reported by Nishant Sharma. Patch by me, reviewed by Nishant Sharma and Richard Guo. Discussion: https://postgr.es/m/CADrsxdbcN1vejBaf8a%2BQhrZY5PXL-04mCd4GDu6qm6FigDZd6Q%40mail.gmail.com
* Eliminate fixed token-length limit in hba.c.Tom Lane2023-07-27
| | | | | | | | | | | | | | | | | | | Historically, hba.c limited tokens in the authentication configuration files (pg_hba.conf and pg_ident.conf) to less than 256 bytes. We have seen a few reports of this limit causing problems; notably, for moderately-complex LDAP configurations. Let's get rid of the fixed limit by using a StringInfo instead of a fixed-size buffer. This actually takes less code than before, since we can get rid of a nontrivial error recovery stanza. It's doubtless a hair slower, but parsing the content of the HBA files should in no way be performance-critical. Although this is a pretty straightforward patch, it doesn't seem worth the risk to back-patch given the small number of complaints to date. In released branches, we'll just raise MAX_TOKEN to ameliorate the problem. Discussion: https://postgr.es/m/1588937.1690221208@sss.pgh.pa.us
* Fix performance problem with new COPY DEFAULT codeDavid Rowley2023-07-27
| | | | | | | | | | | | | | | | 9f8377f7a added code to allow COPY FROM insert a column's default value when the input matches the DEFAULT string specified in the COPY command. Here we fix some inefficient code which needlessly palloc0'd an array to store if we should use the default value or input value for the given column. This array was being palloc0'd and pfree'd once per row. It's much more efficient to allocate this once and just reset the values once per row. Reported-by: Masahiko Sawada Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoDvDmUQeJtZrau1ovnT_smN940%3DKp6mszNGK3bq9yRN6g%40mail.gmail.com Backpatch-through: 16, where 9f8377f7a was introduced.
* Fix crash with RemoveFromWaitQueue() when detecting a deadlock.Masahiko Sawada2023-07-26
| | | | | | | | | | | | | | | | | | Commit 5764f611e used dclist_delete_from() to remove the proc from the wait queue. However, since it doesn't clear dist_node's next/prev to NULL, it could call RemoveFromWaitQueue() twice: when the process detects a deadlock and then when cleaning up locks on aborting the transaction. The waiting lock information is cleared in the first call, so it led to a crash in the second call. Backpatch to v16, where the change was introduced. Bug: #18031 Reported-by: Justin Pryzby, Alexander Lakhin Reviewed-by: Andres Freund Discussion: https://postgr.es/m/ZKy4AdrLEfbqrxGJ%40telsasoft.com Discussion: https://postgr.es/m/18031-ebe2d08cb405f6cc@postgresql.org Backpatch-through: 16
* Remove unnecessary checks for indexes for REPLICA IDENTITY FULL tables.Masahiko Sawada2023-07-25
| | | | | | | | | | | | | | | | | | | | | | Previously, when selecting an usable index for update/delete for the REPLICA IDENTITY FULL table, in IsIndexOnlyExpression(), we used to check if all index fields are not expressions. However, it was not necessary, because it is enough to check if only the leftmost index field is not an expression (and references the remote table column) and this check has already been done by RemoteRelContainsLeftMostColumnOnIdx(). This commit removes IsIndexOnlyExpression() and RemoteRelContainsLeftMostColumnOnIdx() and all checks for usable indexes for REPLICA IDENTITY FULL tables are now performed by IsIndexUsableForReplicaIdentityFull(). Backpatch this to remain the code consistent. Reported-by: Peter Smith Reviewed-by: Amit Kapila, Önder Kalacı Discussion: https://postgr.es/m/CAHut%2BPsGRE5WSsY0jcLHJEoA17MrbP9yy8FxdjC_ZOAACxbt%2BQ%40mail.gmail.com Backpatch-through: 16
* Fix the display of UNKNOWN message type in apply worker.Amit Kapila2023-07-25
| | | | | | | | | | | | | | We include the message type while displaying an error context in the apply worker. Now, while retrieving the message type string if the message type is unknown we throw an error that will hide the original error. So, instead, we need to simply return the string indicating an unknown message type. Reported-by: Ashutosh Bapat Author: Euler Taveira, Amit Kapila Reviewed-by: Ashutosh Bapat Backpatch-through: 15 Discussion: https://postgr.es/m/CAExHW5suAEDW-mBZt_qu4RVxWZ1vL54-L+ci2zreYWebpzxYsA@mail.gmail.com
* Fix off-by-one in LimitAdditionalPins()Andres Freund2023-07-24
| | | | | | | | | | | | | | | Due to the bug LimitAdditionalPins() could return 0, violating LimitAdditionalPins()'s API ("One additional pin is always allowed"). This could be hit when setting shared_buffers very low and using a fair amount of concurrency. This bug was introduced in 31966b151e6a. Author: "Anton A. Melnikov" <aamelnikov@inbox.ru> Reported-by: "Anton A. Melnikov" <aamelnikov@inbox.ru> Reported-by: Victoria Shepard Discussion: https://postgr.es/m/ae46f2fb-5586-3de0-b54b-1bb0f6410ebd@inbox.ru Backpatch: 16-
* Compare only major versions in AdjustUpgrade.pmAlvaro Herrera2023-07-24
| | | | | | | | | | | | | | | | | | | Because PostgreSQL::Version is very nuanced about development version numbers, the comparison to 16beta2 makes it think that that release is older than 16, therefore applying a database tweak that doesn't work there (the comparison is only supposed to match when run on version 15). As suggested by Andrew Dunstan, fix by having AdjustUpgrade.pm public methods create a separate PostgreSQL::Version object to use for these comparisons, that only carries the major version number. While at it, have the same methods ensure that the objects given are of the expected type. Backpatch to 16. This module goes all the way back to 9.2, but there's probably no need for this fix except where betas still live. Co-authored-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://postgr.es/m/20230719110504.zbu74o54bqqlsufb@alvherre.pgsql
* Avoid compiler warning in non-assert builds.Tom Lane2023-07-22
| | | | | | | | | | | After 3c90dcd03, try_partitionwise_join's child_joinrelids variable is read only in an Assert, provoking a compiler warning in non-assert builds. Rearrange code to avoid the warning and eliminate unnecessary work in the non-assert case. Per CI testing (via Jeff Davis and Bharath Rupireddy) Discussion: https://postgr.es/m/ef0de9713e605451f1b60b30648c5ee900b2394c.camel@j-davis.com
* Fix calculation of relid sets for partitionwise child joins.Tom Lane2023-07-21
| | | | | | | | | | | | | | | | | | | Applying add_outer_joins_to_relids() to a child join doesn't actually work, even if we've built a SpecialJoinInfo specialized to the child, because that function will also compare the join's relids to elements of the main join_info_list, which only deal in regular relids not child relids. This mistake escaped detection by the existing partitionwise join tests because they didn't test any cases where add_outer_joins_to_relids() needs to add additional OJ relids (that is, any cases where join reordering per identity 3 is possible). Instead, let's apply adjust_child_relids() to the relids of the parent join. This requires minor code reordering to collect the relevant AppendRelInfo structures first, but that's work we'd do shortly anyway. Report and fix by Richard Guo; cosmetic changes by me Discussion: https://postgr.es/m/CAMbWs49NCNbyubZWgci3o=_OTY=snCfAPtMnM-32f3mm-K-Ckw@mail.gmail.com
* Don't include CaseTestExpr in JsonValueExpr.formatted_exprAmit Langote2023-07-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | A CaseTestExpr is currently being put into JsonValueExpr.formatted_expr as placeholder for the result of evaluating JsonValueExpr.raw_expr, which in turn is evaluated separately. Though, there's no need for this indirection if raw_expr itself can be embedded into formatted_expr and evaluated as part of evaluating the latter, especially as there is no special reason to evaluate it separately. So this commit makes it so. As a result, JsonValueExpr.raw_expr no longer needs to be evaluated in ExecInterpExpr(), eval_const_exprs_mutator() etc. and is now only used for displaying the original "unformatted" expression in ruleutils.c. Comments about and the code manipulating formatted_expr is updated to mention that it is now always set and is the expression that gives a JsonValueExpr its runtime value. While at it, this also removes the function makeCaseTestExpr(), because the code in makeJsonConstructorExpr() looks more readable without it IMO and isn't used by anyone else either. Finally, a note is added in the comment above CaseTestExpr's definition that JsonConstructorExpr is also using it. Backpatched to 16 from the development branch to keep the code in sync across branches. Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Guard against null plan pointer in CachedPlanIsSimplyValid().Tom Lane2023-07-20
| | | | | | | | | | | | | If both the passed-in plan pointer and plansource->gplan are NULL, CachedPlanIsSimplyValid would think that the plan pointer is possibly-valid and try to dereference it. For the one extant call site in plpgsql, this situation doesn't normally happen which is why we've not noticed. However, it appears to be possible if the previous use of the cached plan failed, as per report from Justin Pryzby. Add an extra check to prevent crashing. Back-patch to v13 where this code was added. Discussion: https://postgr.es/m/ZLlV+STFz1l/WhAQ@telsasoft.com
* Pass constructName to transformJsonValueExpr()Amit Langote2023-07-20
| | | | | | | | | | | | This allows it to pass to coerce_to_specific_type() the actual name corresponding to the specific JSON_* function expression being transformed, instead of the currently hardcoded string. Backpatched to 16 from the development branch to keep the code in sync across branches. Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqHu58pO3cJ7rB6ZLwUztVdG1J66xSjDdjfan5uT5NhESw@mail.gmail.com
* Add psql \drg command to display role grants.Tom Lane2023-07-19
| | | | | | | | | | | | | | | | | | | | | | | With the addition of INHERIT and SET options for role grants, the historical display of role memberships in \du/\dg is woefully inadequate. Besides those options, there are pre-existing shortcomings that you can't see the ADMIN option nor the grantor. To fix this, remove the "Member of" column from \du/\dg altogether (making that output usefully narrower), and invent a new meta-command "\drg" that is specifically for displaying role memberships. It shows one row for each role granted to the selected role(s), with the grant options and grantor. We would not normally back-patch such a feature addition post feature freeze, but in this case the change is mainly driven by v16 changes in the server, so it seems appropriate to include it in v16. Pavel Luzanov, with bikeshedding and review from a lot of people, but particularly David Johnston Discussion: https://postgr.es/m/b9be2d0e-a9bc-0a30-492f-a4f68e4f7740@postgrespro.ru
* Fix indentation in twophase.cMichael Paquier2023-07-18
| | | | | | | | | This has been missed in cb0cca1, noticed before buildfarm member koel has been able to complain while poking at a different patch. Like the other commit, backpatch all the way down to limit the odds of merge conflicts. Backpatch-through: 11
* Fix recovery of 2PC transaction during crash recoveryMichael Paquier2023-07-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A crash in the middle of a checkpoint with some two-phase state data already flushed to disk by this checkpoint could cause a follow-up crash recovery to recover twice the same transaction, once from what has been found in pg_twophase/ at the beginning of recovery and a second time when replaying its corresponding record. This would lead to FATAL failures in the startup process during recovery, where the same transaction would have a state recovered twice instead of once: LOG: recovering prepared transaction 731 from shared memory LOG: recovering prepared transaction 731 from shared memory FATAL: lock ExclusiveLock on object 731/0/0 is already held This issue is fixed by skipping the addition of any 2PC state coming from a record whose equivalent 2PC state file has already been loaded in TwoPhaseState at the beginning of recovery by restoreTwoPhaseData(), which is OK as long as the system has not reached a consistent state. The timing to get a messed up recovery processing is very racy, and would very unlikely happen. The thread that has reported the issue has demonstrated the bug using injection points to force a PANIC in the middle of a checkpoint. Issue introduced in 728bd99, so backpatch all the way down. Reported-by: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com> Author: "suyu.cmj" <mengjuan.cmj@alibaba-inc.com> Author: Michael Paquier Discussion: https://postgr.es/m/109e6994-b971-48cb-84f6-829646f18b4c.mengjuan.cmj@alibaba-inc.com Backpatch-through: 11
* Include <limits.h> in fe-auth.c, to get CHAR_BIT reliably.Tom Lane2023-07-17
| | | | | | | | | | | | | fe-auth.c references CHAR_BIT since commit 3a465cc67, but it did not #include <limits.h>, which per POSIX is where that symbol is defined. This escaped notice so far because (a) on most platforms, <sys/param.h> pulls in <limits.h>, (b) even if yours doesn't, OpenSSL pulls it in, so compiling with --with-openssl masks the omission. Per bug #18026 from Marcel Hofstetter. Back-patch to v16. Discussion: https://postgr.es/m/18026-d5bb69f79cd16203@postgresql.org
* Add missing initializations of p_perminfoAmit Langote2023-07-14
| | | | | | | | | | | | In a61b1f74823, we failed to update transformFromClauseItem() and buildNSItemFromLists() to set ParseNamespaceItem.p_perminfo causing it to point to garbage. Pointed out by Tom Lane. Reported-by: Farias de Oliveira <matheusfarias519@gmail.com> Discussion: https://postgr.es/m/3173476.1689286373%40sss.pgh.pa.us Backpatch-through: 16
* Add indisreplident to fields refreshed by RelationReloadIndexInfo()Michael Paquier2023-07-14
| | | | | | | | | | | | | | | | | | | RelationReloadIndexInfo() is a fast-path used for index reloads in the relation cache, and it has always forgotten about updating indisreplident, which is something that would happen after an index is selected for a replica identity. This can lead to incorrect cache information provided when executing a command in a transaction context that updates indisreplident. None of the code paths currently on HEAD that need to check upon pg_index.indisreplident fetch its value from the relation cache, always relying on a fresh copy on the syscache. Unfortunately, this may not be the case of out-of-core code, that could see out-of-date value. Author: Shruthi Gowda Reviewed-by: Robert Haas, Dilip Kumar, Michael Paquier Discussion: https://postgr.es/m/CAASxf_PBcxax0wW-3gErUyftZ0XrCs3Lrpuhq4-Z3Fak1DoW7Q@mail.gmail.com Backpatch-through: 11
* Fix updates of indisvalid for partitioned indexesMichael Paquier2023-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | indisvalid is switched to true for partitioned indexes when all its partitions have valid indexes when attaching a new partition, up to the top-most parent if all its leaves are themselves valid when dealing with multiple layers of partitions. The copy of the tuple from pg_index used to switch indisvalid to true came from the relation cache, which is incorrect. Particularly, in the case reported by Shruthi Gowda, executing a series of commands in a single transaction would cause the validation of partitioned indexes to use an incorrect version of a pg_index tuple, as indexes are reloaded after an invalidation request with RelationReloadIndexInfo(), a much faster version than a full index cache rebuild. In this case, the limited information updated in the cache leads to an incorrect version of the tuple used. One of the symptoms reported was the following error, with a replica identity update, for instance: "ERROR: attempted to update invisible tuple" This is incorrect since 8b08f7d, so backpatch all the way down. Reported-by: Shruthi Gowda Author: Michael Paquier Reviewed-by: Shruthi Gowda, Dilip Kumar Discussion: https://postgr.es/m/CAASxf_PBcxax0wW-3gErUyftZ0XrCs3Lrpuhq4-Z3Fak1DoW7Q@mail.gmail.com Backpatch-through: 11
* Handle DROP DATABASE getting interruptedAndres Freund2023-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Until now, when DROP DATABASE got interrupted in the wrong moment, the removal of the pg_database row would also roll back, even though some irreversible steps have already been taken. E.g. DropDatabaseBuffers() might have thrown out dirty buffers, or files could have been unlinked. But we continued to allow connections to such a corrupted database. To fix this, mark databases invalid with an in-place update, just before starting to perform irreversible steps. As we can't add a new column in the back branches, we use pg_database.datconnlimit = -2 for this purpose. An invalid database cannot be connected to anymore, but can still be dropped. Unfortunately we can't easily add output to psql's \l to indicate that some database is invalid, it doesn't fit in any of the existing columns. Add tests verifying that a interrupted DROP DATABASE is handled correctly in the backend and in various tools. Reported-by: Evgeny Morozov <postgresql3@realityexists.net> Author: Andres Freund <andres@anarazel.de> Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Thomas Munro <thomas.munro@gmail.com> Discussion: https://postgr.es/m/20230509004637.cgvmfwrbht7xm7p6@awork3.anarazel.de Discussion: https://postgr.es/m/20230314174521.74jl6ffqsee5mtug@awork3.anarazel.de Backpatch: 11-, bug present in all supported versions
* Release lock after encountering bogs row in vac_truncate_clog()Andres Freund2023-07-13
| | | | | | | | | | | | | | | | | | | When vac_truncate_clog() encounters bogus datfrozenxid / datminmxid values, it returns early. Unfortunately, until now, it did not release WrapLimitsVacuumLock. If the backend later tries to acquire WrapLimitsVacuumLock, the session / autovacuum worker hangs in an uncancellable way. Similarly, other sessions will hang waiting for the lock. However, if the backend holding the lock exited or errored out for some reason, the lock was released. The bug was introduced as a side effect of 566372b3d643. It is interesting that there are no production reports of this problem. That is likely due to a mix of bugs leading to bogus values having gotten less common, process exit releasing locks and instances of hangs being hard to debug for "normal" users. Discussion: https://postgr.es/m/20230621221208.vhsqgduwfpzwxnpg@awork3.anarazel.de
* Fix untranslatable log message assemblyPeter Eisentraut2023-07-13
| | | | | | | | | We can't inject the name of the logical replication worker into a log message like that. But for these messages we don't really need the precision of knowing what kind of worker it was, so just write "logical replication worker" and keep the message in one piece. Discussion: https://www.postgresql.org/message-id/flat/CAHut%2BPt1xwATviPGjjtJy5L631SGf3qjV9XUCmxLu16cHamfgg%40mail.gmail.com
* Doc: clarify the conditions of usable indexes for REPLICA IDENTITY FULL tables.Masahiko Sawada2023-07-13
| | | | | | | | | | | | | | Commit 89e46da5e allowed REPLICA IDENTITY FULL tables to use an index on the subscriber during apply of update/delete. This commit clarifies in the documentation that the leftmost field of candidate indexes must be a column (not an expression) that references the published relation column. The source code comments are also updated accordingly. Reviewed-by: Peter Smith, Amit Kapila Discussion: https://postgr.es/m/CAD21AoDJjffEvUFKXT27Q5U8-UU9JHv4rrJ9Ke8Zkc5UPWHLvA@mail.gmail.com Backpatch-through: 16
* pg_bsd_indent: Collect test diffs in test.diffs instead of tests.diffAndres Freund2023-07-12
| | | | | | | That way CI knows to pick up the file if the test fails. Discussion: https://postgr.es/m/20230711233307.hu4wetabjm5f7ver@awork3.anarazel.de Backpatch: 16-, where the test was added
* Be more rigorous about local variables in PostgresMain().Tom Lane2023-07-10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since PostgresMain calls sigsetjmp, any local variables that are not marked "volatile" have a risk of unspecified behavior. In practice this means that when control returns via longjmp, such variables might get reset to their values as of the time of sigsetjmp, depending on whether the compiler chose to put them in registers or on the stack. We were careful about this for "send_ready_for_query", but not the other local variables. In the case of the timeout_enabled flags, resetting them to their initial "false" states is actually good, since we do "disable_all_timeouts()" in the longjmp cleanup code path. If that does not happen, we risk uselessly calling "disable_timeout()" later, which is harmless but a little bit expensive. Let's explicitly reset these flags so that the behavior is correct and platform-independent. (This change means that we really don't need the new "volatile" markings after all, but let's install them anyway since any change in this logic could re-introduce a problem.) There is no issue for "firstchar" and "input_message" because those are explicitly reinitialized each time through the query processing loop. To make that clearer, move them to be declared inside the loop. That leaves us with all the function-lifespan locals except the sigjmp_buf itself marked as volatile, which seems like a good policy to have going forward. Because of the possibility of extra disable_timeout() calls, this seems worth back-patching. Sergey Shinderuk and Tom Lane Discussion: https://postgr.es/m/2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru
* Fix pgindentPeter Eisentraut2023-07-10
| | | | for commit e53a611523
* Message wording improvementsPeter Eisentraut2023-07-10
|
* Fix ALTER EXTENSION SET SCHEMA with objects outside an extension's schemaMichael Paquier2023-07-10
| | | | | | | | | | | | | | | | | | | | | | | | As coded, the code would use as a base comparison the namespace OID from the first object scanned in pg_depend when switching its namespace dependency entry to the new one, and use it as a base of comparison for any follow-up checks. It would also be used as the old namespace OID to switch *from* for the extension's pg_depend entry. Hence, if the first object scanned has a namespace different than the one stored in the extension, we would finish by: - Not checking that the extension objects map with the extension's schema. - Not switching the extension -> namespace dependency entry to the new namespace provided by the user, making ALTER EXTENSION ineffective. This issue exists since this command has been introduced in d9572c4 for relocatable extension, so backpatch all the way down to 11. The test case has been provided by Heikki, that I have tweaked a bit to show the effects on pg_depend for the extension. Reported-by: Heikki Linnakangas Author: Michael Paquier, Heikki Linnakangas Discussion: https://postgr.es/m/20eea594-a05b-4c31-491b-007b6fceef28@iki.fi Backpatch-through: 11
* Fix tmpdir issues with commit e213de8e78Andrew Dunstan2023-07-08
| | | | | | | | | | | | Commit e213de8e78 fixed a problem with path lengths to a tempdir on Windows, but caused problems on at least some Unix systems where the system tempdir is on a different file system. To work around this, only used the system temdir for the destination of pg_replslot on Windows, and otherwise restore the old behaviour. Backpatch to relase 14 like the previous patch. Problem exposed by a myriad of buildfarm animals.
* Use shorter location for pg_replslot in pg_basebackup testAndrew Dunstan2023-07-08
| | | | | | | | The symlink to a longer location tripped up some Windows limit on buildfarm animal fairywren when running with meson, which uses slightly longer paths. Backpatch to all live branches to keep the script in sync.
* Fix Perl warningPeter Eisentraut2023-07-08
| | | | Use of uninitialized value $content in concatenation (.) or string
* Make some indentation in gram.y consistentPeter Eisentraut2023-07-08
| | | | | Reviewed-by: Amit Langote <amitlangote09@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Revert MAINTAIN privilege and pg_maintain predefined role.Nathan Bossart2023-07-07
| | | | | | | | | | | | | | | | This reverts the following commits: 4dbdb82513, c2122aae63, 5b1a879943, 9e1e9d6560, ff9618e82a, 60684dd834, 4441fc704d, and b5d6382496. A role with the MAINTAIN privilege may be able to use search_path tricks to escalate privileges to the table owner. Unfortunately, it is too late in the v16 development cycle to apply the proposed fix, i.e., restricting search_path when running maintenance commands. Bumps catversion. Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org Backpatch-through: 16
* Document relaxed HOT for summarizing indexesTomas Vondra2023-07-07
| | | | | | | | | | | | | Commit 19d8e2308b allowed a weaker check for HOT with summarizing indexes, but it did not update README.HOT. So do that now. Patch by Matthias van de Meent, minor changes by me. Backpatch to 16, where the optimization was introduced. Author: Matthias van de Meent Reviewed-by: Tomas Vondra Backpatch-through: 16 Discussion: https://postgr.es/m/CAEze2WiEOm8V+c9kUeYp2BPhbEc5s473fUf51xNeqvSFGv44Ew@mail.gmail.com
* Fix type of iterator variable in SH_START_ITERATEAndres Freund2023-07-06
| | | | | | | | | Also add comment to make the reasoning behind the Assert() more explicit (per Tom). Reported-by: Ranier Vilela Discussion: https://postgr.es/m/CAEudQAocXNJ6s1VLz+hMamLAQAiewRoW17OJ6-+9GACKfj6iPQ@mail.gmail.com Backpatch: 11-
* Skip pg_baseback long filename test if path too long on WindowsAndrew Dunstan2023-07-06
| | | | | | | | | | | | | | On Windows, it's sometimes difficult to create a file with a path longer than 255 chars, and if it can be created it might not be seen by the archiver. This can be triggered by the test for tar backups with filenames greater than 100 bytes. So we skip that test if the path would exceed 255. Backpatch to all live branches. Reviewed by Daniel Gustafsson Discussion: https://postgr.es/m/666ac55b-3400-fb2c-2cea-0281bf36a53c@dunslane.net
* WAL-log the creation of the init fork of unlogged indexes.Heikki Linnakangas2023-07-06
| | | | | | | | | | | | | | | | | | | We create a file, so we better WAL-log it. In practice, all the built-in index AMs and all extensions that I'm aware of write a metapage to the init fork, which is WAL-logged, and replay of the metapage implicitly creates the fork too. But if ambuildempty() didn't write any page, we would miss it. This can be seen with dummy_index_am. Set up replication, create a 'dummy_index_am' index on an unlogged table, and look at the files created in the replica: the init fork is not created on the replica. Dummy_index_am doesn't do anything with the relation files, however, so it doesn't lead to any user-visible errors. Backpatch to all supported versions. Reviewed-by: Robert Haas Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9%40iki.fi
* Revert the commits related to allowing page lock to conflict among parallel ↵Amit Kapila2023-07-06
| | | | | | | | | | | | | | | | | | | | group members. This commit reverts the work done by commits 3ba59ccc89 and 72e78d831a. Those commits were incorrect in asserting that we never acquire any other heavy-weight lock after acquring page lock other than relation extension lock. We can acquire a lock on catalogs while doing catalog look up after acquring page lock. This won't impact any existing feature but we need to think some other way to achieve this before parallelizing other write operations or even improving the parallelism in vacuum (like allowing multiple workers for an index). Reported-by: Jaime Casanova Author: Amit Kapila Backpatch-through: 13 Discussion: https://postgr.es/m/CAJKUy5jffnRKNvRHKQ0LynRb0RJC-o4P8Ku3x9vGAVLwDBWumQ@mail.gmail.com
* Fix leak of LLVM "fatal-on-oom" section counter.Heikki Linnakangas2023-07-05
| | | | | | | | | | | | | | | | llvm_release_context() called llvm_enter_fatal_on_oom(), but was missing the corresponding llvm_leave_fatal_on_oom() call. As a result, if JIT was used at all, we were almost always in the "fatal-on-oom" state. It only makes a difference if you use an extension written in C++, and run out of memory in a C++ 'new' call. In that case, you would get a PostgreSQL FATAL error, instead of the default behavior of throwing a C++ exception. Back-patch to all supported versions. Reviewed-by: Daniel Gustafsson Discussion: https://www.postgresql.org/message-id/54b78cca-bc84-dad8-4a7e-5b56f764fab5@iki.fi