aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* 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
* Revert "Add notBefore and notAfter to SSL cert info display"Daniel Gustafsson2023-07-20
| | | | | | | Due to an oversight in reviewing, this used functionality not compatible with old versions of OpenSSL. This reverts commit 75ec5e7bec700577d39d653c316e3ae6c505842c.
* Add notBefore and notAfter to SSL cert info displayDaniel Gustafsson2023-07-20
| | | | | | | | | This adds the X509 attributes notBefore and notAfter to sslinfo as well as pg_stat_ssl to allow verifying and identifying the validity period of the current client certificate. Author: Cary Huang <cary.huang@highgo.ca> Discussion: https://postgr.es/m/182b8565486.10af1a86f158715.2387262617218380588@highgo.ca
* Unify JSON categorize type API and export for external useAmit Langote2023-07-20
| | | | | | | | | | | | | | | | | | | | | | This essentially removes the JsonbTypeCategory enum and jsonb_categorize_type() and integrates any jsonb-specific logic that was in jsonb_categorize_type() into json_categorize_type(), now moved to jsonfuncs.c. The remaining JsonTypeCategory enum and json_categorize_type() cover the needs of the callers in both json.c and jsonb.c. json_categorize_type() has grown a new parameter named is_jsonb for callers to engage the jsonb-specific behavior of json_categorize_type(). One notable change in the now exported API of json_categorize_type() is that it now always returns *outfuncoid even though a caller may have no need currently to see one. This is in preparation of later commits to implement additional SQL/JSON functions. Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Add missing ObjectIdGetDatum() in syscache lookup calls for OidsMichael Paquier2023-07-20
| | | | | | | | | | Based on how postgres.h foes the Oid <-> Datum conversion, there is no existing bugs but let's be consistent. 17 spots have been noticed as incorrectly passing down Oids rather than Datums. Aleksander got one, Zhang two and I the rest. Author: Michael Paquier, Aleksander Alekseev, Zhang Mingli Discussion: https://postgr.es/m/ZLUhqsqQN1MOaxdw@paquier.xyz
* Support parenthesized syntax for CLUSTER without a table name.Nathan Bossart2023-07-19
| | | | | | | | | | | | | b5913f6120 added a parenthesized syntax for CLUSTER, but it requires specifying a table name. This is unlike commands such as VACUUM and ANALYZE, which do not require specifying a table in the parenthesized syntax. This change resolves this inconsistency. This is preparatory work for a follow-up commit that will move the unparenthesized syntax to the "Compatibility" section of the CLUSTER documentation. Reviewed-by: Melanie Plageman, Michael Paquier Discussion: https://postgr.es/m/CAAKRu_bc5uHieG1976kGqJKxyWtyQt9yvktjsVX%2Bi7NOigDjOA%40mail.gmail.com
* Rearrange CLUSTER rules in gram.y.Nathan Bossart2023-07-19
| | | | | | | | | | | This change moves the unparenthesized syntax for CLUSTER to the end of the ClusterStmt rules in preparation for a follow-up commit that will move this syntax to the "Compatibility" section of the CLUSTER documentation. The documentation for the CLUSTER syntaxes has also been consolidated. Suggested-by: Melanie Plageman Discussion https://postgr.es/m/CAAKRu_bc5uHieG1976kGqJKxyWtyQt9yvktjsVX%2Bi7NOigDjOA%40mail.gmail.com
* 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
* Remove db_user_namespace.Nathan Bossart2023-07-17
| | | | | | | | | | | This feature was intended to be a temporary measure to support per-database user names. A better one hasn't materialized in the ~21 years since it was added, and nobody claims to be using it, so let's just remove it. Reviewed-by: Michael Paquier, Magnus Hagander Discussion: https://postgr.es/m/20230630200509.GA2830328%40nathanxps13 Discussion: https://postgr.es/m/20230630215608.GD2941194%40nathanxps13
* Shrink memory contexts struct sizesDavid Rowley2023-07-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | Here we reduce the block size fields in AllocSetContext, GenerationContext and SlabContext from Size down to uint32. Ever since c6e0fe1f2, blocks for non-dedicated palloc chunks can no longer be larger than 1GB, so there's no need to store the various block size fields as 64-bit values. 32 bits are enough to store 2^30. Here we also further reduce the memory context struct sizes by getting rid of the 'keeper' field which stores a pointer to the context's keeper block. All the context types which have this field always allocate the keeper block in the same allocation as the memory context itself, so the keeper block always comes right at the end of the context struct. Add some macros to calculate that address rather than storing it in the context. Overall, in AllocSetContext and GenerationContext, this saves 20 bytes on 64-bit builds which for ALLOCSET_SMALL_SIZES can sometimes mean the difference between having to allocate a 2nd block and storing all the required allocations on the keeper block alone. Such contexts are used in relcache to store cache entries for indexes, of which there can be a large number in a single backend. Author: Melih Mutlu Reviewed-by: David Rowley Discussion: https://postgr.es/m/CAGPVpCSOW3uJ1QJmsMR9_oE3X7fG_z4q0AoU4R_w+2RzvroPFg@mail.gmail.com
* Allow plan nodes with initPlans to be considered parallel-safe.Tom Lane2023-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the plan itself is parallel-safe, and the initPlans are too, there's no reason anymore to prevent the plan from being marked parallel-safe. That restriction (dating to commit ab77a5a45) was really a special case of the fact that we couldn't transmit subplans to parallel workers at all. We fixed that in commit 5e6d8d2bb and follow-ons, but this case never got addressed. We still forbid attaching initPlans to a Gather node that's inserted pursuant to debug_parallel_query = regress. That's because, when we hide the Gather from EXPLAIN output, we'd hide the initPlans too, causing cosmetic regression diffs. It seems inadvisable to kluge EXPLAIN to the extent required to make the output look the same, so just don't do it in that case. Along the way, this also takes care of some sloppiness about updating path costs to match when we move initplans from one place to another during createplan.c and setrefs.c. Since all the planning decisions are already made by that point, this is just cosmetic; but it seems good to keep EXPLAIN output consistent with where the initplans are. The diff in query_planner() might be worth remarking on. I found that one because after fixing things to allow parallel-safe initplans, one partition_prune test case changed plans (as shown in the patch) --- but only when debug_parallel_query was active. The reason proved to be that we only bothered to mark Result nodes as potentially parallel-safe when debug_parallel_query is on. This neglects the fact that parallel-safety may be of interest for a sub-query even though the Result itself doesn't parallelize. Discussion: https://postgr.es/m/1129530.1681317832@sss.pgh.pa.us
* Account for optimized MinMax aggregates during SS_finalize_plan.Tom Lane2023-07-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We are capable of optimizing MIN() and MAX() aggregates on indexed columns into subqueries that exploit the index, rather than the normal thing of scanning the whole table. When we do this, we replace the Aggref node(s) with Params referencing subquery outputs. Such Params really ought to be included in the per-plan-node extParam/allParam sets computed by SS_finalize_plan. However, we've never done so up to now because of an ancient implementation choice to perform that substitution during set_plan_references, which runs after SS_finalize_plan, so that SS_finalize_plan never sees these Params. This seems like clearly a bug, yet there have been no field reports of problems that could trace to it. This may be because the types of Plan nodes that could contain Aggrefs do not have any of the rescan optimizations that are controlled by extParam/allParam. Nonetheless it seems certain to bite us someday, so let's fix it in a self-contained patch that can be back-patched if we find a case in which there's a live bug pre-v17. The cleanest fix would be to perform a separate tree walk to do these substitutions before SS_finalize_plan runs. That seems unattractive, first because a whole-tree mutation pass is expensive, and second because we lack infrastructure for visiting expression subtrees in a Plan tree, so that we'd need a new function knowing as much as SS_finalize_plan knows about that. I also considered swapping the order of SS_finalize_plan and set_plan_references, but that fell foul of various assumptions that seem tricky to fix. So the approach adopted here is to teach SS_finalize_plan itself to check for such Aggrefs. I refactored things a bit in setrefs.c to avoid having three copies of the code that does that. Given the lack of any currently-known bug, no test case here. Discussion: https://postgr.es/m/2391880.1689025003@sss.pgh.pa.us
* Improve error message for MaxAllocSize overrun in accumArrayResult.Tom Lane2023-07-14
| | | | | | | | | | | | | | | Before, if you went past about 64M array elements in array_agg() and allied functions, you got a generic "invalid memory alloc request size" error. This patch replaces that with "array size exceeds the maximum allowed", which seems more user-friendly since it points you to needing to reduce the size of your array result. (This is the same error text you'd get from construct_md_array in the event of overrunning the maximum physical size for the finished array.) Per question from Shaozhong Shi. Since this hasn't come up often, I don't feel a need to back-patch. Discussion: https://postgr.es/m/CA+i5JwYtVS9z2E71PcNKAVPbOn4R2wuj-LqbJsYr_XOz73q7dQ@mail.gmail.com
* 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
* Fix privilege check for SET SESSION AUTHORIZATION.Nathan Bossart2023-07-13
| | | | | | | | | | | | | | | | | | | Presently, the privilege check for SET SESSION AUTHORIZATION checks whether the original authenticated role was a superuser at connection start time. Even if the role loses the superuser attribute, its existing sessions are permitted to change session authorization to any role. This commit modifies this privilege check to verify the original authenticated role currently has superuser. In the event that the authenticated role loses superuser within a session authorization change, the authorization change will remain in effect, which means the user can still take advantage of the target role's privileges. However, [RE]SET SESSION AUTHORIZATION will only permit switching to the original authenticated role. Author: Joseph Koshakow Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com
* Move privilege check for SET SESSION AUTHORIZATION.Nathan Bossart2023-07-13
| | | | | | | | | | | | | | | | Presently, the privilege check for SET SESSION AUTHORIZATION is performed in session_authorization's assign_hook. A relevant comment states, "It's OK because the check does not require catalog access and can't fail during an end-of-transaction GUC reversion..." However, we plan to add a catalog lookup to this privilege check in a follow-up commit. This commit moves this privilege check to the check_hook for session_authorization. Like check_role(), we do not throw a hard error for insufficient privileges when the source is PGC_S_TEST. Author: Joseph Koshakow Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com
* Allow the use of a hash index on the subscriber during replication.Amit Kapila2023-07-14
| | | | | | | | | | | | | | | Commit 89e46da5e5 allowed using BTREE indexes that are neither PRIMARY KEY nor REPLICA IDENTITY on the subscriber during apply of update/delete. This patch extends that functionality to also allow HASH indexes. We explored supporting other index access methods as well but they don't have a fixed strategy for equality operation which is required by the current infrastructure in logical replication to scan the indexes. Author: Kuroda Hayato Reviewed-by: Peter Smith, Onder Kalaci, Amit Kapila Discussion: https://postgr.es/m/TYAPR01MB58669D7414E59664E17A5827F522A@TYAPR01MB5866.jpnprd01.prod.outlook.com
* 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
* Remove wal_sync_method=fsync_writethrough on Windows.Thomas Munro2023-07-14
| | | | | | | | | | | | The "fsync" level already flushes drive write caches on Windows (as does "fdatasync"), so it only confuses matters to have an apparently higher level that isn't actually different at all. That leaves "fsync_writethrough" only for macOS, where it actually does something different. Reviewed-by: Magnus Hagander <magnus@hagander.net> Discussion: https://postgr.es/m/CA%2BhUKGJ2CG2SouPv2mca2WCTOJxYumvBARRcKPraFMB6GSEMcA%40mail.gmail.com
* Add information about line contents on parsing failure of wait_event_names.txtMichael Paquier2023-07-14
| | | | | | | | | The contents of the line whose parsing failed was not reported in the error message produced by generate-wait_event_types.pl, making harder than necessary the debugging of incorrectly-shaped entries in the file. Reported-by: Andres Freund Discussion: https://postgr.es/m/ZK9S3jFEV1X797Ll@paquier.xyz
* Remove double quotes from the second column of wait_event_names.txtMichael Paquier2023-07-14
| | | | | | | | | | | The double quotes used for the wait event names are not required, as the values quoted are made of single words. The files generated by generate-wait_event_types.pl (pgstat_wait_event.c, wait_event_types.h and wait_event_types.sgml) are exactly the same before and after this commit, hence the wait event names and the enum elements have the same names as before. Discussion: https://postgr.es/m/ZK9S3jFEV1X797Ll@paquier.xyz
* 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
* Add missing const qualifierAmit Langote2023-07-13
| | | | | | | | Missed in commit 785480c9533d9. Pointed out by Tom Lane. Discussion: https://postgr.es/m/2795364.1689221300%40sss.pgh.pa.us
* Fix code indentation violation in commit b6e1157e7dAmit Langote2023-07-13
| | | | Per buildfarm member koel via Andrew Dunstan.
* 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
* Remove duplicated assignment of LLVMJitHandle->lljitMichael Paquier2023-07-13
| | | | | | | | | | This duplicated assignment when emiting some code not yet compiled. Oversight in 6c57f2e. Author: Matheus Alcantara Reviewed-by: Gurjeet Singh Discussion: https://postgr.es/m/La1Tfi7wirg9uGbCx_y7Qb9kl2T15mYouDXjCKAFuDqzQWnTRwH7KNLGigLLcxRs91V6dp2ySs1j_7mB4btzEZJ9NIMEAyOjUyAS7Jx-ydQ=@pm.me
* 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
* Rename session_auth_is_superuser to current_role_is_superuser.Nathan Bossart2023-07-12
| | | | | | | | | | | This variable might've been accurately named when it was added in ea886339b8, but the name hasn't been accurate since at least the introduction of SET ROLE in e5d6b91220. The corresponding documentation was fixed in eedb068c0a. This commit renames the variable accordingly. Suggested-by: Joseph Koshakow Discussion: https://postgr.es/m/CAAvxfHc-HHzONQ2oXdvhFF9ayRnidPwK%2BfVBhRzaBWYYLVQL-g%40mail.gmail.com
* Don't include CaseTestExpr in JsonValueExpr.formatted_exprAmit Langote2023-07-13
| | | | | | | | | | | | | | | | | | | | | | | | 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. 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. Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Pass constructName to transformJsonValueExpr()Amit Langote2023-07-13
| | | | | | | | | 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. Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
* Simplify some conditions related to [LW]Lock in generate-wait_event_types.plMichael Paquier2023-07-13
| | | | | | | | | | | The first check on the enum values was not necessary as the values set in wait_event_names.txt for the classes LWLock and Lock were able to satisfy the check. The second check when generating the C and header files is now based on a match of the class names, making it simpler to understand. Author: Masahiro Ikeda, Michael Paquier Discussion: https://postgr.es/m/eaf82a85c0ef1b55dc3b651d3f7b867a@oss.nttdata.com
* Remove ancient special case code for dropping oid columnsPeter Eisentraut2023-07-12
| | | | | | | | | | The special handling of negative attribute numbers in RemoveAttributeById() was introduced to support SET WITHOUT OIDS (commit 24614a9880). But that feature doesn't exist anymore, so we can revert to the previous, simpler version. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
* Remove ancient special case code for adding oid columnsPeter Eisentraut2023-07-12
| | | | | | | | | | | The special handling of negative attribute numbers in ATExecAddColumn() was introduced to support SET WITH OIDS (commit 6d1e361852). But that feature doesn't exist anymore, so we can revert to the previous, simpler version. In passing, also remove an obsolete comment about OID support. Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
* Remove obsolete comment about OID supportPeter Eisentraut2023-07-12
| | | | | Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
* Allow some exclusion constraints on partitionsPeter Eisentraut2023-07-12
| | | | | | | | | | | | | Previously we only allowed unique B-tree constraints on partitions (and only if the constraint included all the partition keys). But we could allow exclusion constraints with the same restriction. We also require that those columns be compared for equality, not something like &&. Author: Paul A. Jungwirth <pj@illuminatedcomputing.com> Reviewed-by: Ronan Dunklau <ronan.dunklau@aiven.io> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://www.postgresql.org/message-id/flat/ec8b1d9b-502e-d1f8-e909-1bf9dffe6fa5@illuminatedcomputing.com
* Report index vacuum progress.Masahiko Sawada2023-07-11
| | | | | | | | | | | | | | | | | This commit adds two columns: indexes_total and indexes_processed, to pg_stat_progress_vacuum system view to show the index vacuum progress. These numbers are reported in the "vacuuming indexes" and "cleaning up indexes" phases. This uses the new parallel message type for progress reporting added by be06506e7. Bump catversion because this changes the definition of pg_stat_progress_vacuum. Author: Sami Imseih Reviewed by: Masahiko Sawada, Michael Paquier, Nathan Bossart, Andres Freund Discussion: https://www.postgresql.org/message-id/flat/5478DFCD-2333-401A-B2F0-0D186AB09228@amazon.com
* Add new parallel message type to progress reporting.Masahiko Sawada2023-07-11
| | | | | | | | | | | | | | | | | | This commit adds a new type of parallel message 'P' to allow a parallel worker to poke at a leader to update the progress. Currently it supports only incremental progress reporting but it's possible to allow for supporting of other backend progress APIs in the future. There are no users of this new message type as of this commit. That will follow in future commits. Idea from Andres Freund. Author: Sami Imseih Reviewed by: Michael Paquier, Masahiko Sawada Discussion: https://www.postgresql.org/message-id/flat/5478DFCD-2333-401A-B2F0-0D186AB09228@amazon.com
* Don't expose Windows' mbstowcs_l() and wcstombs_l().Thomas Munro2023-07-11
| | | | | | | | | | | | | | | | | | | | Windows has similar functions with leading underscores. Previously, we provided the rename via a macro in win32_port.h. In fact its functions are not always good replacements for the Unix functions, since they can't deal with UTF-8. They are only currently used by pg_locale.c, which is careful to redirect to other Windows routines for UTF-8. Given that portability hazard, it seem unlikely to be a good idea to encourage any other code to think of these functions as being available outside pg_locale.c. Any code that thinks it wants these functions probably wants our wchar2char() or char2wchar() routines instead, or it won't actually work on Windows in UTF-8 databases. Furthermore, some major libc implementations including glibc don't have them (they only have the standard variants without _l), so external code is very unlikely to require them to exist. Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/CA%2BhUKG%2Bt_CHPzEoPnKyARJBJgE9-GxNajJo6ZuSfRK_KWFO%2B6w%40mail.gmail.com
* 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
|
* Add more sanity checks with callers of changeDependencyFor()Michael Paquier2023-07-10
| | | | | | | | | | | | | | | | | | changeDependencyFor() returns the number of pg_depend entries changed, or 0 if there is a problem. The callers of this routine expect only one dependency to change, but they did not check for the result returned. The following code paths gain checks: - Namespace for extensions. - Namespace for various object types (see AlterObjectNamespace). - Planner support function for a function. Some existing error messages related to all that are reworded to be more consistent with the project style, and the new error messages added follow the same style. This change has exposed one bug fixed a bit earlier with bd5ddbe. Reviewed-by: Heikki Linnakangas, Akshat Jaimini Discussion: https://postgr.es/m/ZJzD/rn+UbloKjB7@paquier.xyz
* 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
* Remove unnecessary unbind in LDAP search+bind modePeter Eisentraut2023-07-09
| | | | | | | | | | | | | | | | | | | | | | | | Comments in src/backend/libpq/auth.c say: (after successfully finding the final DN to check the user-supplied password against) /* Unbind and disconnect from the LDAP server */ and later /* * Need to re-initialize the LDAP connection, so that we can bind to * it with a different username. */ But the protocol actually permits multiple subsequent authentications ("binds") over a single connection. So, it seems like the whole connection re-initialization thing was just a confusion and can be safely removed, thus saving quite a few network round-trips, especially for the case of ldaps/starttls. Author: Anatoly Zaretsky <anatoly.zaretsky@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CALbq6kmJ-1+58df4B51ctPfTOSyPbY8Qi2=ct8oR=i4TamkUoQ@mail.gmail.com
* All supported systems have locale_t.Thomas Munro2023-07-09
| | | | | | | | | | | | | | | | | | | | locale_t is defined by POSIX.1-2008 and SUSv4, and available on all targeted systems. For Windows, win32_port.h redirects to a partial implementation called _locale_t. We can now remove a lot of compile-time tests for HAVE_LOCALE_T, and associated comments and dead code branches that were needed for older computers. Since configure + MinGW builds didn't detect locale_t but now we assume that all systems have it, further inconsistencies among the 3 Windows build systems were revealed. With this commit, we no longer define HAVE_WCSTOMBS_L and HAVE_MBSTOWCS_L on any Windows build system, but we have logic to deal with that so that replacements are available where appropriate. Reviewed-by: Noah Misch <noah@leadboat.com> Reviewed-by: Tristan Partin <tristan@neon.tech> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Discussion: https://postgr.es/m/CA%2BhUKGLg7_T2GKwZFAkEf0V7vbnur-NfCjZPKZb%3DZfAXSV1ORw%40mail.gmail.com
* 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