Heikki Linnakangas [Sun, 11 Aug 2024 17:21:16 +0000 (20:21 +0300)]
Initialize HASHCTL differently, to suppress Coverity warning
Coverity complained that the hash_create() call might access
hash_table_ctl->hctl. That's a false alarm, hash_create() only
accesses that field when passed the HASH_SHARED_MEM flag. Try to
silence it by using a plain local variable instead of a const. That's
how the HASHCTL is initialized in all the other hash_create() calls.
Tom Lane [Sun, 11 Aug 2024 16:24:56 +0000 (12:24 -0400)]
Suppress Coverity warnings about Asserts in get_name_for_var_field.
Coverity thinks dpns->plan could be null at these points. That
shouldn't really be possible, but it's easy enough to modify the
Asserts so they'd not core-dump if it were true.
These are new in
b919a97a6. Back-patch to v13; the v12 version
of the patch didn't have these Asserts.
Tom Lane [Sat, 10 Aug 2024 19:51:28 +0000 (15:51 -0400)]
Allow adjusting session_authorization and role in parallel workers.
The code intends to allow GUCs to be set within parallel workers
via function SET clauses, but not otherwise. However, doing so fails
for "session_authorization" and "role", because the assign hooks for
those attempt to set the subsidiary "is_superuser" GUC, and that call
falls foul of the "not otherwise" prohibition. We can't switch to
using GUC_ACTION_SAVE for this, so instead add a new GUC variable
flag GUC_ALLOW_IN_PARALLEL to mark is_superuser as being safe to set
anyway. (This is okay because is_superuser has context PGC_INTERNAL
and thus only hard-wired calls can change it. We'd need more thought
before applying the flag to other GUCs; but maybe there are other
use-cases.) This isn't the prettiest fix perhaps, but other
alternatives we thought of would be much more invasive.
While here, correct a thinko in commit
059de3ca4: when rejecting
a GUC setting within a parallel worker, we should return 0 not -1
if the ereport doesn't longjmp. (This seems to have no consequences
right now because no caller cares, but it's inconsistent.) Improve
the comments to try to forestall future confusion of the same kind.
Despite the lack of field complaints, this seems worth back-patching.
Thanks to Nathan Bossart for the idea to invent a new flag,
and for review.
Discussion: https://postgr.es/m/
2833457.
1723229039@sss.pgh.pa.us
Alexander Korotkov [Sat, 10 Aug 2024 18:43:14 +0000 (21:43 +0300)]
Add tests for pg_wal_replay_wait() errors
Improve test coverage for pg_wal_replay_wait() procedure by adding test
cases when it errors out.
Alexander Korotkov [Sat, 10 Aug 2024 18:43:09 +0000 (21:43 +0300)]
Improve header comment for WaitLSNSetLatches()
Reflect the fact that we remove waiters from the heap, not just set their
latches.
Alexander Korotkov [Sat, 10 Aug 2024 18:43:02 +0000 (21:43 +0300)]
Adjust pg_wal_replay_wait() procedure behavior on promoted standby
pg_wal_replay_wait() is intended to be called on standby. However, standby
can be promoted to primary at any moment, even concurrently with the
pg_wal_replay_wait() call. If recovery is not currently in progress
that doesn't mean the wait was unsuccessful. Thus, we always need to recheck
if the target LSN is replayed.
Reported-by: Kevin Hale Boyes
Discussion: https://postgr.es/m/CAPpHfdu5QN%2BZGACS%2B7foxmr8_nekgA2PA%2B-G3BuOUrdBLBFb6Q%40mail.gmail.com
Author: Alexander Korotkov
John Naylor [Tue, 6 Aug 2024 13:38:33 +0000 (20:38 +0700)]
Lower minimum maintenance_work_mem to 64kB
Since the introduction of TID store, vacuum uses far less memory in
the common case than in versions 16 and earlier. Invoking multiple
rounds of index vacuuming in turn requires a much larger table. It'd
be a good idea anyway to cover this case in regression testing, and a
lower limit is less painful for slow buildfarm animals. The reason to
do it now is to re-enable coverage of the bugfix in commit
83c39a1f7f.
For consistency, give autovacuum_work_mem the same treatment.
Suggested by Andres Freund
Tested by Melanie Plageman
Backpatch to v17, where TID store was introduced
Discussion: https://postgr.es/m/
20240516205458.ohvlzis5b5tvejru@awork3.anarazel.de
Discussion: https://postgr.es/m/
20240722164745.fvaoh6g6zprisqgp%40awork3.anarazel.de
Peter Eisentraut [Sat, 10 Aug 2024 06:12:44 +0000 (08:12 +0200)]
Fix inappropriate uses of atol()
Some code using atol() would not work correctly if sizeof(long)==4:
- src/bin/pg_basebackup/pg_basebackup.c: Would miscount size of a
tablespace over 2 TB.
- src/bin/pg_basebackup/streamutil.c: Would truncate a timeline ID
beyond INT32_MAX.
- src/bin/pg_rewind/libpq_source.c: Would miscount size of files
larger than 2 GB (but this currently cannot happen).
Replace these with atoll().
In one case, the use of atol() did not result in incorrect behavior
but seems inconsistent with related code:
- src/interfaces/ecpg/ecpglib/execute.c: Gratuitous, since it
processes a value from pg_type.typlen, which is int16.
Replace this with atoi().
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/
a52738ad-06bc-4d45-b59f-
b38a8a89de49%40eisentraut.org
Alvaro Herrera [Fri, 9 Aug 2024 21:55:01 +0000 (17:55 -0400)]
libpq: Trace StartupMessage/SSLRequest/GSSENCRequest correctly
libpq tracing via PQtrace would uselessly print the wrong thing for
these types of messages. With this commit, their type and contents
would be correctly listed. (This can be verified with PQconnectStart(),
but we don't use that in libpq_pipeline, so I (Álvaro) haven't bothered
to add any tests.)
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
Heikki Linnakangas [Fri, 9 Aug 2024 21:06:19 +0000 (00:06 +0300)]
Fix comment on processes being kept over a restart
All child processes except the syslogger are killed on a restart. The
archiver might be already running though, if it was started during
recovery.
The split in the comments between "other special children" and the
first group of "background tasks" seemed really arbitrary, so I just
merged them all into one group.
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/
8f2118b9-79e3-4af7-b2c9-
bd5818193ca4@iki.fi
Heikki Linnakangas [Fri, 9 Aug 2024 21:04:43 +0000 (00:04 +0300)]
Refactor code to handle death of a backend or bgworker in postmaster
Currently, when a child process exits, the postmaster first scans
through BackgroundWorkerList, to see if it the child process was a
background worker. If not found, then it scans through BackendList to
see if it was a regular backend. That leads to some duplication
between the bgworker and regular backend cleanup code, as both have an
entry in the BackendList that needs to be cleaned up in the same way.
Refactor that so that we scan just the BackendList to find the child
process, and if it was a background worker, do the additional
bgworker-specific cleanup in addition to the normal Backend cleanup.
Change HandleChildCrash so that it doesn't try to handle the cleanup
of the process that already exited, only the signaling of all the
other processes. When called for any of the aux processes, the caller
had already cleared the *PID global variable, so the code in
HandleChildCrash() to do that was unused.
On Windows, if a child process exits with ERROR_WAIT_NO_CHILDREN, it's
now logged with that exit code, instead of 0. Also, if a bgworker
exits with ERROR_WAIT_NO_CHILDREN, it's now treated as crashed and is
restarted. Previously it was treated as a normal exit.
If a child process is not found in the BackendList, the log message
now calls it "untracked child process" rather than "server process".
Arguably that should be a PANIC, because we do track all the child
processes in the list, so failing to find a child process is highly
unexpected. But if we want to change that, let's discuss and do that
as a separate commit.
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/
835232c0-a5f7-4f20-b95b-
5b56ba57d741@iki.fi
Heikki Linnakangas [Fri, 9 Aug 2024 19:44:20 +0000 (22:44 +0300)]
Make BackgroundWorkerList doubly-linked
This allows ForgetBackgroundWorker() and ReportBackgroundWorkerExit()
to take a RegisteredBgWorker pointer as argument, rather than a list
iterator. That feels a little more natural. But more importantly, this
paves the way for more refactoring in the next commit.
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/
835232c0-a5f7-4f20-b95b-
5b56ba57d741@iki.fi
Nathan Bossart [Fri, 9 Aug 2024 18:16:33 +0000 (13:16 -0500)]
doc: Standardize use of dashes in references to CRC and SHA.
Presently, we inconsistently use dashes in references to these
algorithms (e.g., CRC32C versus CRC-32C). Some popular web sources
appear to prefer dashes, and with this commit, we will, too.
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/ZrUFpLP-w2zTAHqq%40nathan
Nathan Bossart [Fri, 9 Aug 2024 15:52:37 +0000 (10:52 -0500)]
doc: Fix name of CRC algorithm in "Reliability" section.
This section claims we use CRC-32 for WAL records and two-phase
state files, but we've actually used CRC-32C since v9.5 (commit
5028f22f6e). Fix that.
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/ZrUFpLP-w2zTAHqq%40nathan
Backpatch-through: 12
Tom Lane [Fri, 9 Aug 2024 15:21:39 +0000 (11:21 -0400)]
Fix "failed to find plan for subquery/CTE" errors in EXPLAIN.
To deparse a reference to a field of a RECORD-type output of a
subquery, EXPLAIN normally digs down into the subquery's plan to try
to discover exactly which anonymous RECORD type is meant. However,
this can fail if the subquery has been optimized out of the plan
altogether on the grounds that no rows could pass the WHERE quals,
which has been possible at least since
3fc6e2d7f. There isn't
anything remaining in the plan tree that would help us, so fall back
to printing the field name as "fN" for the N'th column of the record.
(This will actually be the right thing some of the time, since it
matches the column names we assign to RowExprs.)
In passing, fix a comment typo in create_projection_plan, which
I noticed while experimenting with an alternative fix for this.
Per bug #18576 from Vasya B. Back-patch to all supported branches.
Richard Guo and Tom Lane
Discussion: https://postgr.es/m/18576-
9feac34e132fea9e@postgresql.org
Peter Eisentraut [Fri, 9 Aug 2024 05:17:15 +0000 (07:17 +0200)]
Remove obsolete RECHECK keyword completely
This used to be part of CREATE OPERATOR CLASS and ALTER OPERATOR
FAMILY, but it has done nothing (except issue a NOTICE) since
PostgreSQL 8.4. Commit
30e7c175b81 removed support for dumping from
pre-9.2 servers, so this no longer serves any need.
This now removes it completely, and you'd get a normal parse error if
you used it.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
113ef2d2-3657-4353-be97-
f28fceddbca1%40eisentraut.org
Amit Kapila [Fri, 9 Aug 2024 04:53:57 +0000 (10:23 +0530)]
Change the misleading local end_lsn for prepared transactions.
The apply worker was using XactLastCommitEnd as local end_lsn for applying
prepare and rollback_prepare. The XactLastCommitEnd value is the end lsn
of the last commit applied before the prepare transaction which makes no
sense. This LSN is used to decide whether we can send the acknowledgment
of the corresponding remote LSN to the server.
It is okay not to set the local_end LSN with the actual WAL position for
the prepare because we always flush the prepare record. So, we can send
the acknowledgment of the remote_end LSN as soon as prepare is finished.
The current code is misleading but as such doesn't create any problem, so
decided not to backpatch.
Author: Hayato Kuroda
Reviewed-by: Shveta Malik, Amit Kapila
Discussion: https://postgr.es/m/TYAPR01MB5692FA4926754B91E9D7B5F0F5AA2@TYAPR01MB5692.jpnprd01.prod.outlook.com
Alvaro Herrera [Fri, 9 Aug 2024 00:35:12 +0000 (20:35 -0400)]
libpq: Add suppress argument to pqTraceOutputNchar
In future commits we're going to trace authentication related messages.
Some of these messages contain challenge bytes as part of a
challenge-response flow. Since these bytes are different for every
connection, we want to normalize them when the PQTRACE_REGRESS_MODE
trace flag is set. This commit modifies pqTraceOutputNchar to take a
suppress argument, which makes it possible to do so.
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/CAGECzQSoPHtZ4xe0raJ6FYSEiPPS+YWXBhOGo+Y1YecLgknF3g@mail.gmail.com
Alvaro Herrera [Thu, 8 Aug 2024 23:35:13 +0000 (19:35 -0400)]
Refuse ATTACH of a table referenced by a foreign key
Trying to attach a table as a partition which is already on the
referenced side of a foreign key on the partitioned table that it is
being attached to, leads to strange behavior: we try to clone the
foreign key from the parent to the partition, but this new FK points to
the partition itself, and the mix of pg_constraint rows and triggers
doesn't behave well.
Rather than trying to untangle the mess (which might be possible given
sufficient time), I opted to forbid the ATTACH. This doesn't seem a
problematic restriction, given that we already fail to create the
foreign key if you do it the other way around, that is, having the
partition first and the FK second.
Backpatch to all supported branches.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/18541-
628a61bc267cd2d3@postgresql.org
Alvaro Herrera [Thu, 8 Aug 2024 19:17:11 +0000 (15:17 -0400)]
Refactor error messages to reduce duplication
I also took the liberty of changing
errmsg("COPY DEFAULT only available using COPY FROM")
to
errmsg("COPY %s cannot be used with %s", "DEFAULT", "COPY TO")
because the original wording is unlike all other messages that indicate
option incompatibility. This message was added by commit
9f8377f7a279
(16-era), in whose development thread there was no discussion on this
point.
Backpatch to 17.
Alexander Korotkov [Thu, 8 Aug 2024 08:48:57 +0000 (11:48 +0300)]
Add a caveat to hash_seq_init_with_hash_value() header comment
The typical use-case for hash_seq_init_with_hash_value() is syscache
callback. Add a caveat that the default hash function doesn't match syscache
hash function. So, one needs to define a custom hash function.
Reported-by: Pavel Stehule
Discussion: https://postgr.es/m/CAFj8pRAXmv6eyYx%3DE_BTfyK%3DO_%2ByOF8sXB%3D0bn9eOBt90EgWRA%40mail.gmail.com
Reviewed-by: Pavel Stehule
Heikki Linnakangas [Thu, 8 Aug 2024 07:20:25 +0000 (10:20 +0300)]
Fix pg_rewind debug output to print the source timeline history
getTimelineHistory() is called twice, to read the source and the
target timeline history files. However, the loop to print the file
with the --debug option used the wrong variable when dealing with the
source. As a result, the source's history was always printed as empty.
Spotted while debugging bug #18575, but this does not fix that bug,
just the debugging output. Backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/
092dd515-b7b4-4fd0-8407-
ceca2f02f6ec@iki.fi
Noah Misch [Wed, 7 Aug 2024 18:43:08 +0000 (11:43 -0700)]
Fix names of "Visual Studio" and Meson in a documentation sentence.
Commit
3cffe7946c268be91a340ec9a27081cb93d67d35 missed this. Back-patch
to v17, which introduced this.
Discussion: https://postgr.es/m/CAJ7c6TM7ct0EjoCQaLSVYoxxnEw4xCUFebWj77GktWsqEdyCtQ@mail.gmail.com
Tom Lane [Wed, 7 Aug 2024 16:54:39 +0000 (12:54 -0400)]
Fix edge case in plpgsql's make_callstmt_target().
If the plancache entry for the CALL statement is already stale,
it's possible for us to fetch an old procedure OID out of it,
and then fail with "cache lookup failed for function NNN".
In ordinary usage this never happens because make_callstmt_target
is called just once immediately after building the plancache
entry. It can be forced however by setting up an erroneous CALL
(that causes make_callstmt_target itself to report an error),
then dropping/recreating the target procedure, then repeating
the erroneous CALL.
To fix, use SPI_plan_get_cached_plan() to fetch the plancache's
plan, rather than assuming we can use SPI_plan_get_plan_sources().
This shouldn't add any noticeable overhead in the normal case,
and in the stale-plan case we'd have had to replan anyway a little
further down.
The other callers of SPI_plan_get_plan_sources() seem OK, because
either they don't need up-to-date plans or they know that the
query was just (re) planned. But add some commentary in hopes
of not falling into this trap again.
Per bug #18574 from Song Hongyu. Back-patch to v14 where this coding
was introduced. (Older branches have comparable code, but it's run
after any required replanning, so there's no issue.)
Discussion: https://postgr.es/m/18574-
2ce7ba3249221389@postgresql.org
Alvaro Herrera [Wed, 7 Aug 2024 15:30:36 +0000 (11:30 -0400)]
Refactor/reword some error messages to avoid duplicates
Also, remove brackets around "EMPTY [ ARRAY ]". An error message is
not the place to state that a keyword is optional.
Backpatch to 17.
Robert Haas [Wed, 7 Aug 2024 12:49:41 +0000 (08:49 -0400)]
Improve file header comments for astramer code.
Make it clear that "astreamer" stands for "archive streamer".
Generalize comments that still believe this code can only be used
by pg_basebackup. Add some comments explaining the asymmetry
between the gzip, lz4, and zstd astreamers, in the hopes of making
life easier for anyone who hacks on this code in the future.
Robert Haas, reviewed by Amul Sul.
Discussion: http://postgr.es/m/CAAJ_b97O2kkKVTWxt8MxDN1o-cDfbgokqtiN2yqFf48=gXpcxQ@mail.gmail.com
Heikki Linnakangas [Wed, 7 Aug 2024 07:43:52 +0000 (10:43 +0300)]
Make fallback MD5 implementation thread-safe on big-endian systems
Replace a static scratch buffer with a local variable, because a
static buffer makes the function not thread-safe. This function is
used in client-code in libpq, so it needs to be thread-safe. It was
until commit
b67b57a966, which replaced the implementation with the
one from pgcrypto.
Backpatch to v14, where we switched to the new implementation.
Reviewed-by: Robert Haas, Michael Paquier
Discussion: https://www.postgresql.org/message-id/
dfa2015d-ad21-4802-a4cc-
3850fc5fff3f@iki.fi
Peter Eisentraut [Wed, 7 Aug 2024 07:21:07 +0000 (09:21 +0200)]
Revert ECPG's use of pnstrdup()
Commit
0b9466fce added a dependency on fe_memutils' pnstrdup() inside
informix.c. This adds an exit() path in a library, which we don't
want. (Unlike libpq, the ecpg libraries don't have an automated check
for that, but it makes sense to keep them to a similar standard.) The
ecpg code can already handle failure results from the *strdup() call
by itself.
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/CAOYmi+=pg=W5L1h=3MEP_EB24jaBu2FyATrLXqQHGe7cpuvwyg@mail.gmail.com
Alexander Korotkov [Wed, 7 Aug 2024 03:51:29 +0000 (06:51 +0300)]
Optimize InvalidateAttoptCacheCallback() and TypeCacheTypCallback()
These callbacks are receiving hash values as arguments, which doesn't allow
direct lookups for AttoptCacheHash and TypeCacheHash. This is why subject
callbacks currently use full iteration over corresponding hashes.
This commit avoids full hash iteration in InvalidateAttoptCacheCallback(),
and TypeCacheTypCallback(). At first, we switch AttoptCacheHash and
TypeCacheHash to use same hash function as syscache. As second, we
use hash_seq_init_with_hash_value() to iterate only hash entries with matching
hash value.
Discussion: https://postgr.es/m/
5812a6e5-68ae-4d84-9d85-
b443176966a1%40sigaev.ru
Author: Teodor Sigaev
Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov
Reviewed-by: Andrei Lepikhov
Alexander Korotkov [Wed, 7 Aug 2024 03:51:16 +0000 (06:51 +0300)]
Introduce hash_search_with_hash_value() function
This new function iterates hash entries with given hash values. This function
is designed to avoid full sequential hash search in the syscache invalidation
callbacks.
Discussion: https://postgr.es/m/
5812a6e5-68ae-4d84-9d85-
b443176966a1%40sigaev.ru
Author: Teodor Sigaev
Reviewed-by: Aleksander Alekseev, Tom Lane, Michael Paquier, Roman Zharkov
Reviewed-by: Andrei Lepikhov
Heikki Linnakangas [Tue, 6 Aug 2024 20:05:25 +0000 (23:05 +0300)]
Use psprintf to simplify gtsvectorout()
The buffer allocation was correct, but looked archaic and scary:
- It was weird to calculate the buffer size before determining which
format string was used. With the same effort, we could've used the
right-sized buffer for each branch.
- Commit
aa0d3504560 added one more possible return string ("all true
bits"), but didn't adjust the code at the top of the function to
calculate the returned string's max size. It was not a live bug,
because the new string was smaller than the existing ones, but
seemed wrong in principle.
- Use of sprintf() is generally eyebrow-raising these days
Switch to psprintf(). psprintf() allocates a larger buffer than what
was allocated before, 128 bytes vs 80 bytes, which is acceptable as
this code is not performance or space critical.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
54c29fb0-edf2-48ea-9814-
44e918bbd6e8@iki.fi
Heikki Linnakangas [Tue, 6 Aug 2024 20:04:51 +0000 (23:04 +0300)]
Constify fields and parameters in spell.c
I started by marking VoidString as const, and fixing the fallout by
marking more fields and function arguments as const. It proliferated
quite a lot, but all within spell.c and spell.h.
A more narrow patch to get rid of the static VoidString buffer would
be to replace it with '#define VoidString ""', as C99 allows assigning
"" to a non-const pointer, even though you're not allowed to modify
it. But it seems like good hygiene to mark all these as const. In the
structs, the pointers can point to the constant VoidString, or a
buffer allocated with palloc(), or with compact_palloc(), so you
should not modify them.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
54c29fb0-edf2-48ea-9814-
44e918bbd6e8@iki.fi
Heikki Linnakangas [Tue, 6 Aug 2024 20:04:48 +0000 (23:04 +0300)]
Mark misc static global variables as const
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
54c29fb0-edf2-48ea-9814-
44e918bbd6e8@iki.fi
Heikki Linnakangas [Tue, 6 Aug 2024 20:04:22 +0000 (23:04 +0300)]
Make nullSemAction const, add 'const' decorators to related functions
To make it more clear that these should never be modified.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
54c29fb0-edf2-48ea-9814-
44e918bbd6e8@iki.fi
Heikki Linnakangas [Tue, 6 Aug 2024 20:03:43 +0000 (23:03 +0300)]
Turn a few 'validnsps' static variables into locals
There was no need for these to be static buffers, local variables work
just as well. I think they were marked as 'static' to imply that they
are read-only, but 'const' is more appropriate for that, so change
them to const.
To make it possible to mark the variables as 'const', also add 'const'
decorations to the transformRelOptions() signature.
Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/
54c29fb0-edf2-48ea-9814-
44e918bbd6e8@iki.fi
Jeff Davis [Tue, 6 Aug 2024 18:55:21 +0000 (11:55 -0700)]
selfuncs.c: use pg_strxfrm() instead of strxfrm().
pg_strxfrm() takes a pg_locale_t, so it works properly with all
providers. This improves estimates for ICU when performing linear
interpolation within a histogram bin.
Previously, convert_string_datum() always used strxfrm() and relied on
setlocale(). That did not produce good estimates for non-default or
non-libc collations.
Discussion: https://postgr.es/m/
89475ee5487d795124f4e25118ea8f1853edb8cb.camel@j-davis.com
Heikki Linnakangas [Tue, 6 Aug 2024 19:15:55 +0000 (22:15 +0300)]
Fix datatypes in comments in instr_time.h
The INSTR_TIME_GET_NANOSEC(t) and INSTR_TIME_GET_MICROSEC(t) macros
return a signed int64.
Discussion: https://www.postgresql.org/message-id/ZrHkv3MAQfwNSmTG@ip-10-97-1-34.eu-west-3.compute.internal
Heikki Linnakangas [Tue, 6 Aug 2024 19:15:46 +0000 (22:15 +0300)]
Revert "Fix comments in instr_time.h and remove an unneeded cast to int64"
This reverts commit
3dcb09de7b. Tom Lane pointed out that it broke the
abstraction provided by the macros. The callers should not need to
know what the internal type is.
This commit is an exact revert, the next commit will fix the comments
on the macros that incorrectly claim that they return uint64.
Discussion: https://www.postgresql.org/message-id/ZrHkv3MAQfwNSmTG@ip-10-97-1-34.eu-west-3.compute.internal
Tom Lane [Tue, 6 Aug 2024 16:36:42 +0000 (12:36 -0400)]
Allow parallel workers to cope with a newly-created session user ID.
Parallel workers failed after a sequence like
BEGIN;
CREATE USER foo;
SET SESSION AUTHORIZATION foo;
because check_session_authorization could not see the uncommitted
pg_authid row for "foo". This is because we ran RestoreGUCState()
in a separate transaction using an ordinary just-created snapshot.
The same disease afflicts any other GUC that requires catalog lookups
and isn't forgiving about the lookups failing.
To fix, postpone RestoreGUCState() into the worker's main transaction
after we've set up a snapshot duplicating the leader's. This affects
check_transaction_isolation and check_transaction_deferrable, which
think they should only run during transaction start. Make them
act like check_transaction_read_only, which already knows it should
silently accept the value when InitializingParallelWorker.
This un-reverts commit
f5f30c22e. The original plan was to back-patch
that, but the fact that
0ae5b763e proved to be a pre-requisite shows
that the subtle API change for GUC hooks might actually break some of
them. The problem we're trying to fix seems not worth taking such a
risk for in stable branches.
Per bug #18545 from Andrey Rachitskiy.
Discussion: https://postgr.es/m/18545-
feba138862f19aaa@postgresql.org
Tom Lane [Tue, 6 Aug 2024 16:21:23 +0000 (12:21 -0400)]
Clean up handling of client_encoding GUC in parallel workers.
The previous coding here threw an error from assign_client_encoding
if it was invoked in a parallel worker. That's a very fundamental
violation of the GUC hook API: assign hooks must not throw errors.
The place to complain is in the check hook, so move the test to
there, and use the regular check-hook API (ie return false) to
report it.
The reason this coding is a problem is that it breaks GUC rollback,
which may occur after we leave InitializingParallelWorker state.
That case seems not actually reachable before now, but commit
f5f30c22e made it reachable, so we need to fix this before that
can be un-reverted.
In passing, improve the commentary in ParallelWorkerMain, and
add a check for failure of SetClientEncoding. That's another
case that can't happen now but might become possible after
foreseeable code rearrangements (notably, if the shortcut of
skipping PrepareClientEncoding stops being OK).
Discussion: https://postgr.es/m/18545-
feba138862f19aaa@postgresql.org
Nathan Bossart [Tue, 6 Aug 2024 15:56:37 +0000 (10:56 -0500)]
Remove volatile qualifiers from pg_stat_statements.c.
Prior to commit
0709b7ee72, which changed the spinlock primitives
to function as compiler barriers, access to variables within a
spinlock-protected section required using a volatile pointer, but
that is no longer necessary.
Reviewed-by: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/Zqkv9iK7MkNS0KaN%40nathan
Heikki Linnakangas [Tue, 6 Aug 2024 11:28:02 +0000 (14:28 +0300)]
Fix comments in instr_time.h and remove an unneeded cast to int64
03023a2664 represented time as an int64 on all platforms but forgot to
update the comment related to INSTR_TIME_GET_MICROSEC() and provided
an incorrect comment for INSTR_TIME_GET_NANOSEC().
In passing remove an unneeded cast to int64.
Author: Bertrand Drouvot
Discussion: https://www.postgresql.org/message-id/ZrHkv3MAQfwNSmTG@ip-10-97-1-34.eu-west-3.compute.internal
Michael Paquier [Tue, 6 Aug 2024 07:27:38 +0000 (16:27 +0900)]
Remove unnecessary declaration of heapam_methods
This overlaps with the declaration at the end of heapam_handler.c that
lists all the callback routines for the heap table AM.
Author: Japin Li
Discussion: https://postgr.es/m/ME0P300MB04459456D5C4E70D48116896B6B12@ME0P300MB0445.AUSP300.PROD.OUTLOOK.COM
Jeff Davis [Tue, 6 Aug 2024 01:15:57 +0000 (18:15 -0700)]
Remove support for null pg_locale_t most places.
Previously, passing NULL for pg_locale_t meant "use the libc provider
and the server environment". Now that the database collation is
represented as a proper pg_locale_t (not dependent on setlocale()),
remove special cases for NULL.
Leave wchar2char() and char2wchar() unchanged for now, because the
callers don't always have a libc-based pg_locale_t available.
Discussion: https://postgr.es/m/
cfd9eb85-c52a-4ec9-a90e-
a5e4de56e57d@eisentraut.org
Reviewed-by: Peter Eisentraut, Andreas Karlsson
Robert Haas [Mon, 5 Aug 2024 15:40:29 +0000 (11:40 -0400)]
Move astreamer (except astreamer_inject) to fe_utils.
This allows the code to be used by other frontend applications.
Amul Sul, reviewed by Sravan Kumar, Andres Freund (whose input
I specifically solicited regarding the meson.build changes),
and me.
Discussion: http://postgr.es/m/CAAJ_b94StvLWrc_p4q-f7n3OPfr6GhL8_XuAg2aAaYZp1tF-nw@mail.gmail.com
Robert Haas [Mon, 5 Aug 2024 14:40:23 +0000 (10:40 -0400)]
Move recovery injector astreamer to a separate header file.
Unlike the rest of the astreamer (formerly bbstreamer) infrastructure
which is reusable by other tools, astreamer_inject.c seems extremely
specific to pg_basebackup. Hence, move the corresponding declarations
to a separate header file, so that we can move the rest of the code
without moving this.
Amul Sul, reviewed by Sravan Kumar and by me.
Discussion: http://postgr.es/m/CAAJ_b94StvLWrc_p4q-f7n3OPfr6GhL8_XuAg2aAaYZp1tF-nw@mail.gmail.com
Robert Haas [Mon, 5 Aug 2024 13:35:42 +0000 (09:35 -0400)]
Rename bbstreamer to astreamer.
I (rhaas) intended "bbstreamer" to stand for "base backup streamer,"
but that implies that this infrastructure can only ever be used by
pg_basebackup. In fact, it is a generally useful way of streaming
data from a tar or compressed tar file, and it could be extended to
work with other archive formats as well if we ever wanted to do that.
Hence, rename it to "astreamer" (archive streamer) in preparation for
reusing the infrastructure from pg_verifybackup (and perhaps
eventually also other utilities, such as pg_combinebackup or
pg_waldump).
This is purely a renaming commit. Comment adjustments and relocation
of the actual code to someplace from which it can be reused are left
to future commits.
Amul Sul, reviewed by Sravan Kumar and by me.
Discussion: http://postgr.es/m/CAAJ_b94StvLWrc_p4q-f7n3OPfr6GhL8_XuAg2aAaYZp1tF-nw@mail.gmail.com
Masahiko Sawada [Mon, 5 Aug 2024 13:05:33 +0000 (06:05 -0700)]
Restrict accesses to non-system views and foreign tables during pg_dump.
When pg_dump retrieves the list of database objects and performs the
data dump, there was possibility that objects are replaced with others
of the same name, such as views, and access them. This vulnerability
could result in code execution with superuser privileges during the
pg_dump process.
This issue can arise when dumping data of sequences, foreign
tables (only 13 or later), or tables registered with a WHERE clause in
the extension configuration table.
To address this, pg_dump now utilizes the newly introduced
restrict_nonsystem_relation_kind GUC parameter to restrict the
accesses to non-system views and foreign tables during the dump
process. This new GUC parameter is added to back branches too, but
these changes do not require cluster recreation.
Back-patch to all supported branches.
Reviewed-by: Noah Misch
Security: CVE-2024-7348
Backpatch-through: 12
David Rowley [Mon, 5 Aug 2024 11:16:44 +0000 (23:16 +1200)]
Optimize JSON escaping using SIMD
Here we adjust escape_json_with_len() to make use of SIMD to allow
processing of up to 16-bytes at a time rather than processing a single
byte at a time. This has been shown to speed up escaping of JSON
strings significantly.
Escaping is required for both JSON string properties and also the
property names themselves, so this should also help improve the speed of
the conversion from JSON into text for JSON objects that have property
names 16 or more bytes long.
Escaping JSON strings was often a significant bottleneck for longer
strings. With these changes, some benchmarking has shown a query
performing nearly 4 times faster when escaping a JSON object with a 1MB
text property. Tests with shorter text properties saw smaller but still
significant performance improvements. For example, a test outputting 1024
JSON strings with a text property length ranging from 1 char to 1024 chars
became around 2 times faster.
Author: David Rowley
Reviewed-by: Melih Mutlu
Discussion: https://postgr.es/m/CAApHDvpLXwMZvbCKcdGfU9XQjGCDm7tFpRdTXuB9PVgpNUYfEQ@mail.gmail.com
Amit Kapila [Mon, 5 Aug 2024 09:08:00 +0000 (14:38 +0530)]
Fix typo in bufpage.h.
Author: Senglee Choi
Reviewed-by: Tender Wang
Discussion: https://postgr.es/m/CACUsy79U0=S5zWEf6D57F=vB7rOEa86xFY6oovDZ58jRcROCxQ@mail.gmail.com
Michael Paquier [Mon, 5 Aug 2024 03:29:22 +0000 (12:29 +0900)]
injection_points: Add some fixed-numbered statistics
Like
75534436a477, this acts mainly as a template to show what can be
achieved with fixed-numbered stats (like WAL, bgwriter, etc.) with the
pluggable cumulative statistics APIs introduced in
7949d9594582.
Fixed-numbered stats are defined in their own file, named
injection_stats_fixed.c, separated entirely from the variable-numbered
case in injection_stats.c. This is mainly for clarity as having both
examples in the same file would be confusing.
Note that this commit uses the helper routines added in
2eff9e678d35.
The stats stored track globally the number of times injection points
have been attached, detached or run. Two more fields should be added
later for the number of times a point has been cached or loaded, but
what's here is enough as a template.
More TAP tests are added, providing coverage for fixed-numbered custom
stats.
Author: Michael Paquier
Reviewed-by: Dmitry Dolgov, Bertrand Drouvot
Discussion: https://postgr.es/m/Zmqm9j5EO0I4W8dx@paquier.xyz
Michael Paquier [Mon, 5 Aug 2024 03:06:54 +0000 (12:06 +0900)]
injection_points: Add some cumulative stats for injection points
This acts as a template of what can be achieved with the pluggable
cumulative stats APIs introduced in
7949d9594582 for the
variable-numbered case where stats entries are stored in the pgstats
dshash, while being potentially useful on its own for injection points,
say to add starting and/or stopping conditions based on the statistics
(want to trigger a callback after N calls, for example?).
Currently, the only data gathered is the number of times an injection
point is run. More fields can always be added as required. All the
routines related to the stats are located in their own file, called
injection_stats.c in the test module injection_points, for clarity.
The stats can be used only if the test module is loaded through
shared_preload_libraries. The key of the dshash uses InvalidOid for the
database, and an int4 hash of the injection point name as object ID.
A TAP test is added to provide coverage for the new custom cumulative
stats APIs, showing the persistency of the data across restarts, for
example.
Author: Michael Paquier
Reviewed-by: Dmitry Dolgov, Bertrand Drouvot
Discussion: https://postgr.es/m/Zmqm9j5EO0I4W8dx@paquier.xyz
Michael Paquier [Mon, 5 Aug 2024 02:43:33 +0000 (11:43 +0900)]
Add helper routines to retrieve data for custom fixed-numbered pgstats
This is useful for extensions to get snapshot and shmem data for custom
cumulative statistics when these have a fixed number of objects, so as
these do not need to know about the snapshot internals, aka pgStatLocal.
An upcoming commit introducing an example template for custom cumulative
stats with fixed-numbered objects will make use of these. I have
noticed that this is useful for extension developers while hacking my
own example, actually.
Author: Michael Paquier
Reviewed-by: Dmitry Dolgov, Bertrand Drouvot
Discussion: https://postgr.es/m/Zmqm9j5EO0I4W8dx@paquier.xyz
Alexander Korotkov [Sun, 4 Aug 2024 17:26:48 +0000 (20:26 +0300)]
pg_wal_replay_wait(): Fix typo in the doc
Reported-by: Kevin Hale Boyes
Discussion: https://postgr.es/m/CADAecHWKpaPuPGXAMOH%3DwmhTpydHWGPOk9KWX97UYhp5GdqCWw%40mail.gmail.com
Michael Paquier [Sun, 4 Aug 2024 10:41:24 +0000 (19:41 +0900)]
Introduce pluggable APIs for Cumulative Statistics
This commit adds support in the backend for $subject, allowing
out-of-core extensions to plug their own custom kinds of cumulative
statistics. This feature has come up a few times into the lists, and
the first, original, suggestion came from Andres Freund, about
pg_stat_statements to use the cumulative statistics APIs in shared
memory rather than its own less efficient internals. The advantage of
this implementation is that this can be extended to any kind of
statistics.
The stats kinds are divided into two parts:
- The in-core "builtin" stats kinds, with designated initializers, able
to use IDs up to 128.
- The "custom" stats kinds, able to use a range of IDs from 128 to 256
(128 slots available as of this patch), with information saved in
TopMemoryContext. This can be made larger, if necessary.
There are two types of cumulative statistics in the backend:
- For fixed-numbered objects (like WAL, archiver, etc.). These are
attached to the snapshot and pgstats shmem control structures for
efficiency, and built-in stats kinds still do that to avoid any
redirection penalty. The data of custom kinds is stored in a first
array in snapshot structure and a second array in the shmem control
structure, both indexed by their ID, acting as an equivalent of the
builtin stats.
- For variable-numbered objects (like tables, functions, etc.). These
are stored in a dshash using the stats kind ID in the hash lookup key.
Internally, the handling of the builtin stats is unchanged, and both
fixed and variabled-numbered objects are supported. Structure
definitions for builtin stats kinds are renamed to reflect better the
differences with custom kinds.
Like custom RMGRs, custom cumulative statistics can only be loaded with
shared_preload_libraries at startup, and must allocate a unique ID
shared across all the PostgreSQL extension ecosystem with the following
wiki page to avoid conflicts:
https://wiki.postgresql.org/wiki/CustomCumulativeStats
This makes the detection of the stats kinds and their handling when
reading and writing stats much easier than, say, allocating IDs for
stats kinds from a shared memory counter, that may change the ID used by
a stats kind across restarts. When under development, extensions can
use PGSTAT_KIND_EXPERIMENTAL.
Two examples that can be used as templates for fixed-numbered and
variable-numbered stats kinds will be added in some follow-up commits,
with tests to provide coverage.
Some documentation is added to explain how to use this plugin facility.
Author: Michael Paquier
Reviewed-by: Dmitry Dolgov, Bertrand Drouvot
Discussion: https://postgr.es/m/Zmqm9j5EO0I4W8dx@paquier.xyz
Peter Eisentraut [Sun, 4 Aug 2024 09:17:46 +0000 (11:17 +0200)]
Use CXXFLAGS instead of CFLAGS for linking C++ code
Otherwise, this would break if using C and C++ compilers from
different families and they understand different options. It already
used the right flags for compiling, this is only for linking. Also,
the meson setup already did this correctly.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/228700.
1722717983@sss.pgh.pa.us
Michael Paquier [Sat, 3 Aug 2024 18:07:20 +0000 (03:07 +0900)]
Fix incorrect format placeholders in pgstat.c
These should have been switched from %d to %u in
3188a4582a8c in the
debugging elogs added in
ca1ba50fcb6f. PgStat_Kind should never be
higher than INT32_MAX, but let's be clean.
Issue noticed while hacking more on this area.
Peter Eisentraut [Sat, 3 Aug 2024 09:42:37 +0000 (11:42 +0200)]
Add -Wmissing-variable-declarations to the standard compilation flags
This warning flag detects global variables not declared in header
files. This is similar to what -Wmissing-prototypes does for
functions. (More correctly, it is similar to what
-Wmissing-declarations does for functions, but -Wmissing-prototypes is
a superset of that in C.)
This flag is new in GCC 14. Clang has supported it for a while.
Several recent commits have cleaned up warnings triggered by this, so
it should now be clean.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/
e0a62134-83da-4ba4-8cdb-
ceb0111c95ce@eisentraut.org
Jeff Davis [Fri, 2 Aug 2024 18:49:03 +0000 (11:49 -0700)]
Small refactoring around ExecCreateTableAs().
Since commit
4b74ebf726, the refresh logic is used to populate
materialized views, so we can simplify the error message in
ExecCreateTableAs().
Also, RefreshMatViewByOid() is moved to just after
create_ctas_nodata() call to improve code readability.
Author: Yugo Nagata
Discussion: https://postgr.es/m/
20240802161301.
d975daca9ba7a706fa05ecd7@sraoss.co.jp
Noah Misch [Fri, 2 Aug 2024 19:49:56 +0000 (12:49 -0700)]
Fix name of "Visual Studio" in documentation.
Back-patch to v17, which introduced this.
Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TM7ct0EjoCQaLSVYoxxnEw4xCUFebWj77GktWsqEdyCtQ@mail.gmail.com
Alexander Korotkov [Fri, 2 Aug 2024 18:13:05 +0000 (21:13 +0300)]
Implement pg_wal_replay_wait() stored procedure
pg_wal_replay_wait() is to be used on standby and specifies waiting for
the specific WAL location to be replayed. This option is useful when
the user makes some data changes on primary and needs a guarantee to see
these changes are on standby.
The queue of waiters is stored in the shared memory as an LSN-ordered pairing
heap, where the waiter with the nearest LSN stays on the top. During
the replay of WAL, waiters whose LSNs have already been replayed are deleted
from the shared memory pairing heap and woken up by setting their latches.
pg_wal_replay_wait() needs to wait without any snapshot held. Otherwise,
the snapshot could prevent the replay of WAL records, implying a kind of
self-deadlock. This is why it is only possible to implement
pg_wal_replay_wait() as a procedure working without an active snapshot,
not a function.
Catversion is bumped.
Discussion: https://postgr.es/m/
eb12f9b03851bb2583adab5df9579b4b%40postgrespro.ru
Author: Kartyshov Ivan, Alexander Korotkov
Reviewed-by: Michael Paquier, Peter Eisentraut, Dilip Kumar, Amit Kapila
Reviewed-by: Alexander Lakhin, Bharath Rupireddy, Euler Taveira
Reviewed-by: Heikki Linnakangas, Kyotaro Horiguchi
Alvaro Herrera [Fri, 2 Aug 2024 16:05:38 +0000 (12:05 -0400)]
Fix NLS file reference in pg_createsubscriber
pg_createsubscriber is referring to a non-existent message translation
file, causing NLS to not work correctly. This command should use the
same file as pg_basebackup.
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20240802.115717.
1083441453338151622.horikyota.ntt@gmail.com
Alvaro Herrera [Fri, 2 Aug 2024 16:01:10 +0000 (12:01 -0400)]
pg_createsubscriber: Fix bogus error message
Also some desultory style improvement
Peter Eisentraut [Fri, 2 Aug 2024 07:59:35 +0000 (09:59 +0200)]
Include bison header files into implementation files
Before Bison 3.4, the generated parser implementation files run afoul
of -Wmissing-variable-declarations (in spite of commit
ab61c40bfa2)
because declarations for yylval and possibly yylloc are missing. The
generated header files contain an extern declaration, but the
implementation files don't include the header files. Since Bison 3.4,
the generated implementation files automatically include the generated
header files, so then it works.
To make this work with older Bison versions as well, include the
generated header file from the .y file.
(With older Bison versions, the generated implementation file contains
effectively a copy of the header file pasted in, so including the
header file is redundant. But we know this works anyway because the
core grammar uses this arrangement already.)
Discussion: https://www.postgresql.org/message-id/flat/
e0a62134-83da-4ba4-8cdb-
ceb0111c95ce@eisentraut.org
Heikki Linnakangas [Thu, 1 Aug 2024 20:23:55 +0000 (23:23 +0300)]
Minor refactoring of assign_backendlist_entry()
Make assign_backendlist_entry() responsible just for allocating the
Backend struct. Linking it to the RegisteredBgWorker is the caller's
responsibility now. Seems more clear that way.
Discussion: https://www.postgresql.org/message-id/
835232c0-a5f7-4f20-b95b-
5b56ba57d741@iki.fi
Heikki Linnakangas [Thu, 1 Aug 2024 20:23:47 +0000 (23:23 +0300)]
Fix outdated comment; all running bgworkers are in BackendList
Before commit
8a02b3d732, only bgworkers that connected to a database
had an entry in the Backendlist. Commit
8a02b3d732 changed that, but
forgot to update this comment.
Discussion: https://www.postgresql.org/message-id/
835232c0-a5f7-4f20-b95b-
5b56ba57d741@iki.fi
Michael Paquier [Thu, 1 Aug 2024 19:49:34 +0000 (04:49 +0900)]
Switch PgStat_Kind from an enum to a uint32 type
A follow-up patch is planned to make cumulative statistics pluggable,
and using a type is useful in the internal routines used by pgstats as
PgStat_Kind may have a value that was not originally in the enum removed
here, once made pluggable.
While on it, this commit switches pgstat_is_kind_valid() to use
PgStat_Kind rather than an int, to be more consistent with its existing
callers. Some loops based on the stats kind IDs are switched to use
PgStat_Kind rather than int, for consistency with the new time.
Author: Michael Paquier
Reviewed-by: Dmitry Dolgov, Bertrand Drouvot
Discussion: https://postgr.es/m/Zmqm9j5EO0I4W8dx@paquier.xyz
Michael Paquier [Thu, 1 Aug 2024 16:57:28 +0000 (01:57 +0900)]
Add redo LSN to pgstats files
This is used in the startup process to check that the pgstats file we
are reading includes the redo LSN referring to the shutdown checkpoint
where it has been written. The redo LSN in the pgstats file needs to
match with what the control file has.
This is intended to be used for an upcoming change that will extend the
write of the stats file to happen during checkpoints, rather than only
shutdown sequences.
Bump PGSTAT_FILE_FORMAT_ID.
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/Zp8o6_cl0KSgsnvS@paquier.xyz
Peter Eisentraut [Thu, 1 Aug 2024 10:41:26 +0000 (12:41 +0200)]
Convert some extern variables to static, Windows code
Similar to
720b0eaae9b, discovered by MinGW.
Peter Eisentraut [Thu, 1 Aug 2024 09:59:29 +0000 (11:59 +0200)]
Convert an extern variable to static
Similar to
720b0eaae9b, fixes new code from
bd15b7db489.
Peter Eisentraut [Thu, 1 Aug 2024 10:09:56 +0000 (12:09 +0200)]
pg_createsubscriber: Rename option --socket-directory to --socketdir
For consistency with the equivalent option in pg_upgrade.
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Discussion: https://www.postgresql.org/message-id/flat/
1ed82b9b-8e20-497d-a2f8-
aebdd793d595%40eisentraut.org
Etsuro Fujita [Thu, 1 Aug 2024 08:45:00 +0000 (17:45 +0900)]
Update comment in portal.h.
We store tuples into the portal's tuple store for a PORTAL_ONE_MOD_WITH
query as well.
Back-patch to all supported branches.
Reviewed by Andy Fan.
Discussion: https://postgr.es/m/CAPmGK14HVYBZYZtHabjeCd-e31VT%3Dwx6rQNq8QfehywLcpZ2Hw%40mail.gmail.com
Peter Eisentraut [Thu, 1 Aug 2024 07:37:44 +0000 (09:37 +0200)]
Convert node test compile-time settings into run-time parameters
This converts
COPY_PARSE_PLAN_TREES
WRITE_READ_PARSE_PLAN_TREES
RAW_EXPRESSION_COVERAGE_TEST
into run-time parameters
debug_copy_parse_plan_trees
debug_write_read_parse_plan_trees
debug_raw_expression_coverage_test
They can be activated for tests using PG_TEST_INITDB_EXTRA_OPTS.
The compile-time symbols are kept for build farm compatibility, but
they now just determine the default value of the run-time settings.
Furthermore, support for these settings is not compiled in at all
unless assertions are enabled, or the new symbol
DEBUG_NODE_TESTS_ENABLED is defined at compile time, or any of the
legacy compile-time setting symbols are defined. So there is no
run-time overhead in production builds. (This is similar to the
handling of DISCARD_CACHES_ENABLED.)
Discussion: https://www.postgresql.org/message-id/flat/
30747bd8-f51e-4e0c-a310-
a6e2c37ec8aa%40eisentraut.org
Amit Kapila [Thu, 1 Aug 2024 04:41:06 +0000 (10:11 +0530)]
Avoid duplicate table scans for cross-partition updates during logical replication.
When performing a cross-partition update in the apply worker, it
needlessly scans the old partition twice, resulting in noticeable
overhead.
This commit optimizes it by removing the redundant table scan.
Author: Hou Zhijie
Reviewed-by: Hayato Kuroda, Amit Kapila
Discussion: https://postgr.es/m/OS0PR01MB571623E39984D94CBB5341D994AB2@OS0PR01MB5716.jpnprd01.prod.outlook.com
Andres Freund [Thu, 1 Aug 2024 01:11:49 +0000 (18:11 -0700)]
Evaluate arguments of correlated SubPlans in the referencing ExprState
Until now we generated an ExprState for each parameter to a SubPlan and
evaluated them one-by-one ExecScanSubPlan. That's sub-optimal as creating lots
of small ExprStates
a) makes JIT compilation more expensive
b) wastes memory
c) is a bit slower to execute
This commit arranges to evaluate parameters to a SubPlan as part of the
ExprState referencing a SubPlan, using the new EEOP_PARAM_SET expression
step. We emit one EEOP_PARAM_SET for each argument to a subplan, just before
the EEOP_SUBPLAN step.
It likely is worth using EEOP_PARAM_SET in other places as well, e.g. for
SubPlan outputs, nestloop parameters and - more ambitiously - to get rid of
ExprContext->domainValue/caseValue/ecxt_agg*. But that's for later.
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru>
Discussion: https://postgr.es/m/
20230225214401.346ancgjqc3zmvek@awork3.anarazel.de
Tom Lane [Thu, 1 Aug 2024 00:53:33 +0000 (20:53 -0400)]
Revert "Allow parallel workers to cope with a newly-created session user ID."
This reverts commit
f5f30c22ed69fb37b896c4d4546b2ab823c3fd61.
Some buildfarm animals are failing with "cannot change
"client_encoding" during a parallel operation". It looks like
assign_client_encoding is unhappy at being asked to roll back a
client_encoding setting after a parallel worker encounters a
failure. There must be more to it though: why didn't I see this
during local testing? In any case, it's clear that moving the
RestoreGUCState() call is not as side-effect-free as I thought.
Given that the bug
f5f30c22e intended to fix has gone unreported
for years, it's not something that's urgent to fix; I'm not
willing to risk messing with it further with only days to our
next release wrap.
Jeff Davis [Wed, 31 Jul 2024 23:42:19 +0000 (16:42 -0700)]
Add is_create parameter to RefreshMatviewByOid().
RefreshMatviewByOid is used for both REFRESH and CREATE MATERIALIZED
VIEW. This flag is currently just used for handling internal error
messages, but also aimed to improve code-readability.
Author: Yugo Nagata
Discussion: https://postgr.es/m/
20240726122630.
70e889f63a4d7e26f8549de8@sraoss.co.jp
Jeff Davis [Wed, 31 Jul 2024 23:15:26 +0000 (16:15 -0700)]
Remove unused ParamListInfo argument from ExecRefreshMatView.
Author: Yugo Nagata
Discussion: https://postgr.es/m/
20240726122630.
70e889f63a4d7e26f8549de8@sraoss.co.jp
Tom Lane [Wed, 31 Jul 2024 22:54:10 +0000 (18:54 -0400)]
Allow parallel workers to cope with a newly-created session user ID.
Parallel workers failed after a sequence like
BEGIN;
CREATE USER foo;
SET SESSION AUTHORIZATION foo;
because check_session_authorization could not see the uncommitted
pg_authid row for "foo". This is because we ran RestoreGUCState()
in a separate transaction using an ordinary just-created snapshot.
The same disease afflicts any other GUC that requires catalog lookups
and isn't forgiving about the lookups failing.
To fix, postpone RestoreGUCState() into the worker's main transaction
after we've set up a snapshot duplicating the leader's. This affects
check_transaction_isolation and check_transaction_deferrable, which
think they should only run during transaction start. Make them
act like check_transaction_read_only, which already knows it should
silently accept the value when InitializingParallelWorker.
Per bug #18545 from Andrey Rachitskiy. Back-patch to all
supported branches, because this has been wrong for awhile.
Discussion: https://postgr.es/m/18545-
feba138862f19aaa@postgresql.org
Nathan Bossart [Wed, 31 Jul 2024 15:12:42 +0000 (10:12 -0500)]
Improve performance of dumpSequenceData().
As one might guess, this function dumps the sequence data. It is
called once per sequence, and each such call executes a query to
retrieve the relevant data for a single sequence. This can cause
pg_dump to take significantly longer, especially when there are
many sequences.
This commit improves the performance of this function by gathering
all the sequence data with a single query at the beginning of
pg_dump. This information is stored in a sorted array that
dumpSequenceData() can bsearch() for what it needs. This follows a
similar approach as previous commits that introduced sorted arrays
for role information, pg_class information, and sequence metadata.
As with those commits, this patch will cause pg_dump to use more
memory, but that isn't expected to be too egregious.
Note that we use the brand new function pg_sequence_read_tuple() in
the query that gathers all sequence data, so we must continue to
use the preexisting query-per-sequence approach for versions older
than 18.
Reviewed-by: Euler Taveira, Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/
20240503025140.GA1227404%40nathanxps13
Nathan Bossart [Wed, 31 Jul 2024 15:12:42 +0000 (10:12 -0500)]
Introduce pg_sequence_read_tuple().
This new function returns the data for the given sequence, i.e.,
the values within the sequence tuple. Since this function is a
substitute for SELECT from the sequence, the SELECT privilege is
required on the sequence in question. It returns all NULLs for
sequences for which we lack privileges, other sessions' temporary
sequences, and unlogged sequences on standbys.
This function is primarily intended for use by pg_dump in a
follow-up commit that will use it to optimize dumpSequenceData().
Like pg_sequence_last_value(), which is a support function for the
pg_sequences system view, pg_sequence_read_tuple() is left
undocumented.
Bumps catversion.
Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/
20240503025140.GA1227404%40nathanxps13
Nathan Bossart [Wed, 31 Jul 2024 15:12:42 +0000 (10:12 -0500)]
Improve performance of dumpSequence().
This function dumps the sequence definitions. It is called once
per sequence, and each such call executes a query to retrieve the
metadata for a single sequence. This can cause pg_dump to take
significantly longer, especially when there are many sequences.
This commit improves the performance of this function by gathering
all the sequence metadata with a single query at the beginning of
pg_dump. This information is stored in a sorted array that
dumpSequence() can bsearch() for what it needs. This follows a
similar approach as commits
d5e8930f50 and
2329cad1b9, which
introduced sorted arrays for role information and pg_class
information, respectively. As with those commits, this patch will
cause pg_dump to use more memory, but that isn't expected to be too
egregious.
Note that before version 10, the sequence metadata was stored in
the sequence relation itself, which makes it difficult to gather
all the sequence metadata with a single query. For those older
versions, we continue to use the preexisting query-per-sequence
approach.
Reviewed-by: Euler Taveira
Discussion: https://postgr.es/m/
20240503025140.GA1227404%40nathanxps13
Nathan Bossart [Wed, 31 Jul 2024 15:12:41 +0000 (10:12 -0500)]
Parse sequence type and integer metadata in dumpSequence().
This commit modifies dumpSequence() to parse all the sequence
metadata into the appropriate types instead of carting around
string pointers to the PGresult data. Besides allowing us to free
the PGresult storage earlier in the function, this eliminates the
need to compare min_value and max_value to their respective
defaults as strings.
This is preparatory work for a follow-up commit that will improve
the performance of dumpSequence() in a similar manner to how commit
2329cad1b9 optimized binary_upgrade_set_pg_class_oids().
Reviewed-by: Euler Taveira
Discussion: https://postgr.es/m/
20240503025140.GA1227404%40nathanxps13
David Rowley [Wed, 31 Jul 2024 13:25:25 +0000 (01:25 +1200)]
Doc: mention executor memory usage for enable_partitionwise* GUCs
Prior to this commit, the docs for enable_partitionwise_aggregate and
enable_partitionwise_join mentioned the additional overheads enabling
these causes for the query planner, but they mentioned nothing about the
possible surge in work_mem-consuming executor nodes that could end up in
the final plan. Dimitrios reported the OOM killer intervened on his
query as a result of using enable_partitionwise_aggregate=on.
Here we adjust the docs to mention the possible increase in the number of
work_mem-consuming executor nodes that can appear in the final plan as a
result of enabling these GUCs.
Reported-by: Dimitrios Apostolou
Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/
3603c380-d094-136e-e333-
610914fb3e80%40gmx.net
Discussion: https://postgr.es/m/CAApHDvoZ0_yqwPFEpb6h261L76BUpmh5GxBQq0LeRzQ5Jh3zzg@mail.gmail.com
Backpatch-through: 12, oldest supported version
Peter Eisentraut [Wed, 31 Jul 2024 09:08:28 +0000 (11:08 +0200)]
Add API and ABI stability guidance to the C language docs
Includes guidance for major and minor version releases, and sets
reasonable expectations for extension developers to follow.
Author: David Wheeler, Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/flat/
5DA9F9D2-B8B2-43DE-BD4D-
53A4160F6E8D%40justatheory.com
Peter Eisentraut [Wed, 31 Jul 2024 05:57:47 +0000 (07:57 +0200)]
doc: Avoid too prominent use of "backup" on pg_dump man page
Some users inadvertently rely on pg_dump as their primary backup tool,
when better solutions exist. The pg_dump man page is arguably
misleading in that it starts with
"pg_dump is a utility for backing up a PostgreSQL database."
This tones this down a little bit, by replacing most uses of "backup"
with "export" and adding a short note that pg_dump is not a
general-purpose backup tool.
Discussion: https://www.postgresql.org/message-id/flat/
70b48475-7706-4268-990d-
fd522b038d96%40eisentraut.org
Peter Eisentraut [Wed, 31 Jul 2024 04:22:02 +0000 (06:22 +0200)]
Make building with LTO work on macOS
When building with -flto, the backend binary must keep many otherwise
unused symbols to make them available to dynamically loaded modules /
extensions. This has been done via -Wl,--export-dynamic on many
platforms for years. This flag is not supported by the macOS linker,
though. Here it's called -Wl,-export_dynamic instead.
Thus, make configure pick up on this variant of the flag as well.
Meson has the logic upstream as of version 1.5.0.
Without this fix, building with -flto fails with errors similar to [1]
and [2].
[1]: https://postgr.es/m/
1581936537572-0.post%40n3.nabble.com
[2]: https://postgr.es/m/21800.
1499270547%40sss.pgh.pa.us
Author: Wolfgang Walther <walther@technowledgy.de>
Discussion: https://www.postgresql.org/message-id/flat/
427c7c25-e8e1-4fc5-a1fb-
01ceff185e5b@technowledgy.de
Amit Kapila [Wed, 31 Jul 2024 03:23:55 +0000 (08:53 +0530)]
Fix random failure in 021_twophase.
After disabling the subscription, the failed test was changing the
two_phase option for the subscription. We can't change the two_phase
option for a subscription till the corresponding apply worker is active.
The check to ensure that the replication apply worker has exited was
incorrect.
Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm3YY+bzj+JWJbY+DsUgJ2mPk8OR1ttjVX2cywKr4BUgxw@mail.gmail.com
Jeff Davis [Tue, 30 Jul 2024 23:23:20 +0000 (16:23 -0700)]
Relax check for return value from second call of pg_strnxfrm().
strxfrm() is not guaranteed to return the exact number of bytes needed
to store the result; it may return a higher value.
Discussion: https://postgr.es/m/
32f85d88d1f64395abfe5a10dd97a62a4d3474ce.camel@j-davis.com
Reviewed-by: Heikki Linnakangas
Backpatch-through: 16
Heikki Linnakangas [Tue, 30 Jul 2024 19:06:07 +0000 (22:06 +0300)]
Refactor getWeights to write to caller-supplied buffer
This gets rid of the static result buffer.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/
7f86e06a-98c5-4ce3-8ec9-
3885c8de0358@iki.fi
Heikki Linnakangas [Tue, 30 Jul 2024 19:06:03 +0000 (22:06 +0300)]
Replace static buf with a stack-allocated one in 'seg' extension
The buffer is used only locally within the function. Also, the
initialization to '0' characters was unnecessary, the initial content
were always overwritten with sprintf(). I don't understand why it was
done that way, but it's been like that since forever.
In the passing, change from sprintf() to snprintf(). The buffer was
long enough so sprintf() was fine, but this makes it more obvious that
there's no risk of a buffer overflow.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/
7f86e06a-98c5-4ce3-8ec9-
3885c8de0358@iki.fi
Heikki Linnakangas [Tue, 30 Jul 2024 19:05:59 +0000 (22:05 +0300)]
Replace static buf with a stack-allocated one in ReadControlFile
It's only used very locally within the function.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/
7f86e06a-98c5-4ce3-8ec9-
3885c8de0358@iki.fi
Heikki Linnakangas [Tue, 30 Jul 2024 19:05:51 +0000 (22:05 +0300)]
Replace static buf with palloc in str_time()
The function is used only once in the startup process, so the leak
into current memory context is harmless.
This is a tiny step in making the server thread-safe.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/
7f86e06a-98c5-4ce3-8ec9-
3885c8de0358@iki.fi
Heikki Linnakangas [Tue, 30 Jul 2024 19:02:58 +0000 (22:02 +0300)]
Replace static bufs with a StringInfo in cash_words()
For clarity. The code was correct, and the buffer was large enough,
but string manipulation with no bounds checking is scary.
This incurs an extra palloc+pfree to every call, but in quick
performance testing, it doesn't seem to be significant.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/
7f86e06a-98c5-4ce3-8ec9-
3885c8de0358@iki.fi
Heikki Linnakangas [Tue, 30 Jul 2024 12:19:46 +0000 (15:19 +0300)]
Remove leftover function declaration
Commit
9d9b9d46f3 removed the function (or rather, moved it to a
different source file and renamed it to SendCancelRequest), but forgot
the declaration in the header file.
Andrew Dunstan [Tue, 30 Jul 2024 11:57:38 +0000 (07:57 -0400)]
Preserve tz when converting to jsonb timestamptz
This removes an inconsistency in the treatment of different datatypes by
the jsonpath timestamp_tz() function. Conversions from data types that
are not timestamp-aware, such as date and timestamp, are now treated
consistently with conversion from those that are such as timestamptz.
Author: David Wheeler
Reviewed-by: Junwang Zhao and Jeevan Chalke
Discussion: https://postgr.es/m/
7DE080CE-6D8C-4794-9BD1-
7D9699172FAB%40justatheory.com
Backpatch to release 17.
Thomas Munro [Tue, 30 Jul 2024 11:31:56 +0000 (23:31 +1200)]
Remove spinlocks and atomics from meson_options.txt.
Commits
e2562667 and
81385261 removed the configure equivalents, but
forgot to remove these options from meson_options.txt.
Revealed by the fact that build farm animals rorqual and francolin
didn't fail, despite being configured to set those options to off. They
should now fail with unknown option, until they are adjusted.
Thomas Munro [Tue, 30 Jul 2024 11:15:09 +0000 (23:15 +1200)]
Remove useless member of BackendParameters.
Oversight in
e2562667, which stopped using SpinlockSemaArray but forgot
to remove it from the array.
Reported-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/
310f4005-91d7-42b2-ac70-
92624260dd28%40iki.fi
Thomas Munro [Tue, 30 Jul 2024 10:16:50 +0000 (22:16 +1200)]
Require memory barrier support.
Previously we had a fallback implementation that made a harmless system
call, based on the assumption that system calls must contain a memory
barrier. That shouldn't be reached on any current system, and it seems
highly likely that we can easily find out how to request explicit memory
barriers, if we've already had to find out how to do atomics on a
hypothetical new system.
Removed comments and a function name referred to a spinlock used for
fallback memory barriers, but that changed in
1b468a13, which left some
misleading words behind in a few places.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
721bf39a-ed8a-44b0-8b8e-
be3bd81db748%40technowledgy.de
Discussion: https://postgr.es/m/
3351991.
1697728588%40sss.pgh.pa.us
Thomas Munro [Tue, 30 Jul 2024 10:12:42 +0000 (22:12 +1200)]
Require compiler barrier support.
Previously we had a fallback implementation of pg_compiler_barrier()
that called an empty function across a translation unit boundary so the
compiler couldn't see what it did. That shouldn't be needed on any
current systems, and might not even work with a link time optimizer.
Since we now require compiler-specific knowledge of how to implement
atomics, we should also know how to implement compiler barriers on a
hypothetical new system.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
721bf39a-ed8a-44b0-8b8e-
be3bd81db748%40technowledgy.de
Discussion: https://postgr.es/m/
3351991.
1697728588%40sss.pgh.pa.us