Peter Eisentraut [Sat, 1 Feb 2025 09:18:46 +0000 (10:18 +0100)]
Rename GistTranslateStratnum() to GistTranslateCompareType()
Follow up to commit
630f9a43cec. The previous name had become
confusing, because it doesn't actually translate a strategy number but
a CompareType into a strategy number. We might add the inverse at
some point, which would then probably be called something like
GistTranslateStratnum.
Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
E72EAA49-354D-4C2E-8EB9-
255197F55330@enterprisedb.com
Peter Eisentraut [Sat, 1 Feb 2025 09:01:16 +0000 (10:01 +0100)]
Add script to keep .editorconfig in sync with .gitattributes
Our repo already contained an .editorconfig file, but it was not kept
up to date with .gitattributes. This adds a script that keeps these
files in sync. A big advantage of the editorconfig file is that it
many editors/IDEs get automatically configured to trim trailing
newlines and add a final newline on save, while .gitattributes only
complains about these problems instead of automatically fixing them.
This also adds rules to .gitattributes for Python files as well as for
C files in pg_bsd_indent directory (which have a different tab_width
than most C files due to being vendored in).
Author: Jelte Fennema-Nio <github-tech@jeltef.nl>
Discussion: https://www.postgresql.org/message-id/flat/CAGECzQQGzbroAXi+Yicp3HvcCo4=g84kaOgjuvQ5MW9F0ubOGg@mail.gmail.com
Amit Langote [Sat, 1 Feb 2025 07:36:18 +0000 (16:36 +0900)]
Add commit
76aa615943 to .git-blame-ignore-revs
Tom Lane [Fri, 31 Jan 2025 20:17:15 +0000 (15:17 -0500)]
Doc: add commentary about cowboy assignment of maintenance_work_mem.
Whilst working on commit
041e8b95b I happened to notice that
parallel_vacuum_main() assigns directly to the maintenance_work_mem
GUC. This is definitely not per project conventions, so I tried to
fix it to use SetConfigOption(). But that fails with "parameter
cannot be set during a parallel operation". It doesn't seem worth
working on a cleaner answer, at least not till we have a few more
instances of similar problems. But add some commentary, just so
nobody gets the idea that this is an approved way to set a GUC.
Tom Lane [Fri, 31 Jan 2025 19:36:56 +0000 (14:36 -0500)]
Remove obsolete restriction on the range of log_rotation_size.
When syslogger.c was first written, we didn't want to assume that
all platforms have 64-bit ftello. But we've been assuming that
since v13 (cf commit
799d22461), so let's use that in syslogger.c
and allow log_rotation_size to range up to INT_MAX kilobytes.
The old code effectively limited log_rotation_size to 2GB regardless
of platform. While nobody's complained, that doesn't seem too far
away from what might be thought reasonable these days.
I noticed this while searching for instances of "1024L" in connection
with commit
041e8b95b. These were the last such instances.
(We still have instances of L-suffixed literals, but most of them
are associated with wait intervals for pg_usleep or similar functions.
I don't see any urgent reason to change that.)
Tom Lane [Fri, 31 Jan 2025 18:52:40 +0000 (13:52 -0500)]
Get rid of our dependency on type "long" for memory size calculations.
Consistently use "Size" (or size_t, or in some places int64 or double)
as the type for variables holding memory allocation sizes. In most
places variables' data types were fine already, but we had an ancient
habit of computing bytes from kilobytes-units GUCs with code like
"work_mem * 1024L". That risks overflow on Win64 where they did not
make "long" as wide as "size_t". We worked around that by restricting
such GUCs' ranges, so you couldn't set work_mem et al higher than 2GB
on Win64. This patch removes that restriction, after replacing such
calculations with "work_mem * (Size) 1024" or variants of that.
It should be noted that this patch was constructed by searching
outwards from the GUCs that have MAX_KILOBYTES as upper limit.
So I can't positively guarantee there are no other places doing
memory-size arithmetic in int or long variables. I do however feel
pretty confident that increasing MAX_KILOBYTES on Win64 is safe now.
Also, nothing in our code should be dealing in multiple-gigabyte
allocations without authorization from a relevant GUC, so it seems
pretty likely that this search caught everything that could be at
risk of overflow.
Author: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/1a01f0-
66ec2d80-3b-
68487680@
27595217
Daniel Gustafsson [Fri, 31 Jan 2025 14:47:28 +0000 (15:47 +0100)]
require_auth: prepare for multiple SASL mechanisms
Prior to this patch, the require_auth implementation assumed that
the AuthenticationSASL protocol message was using SCRAM-SHA-256.
In preparation for future SASL mechanisms, like OAUTHBEARER, split
the implementation into two tiers: the first checks the acceptable
AUTH_REQ_* codes, and the second checks acceptable mechanisms if
AUTH_REQ_SASL et.al are permitted.
conn->allowed_sasl_mechs contains a list of pointers to acceptable
mechanisms, and pg_SASL_init() will bail if the selected mechanism
isn't contained in this array.
Since there's only one mechansism supported right now, one branch
of the second tier cannot be exercised yet and is protected by an
Assert(false) call. This assertion will need to be removed when
the next mechanism is added.
This patch is extracted from a larger body of work aimed at adding
support for OAUTHBEARER in libpq.
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CAOYmi+kJqzo6XsR9TEhvVfeVNQ-TyFM5LATypm9yoQVYk=4Wrw@mail.gmail.com
Daniel Gustafsson [Fri, 31 Jan 2025 14:39:35 +0000 (15:39 +0100)]
Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h
Future SASL mechanism, like OAUTHBEARER, will use this as a limit on
token messages coming from the client, so promote it to the header
file to make it available.
This patch is extracted from a larger body of work aimed at adding
support for OAUTHBEARER in libpq.
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/CAOYmi+kJqzo6XsR9TEhvVfeVNQ-TyFM5LATypm9yoQVYk=4Wrw@mail.gmail.com
Daniel Gustafsson [Fri, 31 Jan 2025 09:44:21 +0000 (10:44 +0100)]
doc: Fix pg_buffercache_evict() title
Use <function> rather than <structname> in the <title> to be consistent
with how other functions in this module are documented. Also suffix the
function name with () for consistency.
Backpatch to v17 where pg_buffercache_evict was introduced.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CAExHW5uKWH8CuZc9NCb8XxSQc6uzvACV0cScebm54kF763ERAw@mail.gmail.com
Backpatch-through: 17
Amit Langote [Fri, 31 Jan 2025 07:44:24 +0000 (16:44 +0900)]
Fix bad indentation introduced in commit
d47cbf474
Per buildfarm member koel
Amit Langote [Fri, 31 Jan 2025 06:47:15 +0000 (15:47 +0900)]
Perform runtime initial pruning outside ExecInitNode()
This commit builds on the prior change that moved PartitionPruneInfos
out of individual plan nodes into a list in PlannedStmt, making it
possible to initialize PartitionPruneStates without traversing the
plan tree and perform runtime initial pruning before ExecInitNode()
initializes the plan trees. These tasks are now handled in a new
routine, ExecDoInitialPruning(), which is called by InitPlan()
before calling ExecInitNode() on various plan trees.
ExecDoInitialPruning() performs the initial pruning and saves the
result -- a Bitmapset of indexes for surviving child subnodes -- in
es_part_prune_results, a list in EState.
PartitionPruneStates created for initial pruning are stored in
es_part_prune_states, another list in EState, for later use during
exec pruning. Both lists are parallel to es_part_prune_infos, which
holds the PartitionPruneInfos from PlannedStmt, enabling shared
indexing.
PartitionPruneStates initialized in ExecDoInitialPruning() now
include only the PartitionPruneContexts for initial pruning steps.
Exec pruning contexts are initialized later in
ExecInitPartitionExecPruning() when the parent plan node is
initialized, as the exec pruning step expressions depend on the parent
node's PlanState.
The existing function PartitionPruneFixSubPlanMap() has been
repurposed for this initialization to avoid duplicating a similar
loop structure for finding PartitionedRelPruningData to initialize
exec pruning contexts for. It has been renamed to
InitExecPruningContexts() to reflect its new primary responsibility.
The original logic to "fix subplan maps" remains intact but is now
encapsulated within the renamed function.
This commit removes two obsolete Asserts in partkey_datum_from_expr().
The ExprContext used for pruning expression evaluation is now
independent of the parent PlanState, making these Asserts unnecessary.
By centralizing pruning logic and decoupling it from the plan
initialization step (ExecInitNode()), this change sets the stage for
future patches that will use the result of initial pruning to
save the overhead of redundant processing for pruned partitions.
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
Amit Kapila [Fri, 31 Jan 2025 04:57:35 +0000 (10:27 +0530)]
Raise an error while trying to acquire an invalid slot.
Once a replication slot is invalidated, it cannot be altered or used to
fetch changes. However, a process could still acquire an invalid slot and
fail later.
For example, if a process acquires a logical slot that was invalidated due
to wal_removed, it will eventually fail in CreateDecodingContext() when
attempting to access the removed WAL. Similarly, for physical replication
slots, even if the slot is invalidated and invalidation_reason is set to
wal_removed, the walsender does not currently check for invalidation when
starting physical replication. Instead, replication starts, and an error
is only reported later while trying to access WAL. Similarly, we prohibit
modifying slot properties for invalid slots but give the error for the
same after acquiring the slot.
This patch improves error handling by detecting invalid slots earlier at
the time of slot acquisition which is the first step. This also helped in
unifying different ERROR messages at different places and gave a
consistent message for invalid slots. This means that the message for
invalid slots will change to a generic message.
This will also be helpful for future patches where we are planning to
invalidate slots due to more reasons like idle_timeout because we don't
have to modify multiple places in such cases and avoid the chances of
missing out on a particular place.
Author: Nisha Moond <nisha.moond412@gmail.com>
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CABdArM6pBL5hPnSQ+5nEVMANcF4FCH7LQmgskXyiLY75TMnKpw@mail.gmail.com
Michael Paquier [Fri, 31 Jan 2025 03:41:39 +0000 (12:41 +0900)]
injection_points: Add routine able to drop all stats
This serves as an example of how to use the new function introduced in
ce5c620fb625, pgstat_drop_matching_entries(), with a callback able to
filter the entries dropped.
A SQL function named injection_points_stats_drop() is added with some
tests.
Author: Lukas Fitti
Discussion: https://postgr.es/m/CAP53PkwuFbo3NkwZgxwNRMjMfqPEqidD-SggaoQ4ijotBVLJAA@mail.gmail.com
Michael Paquier [Fri, 31 Jan 2025 03:27:19 +0000 (12:27 +0900)]
Add pgstat_drop_matching_entries() to pgstats
This allows users of the cumulative statistics to drop entries in the
shared hash stats table, deleting as well local references. Callers of
this function can optionally define a callback able to filter which
entries to drop, similarly to pgstat_reset_matching_entries() with its
callback do_reset().
pgstat_drop_all_entries() is refactored so as it uses this new function.
Author: Lukas Fitti
Discussion: https://postgr.es/m/CAP53PkwuFbo3NkwZgxwNRMjMfqPEqidD-SggaoQ4ijotBVLJAA@mail.gmail.com
Michael Paquier [Fri, 31 Jan 2025 02:05:57 +0000 (11:05 +0900)]
Fix comment of StrategySyncStart()
The top comment of StrategySyncStart() mentions BufferSync(), but this
function calls BgBufferSync(), not BufferSync().
Oversight in
9cd00c457e6a.
Author: Ashutosh Bapat
Discussion: https://postgr.es/m/CAExHW5tgkjag8i-s=RFrCn5KAWDrC4zEPPkfUKczfccPOxBRQQ@mail.gmail.com
Backpatch-through: 13
Tom Lane [Thu, 30 Jan 2025 21:44:47 +0000 (16:44 -0500)]
Use "ssize_t" not "long" in max_stack_depth-related code.
This change adapts these functions to the machine's address width
without depending on "long" to be the right size. (It isn't on
Win64, for example.) While it seems unlikely anyone would care
to run with a stack depth limit exceeding 2GB, this is part of a
general push to avoid using type "long" to represent memory sizes.
It's convenient to use ssize_t rather than the perhaps-more-obvious
choice of size_t/Size, because the code involved depends on working
with a signed data type. Our MAX_KILOBYTES limit already ensures
that ssize_t will be sufficient to represent the maximum value of
max_stack_depth.
Extracted from a larger patch by Vladlen, plus additional hackery
by me.
Author: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/1a01f0-
66ec2d80-3b-
68487680@
27595217
Tom Lane [Thu, 30 Jan 2025 20:36:07 +0000 (15:36 -0500)]
Avoid integer overflow while testing wal_skip_threshold condition.
smgrDoPendingSyncs had two distinct risks of integer overflow while
deciding which way to ensure durability of a newly-created relation.
First, it accumulated the total size of all forks in a variable of
type BlockNumber (uint32). While we restrict an individual fork's
size to fit in that, I don't believe there's such a restriction on
all of them added together. Second, it proceeded to multiply the
sum by BLCKSZ, which most certainly could overflow a uint32.
(The exact expression is total_blocks * BLCKSZ / 1024. The
compiler might choose to optimize that to total_blocks * 8,
which is not at quite as much risk of overflow as a literal
reading would be, but it's still wrong.)
If an overflow did occur it could lead to a poor choice to
shove a very large relation into WAL instead of fsync'ing it.
This wouldn't be fatal, but it could be inefficient.
Change total_blocks to uint64 which should be plenty, and
rearrange the comparison calculation to be overflow-safe.
I noticed this while looking for ramifications of the proposed
change in MAX_KILOBYTES. It's not entirely clear to me why
wal_skip_threshold is limited to MAX_KILOBYTES in the
first place, but in any case this code is unsafe regardless
of the range of wal_skip_threshold.
Oversight in
c6b92041d which introduced wal_skip_threshold,
so back-patch to v13.
Discussion: https://postgr.es/m/1a01f0-
66ec2d80-3b-
68487680@
27595217
Backpatch-through: 13
Melanie Plageman [Thu, 30 Jan 2025 20:26:55 +0000 (15:26 -0500)]
Move BitmapTableScan per-scan setup into a helper
Add BitmapTableScanSetup(), a helper which contains all of the code that
must be done on every scan of the table in a bitmap table scan. This
includes scanning the index, building the bitmap, and setting up the
scan descriptors.
Pushing this setup into a helper function makes BitmapHeapNext() more
readable.
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ1vXu%2BZdT0_MM-i1vbTdfHHf0KR3cK6R5gs6dNNNpyrJw%40mail.gmail.com
Tom Lane [Thu, 30 Jan 2025 18:21:42 +0000 (13:21 -0500)]
Simplify executor's handling of CaseTestExpr & CoerceToDomainValue.
Instead of deciding at runtime whether to read from casetest.value
or caseValue_datum, split EEOP_CASE_TESTVAL into two opcodes and
make the decision during expression compilation. Similarly for
EEOP_DOMAIN_TESTVAL. This actually results in net less code,
mainly because llvmjit_expr.c's code for handling these opcodes
gets shorter. The performance gain is doubtless negligible, but
this seems worth changing anyway on grounds of simplicity and
understandability.
Author: Andreas Karlsson <andreas@proxel.se>
Co-authored-by: Xing Guo <higuoxing@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CACpMh+AiBYAWn+D1aU7Rsy-V1tox06Cbc0H3qA7rwL5zdJ=anQ@mail.gmail.com
Amit Kapila [Thu, 30 Jan 2025 05:39:18 +0000 (11:09 +0530)]
Doc: Generated column replication.
Commit
7054186c4e added the support to publish generated stored columns.
This patch adds detailed documentation for that feature.
Author: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/
B80D17B2-2C8E-4C7D-87F2-
E5B4BE3C069E%40gmail.com
Discussion: https://postgr.es/m/CAHut+PsYmAvKhUjA1AaR1rxLdeSBKiBko8wKyf4_H8nEEqDuOg@mail.gmail.com
Amit Langote [Thu, 30 Jan 2025 02:57:32 +0000 (11:57 +0900)]
Move PartitionPruneInfo out of plan nodes into PlannedStmt
This moves PartitionPruneInfo from plan nodes to PlannedStmt,
simplifying traversal by centralizing all PartitionPruneInfo
structures in a single list in it, which holds all instances for the
main query and its subqueries. Instead of plan nodes (Append or
MergeAppend) storing PartitionPruneInfo pointers, they now reference
an index in this list.
A bitmapset field is added to PartitionPruneInfo to store the RT
indexes corresponding to the apprelids field in Append or MergeAppend.
This allows execution pruning logic to verify that it operates on the
correct plan node, mainly to facilitate debugging.
Duplicated code in set_append_references() and
set_mergeappend_references() is refactored into a new function,
register_pruneinfo(). This updates RT indexes by applying rtoffet
and adds PartitionPruneInfo to the global list in PlannerGlobal.
By allowing pruning to be performed without traversing the plan tree,
this change lays the groundwork for runtime initial pruning to occur
independently of plan tree initialization.
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org> (earlier version)
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
Tom Lane [Wed, 29 Jan 2025 20:42:25 +0000 (15:42 -0500)]
Require callers of coerce_to_domain() to supply base type/typmod.
In view of the issue fixed in commit
0da39aa76, it no longer seems
like a great idea for coerce_to_domain() to offer to perform a lookup
that its caller probably should have done already. The caller should
be providing a value of the domain's base type, so it's hard to
envision a valid case where it hasn't looked up that type. After
0da39aa76 there is only one caller using the option for internal
lookup, and that one can trivially be rearranged to not do that.
So this seems more like a bug-encouraging misfeature than a useful
shortcut; let's get rid of it (in HEAD only, there's no need to
break any external callers in back branches).
Discussion: https://postgr.es/m/
1865579.
1738113656@sss.pgh.pa.us
Tom Lane [Wed, 29 Jan 2025 20:31:55 +0000 (15:31 -0500)]
Handle default NULL insertion a little better.
If a column is omitted in an INSERT, and there's no column default,
the code in preptlist.c generates a NULL Const to be inserted.
Furthermore, if the column is of a domain type, we wrap the Const
in CoerceToDomain, so as to throw a run-time error if the domain
has a NOT NULL constraint. That's fine as far as it goes, but
there are two problems:
1. We're being sloppy about the type/typmod that the Const is
labeled with. It really should have the domain's base type/typmod,
since it's the input to CoerceToDomain not the output. This can
result in coerce_to_domain inserting a useless length-coercion
function (useless because it's being applied to a null). The
coercion would typically get const-folded away later, but it'd
be better not to create it in the first place.
2. We're not applying expression preprocessing (specifically,
eval_const_expressions) to the resulting expression tree.
The planner's primary expression-preprocessing pass already happened,
so that means the length coercion step and CoerceToDomain node miss
preprocessing altogether.
This is at the least inefficient, since it means the length coercion
and CoerceToDomain will actually be executed for each inserted row,
though they could be const-folded away in most cases. Worse, it
seems possible that missing preprocessing for the length coercion
could result in an invalid plan (for example, due to failing to
perform default-function-argument insertion). I'm not aware of
any live bug of that sort with core datatypes, and it might be
unreachable for extension types as well because of restrictions of
CREATE CAST, but I'm not entirely convinced that it's unreachable.
Hence, it seems worth back-patching the fix (although I only went
back to v14, as the patch doesn't apply cleanly at all in v13).
There are several places in the rewriter that are building null
domain constants the same way as preptlist.c. While those are
before the planner and hence don't have any reachable bug, they're
still applying a length coercion that will be const-folded away
later, uselessly wasting cycles. Hence, make a utility routine
that all of these places can call to do it right.
Making this code more careful about the typmod assigned to the
generated NULL constant has visible but cosmetic effects on some
of the plans shown in contrib/postgres_fdw's regression tests.
Discussion: https://postgr.es/m/
1865579.
1738113656@sss.pgh.pa.us
Backpatch-through: 14
Tom Lane [Wed, 29 Jan 2025 19:24:36 +0000 (14:24 -0500)]
Avoid breaking SJIS encoding while de-backslashing Windows paths.
When running on Windows, canonicalize_path() converts '\' to '/'
to prevent confusing the Windows command processor. It was
doing that in a non-encoding-aware fashion; but in SJIS there
are valid two-byte characters whose second byte matches '\'.
So encoding corruption ensues if such a character is used in
the path.
We can fairly easily fix this if we know which encoding is
in use, but a lot of our utilities don't have much of a clue
about that. After some discussion we decided we'd settle for
fixing this only in psql, and assuming that its value of
client_encoding matches what the user is typing.
It seems hopeless to get the server to deal with the problematic
characters in database path names, so we'll just declare that
case to be unsupported. That means nothing need be done in
the server, nor in utility programs whose only contact with
file path names is for database paths. But psql frequently
deals with client-side file paths, so it'd be good if it
didn't mess those up.
Bug: #18735
Reported-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Koichi Suzuki <koichi.suzuki@enterprisedb.com>
Discussion: https://postgr.es/m/18735-
4acdb3998bb9f2b1@postgresql.org
Backpatch-through: 13
Tom Lane [Wed, 29 Jan 2025 18:23:31 +0000 (13:23 -0500)]
Make BufferIsExclusiveLocked and BufferIsDirty work for local buffers.
These functions tried to check the state of the buffer's content lock
even for local buffers. Since we don't use the content lock for a
local buffer, that would lead to a "false" result from
LWLockHeldByMeInMode, which would mean a misleading "false" answer
from BufferIsExclusiveLocked (we'd rather that case always return
"true") or an assertion failure in BufferIsDirty.
The core code never applies these two functions to local buffers,
and apparently no extensions do either, since we've not heard
complaints. Still, in the name of future-proofing, let's fix
them to act as though a pinned local buffer is content-locked.
Author: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://postgr.es/m/
19396ef77f8.
1098c4a1810508.
2255483659262451647@zohocorp.com
John Naylor [Wed, 29 Jan 2025 07:28:20 +0000 (14:28 +0700)]
Fix grammatical typos around possessive "its"
Some places spelled it "it's", which is short for "it is".
In passing, fix a couple other nearby grammatical errors.
Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/CA+COZaAO8g1KJCV0T48=CkJMjAnnfTGLWOATz+2aCh40c2Nm+g@mail.gmail.com
John Naylor [Wed, 29 Jan 2025 06:35:43 +0000 (13:35 +0700)]
Revert "Speed up tail processing when hashing aligned C strings, take two"
This reverts commit
a365d9e2e8c1ead27203a4431211098292777d3b.
Older versions of Valgrind raise an error, so go back to the bytewise
loop for the final word in the input.
Reported-by: Anton A. Melnikov <a.melnikov@postgrespro.ru>
Discussion: https://postgr.es/m/
a3a959f6-14b8-4819-ac04-
eaf2aa2e868d@postgrespro.ru
Backpatch-through: 17
Michael Paquier [Tue, 28 Jan 2025 23:49:48 +0000 (08:49 +0900)]
Improve test coverage of network address functions
The following functions were not covered by any tests:
- abbrev(inet)
- set_masklen(cidr)
- set_masklen(inet)
- netmask(inet)
- hostmask(inet)
While on it, this improves the output of some of the existing queries in
the test inet to use better aliases.
Author: Aleksander Alekseev
Reviewed-by: Jacob Champion, Keisuke Kuroda, Tom Lane
Discussion: https://postgr.es/m/CAJ7c6TOyZ9bGNrDK6Z3Q0gr9ow8ZpOm+=+01mpE0dsdH4C+u9A@mail.gmail.com
Amit Kapila [Tue, 28 Jan 2025 05:12:46 +0000 (10:42 +0530)]
Rename pubgencols_type to pubgencols in pg_publication.
The column added in commit
e65dbc9927, pubgencols_type, was inconsistent
with the naming conventions of other columns in the pg_publication
catalog.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CALDaNm1u-ufVOW-RUsXSooqzkpohxfZYy=z78fbcr_9Pq5hbCg@mail.gmail.com
Michael Paquier [Tue, 28 Jan 2025 00:57:32 +0000 (09:57 +0900)]
Track per-relation cumulative time spent in [auto]vacuum and [auto]analyze
This commit adds four fields to the statistics of relations, aggregating
the amount of time spent for each operation on a relation:
- total_vacuum_time, for manual vacuum.
- total_autovacuum_time, for vacuum done by the autovacuum daemon.
- total_analyze_time, for manual analyze.
- total_autoanalyze_time, for analyze done by the autovacuum daemon.
This gives users the option to derive the average time spent for these
operations with the help of the related "count" fields.
Bump catalog version (for the catalog changes) and PGSTAT_FILE_FORMAT_ID
(for the additions in PgStat_StatTabEntry).
Author: Sami Imseih
Reviewed-by: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/CAA5RZ0uVOGBYmPEeGF2d1B_67tgNjKx_bKDuL+oUftuoz+=Y1g@mail.gmail.com
Peter Eisentraut [Mon, 27 Jan 2025 11:02:00 +0000 (12:02 +0100)]
doc: Meson is not experimental on Windows
The installation documentation stated that using Meson is
experimental. But since this is the only way to build using Visual
Studio on Windows, this would imply that that whole build procedure is
experimental, which isn't true. So qualify this statement a bit more.
We keep the statement that Meson is experimental on other platforms,
since it doesn't have full, confirmed feature parity with the make
build system.
Author: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
a3e76618-4cb5-4d54-a71c-
da4fb8ba571b@eisentraut.org
Michael Paquier [Mon, 27 Jan 2025 04:51:23 +0000 (13:51 +0900)]
Print out error position for some ALTER TABLE ALTER COLUMN type
A ParseState exists in ATPrepAlterColumnType() since its introduction
in
077db40fa1f3, and it has never relied on a query string that could be
used to point at a location in the origin string on error.
The output of some regression tests are updated, showing the error
location where applicable. Six error strings are upgraded with the
error location.
Author: Jian He
Discussion: https://postgr.es/m/CACJufxGfbPfWLjcEz33G9eW_epDW0UDi2H05i9eSTPKGJ4rxSA@mail.gmail.com
Michael Paquier [Sun, 26 Jan 2025 23:00:03 +0000 (08:00 +0900)]
pg_amcheck: Fix test failure on Windows with non-existing role
For SSPI auth extra users need to be explicitly allowed, or we get
"SSPI authentication failed" instead of the expected "role does not
exist" error.
This report also means that the test has never worked on Windows since
its introduction in
9706092839db, because it has always bumped on an
authentication failure rather than an error about the role not existing.
Oversight in
eef4a33f62f7, that has added a pattern check on the error
generated by the command.
Per report from Tom Lane, via buildfarm member drongo.
Author: Dagfinn Ilmari Mannsåker
Reviewed-by: Andrew Dunstan
Discussion: https://postgr.es/m/379085.
1737734611@sss.pgh.pa.us
Noah Misch [Sun, 26 Jan 2025 17:39:05 +0000 (09:39 -0800)]
Test postmaster with program_options_handling_ok() et al.
Most executables already get that testing. To occupy the customary
001_basic.pl name, this renumbers the new-in-October tests of
src/test/postmaster/t.
Reviewed by Thomas Munro.
Discussion: https://postgr.es/m/
20241215022701.a1.nmisch@google.com
Álvaro Herrera [Sun, 26 Jan 2025 16:34:28 +0000 (17:34 +0100)]
Add missing CommandCounterIncrement
For commit
b663b9436e75 I thought this was useless, but turns out not to
be for the case where a partitioned table has two identical foreign key
constraints which can both be matched by the same constraint in a
partition during attach. This CCI makes the match search for the second
constraint in the parent ignore the constraint in the child that has
already been matched by the first constraint in the parent.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/
c599253c-1ccd-4161-80fc-
c9065e037a09@gmail.com
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
At update of non-LP_NORMAL TID, fail instead of corrupting page header.
The right mix of DDL and VACUUM could corrupt a catalog page header such
that PageIsVerified() durably fails, requiring a restore from backup.
This affects only catalogs that both have a syscache and have DDL code
that uses syscache tuples to construct updates. One of the test
permutations shows a variant not yet fixed.
This makes !TransactionIdIsValid(TM_FailureData.xmax) possible with
TM_Deleted. I think core and PGXN are indifferent to that.
Per bug #17821 from Alexander Lakhin. Back-patch to v13 (all supported
versions). The test case is v17+, since it uses INJECTION_POINT.
Discussion: https://postgr.es/m/17821-
dd8c334263399284@postgresql.org
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
Merge copies of converting an XID to a FullTransactionId.
Assume twophase.c is the performance-sensitive caller, and preserve its
choice of unlikely() branch hint. Add some retrospective rationale for
that choice. Back-patch to v17, for the next commit to use it.
Reviewed (in earlier versions) by Michael Paquier.
Discussion: https://postgr.es/m/17821-
dd8c334263399284@postgresql.org
Discussion: https://postgr.es/m/
20250116010051.f3.nmisch@google.com
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
Disable runningcheck for src/test/modules/injection_points/specs.
Directory "injection_points" has specified NO_INSTALLCHECK since before
commit
c35f419d6efbdf1a050250d84b687e6705917711 added the specs, but
that commit neglected to disable the corresponding meson runningcheck.
The alternative would be to enable "make installcheck" for ISOLATION,
but the GNU make build system lacks a concept of setting NO_INSTALLCHECK
for REGRESS without also setting it for ISOLATION. Back-patch to v17,
where that commit first appeared, to avoid surprises when back-patching
additional specs.
Discussion: https://postgr.es/m/17821-
dd8c334263399284@postgresql.org
Noah Misch [Sat, 25 Jan 2025 19:28:14 +0000 (11:28 -0800)]
Test ECPG decadd(), decdiv(), decmul(), and decsub() for risnull() input.
Since commit
757fb0e5a9a61ac8d3a67e334faeea6dc0084b3f, these
Informix-compat functions return 0 without changing the output
parameter. Initialize the output parameter before the test call, making
that obvious. Before this, the expected test output has been depending
on freed stack memory. "gcc -ftrivial-auto-var-init=pattern" revealed
that. Back-patch to v13 (all supported versions).
Discussion: https://postgr.es/m/
20250106192748.cf.nmisch@google.com
Tom Lane [Sat, 25 Jan 2025 17:42:05 +0000 (12:42 -0500)]
Doc: recommend "psql -X" for restoring pg_dump scripts.
This practice avoids possible problems caused by non-default psql
options, such as disabling AUTOCOMMIT.
Author: Shinya Kato <Shinya11.Kato@oss.nttdata.com>
Reviewed-by: Robert Treat <rob@xzilla.net>
Discussion: https://postgr.es/m/
96ff23a5d858ff72ca8e823a014d16fe@oss.nttdata.com
Backpatch-through: 13
Andres Freund [Sat, 25 Jan 2025 16:37:13 +0000 (11:37 -0500)]
Change shutdown sequence to terminate checkpointer last
The main motivation for this change is to have a process that can serialize
stats after all other processes have terminated. Serializing stats already
happens in checkpointer, even though walsenders can be active longer.
The only reason the current shutdown sequence does not actively cause problems
is that walsender currently does not generate any stats. However, there is an
upcoming patch changing that.
Another need for this change originates in the AIO patchset, where IO
workers (which, in some edge cases, can emit stats of their own) need to run
while the shutdown checkpoint is being written.
This commit changes the shutdown sequence so checkpointer is signalled (via
SIGINT) to trigger writing the shutdown checkpoint without also causing
checkpointer to exit. Once checkpointer wrote the shutdown checkpoint it
notifies postmaster via PMSIGNAL_XLOG_IS_SHUTDOWN and waits for the
termination signal (SIGUSR2, as before). Checkpointer now is terminated after
all children, other than dead-end children and logger, have been terminated,
tracked using the new PM_WAIT_CHECKPOINTER PMState.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Tom Lane [Sat, 25 Jan 2025 16:24:16 +0000 (11:24 -0500)]
Tighten pg_restore's recognition of its -F (format) option values.
Instead of checking just the first letter, match the whole string
using pg_strcasecmp. Per the documentation, we allow either just
the first letter (e.g. "c") or the whole name ("custom"); but we
will no longer accept random variations such as "chump". This
matches pg_dump's longstanding parsing code for the same option.
Also for consistency with pg_dump, recognize "p"/"plain". We don't
support it, but we can give a more helpful error message than
"unrecognized archive format".
Author: Srinath Reddy <srinath2133@gmail.com>
Discussion: https://postgr.es/m/CAFC+b6pfK-BGcWW1kQmtxVrCh-JGjB2X02rLPQs_ZFaDGjZDsQ@mail.gmail.com
Jeff Davis [Sat, 25 Jan 2025 08:12:30 +0000 (00:12 -0800)]
Fix PDF doc build.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/608525.
1737781222@sss.pgh.pa.us
Tomas Vondra [Fri, 24 Jan 2025 23:36:48 +0000 (00:36 +0100)]
Use the correct sizeof() in BufFileLoadBuffer
The sizeof() call should reference buffer.data, because that's the
buffer we're reading data into, not the whole PGAlignedBuffer union.
This was introduced by
44cac93464, which replaced the simple buffer
with a PGAlignedBuffer field.
It's benign, because the buffer is the largest field of the union, so
the sizes are the same. But it's easy to trip over this in a patch, so
fix and backpatch. Commit
44cac93464 went into 12, but that's EOL.
Backpatch-through: 13
Discussion: https://postgr.es/m/
928bdab1-6567-449f-98c4-
339cd2203b87@vondra.me
Jeff Davis [Fri, 24 Jan 2025 22:56:22 +0000 (14:56 -0800)]
Add SQL function CASEFOLD().
Useful for caseless matching. Similar to LOWER(), but avoids edge-case
problems with using LOWER() for caseless matching.
For collations that support it, CASEFOLD() handles characters with
more than two case variations or multi-character case variations. Some
characters may fold to uppercase. The results of case folding are also
more stable across Unicode versions than LOWER() or UPPER().
Discussion: https://postgr.es/m/
a1886ddfcd8f60cb3e905c93009b646b4cfb74c5.camel%40j-davis.com
Reviewed-by: Ian Lawrence Barwick
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Adjust which processes we expect to have exited
Comments and code stated that we expect checkpointer to have been signalled in
case of immediate shutdown / fatal errors, but didn't treat archiver and
walsenders the same. That doesn't seem right.
I had started digging through the history to see where this oddity was
introduced, but it's not the fault of a single commit.
Instead treat archiver, checkpointer, and walsenders the same.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:08:31 +0000 (17:08 -0500)]
postmaster: Commonalize FatalError paths
This includes some behavioral changes:
- Previously PM_WAIT_XLOG_ARCHIVAL wasn't handled in HandleFatalError(), that
doesn't seem quite right.
- Previously a fatal error in PM_WAIT_XLOG_SHUTDOWN lead to jumping back to
PM_WAIT_BACKENDS, no we go to PM_WAIT_DEAD_END. Jumping backwards doesn't
seem quite right and we didn't do so when checkpointer failed to fork during
a shutdown.
- Previously a checkpointer fork failure didn't call SetQuitSignalReason(),
which would lead to quickdie() reporting
"terminating connection because of unexpected SIGQUIT signal"
which seems even worse than the PMQUIT_FOR_CRASH message. If I saw that in
the log I'd suspect somebody outside of postgres sent SIGQUITs
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Move code to switch into FatalError state into function
There are two places switching to FatalError mode, behaving somewhat
differently. An upcoming commit will introduce a third. That doesn't seem seem
like a good idea.
This commit just moves the FatalError related code from HandleChildCrash()
into its own function, a subsequent commit will evolve the state machine
change to be suitable for other callers.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Don't repeatedly transition to crashing state
Previously HandleChildCrash() skipped logging and signalling child exits if
already in an immediate shutdown or in FatalError state, but still
transitioned server state in response to a crash. That's redundant.
In the other place we transition to FatalError, we do take care to not do so
when already in FatalError state.
To make it easier to combine different paths for entering FatalError state,
only do so once in HandleChildCrash().
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
postmaster: Don't open-code TerminateChildren() in HandleChildCrash()
After removing the duplication no user of sigquit_child() remains, therefore
remove it.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Andres Freund [Fri, 24 Jan 2025 22:00:10 +0000 (17:00 -0500)]
checkpointer: Request checkpoint via latch instead of signal
The motivation for this change is that a future commit will use SIGINT for
another purpose (postmaster requesting WAL access to be shut down) and that
there no other signals that we could readily use (see code comment for the
reason why SIGTERM shouldn't be used). But it's also a tad nicer / more
efficient to use SetLatch(), as it avoids sending signals when checkpointer
already is busy.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/kgng5nrvnlv335evmsuvpnh354rw7qyazl73kdysev2cr2v5zu@m3cfzxicm5kp
Tom Lane [Fri, 24 Jan 2025 18:20:44 +0000 (13:20 -0500)]
Make jsonb casts to scalar types translate JSON null to SQL NULL.
Formerly, these cases threw an error "cannot cast jsonb null to type
<whatever>". That seems less than helpful though. It's also
inconsistent with the behavior of the ->> operator, which translates
JSON null to SQL NULL, as do some other jsonb functions.
Discussion: https://postgr.es/m/
3851203.
1722552717@sss.pgh.pa.us
Peter Eisentraut [Fri, 24 Jan 2025 16:45:29 +0000 (17:45 +0100)]
Fix copy-and-paste typo
Daniel Gustafsson [Fri, 24 Jan 2025 13:25:08 +0000 (14:25 +0100)]
pgcrypto: Make it possible to disable built-in crypto
When using OpenSSL and/or the underlying operating system in FIPS
mode no non-FIPS certified crypto implementations should be used.
While that is already possible by just not invoking the built-in
crypto in pgcrypto, this adds a GUC which prohibit the code from
being called. This doesn't change the FIPS status of PostgreSQL
but can make it easier for sites which target FIPS compliance to
ensure that violations cannot occur.
Author: Daniel Gustafsson <daniel@yesql.se>
Author: Joe Conway <mail@joeconway.com>
Reviewed-by: Joe Conway <mail@joeconway.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/
16b4a157-9ea1-44d0-b7b3-
4c85df5de97b@joeconway.com
Daniel Gustafsson [Fri, 24 Jan 2025 13:18:40 +0000 (14:18 +0100)]
pgcrypto: Add function to check FIPS mode
This adds a SQL callable function for reading and returning the status
of FIPS configuration of OpenSSL. If OpenSSL is operating with FIPS
enabled it will return true, otherwise false. As this adds a function
to the SQL file, bump the extension version to 1.4.
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Joe Conway <mail@joeconway.com>
Discussion: https://postgr.es/m/
8f979145-e206-475a-a31b-
73c977a4134c@joeconway.com
Álvaro Herrera [Fri, 24 Jan 2025 11:54:46 +0000 (12:54 +0100)]
Fix instability in recently added regression tests
We missed the usual ORDER BY clause.
Author: Amul Sul <amul.sul@enterprisedb.com>
Discussion: https://postgr.es/m/CAAJ_b974U3Vvf-qGwFyZ73DFHqyFJP9TOmuiXR2Kp8KVcJtP6w@mail.gmail.com
Peter Eisentraut [Fri, 24 Jan 2025 10:37:20 +0000 (11:37 +0100)]
Convert sepgsql tests to TAP
Add a TAP test for sepgsql. This automates the previously required
manual setup before the test. The actual tests are still run by
pg_regress, as before, but now called from within the TAP Perl script.
The previous manual test script (test_sepgsql) is left in place, since
its purpose is (also) to test whether a running instance was properly
initialized for sepgsql. But it has been changed to call pg_regress
directly and no longer require make.
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://www.postgresql.org/message-id/flat/
651a5baf-5c45-4a5a-a202-
0c8453a4ebf8@eisentraut.org
Peter Eisentraut [Fri, 24 Jan 2025 09:26:12 +0000 (10:26 +0100)]
meson: Fix sepgsql installation
The sepgsql.sql file should be installed under share/contrib/, not
share/extension/, since it is not an extension. This makes it match
what make install does.
Discussion: https://www.postgresql.org/message-id/flat/
651a5baf-5c45-4a5a-a202-
0c8453a4ebf8@eisentraut.org
Michael Paquier [Fri, 24 Jan 2025 06:19:38 +0000 (15:19 +0900)]
initdb: Convert tests to use long options with fat comma style
This is similar to
ce1b0f9da03e, but this time this rule is applied to
some of the TAP tests of initdb.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/878qr146ra.fsf@wibble.ilmari.org
Peter Eisentraut [Fri, 24 Jan 2025 05:55:39 +0000 (06:55 +0100)]
Return yyparse() result not via global variable
Instead of passing the parse result from yyparse() via a global
variable, pass it via a function output argument.
This complements earlier work to make the parsers reentrant.
Discussion: Discussion: https://www.postgresql.org/message-id/flat/
eb6faeac-2a8a-4b69-9189-
c33c520e5b7b@eisentraut.org
Amit Kapila [Fri, 24 Jan 2025 02:55:21 +0000 (08:25 +0530)]
Doc: Fix a typo introduced in
4a0e7314f1.
Author: Erik Rijkers <er@xs4all.nl>
Discussion: https://postgr.es/m/
6e625c81-968e-42d0-802d-
edfaf9cfac11@xs4all.nl
Amit Kapila [Fri, 24 Jan 2025 02:41:29 +0000 (08:11 +0530)]
Doc: Fix column name in pg_publication catalog.
Commit
e65dbc9927 incorrectly spelled the column name in the
pg_publication catalog. In passing make the order of columns in the doc
match the actual catalog.
Author: Shinoda, Noriyoshi <noriyoshi.shinoda@hpe.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/DM4PR84MB1734F8F140E4477580761F93EEE02@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM
Tom Lane [Thu, 23 Jan 2025 19:23:04 +0000 (14:23 -0500)]
Don't ask for bug reports about pthread_is_threaded_np() != 0.
We thought that this condition was unreachable in ExitPostmaster,
but actually it's possible if you have both a misconfigured locale
setting and some other mistake that causes PostmasterMain to bail
out before reaching its own check of pthread_is_threaded_np().
Given the lack of other reports, let's not ask for bug reports if
this occurs; instead just give the same hint as in PostmasterMain.
Bug: #18783
Reported-by: anani191181515@gmail.com
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/18783-
d1873b95a59b9103@postgresql.org
Discussion: https://postgr.es/m/206317.
1737656533@sss.pgh.pa.us
Backpatch-through: 13
Tom Lane [Thu, 23 Jan 2025 17:25:45 +0000 (12:25 -0500)]
Ensure that AFTER triggers run as the instigating user.
With deferred triggers, it is possible that the current role changes
between the time when the trigger is queued and the time it is
executed (for example, the triggering data modification could have
been executed in a SECURITY DEFINER function).
Up to now, deferred trigger functions would run with the current role
set to whatever was active at commit time. That does not matter for
foreign-key constraints, whose correctness doesn't depend on the
current role. But for user-written triggers, the current role
certainly can matter.
Hence, fix things so that AFTER triggers are fired under the role
that was active when they were queued, matching the behavior of
BEFORE triggers which would have actually fired at that time.
(If the trigger function is marked SECURITY DEFINER, that of course
overrides this, as it always has.)
This does not create any new security exposure: if you do DML on a
table owned by a hostile user, that user has always had various ways
to exploit your permissions, such as the aforementioned BEFORE
triggers, default expressions, etc. It might remove some security
exposure, because the old behavior could potentially expose some
other role besides the one directly modifying the table.
There was discussion of making a larger change, such as running as
the trigger's owner. However, that would break the common idiom of
capturing the value of CURRENT_USER in a trigger for auditing/logging
purposes. This change will make no difference in the typical scenario
where the current role doesn't change before commit.
Arguably this is a bug fix, but it seems too big a semantic change
to consider for back-patching.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Joseph Koshakow <koshy44@gmail.com>
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://postgr.es/m/
77ee784cf248e842f74588418f55c2931e47bd78.camel@cybertec.at
Jeff Davis [Thu, 23 Jan 2025 17:06:50 +0000 (09:06 -0800)]
Add support for Unicode case folding.
Expand case mapping tables to include entries for case folding, which
are parsed from CaseFolding.txt.
Discussion: https://postgr.es/m/
a1886ddfcd8f60cb3e905c93009b646b4cfb74c5.camel%40j-davis.com
Tom Lane [Thu, 23 Jan 2025 16:08:05 +0000 (11:08 -0500)]
Reverse the search order in afterTriggerAddEvent().
When scanning existing AfterTriggerSharedData records in search
of a match to the event being queued, we were examining the
records from oldest to newest. But it makes more sense to do
the opposite. The newest record is likely to be from the current
query, while the oldest is likely to be from some previous command
in the same transaction, which will likely have different details.
There aren't expected to be very many active AfterTriggerSharedData
records at once, so that this change is unlikely to make any
spectacular difference. Still, having added a nontrivially-expensive
bms_equal call to this loop yesterday, I feel a need to shave cycles
where possible.
Discussion: https://postgr.es/m/
4166712.
1737583961@sss.pgh.pa.us
Álvaro Herrera [Thu, 23 Jan 2025 14:54:38 +0000 (15:54 +0100)]
Allow NOT VALID foreign key constraints on partitioned tables
This feature was intentionally omitted when FKs were first implemented
for partitioned tables, and had been requested a few times; the
usefulness is clear.
Validation can happen for each partition individually, which is useful
to contain the number of locks held and the duration; or it can be
executed for the partitioning hierarchy as a single command, which
validates all child constraints that haven't been validated already.
This is also useful to implement NOT ENFORCED constraints on top.
Author: Amul Sul <sulamul@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b96Bp=-ZwihPPtuaNX=SrZ0U6ZsXD3+fgARO0JuKa8v2jQ@mail.gmail.com
Amit Kapila [Thu, 23 Jan 2025 12:17:15 +0000 (17:47 +0530)]
Fix buildfarm failure introduced by commit
e65dbc9927.
The patch had incorrectly specified the default value for
publish_generated_columns during the query formation in pg_dump.
Author: Vignesh C <vignesh21@gmail.com>
Discussion: https://postgr.es/m/CAA4eK1KfZYTD8Hpi9TD1KaB8rNUBR9baUvTxa5wYyZDGbEaa6g@mail.gmail.com
Peter Eisentraut [Thu, 23 Jan 2025 11:07:38 +0000 (12:07 +0100)]
Convert macros to static inline functions (htup_details.h, itup.h)
Discussion: https://www.postgresql.org/message-id/flat/
5b558da8-99fb-0a99-83dd-
f72f05388517@enterprisedb.com
Peter Eisentraut [Thu, 23 Jan 2025 11:07:38 +0000 (12:07 +0100)]
Add some const decorations (htup.h)
Discussion: https://www.postgresql.org/message-id/flat/
5b558da8-99fb-0a99-83dd-
f72f05388517@enterprisedb.com
Amit Kapila [Thu, 23 Jan 2025 09:58:37 +0000 (15:28 +0530)]
Change publication's publish_generated_columns option type to enum.
The current boolean publish_generated_columns option only supports a
binary choice, which is insufficient for future enhancements where
generated columns can be of different types (e.g., stored or virtual). The
supported values for the publish_generated_columns option are 'none' and
'stored'.
Author: Vignesh C <vignesh21@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/
d718d219-dd47-4a33-bb97-
56e8fc4da994@eisentraut.org
Discussion: https://postgr.es/m/
B80D17B2-2C8E-4C7D-87F2-
E5B4BE3C069E@gmail.com
Michael Paquier [Thu, 23 Jan 2025 07:03:48 +0000 (16:03 +0900)]
Add error pattern checks for some TAP tests for non-existing objects
Some tests are updated to use command_fails_like(), gaining a check for
the error output generated. The test changed in pg_amcheck has come up
after noticing that an incorrect option name still made the test to
pass, while the command failed. The three other tests changed in
src/bin/scripts/ have been noticed by me, in passing.
Author: Dagfinn Ilmari Mannsåker, Michael Paquier
Discussion: https://postgr.es/m/87bjvy50cs.fsf@wibble.ilmari.org
Michael Paquier [Thu, 23 Jan 2025 06:15:36 +0000 (15:15 +0900)]
Improve TAP tests of pg_basebackup
This addresses some minor issues with the TAP tests of pg_basebackup:
- Remove three duplicated tests used for incorrect option combinations.
- Add more pattern checks for commands doomed to fail, to make sure that
the error generated is the expected one. These are for tests related to
the tablespace mapping and incorrect option combinations.
- Fix the description of one test for the case of backup target versus
format.
Issues noticed while reviewing this area of the tests.
Discussion: https://postgr.es/m/87bjvy50cs.fsf@wibble.ilmari.org
Tom Lane [Wed, 22 Jan 2025 20:18:40 +0000 (15:18 -0500)]
Support RN (roman-numeral format) in to_number().
We've long had roman-numeral output support in to_char(),
but lacked the reverse conversion. Here it is.
Author: Hunaid Sohail <hunaidpgml@gmail.com>
Reviewed-by: Maciek Sakrejda <m.sakrejda@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/CAMWA6ybh4M1VQqpmnu2tfSwO+3gAPeA8YKnMHVADeB=XDEvT_A@mail.gmail.com
Nathan Bossart [Wed, 22 Jan 2025 20:11:37 +0000 (14:11 -0600)]
Fix comment about AVX-512 popcount support.
Since commit
f78667bd91, we've used __attribute__((target(...)))
instead of extra compiler flags for AVX-512 support, but this
comment still says that we put the code in a separate file because
it might require extra compiler flags. Let's just remove that part
of the comment.
Tom Lane [Wed, 22 Jan 2025 16:58:20 +0000 (11:58 -0500)]
Repair incorrect handling of AfterTriggerSharedData.ats_modifiedcols.
This patch fixes two distinct errors that both ultimately trace
to commit
71d60e2aa, which added the ats_modifiedcols field.
The more severe error is that ats_modifiedcols wasn't accounted for
in afterTriggerAddEvent's scanning loop that looks for a pre-existing
duplicate AfterTriggerSharedData. Thus, a new event could be
incorrectly matched to an AfterTriggerSharedData that has a different
value of ats_modifiedcols, resulting in the wrong tg_updatedcols
bitmap getting passed to the trigger whenever it finally gets fired.
We'd not noticed because (a) few triggers consult tg_updatedcols,
and (b) we had no tests exercising a case where such a trigger was
called as an AFTER trigger. In the test case added by this commit,
contrib/lo's trigger fails to remove a large object when expected
because (without this fix) it thinks the LO OID column hasn't changed.
The other problem was introduced by commit
ce5aaea8c, which copied the
modified-columns bitmap into trigger-related storage. It made a copy
for every trigger event, whereas what we really want is to make a new
copy only when we make a new AfterTriggerSharedData entry. (We could
imagine adding extra logic to reduce the number of bitmap copies still
more, but it doesn't look worthwhile at the moment.) In a simple test
of an UPDATE of
10000000 rows with a single AFTER trigger, this thinko
roughly tripled the amount of memory consumed by the pending-triggers
data structures, from
160446744 to
480443440 bytes.
Fixing the first problem requires introducing a bms_equal() call into
afterTriggerAddEvent's scanning loop, which is slightly annoying from
a speed perspective. However, getting rid of the excessive bms_copy()
calls from the second problem balances that out; overall speed of
trigger operations is the same or slightly better, in my tests.
Discussion: https://postgr.es/m/
3496294.
1737501591@sss.pgh.pa.us
Backpatch-through: 13
Amit Kapila [Wed, 22 Jan 2025 09:57:37 +0000 (15:27 +0530)]
Fix \dRp+ output when describing publications with a lower server version.
The psql was not careful that the new column "Generated columns" won't be
present in the lower version. This was introduced in recent commit
7054186c4e.
Author: Vignesh C
Reviewed-by: Peter Smith
Discussion: https://postgr.es/m/CALDaNm3OcXdY0EzDEKAfaK9gq2B67Mfsgxu93+_249ohyts=0g@mail.gmail.com
Peter Eisentraut [Wed, 22 Jan 2025 06:32:21 +0000 (07:32 +0100)]
Additional tests for stored generated columns
Some additional tests have been created during the development of
virtual generated columns (not included here). This commit adds
equivalent tests to the existing test set for stored generated
columns. This includes expanded tests related to MERGE, subqueries,
whole-row references, permissions, domains, partitioning, and
triggers.
Author: Peter Eisentraut <peter@eisentraut.org>
Co-authored-by: jian he <jian.universality@gmail.com>
Co-authored-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
a368248e-69e4-40be-9c07-
6c3b5880b0a6@eisentraut.org
Michael Paquier [Wed, 22 Jan 2025 05:47:13 +0000 (14:47 +0900)]
Improve grammar of options for command arrays in TAP tests
This commit rewrites a good chunk of the command arrays in TAP tests
with a grammar based on the following rules:
- Fat commas are used between option names and their values, making it
clear to both humans and perltidy that values and names are bound
together. This is particularly useful for the readability of multi-line
command arrays, and there are plenty of them in the TAP tests. Most of
the test code is updated to use this style. Some commands used
parenthesis to show the link, or attached values and options in a single
string. These are updated to use fat commas instead.
- Option names are switched to use their long names, making them more
self-documented. Based on a suggestion by Andrew Dunstan.
- Add some trailing commas after the last item in multi-line arrays,
which is a common perl style.
Not all the places are taken care of, but this covers a very good chunk
of them.
Author: Dagfinn Ilmari Mannsåker
Reviewed-by: Michael Paquier, Peter Smith, Euler Taveira
Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org
Amit Kapila [Wed, 22 Jan 2025 05:24:53 +0000 (10:54 +0530)]
Doc: Update the interaction of tablesync with wal_retrieve_retry_interval.
In passing, update the documentation that explains the process of initial
data replication to explicitly state that it uses a table synchronization
worker.
Author: Vignesh C
Reviewed-by: Peter Smith, Shlok Kyal, Amit Kapila
Discussion: https://postgr.es/m/CALDaNm3RxGcD4cDAV5Q0_A4n06F3+AAMpxiyND9Zn0dB86hFmg@mail.gmail.com
Michael Paquier [Wed, 22 Jan 2025 01:13:59 +0000 (10:13 +0900)]
Run perltidy
A follow-up patch will adjust the TAP tests to follow a more-structured
format for option lists in commands, that perltidy is able to cope
better with. Putting the tree first in a clean state makes the next
change a bit easier. v20230309 has been used.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87jzc46d8u.fsf@wibble.ilmari.org
Tom Lane [Tue, 21 Jan 2025 19:43:21 +0000 (14:43 -0500)]
Doc: simplify the tutorial's window-function examples.
For the purposes of this discussion, row_number() is just as good
as rank(), and its behavior is easier to understand and describe.
So let's switch the examples to using row_number().
Along the way to checking the results given in the tutorial,
I found it helpful to extract the empsalary table we use in the
regression tests, which is evidently the same data that was used
to make these results. So I shoved that into advanced.source
to improve the coverage of that file a little. (There's still
several pages of the tutorial that are not included in it,
but at least now 3.5 Window Functions is covered.)
Suggested-by: "David G. Johnston" <david.g.johnston@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
173737973383.1070.
1832752929070067441@wrigleys.postgresql.org
Álvaro Herrera [Tue, 21 Jan 2025 14:24:49 +0000 (15:24 +0100)]
Reword recent error messages: "should" -> "must"
Most were introduced in the 17 timeframe. The ones in wparser_def.c are
very old.
I also changed "JSON path expression for column \"%s\" should return
single item without wrapper" to "JSON path expression for column \"%s\"
must return single item when no wrapper is requested" to avoid
ambiguity.
Backpatch to 17.
Crickets: https://postgr.es/m/
202501131819.26ors7oouafu@alvherre.pgsql
Álvaro Herrera [Tue, 21 Jan 2025 13:53:46 +0000 (14:53 +0100)]
Fix detach of a partition that has a toplevel FK to a partitioned table
In common cases, foreign keys are defined on the toplevel partitioned
table; but if instead one is defined on a partition and references a
partitioned table, and the referencing partition is detached, we would
examine the pg_constraint row on the partition being detached, and fail
to realize that the sub-constraints must be left alone. This causes the
ALTER TABLE DETACH process to fail with
ERROR: could not find ON INSERT check triggers of foreign key constraint NNN
This is similar but not quite the same as what was fixed by
53af9491a043. This bug doesn't affect branches earlier than 15, because
the detach procedure was different there, so we only backpatch down to
15.
Fix by skipping such modifying constraints that are children of other
constraints being detached.
Author: Amul Sul <sulamul@gmail.com>
Diagnosys-by: Sami Imseih <samimseih@gmail.com>
Discussion: https://postgr.es/m/CAAJ_b97GuPh6wQPbxQS-Zpy16Oh+0aMv-w64QcGrLhCOZZ6p+g@mail.gmail.com
Peter Eisentraut [Tue, 21 Jan 2025 13:34:44 +0000 (14:34 +0100)]
Fix NO ACTION temporal foreign keys when the referenced endpoints change
If a referenced UPDATE changes the temporal start/end times, shrinking
the span the row is valid, we get a false return from
ri_Check_Pk_Match(), but overlapping references may still be valid, if
their reference didn't overlap with the removed span.
We need to consider what span(s) are still provided in the referenced
table. Instead of returning that from ri_Check_Pk_Match(), we can
just look it up in the main SQL query.
Reported-by: Sam Gabrielsson <sam@movsom.se>
Author: Paul Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
Peter Eisentraut [Tue, 21 Jan 2025 11:14:49 +0000 (12:14 +0100)]
Improve whitespace in without_overlaps test
Make some indentation better and more consistent. Extracted from
another patch with some actual test changes.
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
Peter Eisentraut [Tue, 21 Jan 2025 07:13:40 +0000 (08:13 +0100)]
Improve generated_stored test
The test table names gtest11s and gtest12s were way originally chosen
to signify "stored", when the idea was to have virtual columns in the
same test file. This is no longer the idea, so this naming is
irrelevant. (The upcoming feature of virtual generated columns will
have a test file that is initially a copy of generated_stored.sql, and
this random difference will be even more annoying then.) Clean this
up by dropping the suffix.
Discussion: https://www.postgresql.org/message-id/flat/
a368248e-69e4-40be-9c07-
6c3b5880b0a6@eisentraut.org
Amit Langote [Tue, 21 Jan 2025 03:53:03 +0000 (12:53 +0900)]
Refactor ExecScan() to allow inlining of its core logic
This commit refactors ExecScan() by moving its tuple-fetching,
filtering, and projection logic into an inline-able function,
ExecScanExtended(), defined in src/include/executor/execScan.h.
ExecScanExtended() accepts parameters for EvalPlanQual state,
qualifiers (ExprState), and projection (ProjectionInfo).
Specialized variants of the execution function of a given Scan node
(for example, ExecSeqScan() for SeqScan) can then pass const-NULL for
unused parameters. This allows the compiler to inline the logic and
eliminate unnecessary branches or checks. Each variant function thus
contains only the necessary code, optimizing execution for scans
where these features are not needed.
The variant function to be used is determined in the ExecInit*()
function of the node and assigned to the ExecProcNode function pointer
in the node's PlanState, effectively turning runtime checks and
conditional branches on the NULLness of epqstate, qual, and projInfo
into static ones, provided the compiler successfully eliminates
unnecessary checks from the inlined code of ExecScanExtended().
Currently, only ExecSeqScan() is modified to take advantage of this
inline-ability. Other Scan nodes might benefit from such specialized
variant functions but that is left as future work.
Benchmarks performed by Junwang Zhao, David Rowley and myself show up
to a 5% reduction in execution time for queries that rely heavily on
Seq Scans. The most significant improvements were observed in
scenarios where EvalPlanQual, qualifiers, and projection were not
required, but other cases also benefit from reduced runtime overhead
due to the inlining and removal of unnecessary code paths.
The idea for this patch first came from Andres Freund in an off-list
discussion. The refactoring approach implemented here is based on a
proposal by David Rowley, significantly improving upon the patch I
(amitlan) initially proposed.
Suggested-by: Andres Freund <andres@anarazel.de>
Co-authored-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Tested-by: Junwang Zhao <zhjwpku@gmail.com>
Tested-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CA+HiwqGaH-otvqW_ce-paL=96JvU4j+Xbuk+14esJNDwefdkOg@mail.gmail.com
Michael Paquier [Tue, 21 Jan 2025 02:30:42 +0000 (11:30 +0900)]
Rework handling of pending data for backend statistics
9aea73fc61d4 has added support for backend statistics, relying on
PgStat_EntryRef->pending for its data pending for flush. This design
lacks in flexibility, because the pending list does some memory
allocation, making it unsuitable if incrementing counters in critical
sections.
Pending data of backend statistics is reworked so the implementation
does not depend on PgStat_EntryRef->pending anymore, relying on a static
area of memory to store the counters that are flushed when stats are
reported to the pgstats dshash. An advantage of this approach is to
allow the pending data to be manipulated in critical sections; some
patches are under discussion and require that.
The pending data is tracked by PendingBackendStats, local to
pgstat_backend.c. Two routines are introduced to allow IO statistics to
update the backend-side counters. have_static_pending_cb and
flush_static_cb are used for the flush, instead of flush_pending_cb.
Author: Bertrand Drouvot, Michael Paquier
Discussion: https://postgr.es/m/66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5
Michael Paquier [Tue, 21 Jan 2025 01:12:39 +0000 (10:12 +0900)]
Rename some pgstats callbacks related to flush of entries
The two callbacks have_fixed_pending_cb and flush_fixed_cb have been
introduced in
fc415edf8ca8 to provide a way for fixed-numbered
statistics to control the flush of their data. These are renamed to
respectively have_static_pending_cb and flush_static_cb. The
restriction that these only apply to fixed-numbered stats is removed.
A follow-up patch will make use of them for backend statistics. This
stats kind is variable-numbered, and patches are under discussion to
track WAL data for IO and backend stats which cannot use
PgStat_EntryRef->pending as pending data would be touched in critical
sections, where no memory allocation can happen.
Per discussion with Andres Freund.
Author: Bertrand Drouvot
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/66efowskppsns35v5u2m7k4sdnl7yoz5bo64tdjwq7r5lhplrz@y7dme5xwh2r5
Tom Lane [Mon, 20 Jan 2025 21:49:15 +0000 (16:49 -0500)]
Update time zone data files to tzdata release 2025a.
DST law changes in Paraguay.
Historical corrections for the Philippines.
Backpatch-through: 13
Tom Lane [Mon, 20 Jan 2025 20:47:53 +0000 (15:47 -0500)]
Avoid using timezone Asia/Manila in regression tests.
The freshly-released 2025a version of tzdata has a refined estimate
for the longitude of Manila, changing their value for LMT in
pre-standardized-timezone days. This changes the output of one of
our test cases. Since we need to be able to run with system tzdata
files that may or may not contain this update, we'd better stop
making that specific test.
I switched it to use Asia/Singapore, which has a roughly similar UTC
offset. That LMT value hasn't changed in tzdb since 2003, so we can
hope that it's well established.
I also noticed that this set of make_timestamptz tests only exercises
zones east of Greenwich, which seems rather sad, and was not the
original intent AFAICS. (We've already changed these tests once
to stabilize their results across tzdata updates, cf
66b737cd9;
it looks like I failed to consider the UTC-offset-sign aspect then.)
To improve that, add a test with Pacific/Honolulu. That LMT offset
is also quite old in tzdb, so we'll cross our fingers that it doesn't
get improved.
Reported-by: Christoph Berg <cb@df7cb.de>
Discussion: https://postgr.es/m/Z46inkznCxesvDEb@msg.df7cb.de
Backpatch-through: 13
Peter Eisentraut [Mon, 20 Jan 2025 14:27:33 +0000 (15:27 +0100)]
Improve generated_stored test
It makes more sense to put the catalog sanity check at the end of the
test rather than at the beginning, so that it can also check whatever
the tests did rather than just whatever happened before the tests.
Suggested-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
a368248e-69e4-40be-9c07-
6c3b5880b0a6@eisentraut.org
Peter Eisentraut [Mon, 20 Jan 2025 12:05:50 +0000 (13:05 +0100)]
Add some more use of Page/PageData rather than char *
Discussion: https://www.postgresql.org/message-id/flat/
692ee0da-49da-4d32-8dca-
da224cc2800e@eisentraut.org
Peter Eisentraut [Mon, 20 Jan 2025 09:53:47 +0000 (10:53 +0100)]
Add const qualifiers to bufpage.h
This makes use of the new PageData type.
PageGetSpecialPointer() had to be turned back into a macro, because it
is used in a way that sometimes it takes const and returns const and
sometimes takes non-const and returns non-const.
Discussion: https://www.postgresql.org/message-id/flat/
692ee0da-49da-4d32-8dca-
da224cc2800e@eisentraut.org
Peter Eisentraut [Mon, 20 Jan 2025 09:53:47 +0000 (10:53 +0100)]
Add PageData C type
This adds the C type PageData and makes the existing type Page a
pointer to it. This follows the usual PostgreSQL C type naming scheme
of Foo/FooData pairs. (Prior to commit
ddbba3aac86, PageData existed
as an unrelated type.) The type definitions are compatible, so this
doesn't change anything except some of the naming.
Discussion: https://www.postgresql.org/message-id/flat/
692ee0da-49da-4d32-8dca-
da224cc2800e@eisentraut.org
Thomas Munro [Mon, 20 Jan 2025 02:17:47 +0000 (15:17 +1300)]
Fix latch event policy that hid socket events.
If a WaitEventSetWait() caller asks for multiple events, an already set
latch would previously prevent other events from being reported at the
same time. Now, we'll also poll the kernel for other events that would
fit in the caller's output buffer with a zero wait time. This policy
change doesn't affect callers that ask for only one event.
The main caller affected is the postmaster. If its latch is set
extremely frequently by backends launching workers and workers exiting,
we don't want it to handle only those jobs and ignore incoming client
connections.
Back-patch to 16 where the postmaster began using the API. The
fast-return policy changed here is older than that, but doesn't cause
any known problems in earlier releases.
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://postgr.es/m/Z1n5UpAiGDmFcMmd%40nathan
Michael Paquier [Mon, 20 Jan 2025 00:29:42 +0000 (09:29 +0900)]
Fix header check for continuation records where standbys could be stuck
XLogPageRead() checks immediately for an invalid WAL record header on a
standby, to be able to handle the case of continuation records that need
to be read across two different sources. As written, the check was too
generic, applying to any target LSN. Based on an analysis by Kyotaro
Horiguchi, what really matters is to make sure that the page header is
checked when attempting to read a LSN at the boundary of a segment, to
handle the case of a continuation record that spawns across multiple
pages when dealing with multiple segments, as WAL receivers are spawned
they request WAL from the beginning of a segment. This fix has been
proposed by Kyotaro Horiguchi.
This could cause standbys to loop infinitely when dealing with a
continuation record during a timeline jump, in the case where the
contents of the record in the follow-up page are invalid.
Some regression tests are added to check such scenarios, able to
reproduce the original problem. In the test, the contents of a
continuation record are overwritten with junk zeros on its follow-up
page, and replayed on standbys. This is inspired by 039_end_of_wal.pl,
and is enough to show how standbys should react on promotion by not
being stuck. Without the fix, the test would fail with a timeout. The
test to reproduce the problem has been written by Alexander Kukushkin.
The original check has been introduced in
066871980183, for a similar
problem.
Author: Kyotaro Horiguchi, Alexander Kukushkin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAFh8B=mozC+e1wGJq0H=0O65goZju+6ab5AU7DEWCSUA2OtwDg@mail.gmail.com
Backpatch-through: 13
Tom Lane [Sun, 19 Jan 2025 19:00:22 +0000 (14:00 -0500)]
Remove PrintBufferDescs() and PrintPinnedBufs().
These have been #ifdef'd out for a long time, and in fact have
been uncompilable since commit
48354581a of 2016-04-10. The
fact that nobody noticed for so long demonstrates their lack of
usefulness, so let's remove them rather than fix them.
Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/CA+COZaB+9CN_f63PPRoVhHjYmCwwmb_9CWLxqCJdMWDqs1a-JA@mail.gmail.com
Andrew Dunstan [Sun, 19 Jan 2025 14:09:58 +0000 (09:09 -0500)]
Be clearer about when jsonapi's need_escapes is needed
Most operations beyond pure json parsing need to set need_escapes to
true to get access to field names and string scalars. Document this
fact more explicitly.
Slightly tweaked patch from:
Author: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CADkLM=c49Vkfg2+A8ubSuEtaGEjuaKZXCA6SrXA8kdwHjx3uxQ@mail.gmail.com