Commit Graph

58812 Commits

Author SHA1 Message Date
Noah Misch
b99414de90 Remove configuration-dependent output from new inplace-inval test.
Per buildfarm members prion and trilobite.  Back-patch to v12 (all
supported versions), like commit
0844b39689.

Strategy reviewed by Tom Lane.

Discussion: https://postgr.es/m/20240628051353.a0.nmisch@google.com
2024-06-28 09:33:40 -07:00
Robert Haas
b48f275f18 pgindent, because I forgot to do that.
Commit 065583cf46 should have
included these changes.
2024-06-28 10:51:05 -04:00
Amit Langote
716bd12d22 SQL/JSON: Always coerce JsonExpr result at runtime
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
2024-06-28 21:58:13 +09:00
Amit Langote
c2d93c3802 SQL/JSON: Fix coercion of constructor outputs to types with typmod
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
2024-06-28 21:48:44 +09:00
Robert Haas
065583cf46 Prevent summarizer hang when summarize_wal turned off and back on.
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
2024-06-28 08:29:05 -04:00
Amit Langote
55e56c84da SQL/JSON: Validate values in ON ERROR/EMPTY clauses
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
2024-06-28 14:01:43 +09:00
Amit Langote
e3c1393efc SQL/JSON: Prevent ON EMPTY for EXISTS columns in JSON_TABLE()
Due to an oversight in de3600452b, 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
2024-06-28 14:01:43 +09:00
Michael Paquier
0ad8153c1f Update modules/injection_points/.gitignore
Thinko in c35f419d6e, where an isolation test has been added to the
module.
2024-06-28 13:41:39 +09:00
Michael Paquier
526b54ece3 Fix comments in heaptuple.c
Since e27f4ee0a7, 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
2024-06-28 13:30:47 +09:00
Michael Paquier
d85fc4be11 Improve locking around InjectionPointRun()
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
2024-06-28 12:31:29 +09:00
Noah Misch
4a7f91b3d3 Remove comment about xl_heap_inplace "AT END OF STRUCT".
Commit 2c03216d83 moved the tuple data
from there to the buffer-0 data.  Back-patch to v12 (all supported
versions), the plan for the next change to this struct.

Discussion: https://postgr.es/m/20240523000548.58.nmisch@google.com
2024-06-27 19:21:06 -07:00
Noah Misch
f9f47f0d93 Cope with inplace update making catcache stale during TOAST fetch.
This extends ad98fb1422 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
2024-06-27 19:21:06 -07:00
Noah Misch
5b823b179e AccessExclusiveLock new relations just after assigning the OID.
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
2024-06-27 19:21:05 -07:00
Noah Misch
0cecc908e9 Lock before setting relhassubclass on RELKIND_PARTITIONED_INDEX.
Commit 5b562644fe added a comment that
SetRelationHasSubclass() callers must hold this lock.  When commit
17f206fbc8 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
2024-06-27 19:21:05 -07:00
Noah Misch
f88cdb36c4 Lock owned sequences during ALTER TABLE SET { LOGGED | UNLOGGED }.
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 344d62fb9a introduced
unlogged sequences.

Reviewed (in an earlier version) by Robert Haas.

Discussion: https://postgr.es/m/20240611024525.9f.nmisch@google.com
2024-06-27 19:21:05 -07:00
Noah Misch
d5f788b41d Expand comments and add an assertion in nodeModifyTable.c.
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
2024-06-27 19:21:05 -07:00
Noah Misch
c35f419d6e Add an injection_points isolation test suite.
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
2024-06-27 19:21:05 -07:00
Noah Misch
abfbd13af0 Create waitfuncs.c for pg_isolation_test_session_is_blocked().
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
2024-06-27 19:21:05 -07:00
Noah Misch
bb93640a68 Add wait event type "InjectionPoint", a custom type like "Extension".
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
2024-06-27 19:21:05 -07:00
Noah Misch
0844b39689 Improve test coverage for changes to inplace-updated catalogs.
This covers both regular and inplace changes, since bugs arise at their
intersection.  Where marked, these witness extant bugs.  Back-patch to
v12 (all supported versions).

Reviewed (in an earlier version) by Robert Haas.

Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
2024-06-27 19:21:05 -07:00
Noah Misch
22a4b104ba Make TAP todo_start effects the same under Meson and prove_check.
This could have caused spurious failures only on SPARC Linux, because
today's only todo_start tests for that platform.  Back-patch to v16,
where Meson support first appeared.

Reviewed by Robert Haas.

Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com
2024-06-27 19:21:04 -07:00
Amit Langote
473a352fb3 SQL/JSON: Document behavior when input document is not jsonb
The input document to functions JSON_EXISTS(), JSON_QUERY(),
JSON_VALUE(), and JSON_TABLE() can be specified as character or
UTF8-encoded bytea strings. These are automatically converted to
jsonb with an implicit cast before being passed to the jsonpath
machinery.

In the current implementation, errors that occur when parsing the
specified string into a valid JSON document are thrown
unconditionally. This means they are not subject to the explicit or
implicit ON ERROR clause of those functions, which is a standard-
conforming behavior.  Add a note to the documentation to mention
that.

Reported-by: Markus Winand
Discussion: https://postgr.es/m/F7DD1442-265C-4220-A603-CB0DEB77E91D%40winand.at
2024-06-28 09:45:03 +09:00
Tom Lane
5d6c64d290 Avoid crashing when a JIT-inlined backend function throws an error.
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
2024-06-27 14:44:02 -04:00
Heikki Linnakangas
cbfbda7841 Fix MVCC bug with prepared xact with subxacts on standby
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
2024-06-27 21:09:58 +03:00
Heikki Linnakangas
ecbf6ac51d tests: Trim newline from result returned by BackgroundPsql->query
This went unnoticed, because only a few existing callers of
BackgroundPsql->query used the result, and the ones that did were not
bothered by an extra newline. I noticed because I was about to add a
new test that checks the result.

Backport to all supported versions, since I just backported the
BackgroundPsql facility to all supported versions too.
2024-06-27 21:09:58 +03:00
Alvaro Herrera
a2dff271eb
Fix thinkos in comments
The first one was noticed by Tender Wang and introduced with
8aba9322511f; the other one was newly introduced with dbca3469eb.
2024-06-27 19:51:47 +02:00
Amit Kapila
3e53492aa7 Drop the temporary tuple slots allocated by pgoutput.
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
2024-06-27 11:35:00 +05:30
Michael Paquier
7467939ea2 Fix overflow with pgstats DSA reference count
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 5891c7a8ed, so backpatch down to 15.

Author: Anthonin Bonnefoy
Discussion: https://postgr.es/m/CAO6_XqqJbJBL=M7Ym13TcB4Xnq58vRa2jcC+gwEPBgbAda6B1Q@mail.gmail.com
Backpatch-through: 15
2024-06-27 09:44:47 +09:00
Heikki Linnakangas
b1ffe3ff0b Fix bugs in MultiXact truncation
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
2024-06-26 23:02:06 +03:00
Bruce Momjian
f92fd18307 doc PG 17 relnotes: fix system catalog name mistake
Reported-by: David G. Johnston

Discussion: https://postgr.es/m/CAKFQuwYgkaOuao4DXuQwhbg+vyu4Xb5TGpuDNDOfMa0AftyweQ@mail.gmail.com

Backpatch-through: master
2024-06-26 15:08:06 -04:00
Bruce Momjian
d537b2e037 doc PG 17 relnotes: add item about pg_collation column renames
Reported-by: David G. Johnston

Discussion: https://postgr.es/m/CAKFQuwYRw30QaWrSsL57k3L_=zdQ4JTgY9pGnnhm42B7fGJX1A@mail.gmail.com

Backpatch-through: master
2024-06-26 13:13:46 -04:00
Nathan Bossart
32f07991b7 Use PqMsg_* macros in fe-auth.c.
Commit f4b54e1ed9, which introduced macros for protocol characters,
missed updating a few places in fe-auth.c.

Author: Jelte Fennema-Nio
Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS%2BYWXBhOGo%2BY1YecLgknF3g%40mail.gmail.com
2024-06-26 11:25:38 -05:00
Peter Geoghegan
486c2ea25c Fix nbtree array unsatisfied inequality check.
_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
2024-06-26 10:45:52 -04:00
Alvaro Herrera
dbca3469eb
Fix partition pruning setup during DETACH CONCURRENTLY
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
2024-06-26 13:40:26 +02:00
Tom Lane
1bf29f51fa Improve comment in gram.y.
"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
2024-06-25 17:53:41 -04:00
Joe Conway
23c5a0e7d4 Stamp 17beta2. 2024-06-24 17:24:14 -04:00
Alvaro Herrera
b0ea16528c
Revert "Fix partition pruning setup during DETACH CONCURRENTLY"
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
2024-06-24 17:20:21 +02:00
Alvaro Herrera
27162a64b3
Fix partition pruning setup during DETACH CONCURRENTLY
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
2024-06-24 15:56:32 +02:00
Peter Eisentraut
f7f4e7e6fa Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 4409d73e450606ff15b428303d706f1d15c1f597
2024-06-24 13:11:27 +02:00
Alexander Korotkov
70a845c04a Remove extra comment at TableAmRoutine.scan_analyze_next_block
The extra comment was accidentally copied here by 6377e12a from
heapam_scan_analyze_next_block().

Reported-by: Matthias van de Meent
Discussion: https://postgr.es/m/CAEze2WjC5PiweG-4oe0hB_Zr59iF3tRE1gURm8w4Cg5b6JEBGw%40mail.gmail.com
2024-06-22 16:17:50 +03:00
Bruce Momjian
a8ffa32377 doc PG 17 relnotes: wording improvements, add links, merge item
Backpatch-through: master
2024-06-21 12:08:14 -04:00
Heikki Linnakangas
441ef5e1ba Fix relcache invalidation when relfilelocator is updated
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
2024-06-21 17:13:10 +03:00
Bruce Momjian
90fe7b74df doc PG 17 relnotes: add link to enable_group_by_reordering GUC
Backpatch-through: master
2024-06-21 10:11:12 -04:00
Alexander Korotkov
82e79ee46b Add doc entry for the new GUC paramenter enable_group_by_reordering
0452b461bc adds alternative orderings of group-by keys during the query
optimization. This new feature is controlled by the new GUC parameter
enable_group_by_reordering, which accidentally came without the documentation.
This commit adds the missing documentation for that GUC.

Reported-by: Bruce Momjian
Discussion: https://postgr.es/m/ZnDx2FYlba_OafQd%40momjian.us
Author: Andrei Lepikhov
Reviewed-by: Pavel Borisov, Alexander Korotkov
2024-06-21 15:39:13 +03:00
John Naylor
fd49e8f323 Prevent access of uninitialized memory in radix tree nodes
RT_NODE_16_SEARCH_EQ() performs comparisions using vector registers
on x64-64 and aarch64. We apply a mask to the resulting bitfield
to eliminate irrelevant bits that may be set. This ensures correct
behavior, but Valgrind complains of the partially-uninitialised
values. So far the warnings have only occurred on aarch64, which
explains why this hasn't been seen earlier.

To fix this warning, initialize the whole fixed-sized part of the nodes
upon allocation, rather than just do the minimum initialization to
function correctly. The initialization for node48 is a bit different
in that the 256-byte slot index array must be populated with "invalid
index" rather than zero. Experimentation has shown that compilers
tend to emit code that uselessly memsets that array twice. To avoid
pessimizing this path, swap the order of the slot_idxs[] and isset[]
arrays so we can initialize with two non-overlapping memset calls.

Reported by Tomas Vondra
Analysis and patch by Tom Lane, reviewed by Masahiko Sawada. I
investigated the behavior of memset calls to overlapping regions,
leading to the above tweaks to node48 as discussed in the thread.

Discussion: https://postgr.es/m/120c63ad-3d12-415f-a7bf-3da451c31bf6%40enterprisedb.com
2024-06-21 17:29:39 +07:00
Peter Eisentraut
c5c82123d3 pg_combinebackup: Error message improvements
Make the wordings of some file-related error messages more like those
used in other files.
2024-06-21 09:40:44 +02:00
Peter Eisentraut
aea79883c5 Remove redundant newlines from error messages 2024-06-21 09:29:34 +02:00
Peter Eisentraut
58445651db Fix make build on MinGW
Revert a couple of the simplifications done in commit 721856ff24
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
2024-06-21 08:17:23 +02:00
Peter Eisentraut
02bbc3c83a parse_manifest: Use const char *
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
2024-06-21 07:53:30 +02:00
Peter Eisentraut
15cd9a3881 jsonapi: Use const char *
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
2024-06-21 07:53:30 +02:00