aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Remove useless extern keywordsPeter Eisentraut2024-07-01
| | | | | | | An extern keyword on a function definition (not declaration) is useless and not the normal style. Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
* Add context type field to pg_backend_memory_contextsDavid Rowley2024-07-01
| | | | | | | | | Since we now (as of v17) have 4 MemoryContext types, the type of context seems like useful information to include in the pg_backend_memory_contexts view. Here we add that. Reviewed-by: David Christensen, Michael Paquier Discussion: https://postgr.es/m/CAApHDvrXX1OR09Zjb5TnB0AwCKze9exZN%3D9Nxxg1ZCVV8W-3BA%40mail.gmail.com
* Remove useless codePeter Eisentraut2024-07-01
| | | | | | | | | | | BuildDescForRelation() goes out of its way to fill in ->constr->has_not_null, but that value is not used for anything later, so this code can all be removed. Note that BuildDescForRelation() doesn't make any effort to fill in the rest of ->constr, so there is no claim that that structure is completely filled in. Reviewed-by: Tomasz Rybak <tomasz.rybak@post.pl> Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org
* Remove useless initializationsPeter Eisentraut2024-07-01
| | | | | | | | | The struct is already initialized to all zeros right before this, and randomly initializing a few but not all fields to zero again has no technical or educational value. Reviewed-by: Tomasz Rybak <tomasz.rybak@post.pl> Discussion: https://www.postgresql.org/message-id/flat/a368248e-69e4-40be-9c07-6c3b5880b0a6@eisentraut.org
* Rename standby_slot_names to synchronized_standby_slots.Amit Kapila2024-07-01
| | | | | | | | | | | | The standby_slot_names GUC allows the specification of physical standby slots that must be synchronized before the logical walsenders associated with logical failover slots. However, for this purpose, the GUC name is too generic. Author: Hou Zhijie Reviewed-by: Bertrand Drouvot, Masahiko Sawada Backpatch-through: 17 Discussion: https://postgr.es/m/ZnWeUgdHong93fQN@momjian.us
* Use pgstat_kind_infos to read fixed shared statisticsMichael Paquier2024-07-01
| | | | | | | | | | | | | | | | | | | | Shared statistics with a fixed number of objects are read from the stats file in pgstat_read_statsfile() using members of PgStat_ShmemControl and following an order based on their PgStat_Kind value. Instead of being explicit, this commit changes the stats read to iterate over the pgstat_kind_infos array to find the memory locations to read into, based on a new shared_ctl_off in PgStat_KindInfo that can be used to define the position of this stats kind in shared memory. This makes the read logic simpler, and eases the introduction of future improvements aimed at making this area more pluggable for external modules. Original idea suggested by Andres Freund. Author: Tristan Partin Reviewed-by: Andres Freund, Michael Paquier Discussion: https://postgr.es/m/D12SQ7OYCD85.20BUVF3DWU5K7@neon.tech
* Format better code for xact_decode()'s XLOG_XACT_INVALIDATIONSMichael Paquier2024-07-01
| | | | | | | | This makes the code more consistent with the surroundings. Author: ChangAo Chen Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/CAExHW5tNTevUh58SKddTtcX3yU_5_PDSC8Mdp-Q2hc9PpZHRJg@mail.gmail.com
* Remove PgStat_KindInfo.named_on_diskMichael Paquier2024-07-01
| | | | | | | | | | | | | | This field is used to track if a stats kind can use a custom format representation on disk when reading or writing its stats case. On HEAD, this exists for replication slots stats, that need a mapping between an internal index ID and the slot names. named_on_disk is currently used nowhere and the callbacks to_serialized_name and from_serialized_name are in charge of checking if the serialization of the stats data should apply, so let's remove it. Reviewed-by: Andres Freund Discussion: https://postgr.es/m/ZmKVlSX_T5YvIOsd@paquier.xyz
* pgindent, because I forgot to do that.Robert Haas2024-06-28
| | | | | Commit 065583cf460f980a182498941ac52810f709a897 should have included these changes.
* SQL/JSON: Always coerce JsonExpr result at runtimeAmit Langote2024-06-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of looking up casts at parse time for converting the result of JsonPath* query functions to the specified or the default RETURNING type, always perform the conversion at runtime using either the target type's input function or the function json_populate_type(). There are two motivations for this change: 1. json_populate_type() coerces to types with typmod such that any string values that exceed length limit cause an error instead of silent truncation, which is necessary to be standard-conforming. 2. It was possible to end up with a cast expression that doesn't support soft handling of errors causing bugs in the of handling ON ERROR clause. JsonExpr.coercion_expr which would store the cast expression is no longer necessary, so remove. Bump catversion because stored rules change because of the above removal. Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: Discussion: https://postgr.es/m/202405271326.5a5rprki64aw%40alvherre.pgsql
* SQL/JSON: Fix coercion of constructor outputs to types with typmodAmit Langote2024-06-28
| | | | | | | | | | | | Ensure SQL/JSON constructor functions that allow specifying the target type using the RETURNING clause perform implicit cast to that type. This ensures that output values that exceed the specified length produce an error rather than being silently truncated. This behavior conforms to the SQL standard. Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/202405271326.5a5rprki64aw%40alvherre.pgsql
* Prevent summarizer hang when summarize_wal turned off and back on.Robert Haas2024-06-28
| | | | | | | | | | | | | | | Before this commit, when the WAL summarizer started up or recovered from an error, it would resume summarization from wherever it left off. That was OK normally, but wrong if summarize_wal=off had been turned off temporary, allowing some WAL to be removed, and then turned back on again. In such cases, the WAL summarizer would simply hang forever. This commit changes the reinitialization sequence for WAL summarizer to rederive the starting position in the way we were already doing at initial startup, fixing the problem. Per report from Israel Barth Rubio. Reviewed by Tom Lane. Discussion: http://postgr.es/m/CA+TgmoYN6x=YS+FoFOS6=nr6=qkXZFWhdiL7k0oatGwug2hcuA@mail.gmail.com
* SQL/JSON: Validate values in ON ERROR/EMPTY clausesAmit Langote2024-06-28
| | | | | | | | | | | | | | | | Currently, the grammar allows any supported values in the ON ERROR and ON EMPTY clauses for SQL/JSON functions, regardless of whether the values are appropriate for the function. This commit ensures that during parse analysis, the provided value is checked for validity for the given function and throws a syntax error if it is not. While at it, this fixes some omissions in the documentation of the ON ERROR/EMPTY clauses for JSON_TABLE(). Reported-by: Jian He <jian.universality@gmail.com> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/CACJufxFgWGqpESSYzyJ6tSurr3vFYBSNEmCfkGyB_dMdptFnZQ%40mail.gmail.com
* SQL/JSON: Prevent ON EMPTY for EXISTS columns in JSON_TABLE()Amit Langote2024-06-28
| | | | | | | | Due to an oversight in de3600452b61, the ON EMPTY clause was incorrectly allowed in the EXISTS column. Fix the grammar to prevent this. Discussion: https://postgr.es/m/CA%2BHiwqHh3YDXTpccgAo4CdfV9Mhy%2Bmg%3Doh6t8rfM5uLW1BJN4g%40mail.gmail.com
* Fix comments in heaptuple.cMichael Paquier2024-06-28
| | | | | | | | | Since e27f4ee0a701, fastgetattr() and heap_getattr() are not macros, but inlined functions. Author: Junwang Zhao Reviewed-by: Stepan Neretin Discussion: https://postgr.es/m/CAEG8a3JS-JKWWyOcM7BU=vPqFXa3W7mZSHnvc3CBqx=tC+3SCA@mail.gmail.com
* Improve locking around InjectionPointRun()Michael Paquier2024-06-28
| | | | | | | | | | | | | | | | As coded, an injection point could be loaded into the local cache without the LWLock InjectionPointLock taken, hence a point detached and re-attached concurrently of a point running calling InjectionPointRun() may finish by loading a callback it did no set initially. Based on all the cases discussed until now on the lists, it is fine to delay the lock release until the callback is run, so let's do that. While on it, remove a useless LWLockRelease() called before an error in InjectionPointAttach(). Per discussion with Heikki Linnakangas and Noah Misch. Discussion: https://postgr.es/m/e1ffb822-054e-4006-ac06-50532767f75b@iki.fi
* Cope with inplace update making catcache stale during TOAST fetch.Noah Misch2024-06-27
| | | | | | | | | | | | | | | | This extends ad98fb14226ae6456fbaed7990ee7591cbe5efd2 to invals of inplace updates. Trouble requires an inplace update of a catalog having a TOAST table, so only pg_database was at risk. (The other catalog on which core code performs inplace updates, pg_class, has no TOAST table.) Trouble would require something like the inplace-inval.spec test. Consider GRANT ... ON DATABASE fetching a stale row from cache and discarding a datfrozenxid update that vac_truncate_clog() has already relied upon. Back-patch to v12 (all supported versions). Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240114201411.d0@rfd.leadboat.com Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* AccessExclusiveLock new relations just after assigning the OID.Noah Misch2024-06-27
| | | | | | | | | | | | | | This has no user-visible, important consequences, since other sessions' catalog scans can't find the relation until we commit. However, this unblocks introducing a rule about locks required to heap_update() a pg_class row. CREATE TABLE has been acquiring this lock eventually, but it can heap_update() pg_class.relchecks earlier. create_toast_table() has been acquiring only ShareLock. Back-patch to v12 (all supported versions), the plan for the commit relying on the new rule. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
* Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.Noah Misch2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 5b562644fec696977df4a82790064e8287927891 added a comment that SetRelationHasSubclass() callers must hold this lock. When commit 17f206fbc824d2b4b14480199ca9ff7dea417eda extended use of this column to partitioned indexes, it didn't take the lock. As the latter commit message mentioned, we currently never reset a partitioned index to relhassubclass=f. That largely avoids harm from the lock omission. The cause for fixing this now is to unblock introducing a rule about locks required to heap_update() a pg_class row. This might cause more deadlocks. It gives minor user-visible benefits: - If an ALTER INDEX SET TABLESPACE runs concurrently with ALTER TABLE ATTACH PARTITION or CREATE PARTITION OF, one transaction blocks instead of failing with "tuple concurrently updated". (Many cases of DDL concurrency still fail that way.) - Match ALTER INDEX ATTACH PARTITION in choosing to lock the index. While not user-visible today, we'll need this if we ever make something set the flag to false for a partitioned index, like ANALYZE does today for tables. Back-patch to v12 (all supported versions), the plan for the commit relying on the new rule. In back branches, add LockOrStrongerHeldByMe() instead of adding a LockHeldByMe() parameter. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
* Lock owned sequences during ALTER TABLE SET { LOGGED | UNLOGGED }.Noah Misch2024-06-27
| | | | | | | | | | | | | | | These commands already make the persistence of owned sequences follow owned table persistence changes. They didn't lock those sequences. They lost the effect of nextval() calls that other sessions make after the ALTER TABLE command, before the ALTER TABLE transaction commits. Fix by acquiring the same lock that ALTER SEQUENCE SET { LOGGED | UNLOGGED } acquires. This might cause more deadlocks. Back-patch to v15, where commit 344d62fb9a978a72cf8347f0369b9ee643fd0b31 introduced unlogged sequences. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
* Expand comments and add an assertion in nodeModifyTable.c.Noah Misch2024-06-27
| | | | | | | | | | | Most comments concern RELKIND_VIEW. One addresses the ExecUpdate() "tupleid" parameter. A later commit will rely on these facts, but they hold already. Back-patch to v12 (all supported versions), the plan for that commit. Reviewed (in an earlier version) by Robert Haas. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Add an injection_points isolation test suite.Noah Misch2024-06-27
| | | | | | | | | Make the isolation harness recognize injection_points wait events as a type of blocked state. Test an extant inplace-update bug. Reviewed by Robert Haas and Michael Paquier. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Create waitfuncs.c for pg_isolation_test_session_is_blocked().Noah Misch2024-06-27
| | | | | | | | | The next commit makes the function inspect an additional non-lock contention source, so it no longer fits in lockfuncs.c. Reviewed by Robert Haas. Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
* Add wait event type "InjectionPoint", a custom type like "Extension".Noah Misch2024-06-27
| | | | | | | | | Both injection points and customization of type "Extension" are new in v17, so this just changes a detail of an unreleased feature. Reported by Robert Haas. Reviewed by Michael Paquier. Discussion: https://postgr.es/m/CA+TgmobfMU5pdXP36D5iAwxV5WKE_vuDLtp_1QyH+H5jMMt21g@mail.gmail.com
* Avoid crashing when a JIT-inlined backend function throws an error.Tom Lane2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | errfinish() assumes that the __FUNC__ and __FILE__ arguments it's passed are compile-time constant strings that can just be pointed to rather than physically copied. However, it's possible for LLVM to generate code in which those pointers point into a dynamically loaded code segment. If that segment gets unloaded before we're done with the ErrorData struct, we have dangling pointers that will lead to SIGSEGV. In simple cases that won't happen, because we won't unload LLVM code before end of transaction. But it's possible to happen if the error is thrown within end-of-transaction code run by _SPI_commit or _SPI_rollback, because since commit 2e517818f those functions clean up by ending the transaction and starting a new one. Rather than fixing this by adding pstrdup() overhead to every elog/ereport sequence, let's fix it by copying the risky pointers in CopyErrorData(). That solves it for _SPI_commit/_SPI_rollback because they use that function to preserve the error data across the transaction end/restart sequence; and it seems likely that any other code doing something similar would need to do that too. I'm suspicious that this behavior amounts to an LLVM bug (or a bug in our use of it?), because it implies that string constant references that should be pointer-equal according to a naive understanding of C semantics will sometimes not be equal. However, even if it is a bug and someday gets fixed, we'll have to cope with the current behavior for a long time to come. Report and patch by me. Back-patch to all supported branches. Discussion: https://postgr.es/m/1565654.1719425368@sss.pgh.pa.us
* Fix MVCC bug with prepared xact with subxacts on standbyHeikki Linnakangas2024-06-27
| | | | | | | | | | | | | | | | | | We did not recover the subtransaction IDs of prepared transactions when starting a hot standby from a shutdown checkpoint. As a result, such subtransactions were considered as aborted, rather than in-progress. That would lead to hint bits being set incorrectly, and the subtransactions suddenly becoming visible to old snapshots when the prepared transaction was committed. To fix, update pg_subtrans with prepared transactions's subxids when starting hot standby from a shutdown checkpoint. The snapshots taken from that state need to be marked as "suboverflowed", so that we also check the pg_subtrans. Backport to all supported versions. Discussion: https://www.postgresql.org/message-id/6b852e98-2d49-4ca1-9e95-db419a2696e0@iki.fi
* Fix thinkos in commentsAlvaro Herrera2024-06-27
| | | | | The first one was noticed by Tender Wang and introduced with 8aba9322511f; the other one was newly introduced with dbca3469ebf8.
* Drop the temporary tuple slots allocated by pgoutput.Amit Kapila2024-06-27
| | | | | | | | | | | | | In pgoutput, when converting the child table's tuple format to match the parent table's, we temporarily create a new slot to store the converted tuple. However, we missed to drop such temporary slots, leading to resource leakage. Reported-by: Bowen Shi Author: Hou Zhijie Reviewed-by: Amit Kapila Backpatch-through: 15 Discussion: https://postgr.es/m/CAM_vCudv8dc3sjWiPkXx5F2b27UV7_YRKRbtSCcE-pv=cVACGA@mail.gmail.com
* Fix overflow with pgstats DSA reference countMichael Paquier2024-06-27
| | | | | | | | | | | | | | | | | | | | | | | | When pgstats is initialized for a backend, it uses dsa_attach_in_place() without a "segment" provided. Hence, no callback is registered to automatically release the DSA attached once a backend exits. Not doing any cleanup causes the reference count of the pgstats DSA to continuously increment, at some point overflowing it (the more the number of connections, the faster it is to reach this state). Once the reference count overflows and then gets back to 0, new backends are not able to attach to the pgstats DSA, failing startup. This issue is resolved by adding in the pgstats shutdown hook a call to dsa_release_in_place(), ensuring that the DSA attached at backend startup is correctly released, keeping the reference count at bay. The author of this patch has been able to see this issue on a server with a long uptime and a high connection turnover. Issue introduced by 5891c7a8ed8f, so backpatch down to 15. Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/CAO6_XqqJbJBL=M7Ym13TcB4Xnq58vRa2jcC+gwEPBgbAda6B1Q@mail.gmail.com Backpatch-through: 15
* Fix bugs in MultiXact truncationHeikki Linnakangas2024-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 1. TruncateMultiXact() performs the SLRU truncations in a critical section. Deleting the SLRU segments calls ForwardSyncRequest(), which will try to compact the request queue if it's full (CompactCheckpointerRequestQueue()). That in turn allocates memory, which is not allowed in a critical section. Backtrace: TRAP: failed Assert("CritSectionCount == 0 || (context)->allowInCritSection"), File: "../src/backend/utils/mmgr/mcxt.c", Line: 1353, PID: 920981 postgres: autovacuum worker template0(ExceptionalCondition+0x6e)[0x560a501e866e] postgres: autovacuum worker template0(+0x5dce3d)[0x560a50217e3d] postgres: autovacuum worker template0(ForwardSyncRequest+0x8e)[0x560a4ffec95e] postgres: autovacuum worker template0(RegisterSyncRequest+0x2b)[0x560a50091eeb] postgres: autovacuum worker template0(+0x187b0a)[0x560a4fdc2b0a] postgres: autovacuum worker template0(SlruDeleteSegment+0x101)[0x560a4fdc2ab1] postgres: autovacuum worker template0(TruncateMultiXact+0x2fb)[0x560a4fdbde1b] postgres: autovacuum worker template0(vac_update_datfrozenxid+0x4b3)[0x560a4febd2f3] postgres: autovacuum worker template0(+0x3adf66)[0x560a4ffe8f66] postgres: autovacuum worker template0(AutoVacWorkerMain+0x3ed)[0x560a4ffe7c2d] postgres: autovacuum worker template0(+0x3b1ead)[0x560a4ffecead] postgres: autovacuum worker template0(+0x3b620e)[0x560a4fff120e] postgres: autovacuum worker template0(+0x3b3fbb)[0x560a4ffeefbb] postgres: autovacuum worker template0(+0x2f724e)[0x560a4ff3224e] /lib/x86_64-linux-gnu/libc.so.6(+0x27c8a)[0x7f62cc642c8a] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85)[0x7f62cc642d45] postgres: autovacuum worker template0(_start+0x21)[0x560a4fd16f31] To fix, bail out in CompactCheckpointerRequestQueue() without doing anything, if it's called in a critical section. That covers the above call path, as well as any other similar cases where RegisterSyncRequest might be called in a critical section. 2. After fixing that, another problem became apparent: Autovacuum process doing that truncation can deadlock with the checkpointer process. TruncateMultiXact() sets "MyProc->delayChkptFlags |= DELAY_CHKPT_START". If the sync request queue is full and cannot be compacted, the process will repeatedly sleep and retry, until there is room in the queue. However, if the checkpointer is trying to start a checkpoint at the same time, and is waiting for the DELAY_CHKPT_START processes to finish, the queue will never shrink. More concretely, the autovacuum process is stuck here: #0 0x00007fc934926dc3 in epoll_wait () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x000056220b24348b in WaitEventSetWaitBlock (set=0x56220c2e4b50, occurred_events=0x7ffe7856d040, nevents=1, cur_timeout=<optimized out>) at ../src/backend/storage/ipc/latch.c:1570 #2 WaitEventSetWait (set=0x56220c2e4b50, timeout=timeout@entry=10, occurred_events=<optimized out>, occurred_events@entry=0x7ffe7856d040, nevents=nevents@entry=1, wait_event_info=wait_event_info@entry=150994949) at ../src/backend/storage/ipc/latch.c:1516 #3 0x000056220b243224 in WaitLatch (latch=<optimized out>, latch@entry=0x0, wakeEvents=wakeEvents@entry=40, timeout=timeout@entry=10, wait_event_info=wait_event_info@entry=150994949) at ../src/backend/storage/ipc/latch.c:538 #4 0x000056220b26cf46 in RegisterSyncRequest (ftag=ftag@entry=0x7ffe7856d0a0, type=type@entry=SYNC_FORGET_REQUEST, retryOnError=true) at ../src/backend/storage/sync/sync.c:614 #5 0x000056220af9db0a in SlruInternalDeleteSegment (ctl=ctl@entry=0x56220b7beb60 <MultiXactMemberCtlData>, segno=segno@entry=11350) at ../src/backend/access/transam/slru.c:1495 #6 0x000056220af9dab1 in SlruDeleteSegment (ctl=ctl@entry=0x56220b7beb60 <MultiXactMemberCtlData>, segno=segno@entry=11350) at ../src/backend/access/transam/slru.c:1566 #7 0x000056220af98e1b in PerformMembersTruncation (oldestOffset=<optimized out>, newOldestOffset=<optimized out>) at ../src/backend/access/transam/multixact.c:3006 #8 TruncateMultiXact (newOldestMulti=newOldestMulti@entry=3221225472, newOldestMultiDB=newOldestMultiDB@entry=4) at ../src/backend/access/transam/multixact.c:3201 #9 0x000056220b098303 in vac_truncate_clog (frozenXID=749, minMulti=<optimized out>, lastSaneFrozenXid=749, lastSaneMinMulti=3221225472) at ../src/backend/commands/vacuum.c:1917 #10 vac_update_datfrozenxid () at ../src/backend/commands/vacuum.c:1760 #11 0x000056220b1c3f76 in do_autovacuum () at ../src/backend/postmaster/autovacuum.c:2550 #12 0x000056220b1c2c3d in AutoVacWorkerMain (startup_data=<optimized out>, startup_data_len=<optimized out>) at ../src/backend/postmaster/autovacuum.c:1569 and the checkpointer is stuck here: #0 0x00007fc9348ebf93 in clock_nanosleep () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x00007fc9348fe353 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x000056220b40ecb4 in pg_usleep (microsec=microsec@entry=10000) at ../src/port/pgsleep.c:50 #3 0x000056220afb43c3 in CreateCheckPoint (flags=flags@entry=108) at ../src/backend/access/transam/xlog.c:7098 #4 0x000056220b1c6e86 in CheckpointerMain (startup_data=<optimized out>, startup_data_len=<optimized out>) at ../src/backend/postmaster/checkpointer.c:464 To fix, add AbsorbSyncRequests() to the loops where the checkpointer waits for DELAY_CHKPT_START or DELAY_CHKPT_COMPLETE operations to finish. Backpatch to v14. Before that, SLRU deletion didn't call RegisterSyncRequest, which avoided this failure. I'm not sure if there are other similar scenarios on older versions, but we haven't had any such reports. Discussion: https://www.postgresql.org/message-id/ccc66933-31c1-4f6a-bf4b-45fef0d4f22e@iki.fi
* Fix nbtree array unsatisfied inequality check.Peter Geoghegan2024-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | _bt_advance_array_keys didn't take sufficient care at the point where it decides whether to start a new primitive index scan based on a call to _bt_check_compare against finaltup (a call with the scan direction flipped around). The final decision was conditioned on rules about how the scan key offset sktrig that initially triggered array advancement (passed to _bt_advance_array_keys from its _bt_checkkeys caller) compared to the offset set by its own _bt_check_compare finaltup call. This approach was faulty, in that it allowed _bt_advance_array_keys to incorrectly start a new primitive index scan, that landed on the same leaf page (on assert-enabled builds it led to an assertion failure). In general, scans with array keys are expected to never have to read the same leaf page more than once (barring cases involving cursors, and cases where the scan restores a marked position for the inner side of a merge join). This principle was established by commit 5bf748b8. To fix, make the final decision based on whether the scan key offset set by the _bt_check_compare finaltup call is an offset to an inequality strategy scan key. An unsatisfied required inequality strategy scan key indicates that all of the scan's required equality strategy scan keys must also be satisfied by finaltup (not just by caller's tuple), and that there is a decent chance that _bt_first will be able to reposition the scan to a position many leaf pages ahead of the current leaf page. Oversight in commit 5bf748b8. Discussion: https://postgr.es/m/CAH2-Wz=DyHbcg7o6zXqzyiin8WE8vzk4tvU8Lrnh-a=EAvO0TQ@mail.gmail.com
* Fix partition pruning setup during DETACH CONCURRENTLYAlvaro Herrera2024-06-26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When detaching partition in concurrent mode, it's possible for partition descriptors to not match the set that was recently seen when the plan was made, causing an assertion failure or (in production builds) failure to construct a working plan. The case that was reported involves prepared statements, but I think it may be possible to hit this bug without that too. The problem is that CreatePartitionPruneState is constructing a PartitionPruneState under the assumption that new partitions can be added, but never removed, but it turns out that this isn't true: a prepared statement gets replanned when the DETACH CONCURRENTLY session sends out its invalidation message, but if the invalidation message arrives after ExecInitAppend started, we would build a partition descriptor without the partition, and then CreatePartitionPruneState would refuse to work with it. CreatePartitionPruneState already contains code to deal with the new descriptor having more partitions than before (and behaving for the extra partitions as if they had been pruned), but doesn't have code to deal with less partitions than before, and it is naïve about the case where the number of partitions is the same. We could simply add that a new stanza for less partitions than before, and in simple testing it works to do that; but it's possible to press the test scripts even further and hit the case where one partition is added and a partition is removed quickly enough that we see the same number of partitions, but they don't actually match, causing hangs during execution. To cope with both these problems, we now memcmp() the arrays of partition OIDs, and do a more elaborate mapping (relying on the fact that both OID arrays are in partition-bounds order) if they're not identical. This fix was already pushed in backbranches earlier. Reported-by: yajun Hu <1026592243@qq.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/18377-e0324601cfebdfe5@postgresql.org
* Improve comment in gram.y.Tom Lane2024-06-25
| | | | | | | | | | "As so-and-so" isn't bad English, but it has a faintly archaic whiff to it, and confuses some non-native speakers. Write "Like so-and-so" instead. Per complaint from Tatsuo Ishii. Discussion: https://postgr.es/m/20240623.130154.1867056921698616251.t-ishii@sranhm.sra.co.jp.sranhm
* Revert "Fix partition pruning setup during DETACH CONCURRENTLY"Alvaro Herrera2024-06-24
| | | | | | | This reverts commit 27162a64b386; this branch is in code freeze due to a nearing release. We can commit again after the release is out. Discussion: https://postgr.es/m/1158256.1719239648@sss.pgh.pa.us
* Fix partition pruning setup during DETACH CONCURRENTLYAlvaro Herrera2024-06-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When detaching partition in concurrent mode, it's possible for partition descriptors to not match the set that was recently seen when the plan was made, causing an assertion failure or (in production builds) failure to construct a working plan. The case that was reported involves prepared statements, but I think it may be possible to hit this bug without that too. The problem is that CreatePartitionPruneState is constructing a PartitionPruneState under the assumption that new partitions can be added, but never removed, but it turns out that this isn't true: a prepared statement gets replanned when the DETACH CONCURRENTLY session sends out its invalidation message, but if the invalidation message arrives after ExecInitAppend started, we would build a partition descriptor without the partition, and then CreatePartitionPruneState would refuse to work with it. CreatePartitionPruneState already contains code to deal with the new descriptor having more partitions than before (and behaving for the extra partitions as if they had been pruned), but doesn't have code to deal with less partitions than before, and it is naïve about the case where the number of partitions is the same. We could simply add that a new stanza for less partitions than before, and in simple testing it works to do that; but it's possible to press the test scripts even further and hit the case where one partition is added and a partition is removed quickly enough that we see the same number of partitions, but they don't actually match, causing hangs during execution. To cope with both these problems, we now memcmp() the arrays of partition OIDs, and do a more elaborate mapping (relying on the fact that both OID arrays are in partition-bounds order) if they're not identical. Backpatch to 14, where DETACH CONCURRENTLY appeared. Reported-by: yajun Hu <1026592243@qq.com> Reviewed-by: Tender Wang <tndrwang@gmail.com> Discussion: https://postgr.es/m/18377-e0324601cfebdfe5@postgresql.org
* Translation updatesPeter Eisentraut2024-06-24
| | | | | Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 4409d73e450606ff15b428303d706f1d15c1f597
* Fix relcache invalidation when relfilelocator is updatedHeikki Linnakangas2024-06-21
| | | | | | | | | | In commit af0e7deb4a, I removed a call to RelationCloseSmgr(), because the dangling SMgrRelation was no longer an issue. However, we still need the call when the relation's relfilelocator changes, so that the new relfilelocator takes effect immediately. Reported-by: Alexander Lakhin <exclusion@gmail.com> Discussion: https://www.postgresql.org/message-id/987b1c8c-8c91-4847-ca0e-879f421680ff%40gmail.com
* Fix make build on MinGWPeter Eisentraut2024-06-21
| | | | | | | | | Revert a couple of the simplifications done in commit 721856ff24b because platforms without ln -s, where LN_S='cp -pR', such as MinGW, required the specific previous incantations. Reported-by: Noah Misch <noah@leadboat.com> Discussion: https://www.postgresql.org/message-id/20240616193448.28@rfd.leadboat.com
* parse_manifest: Use const char *Peter Eisentraut2024-06-21
| | | | | | | | This adapts the manifest parsing code to take advantage of the const-ified jsonapi. Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://www.postgresql.org/message-id/flat/f732b014-f614-4600-a437-dba5a2c3738b%40eisentraut.org
* jsonapi: Use const char *Peter Eisentraut2024-06-21
| | | | | | | | | | Apply const qualifiers to char * arguments and fields throughout the jsonapi. This allows the top-level APIs such as pg_parse_json_incremental() to declare their input argument as const. It also reduces the number of unconstify() calls. Reviewed-by: Andrew Dunstan <andrew@dunslane.net> Discussion: https://www.postgresql.org/message-id/flat/f732b014-f614-4600-a437-dba5a2c3738b%40eisentraut.org
* Don't throw an error if a queued AFTER trigger no longer exists.Tom Lane2024-06-20
| | | | | | | | | | | | | | | | | | | | | afterTriggerInvokeEvents and AfterTriggerExecute have always treated it as an error if the trigger OID mentioned in a queued after-trigger event can't be found. However, that fails to account for the edge case where the trigger's been dropped in the current transaction since queueing the event. There seems no very good reason to disallow that case, so instead silently do nothing if the trigger OID can't be found. This does give up a little bit of bug-detection ability, but I don't recall that these error messages have ever actually revealed a bug, so it seems mostly theoretical. Alternatives such as marking pending events DONE at the time of dropping a trigger would be complicated and perhaps introduce bugs of their own. Per bug #18517 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18517-af2d19882240902c@postgresql.org
* Unify some error messagesPeter Eisentraut2024-06-20
|
* meson: Fix import library name in WindowsPeter Eisentraut2024-06-20
| | | | | | | | | This changes the import library name from 'postgres.exe.lib' to 'postgres.lib', which is what it was with the old MSVC build system. Extension builds use that name. Bug: #18513 Reported-by: Muralikrishna Bandaru <muralikrishna.bandaru@enterprisedb.com>
* SQL/JSON: Correctly enforce the default ON EMPTY behaviorAmit Langote2024-06-19
| | | | | | | | | | | | | | Currently, when the ON EMPTY clause is not present, the ON ERROR clause (implicit or explicit) dictates the behavior when jsonpath evaluation in ExecEvalJsonExprPath() results in an empty sequence. That is an oversight in the commit 6185c9737c. This commit fixes things so that a NULL is returned instead in that case which is the default behavior when the ON EMPTY clause is not present. Reported-by: Markus Winand Discussion: https://postgr.es/m/F7DD1442-265C-4220-A603-CB0DEB77E91D%40winand.at
* SQL/JSON: Correct jsonpath variable name matchingAmit Langote2024-06-19
| | | | | | | | | | | | | | | Previously, GetJsonPathVar() allowed a jsonpath expression to reference any prefix of a PASSING variable's name. For example, the following query would incorrectly work: SELECT JSON_QUERY(context_item, jsonpath '$xy' PASSING val AS xyz); The fix ensures that the length of the variable name mentioned in a jsonpath expression matches exactly with the name of the PASSING variable before comparing the strings using strncmp(). Reported-by: Alvaro Herrera (off-list) Discussion: https://postgr.es/m/CA+HiwqFGkLWMvELBH6E4SQ45qUHthgcRH6gCJL20OsYDRtFx_w@mail.gmail.com
* Fix possible Assert failure in cost_memoize_rescanDavid Rowley2024-06-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | In cost_memoize_rescan(), when calculating the hit_ratio using the calls and ndistinct estimations, if the value that was set in MemoizePath.calls had not been processed through clamp_row_est(), then it was possible that it was set to some non-integer value which could result in ndistinct being 1 higher than calls due to estimate_num_groups() performing clamp_row_est() on its input_rows. This could result in hit_ratio values slightly below 0.0, which would cause an Assert failure. The value of MemoizePath.calls comes from the final parameter in the create_memoize_path() function, of which we only have one true caller of. That caller passes outer_path->rows. All the core code I looked at always seems to call clamp_row_est() on the Path.rows, so there might have been no issues with any core Paths causing troubles here. The bug report was about a CustomPath with a non-clamped row estimated. The misbehavior as a result of this seems to be mostly limited to the Assert() failing. Aside from that, it seems the Memoize costs would just come out slightly higher than they should have, which is likely fairly harmless. Reported-by: Kohei KaiGai <kaigai@heterodb.com> Discussion: https://postgr.es/m/CAOP8fzZnTU+N64UYJYogb1hN-5hFP+PwTb3m_cnGAD7EsQwrKw@mail.gmail.com Reviewed-by: Richard Guo Backpatch-through: 14, where Memoize was introduced
* Fix incorrect punctuation in error messagePeter Eisentraut2024-06-18
|
* Fix insertion of SP-GiST REDIRECT tuples during REINDEX CONCURRENTLY.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reconstruction of an SP-GiST index by REINDEX CONCURRENTLY may insert some REDIRECT tuples. This will typically happen in a transaction that lacks an XID, which leads either to assertion failure in spgFormDeadTuple or to insertion of a REDIRECT tuple with zero xid. The latter's not good either, since eventually VACUUM will apply GlobalVisTestIsRemovableXid() to the zero xid, resulting in either an assertion failure or a garbage answer. In practice, since REINDEX CONCURRENTLY locks out index scans till it's done, it doesn't matter whether it inserts REDIRECTs or PLACEHOLDERs; and likewise it doesn't matter how soon VACUUM reduces such a REDIRECT to a PLACEHOLDER. So in non-assert builds there's no observable problem here, other than perhaps a little index bloat. But it's not behaving as intended. To fix, remove the failing Assert in spgFormDeadTuple, acknowledging that we might sometimes insert a zero XID; and guard VACUUM's GlobalVisTestIsRemovableXid() call with a test for valid XID, ensuring that we'll reduce such a REDIRECT the first time VACUUM sees it. (Versions before v14 use TransactionIdPrecedes here, which won't fail on zero xid, so they really have no bug at all in non-assert builds.) Another solution could be to not create REDIRECTs at all during REINDEX CONCURRENTLY, making the relevant code paths treat that case like index build (which likewise knows that no concurrent index scans can be happening). That would allow restoring the Assert in spgFormDeadTuple, but we'd still need the VACUUM change because redirection tuples with zero xid may be out there already. But there doesn't seem to be a nice way for spginsert() to tell that it's being called in REINDEX CONCURRENTLY without some API changes, so we'll leave that as a possible future improvement. In HEAD, also rename the SpGistState.myXid field to redirectXid, which seems less misleading (since it might not in fact be our transaction's XID) and is certainly less uninformatively generic. Per bug #18499 from Alexander Lakhin. Back-patch to all supported branches. Discussion: https://postgr.es/m/18499-8a519c280f956480@postgresql.org
* Remove recordExtensionInitPriv[Worker]'s ownerId argument.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | In the wake of the previous commit, we're not doing anything with that argument. Hence, revert the portions of 534287403 that added that argument and taught the callers to pass it. Passing the ownerId requires additional syscache lookups in some code paths, which'd be fine if we were doing anything useful with the info, but it seems inadvisable if we're not. Committed separately since there's some thought that we might want to un-revert this in future, in case it's decided that storing the original owner ID explicitly in pg_init_privs is worth doing. Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
* Improve tracking of role dependencies of pg_init_privs entries.Tom Lane2024-06-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 534287403 invented SHARED_DEPENDENCY_INITACL entries in pg_shdepend, but installed them only for non-owner roles mentioned in a pg_init_privs entry. This turns out to be the wrong thing, because there is nothing to cue REASSIGN OWNED to go and update pg_init_privs entries when the object's ownership is reassigned. That leads to leaving dangling entries in pg_init_privs, as reported by Hannu Krosing. Instead, install INITACL entries for all roles mentioned in pg_init_privs entries (except pinned roles), and change ALTER OWNER to not touch them, just as it doesn't touch pg_init_privs entries. REASSIGN OWNED will now substitute the new owner OID for the old in pg_init_privs entries. This feels like perhaps not quite the right thing, since pg_init_privs ought to be a historical record of the state of affairs just after CREATE EXTENSION. However, it's hard to see what else to do, if we don't want to disallow dropping the object's original owner. In any case this is better than the previous do-nothing behavior, and we're unlikely to come up with a superior solution in time for v17. While here, tighten up some coding rules about how ACLs in pg_init_privs should never be null or empty. There's not any obvious reason to allow that, and perhaps asserting that it's not so will catch some bugs. (We were previously inconsistent on the point, with some code paths taking care not to store empty ACLs and others not.) This leaves recordExtensionInitPrivWorker not doing anything with its ownerId argument, but we'll deal with that separately. catversion bump forced because of change of expected contents of pg_shdepend when pg_init_privs entries exist. Discussion: https://postgr.es/m/CAMT0RQSVgv48G5GArUvOVhottWqZLrvC5wBzBa4HrUdXe9VRXw@mail.gmail.com
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-16 01:51:42 +0000