aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Use more consistently int64 for page numbers in SLRU-related codeMichael Paquier2024-07-23
| | | | | | | | | | | | | | clog.c, async.c and predicate.c included some SLRU page numbers still handled as 4-byte integers, while int64 should be used for this purpose. These holes have been introduced in 4ed8f0913bfd, that has introduced the use of 8-byte integers for SLRU page numbers, still forgot about the code paths updated by this commit. Reported-by: Noah Misch Author: Aleksander Alekseev, Michael Paquier Discussion: https://postgr.es/m/20240626002747.dc.nmisch@google.com Backpatch-through: 17
* ldapurl is supported with simple bindPeter Eisentraut2024-07-23
| | | | | | | | | | | | | | | The docs currently imply that ldapurl is for search+bind only, but that's not true. Rearrange the docs to cover this better. Add a test ldapurl with simple bind. This was previously allowed but unexercised, and now that it's documented it'd be good to pin the behavior. Improve error when mixing LDAP bind modes. The option names had gone stale; replace them with a more general statement. Author: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://www.postgresql.org/message-id/flat/CAOYmi+nyg9gE0LeP=xQ3AgyQGR=5ZZMkVVbWd0uR8XQmg_dd5Q@mail.gmail.com
* Get rid of a global variablePeter Eisentraut2024-07-23
| | | | | | | | bootstrap_data_checksum_version can just as easily be passed to where it is used via function arguments. Reviewed-by: Andres Freund <andres@anarazel.de> Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
* Improve comments in slru.{c,h} about segment name formatMichael Paquier2024-07-23
| | | | | | | | | | | | slru.h described incorrectly how SLRU segment names are formatted depending on the segment number and if long or short segment names are used. This commit closes the gap with a better description, fitting with the reality. Reported-by: Noah Misch Author: Aleksander Alekseev Discussion: https://postgr.es/m/20240626002747.dc.nmisch@google.com Backpatch-through: 17
* Replace remaining strtok() with strtok_r()Peter Eisentraut2024-07-23
| | | | | | | | for thread-safety in the server in the future Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: David Steele <david@pgmasters.net> Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org
* Remove redundant code in create_gather_merge_pathRichard Guo2024-07-23
| | | | | | | | | | | | | | | | In create_gather_merge_path, we should always guarantee that the subpath is adequately ordered, and we do not add a Sort node in createplan.c for a Gather Merge node. Therefore, the 'else' branch in create_gather_merge_path, which computes the cost for a Sort node, is redundant. This patch removes the redundant code and emits an error if the subpath is not sufficiently ordered. Meanwhile, this patch changes the check for the subpath's pathkeys in create_gather_merge_plan to an Assert. Author: Richard Guo Discussion: https://postgr.es/m/CAMbWs48u=0bWf3epVtULjJ-=M9Hbkz+ieZQAOS=BfbXZFqbDCg@mail.gmail.com
* Fix rowcount estimate for gather (merge) pathsRichard Guo2024-07-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the case of a parallel plan, when computing the number of tuples processed per worker, we divide the total number of tuples by the parallel_divisor obtained from get_parallel_divisor(), which accounts for the leader's contribution in addition to the number of workers. Accordingly, when estimating the number of tuples for gather (merge) nodes, we should multiply the number of tuples per worker by the same parallel_divisor to reverse the division. However, currently we use parallel_workers rather than parallel_divisor for the multiplication. This could result in an underestimation of the number of tuples for gather (merge) nodes, especially when there are fewer than four workers. This patch fixes this issue by using the same parallel_divisor for the multiplication. There is one ensuing plan change in the regression tests, but it looks reasonable and does not compromise its original purpose of testing parallel-aware hash join. In passing, this patch removes an unnecessary assignment for path.rows in create_gather_merge_path, and fixes an uninitialized-variable issue in generate_useful_gather_paths. No backpatch as this could result in plan changes. Author: Anthonin Bonnefoy Reviewed-by: Rafia Sabih, Richard Guo Discussion: https://postgr.es/m/CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com
* Initialize wal_level in the initial checkpoint record.Robert Haas2024-07-22
| | | | | | As per Coverity and Tom Lane, commit 402b586d0 (back-patched to v17 as 2b5819e2b) forgot to initialize this new structure member in this code path.
* Remove grotty use of disable_cost for TID scan plans.Robert Haas2024-07-22
| | | | | | | | | | | | | | | | | | | Previously, the code charged disable_cost for CurrentOfExpr, and then subtracted disable_cost from the cost of a TID path that used CurrentOfExpr as the TID qual, effectively disabling all paths except that one. Now, we instead suppress generation of the disabled paths entirely, and generate only the one that the executor will actually understand. With this approach, we do not need to rely on disable_cost being large enough to prevent the wrong path from being chosen, and we save some CPU cycle by avoiding generating paths that we can't actually use. In my opinion, the code is also easier to understand like this. Patch by me. Review by Heikki Linnakangas. Discussion: http://postgr.es/m/591b3596-2ea0-4b8e-99c6-fad0ef2801f5@iki.fi
* Add missing call to ConditionVariableCancelSleep().Robert Haas2024-07-22
| | | | | | | | | | After calling ConditionVariableSleep() or ConditionVariableTimedSleep() one or more times, code is supposed to call ConditionVariableCancelSleep() to remove itself from the waitlist. This code neglected to do so. As far as I know, that had no observable consequences, but let's make the code correct. Discussion: http://postgr.es/m/CA+TgmoYW8eR+KN6zhVH0sin7QH6AvENqw_bkN-bB4yLYKAnsew@mail.gmail.com
* Replace some strtok() with strsep()Peter Eisentraut2024-07-22
| | | | | | | | | | | | | | | | | | | | | | | | | | strtok() considers adjacent delimiters to be one delimiter, which is arguably the wrong behavior in some cases. Replace with strsep(), which has the right behavior: Adjacent delimiters create an empty token. Affected by this are parsing of: - Stored SCRAM secrets ("SCRAM-SHA-256$<iterations>:<salt>$<storedkey>:<serverkey>") - ICU collation attributes ("und@colStrength=primary;colCaseLevel=yes") for ICU older than version 54 - PG_COLORS environment variable ("error=01;31:warning=01;35:note=01;36:locus=01") - pg_regress command-line options with comma-separated list arguments (--dbname, --create-role) (currently only used pg_regress_ecpg) Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Reviewed-by: David Steele <david@pgmasters.net> Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org
* Add new error code for "file name too long"Michael Paquier2024-07-22
| | | | | | | | | | | This new error code, named file_name_too_long, maps internally to the errno ENAMETOOLONG to produce a proper error code rather than an internal code under errcode_for_file_access(). This error code can be reached with some SQL command patterns, like a snapshot file name. Reported-by: Alexander Lakhin Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/Zo4ROR9mgy8bowMo@paquier.xyz
* Correctly check updatability of columns targeted by INSERT...DEFAULT.Tom Lane2024-07-20
| | | | | | | | | | | | | | | If a view has some updatable and some non-updatable columns, we failed to verify updatability of any columns for which an INSERT or UPDATE on the view explicitly specifies a DEFAULT item (unless the view has a declared default for that column, which is rare anyway, and one would almost certainly not write one for a non-updatable column). This would lead to an unexpected "attribute number N not found in view targetlist" error rather than the intended error. Per bug #18546 from Alexander Lakhin. This bug is old, so back-patch to all supported branches. Discussion: https://postgr.es/m/18546-84a292e759a9361d@postgresql.org
* Use read streams in CREATE DATABASE when STRATEGY=WAL_LOG.Noah Misch2024-07-20
| | | | | | | | | | | | While this doesn't significantly change runtime now, it arranges for STRATEGY=WAL_LOG to benefit automatically from future optimizations to the read_stream subsystem. For large tables in the template database, this does read 16x as many bytes per system call. Platforms with high per-call overhead, if any, may see an immediate benefit. Nazir Bilal Yavuz Discussion: https://postgr.es/m/CAN55FZ0JKL6vk1xQp6rfOXiNFV1u1H0tJDPPGHWoiO3ea2Wc=A@mail.gmail.com
* Add a way to create read stream object by using SMgrRelation.Noah Misch2024-07-20
| | | | | | | | Currently read stream object can be created only by using Relation. Nazir Bilal Yavuz Discussion: https://postgr.es/m/CAN55FZ0JKL6vk1xQp6rfOXiNFV1u1H0tJDPPGHWoiO3ea2Wc=A@mail.gmail.com
* Refactor PinBufferForBlock() to remove checks about persistence.Noah Misch2024-07-20
| | | | | | | | | | There are checks in PinBufferForBlock() function to set persistence of the relation. This function is called for each block in the relation. Instead, set persistence of the relation before PinBufferForBlock(). Nazir Bilal Yavuz Discussion: https://postgr.es/m/CAN55FZ0JKL6vk1xQp6rfOXiNFV1u1H0tJDPPGHWoiO3ea2Wc=A@mail.gmail.com
* Remove "smgr_persistence == 0" dead code.Noah Misch2024-07-20
| | | | | | | | | Reaching that code would have required multiple processes performing relation extension during recovery, which does not happen. That caller has the persistence available, so pass it. This was dead code as soon as commit 210622c60e1a9db2e2730140b8106ab57d259d15 added it. Discussion: https://postgr.es/m/CAN55FZ0JKL6vk1xQp6rfOXiNFV1u1H0tJDPPGHWoiO3ea2Wc=A@mail.gmail.com
* Add overflow checks to money type.Nathan Bossart2024-07-19
| | | | | | | | | | | | | | | None of the arithmetic functions for the the money type handle overflow. This commit introduces several helper functions with overflow checking and makes use of them in the money type's arithmetic functions. Fixes bug #18240. Reported-by: Alexander Lakhin Author: Joseph Koshakow Discussion: https://postgr.es/m/18240-c5da758d7dc1ecf0%40postgresql.org Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com Backpatch-through: 12
* Ensure vacuum removes all visibly dead tuples older than OldestXminMelanie Plageman2024-07-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If vacuum fails to remove a tuple with xmax older than VacuumCutoffs->OldestXmin and younger than GlobalVisState->maybe_needed, it may attempt to freeze the tuple's xmax and then ERROR out in pre-freeze checks with "cannot freeze committed xmax". Fix this by having vacuum always remove tuples older than OldestXmin. It is possible for GlobalVisState->maybe_needed to precede OldestXmin if maybe_needed is forced to go backward while vacuum is running. This can happen if a disconnected standby with a running transaction older than VacuumCutoffs->OldestXmin reconnects to the primary after vacuum initially calculates GlobalVisState and OldestXmin. In back branches starting with 14, the first version using GlobalVisState, failing to remove tuples older than OldestXmin during pruning caused vacuum to infinitely loop in lazy_scan_prune(), as investigated on this [1] thread. After 1ccc1e05ae removed the retry loop in lazy_scan_prune() and stopped comparing tuples to OldestXmin, the hang could no longer happen, but we could still attempt to freeze dead tuples with xmax older than OldestXmin -- resulting in an ERROR. Fix this by always removing dead tuples with xmax older than VacuumCutoffs->OldestXmin. This is okay because the standby won't replay the tuple removal until the tuple is removable. Thus, the worst that can happen is a recovery conflict. [1] https://postgr.es/m/20240415173913.4zyyrwaftujxthf2%40awork3.anarazel.de#1b216b7768b5bd577a3d3d51bd5aadee Back-patch through 14 Author: Melanie Plageman Reviewed-by: Peter Geoghegan, Robert Haas, Andres Freund, Heikki Linnakangas, and Noah Misch Discussion: https://postgr.es/m/CAAKRu_bDD7oq9ZwB2OJqub5BovMG6UjEYsoK2LVttadjEqyRGg%40mail.gmail.com
* Move resowner from common JitContext to LLVM specificHeikki Linnakangas2024-07-19
| | | | | | | | | | Only the LLVM specific code uses it since resource owners were made extensible in commit b8bff07daa85c837a2747b4d35cd5a27e73fb7b2. This is new in v17, so backpatch there to keep the branches from diverging just yet. Author: Andreas Karlsson <andreas@proxel.se> Discussion: https://www.postgresql.org/message-id/fd3a2a00-6605-4e30-a118-48418b478e6e@proxel.se
* Propagate query IDs of utility statements in functionsMichael Paquier2024-07-19
| | | | | | | | | | | | | | | | | | | For utility statements defined within a function, the query tree is copied to a PlannedStmt as utility commands do not require planning. However, the query ID was missing from the information passed down. This leads to plugins relying on the query ID like pg_stat_statements to not be able to track utility statements within function calls. Tests are added to check this behavior, depending on pg_stat_statements.track. This is an old bug. Now, query IDs for utilities are compiled using their parsed trees rather than the query string since v16 (3db72ebcbe20), leading to less bloat with utilities, so backpatch down only to this version. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_XqrGp-uwBqi3vBPLuRULKkddjC7R5QZCgsFren=8E+m2Sg@mail.gmail.com Backpatch-through: 16
* Do not summarize WAL if generated with wal_level=minimal.Robert Haas2024-07-18
| | | | | | | | | | | | | | | | | | | | | | | | | | To do this, we must include the wal_level in the first WAL record covered by each summary file; so add wal_level to struct Checkpoint and the payload of XLOG_CHECKPOINT_REDO and XLOG_END_OF_RECOVERY. This, in turn, requires bumping XLOG_PAGE_MAGIC and, since the Checkpoint is also stored in the control file, also PG_CONTROL_VERSION. It's not great to do that so late in the release cycle, but the alternative seems to ship v17 without robust protections against this scenario, which could result in corrupted incremental backups. A side effect of this patch is that, when a server with wal_level=replica is started with summarize_wal=on for the first time, summarization will no longer begin with the oldest WAL that still exists in pg_wal, but rather from the first checkpoint after that. This change should be harmless, because a WAL summary for a partial checkpoint cycle can never make an incremental backup possible when it would otherwise not have been. Report by Fujii Masao. Patch by me. Review and/or testing by Jakub Wartak and Fujii Masao. Discussion: http://postgr.es/m/6e30082e-041b-4e31-9633-95a66de76f5d@oss.nttdata.com
* Add INJECTION_POINT_CACHED() to run injection points directly from cacheMichael Paquier2024-07-18
| | | | | | | | | | | | | | | | | | | | | | This new macro is able to perform a direct lookup from the local cache of injection points (refreshed each time a point is loaded or run), without touching the shared memory state of injection points at all. This works in combination with INJECTION_POINT_LOAD(), and it is better than INJECTION_POINT() in a critical section due to the fact that it would avoid all memory allocations should a concurrent detach happen since a LOAD(), as it retrieves a callback from the backend-private memory. The documentation is updated to describe in more details how to use this new macro with a load. Some tests are added to the module injection_points based on a new SQL function that acts as a wrapper of INJECTION_POINT_CACHED(). Based on a suggestion from Heikki Linnakangas. Author: Heikki Linnakangas, Michael Paquier Discussion: https://postgr.es/m/58d588d0-e63f-432f-9181-bed29313dece@iki.fi
* Use PqMsg_* macros in more places.Nathan Bossart2024-07-17
| | | | | | | | | | | Commit f4b54e1ed9, which introduced macros for protocol characters, missed updating a few places. It also did not introduce macros for messages sent from parallel workers to their leader processes. This commit adds a new section in protocol.h for those. Author: Aleksander Alekseev Discussion: https://postgr.es/m/CAJ7c6TNTd09AZq8tGaHS3LDyH_CCnpv0oOz2wN1dGe8zekxrdQ%40mail.gmail.com Backpatch-through: 17
* SQL/JSON: Rethink c2d93c3802bAmit Langote2024-07-17
| | | | | | | | | | | | | | | | | | | This essentially reverts c2d93c3802b except tests. The problem with c2d93c3802b was that it only changed the casting behavior for types with typmod, and had coding issues noted in the post-commit review. This commit changes coerceJsonFuncExpr() to use assignment-level casts instead of explicit casts to coerce the result of JSON constructor functions to the specified or the default RETURNING type. Using assignment-level casts fixes the problem that using explicit casts was leading to the wrong typmod / length coercion behavior -- truncating results longer than the specified length instead of erroring out -- which c2d93c3802b aimed to solve. That restricts the set of allowed target types to string types, the same set that's currently allowed. Discussion: https://postgr.es/m/202406291824.reofujy7xdj3@alvherre.pgsql
* Make write of pgstats file durable at shutdownMichael Paquier2024-07-17
| | | | | | | | | | | | | | This switches the pgstats write code to use durable_rename() rather than rename(). This ensures that the stats file's data is durable when the statistics are written, which is something only happening at shutdown now with the checkpointer doing the job. This could cause the statistics to be lost even after PostgreSQL is shut down, should a host failure happen, for example. Suggested-by: Konstantin Knizhnik Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/ZpDQTZ0cAz0WEbh7@paquier.xyz
* When creating materialized views, use REFRESH to load data.Jeff Davis2024-07-16
| | | | | | | | | | | | | Previously, CREATE MATERIALIZED VIEW ... WITH DATA populated the MV the same way as CREATE TABLE ... AS. Instead, reuse the REFRESH logic, which locks down security-restricted operations and restricts the search_path. This reduces the chance that a subsequent refresh will fail. Reported-by: Noah Misch Backpatch-through: 17 Discussion: https://postgr.es/m/20240630222344.db.nmisch@google.com
* Add tap test for pg_signal_autovacuum roleMichael Paquier2024-07-16
| | | | | | | | | | | | | | This commit provides testig coverage for ccd38024bc3c, checking that a role granted pg_signal_autovacuum_worker is able to stop a vacuum worker. An injection point with a wait is placed at the beginning of autovacuum worker startup to make sure that a worker is still alive when sending and processing the signal sent. Author: Anthony Leung, Michael Paquier, Kirill Reshke Reviewed-by: Andrey Borodin, Nathan Bossart Discussion: https://postgr.es/m/CALdSSPiQPuuQpOkF7x0g2QkA5eE-3xXt7hiJFvShV1bHKDvf8w@mail.gmail.com
* Fix bad indentation introduced in 43cd30bcd1cAndres Freund2024-07-15
| | | | | | | | Oops. Reported-by: Nathan Bossart <nathandbossart@gmail.com> Discussion: https://postgr.es/m/ZpVZB9rH5tHllO75@nathan Backpatch: 12-, like 43cd30bcd1c
* Add missing RestrictSearchPath() calls.Jeff Davis2024-07-15
| | | | | | Reported-by: Noah Misch Backpatch-through: 17 Discussion: https://postgr.es/m/20240630222344.db.nmisch@google.com
* Fix type confusion in guc_var_compare()Andres Freund2024-07-15
| | | | | | | | | | | | | | | | | Before this change guc_var_compare() cast the input arguments to const struct config_generic *. That's not quite right however, as the input on one side is often just a char * on one side. Instead just use char *, the first field in config_generic. This fixes a -Warray-bounds warning with some versions of gcc. While the warning is only known to be triggered for <= 15, the issue the warning points out seems real, so apply the fix everywhere. Author: Nazir Bilal Yavuz <byavuz81@gmail.com> Reported-by: Erik Rijkers <er@xs4all.nl> Suggested-by: Andres Freund <andres@anarazel.de> Discussion: https://postgr.es/m/a74a1a0d-0fd2-3649-5224-4f754e8f91aa%40xs4all.nl
* Run LLVM verify pass on IR in assert builds.Thomas Munro2024-07-15
| | | | | | | | | | The problem fixed by commit 53c8d6c9 would have been noticed if we'd been running LLVM's verify pass on generated IR. Doing so also reveals a complaint about incorrect name mangling, fixed here. Only enabled for LLVM 17+ because it uses the new pass manager API. Suggested-by: Dmitry Dolgov <9erthalion6@gmail.com> Discussion: https://postgr.es/m/CAFj8pRACpVFr7LMdVYENUkScG5FCYMZDDdSGNU-tch%2Bw98OxYg%40mail.gmail.com
* Use correct type for pq_mq_parallel_leader_proc_number variableHeikki Linnakangas2024-07-15
| | | | | | | | | It's a ProcNumber, not a process id. Both are integers, so it's harmless, but clearly wrong. It's been wrong since forever, the mistake has survived through a couple of refactorings already. Spotted-by: Thomas Munro Discussion: https://www.postgresql.org/message-id/CA+hUKGKPTLSGMyE4Brin-osY8omPLNXmVWDMfrRABLp=6QrR_Q@mail.gmail.com
* Use atomics to avoid locking in InjectionPointRun()Heikki Linnakangas2024-07-15
| | | | | | | | | | This allows using injection points without having a PGPROC, like early at backend startup, or in the postmaster. The injection points facility is new in v17, so backpatch there. Reviewed-by: Michael Paquier <michael@paquier.xyz> Disussion: https://www.postgresql.org/message-id/4317a7f7-8d24-435e-9e49-29b72a3dc418@iki.fi
* Fix tablespace handling in MERGE/SPLIT partition commands.Fujii Masao2024-07-15
| | | | | | | | | | | | | | | As commit ca4103025d stated, new partitions without a specified tablespace should inherit the parent relation's tablespace. However, previously, ALTER TABLE MERGE PARTITIONS and ALTER TABLE SPLIT PARTITION commands always created new partitions in the default tablespace, ignoring the parent's tablespace. This commit ensures new partitions inherit the parent's tablespace. Backpatch to v17 where these commands were introduced. Author: Fujii Masao Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com
* Check lateral references within PHVs for memoize cache keysRichard Guo2024-07-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | If we intend to generate a Memoize node on top of a path, we need cache keys of some sort. Currently we search for the cache keys in the parameterized clauses of the path as well as the lateral_vars of its parent. However, it turns out that this is not sufficient because there might be lateral references derived from PlaceHolderVars, which we fail to take into consideration. This oversight can cause us to miss opportunities to utilize the Memoize node. Moreover, in some plans, failing to recognize all the cache keys could result in performance regressions. This is because without identifying all the cache keys, we would need to purge the entire cache every time we get a new outer tuple during execution. This patch fixes this issue by extracting lateral Vars from within PlaceHolderVars and subsequently including them in the cache keys. In passing, this patch also includes a comment clarifying that Memoize nodes are currently not added on top of join relation paths. This explains why this patch only considers PlaceHolderVars that are due to be evaluated at baserels. Author: Richard Guo Reviewed-by: Tom Lane, David Rowley, Andrei Lepikhov Discussion: https://postgr.es/m/CAMbWs48jLxn0pAPZpJ50EThZ569Xrw+=4Ac3QvkpQvNszbeoNg@mail.gmail.com
* Avoid unhelpful internal error for incorrect recursive-WITH queries.Tom Lane2024-07-14
| | | | | | | | | | | | | | | | checkWellFormedRecursion would issue "missing recursive reference" if a WITH RECURSIVE query contained a single self-reference but that self-reference was inside a top-level WITH, ORDER BY, LIMIT, etc, rather than inside the second arm of the UNION as expected. We already intended to throw more-on-point errors for such cases, but those error checks must be done before examining the UNION arm in order to have the desired results. So this patch need only move some code (and improve the comments). Per bug #18536 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18536-0a342ec07901203e@postgresql.org
* Fix new assertion for MERGE view_name ... DO NOTHING.Noah Misch2024-07-13
| | | | | | | | | | | | | Such queries don't expand automatically updatable views, and ModifyTable uses the wholerow attribute unconditionally. The user-visible behavior is fine, so change to more-specific assertions. Commit d5f788b41dc2cbdde6e7694c70dda54d829a5ed5 added the wrong assertion. Back-patch to v17, where commit 5f2e179bd31e5f5803005101eb12a8d7bf8db8f3 introduced MERGE view_name. Reported by Alexander Lakhin. Discussion: https://postgr.es/m/e4b40a88-c134-6926-3196-bc4501cb87a2@gmail.com
* Don't lose partitioned table reltuples=0 after relhassubclass=f.Noah Misch2024-07-13
| | | | | | | | | | | | | | ANALYZE sets relhassubclass=f when a partitioned table no longer has partitions. An ANALYZE doing that proceeded to apply the inplace update of pg_class.reltuples to the old pg_class tuple instead of the new tuple, losing that reltuples=0 change if the ANALYZE committed. Non-partitioning inheritance trees were unaffected. Back-patch to v14, where commit 375aed36ad83f0e021e9bdd3a0034c0c992c66dc introduced maintenance of partitioned table pg_class.reltuples. Reported by Alexander Lakhin. Discussion: https://postgr.es/m/a295b499-dcab-6a99-c06e-01cf60593344@gmail.com
* Fix lost Windows socket EOF events.Thomas Munro2024-07-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Winsock only signals an FD_CLOSE event once if the other end of the socket shuts down gracefully. Because each WaitLatchOrSocket() call constructs and destroys a new event handle every time, with unlucky timing we can lose it and hang. We get away with this only if the other end disconnects non-gracefully, because FD_CLOSE is repeatedly signaled in that case. To fix this design flaw in our Windows socket support fundamentally, we'd probably need to rearchitect it so that a single event handle exists for the lifetime of a socket, or switch to completely different multiplexing or async I/O APIs. That's going to be a bigger job and probably wouldn't be back-patchable. This brute force kludge closes the race by explicitly polling with MSG_PEEK before sleeping. Back-patch to all supported releases. This should hopefully clear up some random build farm and CI hang failures reported over the years. It might also allow us to try using graceful shutdown in more places again (reverted in commit 29992a6) to fix instability in the transmission of FATAL error messages, but that isn't done by this commit. Reported-by: Tom Lane <tgl@sss.pgh.pa.us> Tested-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://postgr.es/m/176008.1715492071%40sss.pgh.pa.us
* Fix ALTER TABLE DETACH for inconsistent indexesAlvaro Herrera2024-07-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a partitioned table has an index that doesn't support a constraint, but a partition has an equivalent index that does, then a DETACH operation would misbehave: a crash in assertion-enabled systems (because we fail to find the constraint in the parent that we expect to), or a broken coninhcount value (-1) in production systems (because we blindly believe that we've successfully detached the parent). While we should reject an ATTACH of a partition with such an index, we have failed to do so in existing releases, so adding an error in stable releases might break the (unlikely) existing applications that rely on this behavior. At this point I don't even want to reject them in master, because it'd break pg_upgrade if such databases exist, and there would be no easy way to fix existing databases without expensive index rebuilds. (Later on we could add ALTER TABLE ... ADD CONSTRAINT USING INDEX to partitioned tables, which would allow the user to fix such patterns. At that point we could add more restrictions to prevent the problem from its root.) Also, add a test case that leaves one table in this condition, so that we can verify that pg_upgrade continues to work if we later decide to change the policy on the master branch. Backpatch to all supported branches. Co-authored-by: Tender Wang <tndrwang@gmail.com> Reported-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/18500-62948b6fe5522f56@postgresql.org
* Add assertion in pgstat_write_statsfile() about processes allowedMichael Paquier2024-07-12
| | | | | | | | | | | | This routine can currently only be called from the postmaster in single-user mode or the checkpointer, but there was no sanity check to make sure that this was always the case. This has proved to be useful when hacking the zone (at least to me), to make sure that the write of the pgstats file happens at shutdown, as wanted by design, in the correct process context. Discussion: https://postgr.es/m/ZnEiqAITL-VgZDoY@paquier.xyz
* Fix a typo in logicalrep_write_typ().Amit Kapila2024-07-12
| | | | | Author: ChangAo Chen Discussion: https://postgr.es/m/tencent_CDECB843B30A8B6B5152FA6458F0F00FDE09@qq.com
* Consider materializing the cheapest inner path in parallel nestloopRichard Guo2024-07-12
| | | | | | | | | | | | | | When generating non-parallel nestloop paths for each available outer path, we always consider materializing the cheapest inner path if feasible. Similarly, in this patch, we also consider materializing the cheapest inner path when building partial nestloop paths. This approach potentially reduces the need to rescan the inner side of a partial nestloop path for each outer tuple. Author: Tender Wang Reviewed-by: Richard Guo, Robert Haas, David Rowley, Alena Rybakina Reviewed-by: Tomasz Rybak, Paul Jungwirth, Yuki Fujii Discussion: https://postgr.es/m/CAHewXNkPmtEXNfVQMou_7NqQmFABca9f4etjBtdbbm0ZKDmWvw@mail.gmail.com
* Improve comment of pgstat_read_statsfile()Michael Paquier2024-07-12
| | | | | | | | | | The comment at the top of pgstat_read_statsfile() mentioned that the stats are read from the on-disk file into the pgstats dshash. This is incorrect for fix-numbered stats as these are loaded directly into shared memory. This commit simplifies the comment to be more general. Author: Bertrand Drouvot Discussion: https://postgr.es/m/Zo/eJIHUcqKxeSgv@ip-10-97-1-34.eu-west-3.compute.internal
* Improve logical replication connection-failure messages.Tom Lane2024-07-11
| | | | | | | | | | These messages mostly said "could not connect to the publisher: %s" which is lacking context. Add some verbiage to indicate which subscription or worker process is failing. Nisha Moond Discussion: https://postgr.es/m/CABdArM7q1=zqL++cYd0hVMg3u_tc0S=0Of=Um-KvDhLony0cSg@mail.gmail.com
* Add min and max aggregates for composite types (records).Tom Lane2024-07-11
| | | | | | | | | Like min/max for arrays, these are just thin wrappers around the existing btree comparison function for records. Aleksander Alekseev Discussion: https://postgr.es/m/CAO=iB8L4WYSNxCJ8GURRjQsrXEQ2-zn3FiCsh2LMqvWq2WcONg@mail.gmail.com
* Fix possibility of logical decoding partial transaction changes.Masahiko Sawada2024-07-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When creating and initializing a logical slot, the restart_lsn is set to the latest WAL insertion point (or the latest replay point on standbys). Subsequently, WAL records are decoded from that point to find the start point for extracting changes in the DecodingContextFindStartpoint() function. Since the initial restart_lsn could be in the middle of a transaction, the start point must be a consistent point where we won't see the data for partial transactions. Previously, when not building a full snapshot, serialized snapshots were restored, and the SnapBuild jumps to the consistent state even while finding the start point. Consequently, the slot's restart_lsn and confirmed_flush could be set to the middle of a transaction. This could lead to various unexpected consequences. Specifically, there were reports of logical decoding decoding partial transactions, and assertion failures occurred because only subtransactions were decoded without decoding their top-level transaction until decoding the commit record. To resolve this issue, the changes prevent restoring the serialized snapshot and jumping to the consistent state while finding the start point. On v17 and HEAD, a flag indicating whether snapshot restores should be skipped has been added to the SnapBuild struct, and SNAPBUILD_VERSION has been bumpded. On backbranches, the flag is stored in the LogicalDecodingContext instead, preserving on-disk compatibility. Backpatch to all supported versions. Reported-by: Drew Callahan Reviewed-by: Amit Kapila, Hayato Kuroda Discussion: https://postgr.es/m/2444AA15-D21B-4CCE-8052-52C7C2DAFE5C%40amazon.com Backpatch-through: 12
* Add a new 'F' entry type for fixed-numbered stats in pgstats fileMichael Paquier2024-07-11
| | | | | | | | | | | | | | | | | | | This new entry type is used for all the fixed-numbered statistics, making possible support for custom pluggable stats. In short, we need to be able to detect more easily if a stats kind exists or not when reading back its data from the pgstats file without a dependency on the order of the entries read. The kind ID of the stats is added to the data written. The data is written in the same fashion as previously, with the fixed-numbered stats first and the dshash entries next. The read part becomes more flexible, loading fixed-numbered stats into shared memory based on the new entry type found. Bump PGSTAT_FILE_FORMAT_ID. Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Zot5bxoPYdS7yaoy@paquier.xyz
* Add PgStat_KindInfo.init_shmem_cbMichael Paquier2024-07-11
| | | | | | | | | | | | | | This new callback gives fixed-numbered stats the possibility to take actions based on the area of shared memory allocated for them. This removes from pgstat_shmem.c any knowledge specific to the types of fixed-numbered stats, and the initializations happen in their own files. Like b68b29bc8fec, this change is useful to make this area of the code more pluggable, so as custom fixed-numbered stats can take actions after their shared memory area is initialized. Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/Zot5bxoPYdS7yaoy@paquier.xyz