Nathan Bossart [Thu, 13 Mar 2025 16:20:53 +0000 (11:20 -0500)]
Add reverse(bytea).
This commit introduces a function for reversing the order of the
bytes in binary strings.
Bumps catversion.
Author: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://postgr.es/m/CAJ7c6TMe0QVRuNssUArbMi0bJJK32%2BzNA3at5m3osrBQ25MHuw%40mail.gmail.com
Peter Eisentraut [Thu, 13 Mar 2025 14:17:08 +0000 (15:17 +0100)]
Fix copy-and-paste mistake in error message
Introduced in commit
a68159ff2b3.
Peter Eisentraut [Thu, 13 Mar 2025 11:25:14 +0000 (12:25 +0100)]
pg_noreturn to replace pg_attribute_noreturn()
We want to support a "noreturn" decoration on more compilers besides
just GCC-compatible ones, but for that we need to move the decoration
in front of the function declaration instead of either behind it or
wherever, which is the current style afforded by GCC-style attributes.
Also rename the macro to "pg_noreturn" to be similar to the C11
standard "noreturn".
pg_noreturn is now supported on all compilers that support C11 (using
_Noreturn), as well as GCC-compatible ones (using __attribute__, as
before), as well as MSVC (using __declspec). (When PostgreSQL
requires C11, the latter two variants can be dropped.)
Now, all supported compilers effectively support pg_noreturn, so the
extra code for !HAVE_PG_ATTRIBUTE_NORETURN can be dropped.
This also fixes a possible problem if third-party code includes
stdnoreturn.h, because then the current definition of
#define pg_attribute_noreturn() __attribute__((noreturn))
would cause an error.
Note that the C standard does not support a noreturn attribute on
function pointer types. So we have to drop these here. There are
only two instances at this time, so it's not a big loss. In one case,
we can make up for it by adding the pg_noreturn to a wrapper function
and adding a pg_unreachable(), in the other case, the latter was
already done before.
Reviewed-by: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/pxr5b3z7jmkpenssra5zroxi7qzzp6eswuggokw64axmdixpnk@zbwxuq7gbbcw
Richard Guo [Thu, 13 Mar 2025 07:36:03 +0000 (16:36 +0900)]
Fix incorrect handling of subquery pullup
When pulling up a subquery, if the subquery's target list items are
used in grouping set columns, we need to wrap them in PlaceHolderVars.
This ensures that expressions retain their separate identity so that
they will match grouping set columns when appropriate.
In
90947674f, we decided to wrap subquery outputs that are non-var
expressions in PlaceHolderVars. This prevents const-simplification
from merging them into the surrounding expressions after subquery
pullup, which could otherwise lead to failing to match those
subexpressions to grouping set columns, with the effect that they'd
not go to null when expected.
However, that left some loose ends. If the subquery's target list
contains two or more identical Var expressions, we can still fail to
match the Var expression to the expected grouping set expression.
This is not related to const-simplification, but rather to how we
match expressions to lower target items in setrefs.c.
For sort/group expressions, we use ressortgroupref matching, which
works well. For other expressions, we primarily rely on comparing the
expressions to determine if they are the same. Therefore, we need a
way to prevent setrefs.c from matching the expression to some other
identical ones.
To fix, wrap all subquery outputs in PlaceHolderVars if the parent
query uses grouping sets, ensuring that they preserve their separate
identity throughout the whole planning process.
Reported-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4-meSahaanKskpBn0KKxdHAXC1_EJCVWHxEodqirrGJnw@mail.gmail.com
Richard Guo [Thu, 13 Mar 2025 07:34:28 +0000 (16:34 +0900)]
Remove code setting wrap_non_vars to true for UNION ALL subqueries
In pull_up_simple_subquery and pull_up_constant_function, there is
code that sets wrap_non_vars to true when dealing with an appendrel
member. The goal is to wrap subquery outputs that are not simple Vars
in PlaceHolderVars, ensuring that what we pull up doesn't get merged
into a surrounding expression during later processing, which could
cause it to fail to match the expression actually available from the
appendrel.
However, this is unnecessary. When pulling up an appendrel child
subquery, the only part of the upper query that could reference the
appendrel child yet is the translated_vars list of the associated
AppendRelInfo that we just made for this child. Furthermore, we do
not want to force use of PHVs in the AppendRelInfo, as there is no
outer join between. In fact, perform_pullup_replace_vars always sets
wrap_non_vars to false before performing pullup_replace_vars on the
AppendRelInfo.
This patch simply removes the code that sets wrap_non_vars to true for
UNION ALL subqueries.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/CAMbWs4-VXDEi1v+hZYLxpOv0riJxHsCkCH1f46tLnhonEAyGCQ@mail.gmail.com
Jeff Davis [Thu, 13 Mar 2025 04:51:52 +0000 (21:51 -0700)]
Refactor convert_case() to prepare for optimizations.
Upcoming optimizations will add complexity to convert_case(). This
patch reorganizes slightly so that the complexity can be contained
within the logic to convert the case of a single character, rather
than mixing it in with logic to iterate through the string.
Reviewed-by: Alexander Borisov <lex.borisov@gmail.com>
Discussion: https://postgr.es/m/
44005c3d-88f4-4a26-981f-
fd82dfa8e313@gmail.com
Amit Kapila [Thu, 13 Mar 2025 03:33:45 +0000 (09:03 +0530)]
Avoid invalidating all RelationSyncCache entries on publication rename.
On Publication rename, we need to only invalidate the RelationSyncCache
entries corresponding to relations that are part of the publication being
renamed.
As part of this patch, we introduce a new invalidation message to
invalidate the cache maintained by the logical decoding output plugin. We
can't use existing relcache invalidation for this purpose, as that would
unnecessarily cause relcache invalidations in other backends.
This will improve performance by building fewer relation cache entries
during logical replication.
Author: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Author: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Hou Zhijie <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/OSCPR01MB14966C09AA201EFFA706576A7F5C92@OSCPR01MB14966.jpnprd01.prod.outlook.com
Thomas Munro [Thu, 13 Mar 2025 02:43:34 +0000 (15:43 +1300)]
Fix read_stream.c for changing io_combine_limit.
In a couple of places, read_stream.c assumed that io_combine_limit would
be stable during the lifetime of a stream. That is not true in at least
one unusual case: streams held by CURSORs where you could change the GUC
between FETCH commands, with unpredictable results.
Fix, by storing stream->io_combine_limit and referring only to that
after construction. This mirrors the treatment of the other important
setting {effective,maintenance}_io_concurrency, which is stored in
stream->max_ios.
One of the cases was the queue overflow space, which was sized for
io_combine_limit and could be overrun if the GUC was increased. Since
that coding was a little hard to follow, also introduce a variable for
better readability instead of open-coding the arithmetic. Doing so
revealed an off-by-one thinko while clamping max_pinned_buffers to
INT16_MAX, though that wasn't a live bug due to the current limits on
GUC values.
Back-patch to 17.
Discussion: https://postgr.es/m/CA%2BhUKG%2B2T9p-%2BzM6Eeou-RAJjTML6eit1qn26f9twznX59qtCA%40mail.gmail.com
Amit Langote [Thu, 13 Mar 2025 00:56:36 +0000 (09:56 +0900)]
Fix copy-paste error in datum_to_jsonb_internal()
Commit
3c152a27b06 mistakenly repeated JSONTYPE_JSON in a condition,
omitting JSONTYPE_CAST. As a result, datum_to_jsonb_internal() failed
to reject inputs that were casts (e.g., from an enum to json as in the
example below) when used as keys in JSON constructors.
This led to a crash in cases like:
SELECT JSON_OBJECT('happy'::mood: '123'::jsonb);
where 'happy'::mood is implicitly cast to json. The missing check
meant such casted values weren’t properly rejected as invalid
(non-scalar) JSON keys.
Reported-by: Maciek Sakrejda <maciek@pganalyze.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Maciek Sakrejda <maciek@pganalyze.com>
Discussion: https://postgr.es/m/CADXhmgTJtJZK9A3Na_ry+Xrq-ghjcejBRhcRMzWZvbd__QdgJA@mail.gmail.com
Backpatch-through: 17
Masahiko Sawada [Wed, 12 Mar 2025 23:56:04 +0000 (16:56 -0700)]
pg_rewind: Add dbname to primary_conninfo when using --write-recovery-conf.
This commit enhances pg_rewind's --write-recovery-conf option to
include the dbname in the generated primary_conninfo value when
specified in the --source-server option. With this modification, the
rewound server can connect to the primary server without manual
configuration file modifications when sync_replication_slots is
enabled.
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CAD21AoAkW=Ht0k9dVoBTCcqLiiZ2MXhVr+d=j2T_EZMerGrLWQ@mail.gmail.com
David Rowley [Wed, 12 Mar 2025 23:44:26 +0000 (12:44 +1300)]
Add
b955df443 to .git-blame-ignore-revs
David Rowley [Wed, 12 Mar 2025 23:41:44 +0000 (12:41 +1300)]
Fix indentation issue
Introduced recently by
9e088f7dd
Per buildfarm member koel
Masahiko Sawada [Wed, 12 Mar 2025 21:23:56 +0000 (14:23 -0700)]
Fix compiler warning in pg_logicalinspect.
Oversight in
bd65cb3cd48.
Reported-by: David Rowley <dgrowleyml@gmail.com>
Reported-by: Nathan Bossart <nathandbossart@gmail.com>
Author: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAApHDvqrhFfnetbcwgGkJ=z63T8HfQ_OyP=vX8BYiXyxFKt67w@mail.gmail.com
Heikki Linnakangas [Wed, 12 Mar 2025 20:03:39 +0000 (22:03 +0200)]
Rename alloc/free functions in reorderbuffer.c
There used to be bespoken pools for these structs to reduce the
palloc/pfree overhead, but that was ripped out a long time ago and
replaced with the generic, cheaper generational memory allocator
(commit
a4ccc1cef5). The Get/Return terminology made sense with the
pools, as you "got" an object from the pool and "returned" it later,
but now it just looks weird. Rename to Alloc/Free.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/
c9e43d2d-8e83-444f-b111-
430377368989@iki.fi
Nathan Bossart [Wed, 12 Mar 2025 20:01:52 +0000 (15:01 -0500)]
Remove count_one_bits() in acl.c.
The only caller, select_best_grantor(), can instead use
pg_popcount64(). This isn't performance-critical code, but we
might as well use the centralized implementation. While at it, add
some test coverage for this part of select_best_grantor().
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/Z9GtL7Nm6hsYyJnF%40nathan
Melanie Plageman [Wed, 12 Mar 2025 19:56:59 +0000 (15:56 -0400)]
Increase default effective_io_concurrency to 16
The default effective_io_concurrency has been 1 since it was introduced
in
b7b8f0b6096d2ab6e. Referencing the associated discussion [1], it
seems 1 was chosen as a conservative value that seemed unlikely to cause
regressions.
Experimentation on high latency cloud storage as well as fast, local
nvme storage (see Discussion link) shows that even slightly higher
values improve query timings substantially. 1 actually performs worse
than 0 [2]. With effective_io_concurrency 1, we are not prefetching
enough to avoid I/O stalls, but we are issuing extra syscalls.
The new default is 16, which should be more appropriate for common
hardware while still avoiding flooding low IOPs devices with I/O
requests.
[1] https://www.postgresql.org/message-id/flat/
FDDBA24E-FF4D-4654-BA75-
692B3BA71B97%40enterprisedb.com
[2] https://www.postgresql.org/message-id/CAAKRu_Zv08Cic%3DqdCfzrQabpEXGrd9Z9UOW5svEVkCM6%3DFXA9g%40mail.gmail.com
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAAKRu_Z%2BJa-mwXebOoOERMMUMvJeRhzTjad4dSThxG0JLXESxw%40mail.gmail.com
Heikki Linnakangas [Wed, 12 Mar 2025 18:53:09 +0000 (20:53 +0200)]
Handle interrupts while waiting on Append's async subplans
We did not wake up on interrupts while waiting on async events on an
async-capable append node. For example, if you tried to cancel the
query, nothing would happen until one of the async subplans becomes
readable. To fix, add WL_LATCH_SET to the WaitEventSet.
Backpatch down to v14 where async Append execution was introduced.
Discussion: https://www.postgresql.org/message-id/
37a40570-f558-40d3-b5ea-
5c2079b3b30b@iki.fi
Tom Lane [Wed, 12 Mar 2025 15:47:19 +0000 (11:47 -0400)]
Build whole-row Vars the same way during parsing and planning.
makeWholeRowVar() has different rules for constructing a
whole-row Var depending on the kind of RTE it's representing.
This turns out to be problematic because the rewriter and planner
can convert view RTEs and set-returning-function RTEs into
subquery RTEs; so a whole-row Var made during planning might
look different from one made by the parser. In isolation this
doesn't cause any problem, but if a query contains Vars made
both ways for the same varno, there are cross-checks in the
executor that will complain. This manifests for UPDATE, DELETE,
and MERGE queries that use whole-row table references.
To fix, we need makeWholeRowVar() to produce the same result
from an inlined RTE as it would have for the original. For
an inlined view, we can use RangeTblEntry.relid to detect
that this had been a view RTE. For inlined SRFs, make a
data structure definition change akin to commit
47bb9db75,
and say that we won't clear RangeTblEntry.functions until
the end of planning. That allows makeWholeRowVar() to
repeat what it would have done with the unmodified RTE.
Reported-by: Duncan Sands <duncan.sands@deepbluecap.com>
Reported-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Diagnosed-by: Tender Wang <tndrwang@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/
3518c50a-ab18-482f-b916-
a37263622501@deepbluecap.com
Backpatch-through: 13
Melanie Plageman [Wed, 12 Mar 2025 15:33:08 +0000 (11:33 -0400)]
Add connection establishment duration logging
Add log_connections option 'setup_durations' which logs durations of
several key parts of connection establishment and backend setup.
For an incoming connection, starting from when the postmaster gets a
socket from accept() and ending when the forked child backend is first
ready for query, there are multiple steps that could each take longer
than expected due to external factors. This logging provides visibility
into authentication and fork duration as well as the end-to-end
connection establishment and backend initialization time.
To make this portable, the timings captured in the postmaster (socket
creation time, fork initiation time) are passed through the
BackendStartupData.
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Guillaume Lelarge <guillaume.lelarge@dalibo.com>
Discussion: https://postgr.es/m/flat/CAAKRu_b_smAHK0ZjrnL5GRxnAVWujEXQWpLXYzGbmpcZd3nLYw%40mail.gmail.com
Melanie Plageman [Wed, 12 Mar 2025 15:33:01 +0000 (11:33 -0400)]
Modularize log_connections output
Convert the boolean log_connections GUC into a list GUC comprised of the
connection aspects to log.
This gives users more control over the volume and kind of connection
logging.
The current log_connections options are 'receipt', 'authentication', and
'authorization'. The empty string disables all connection logging. 'all'
enables all available connection logging.
For backwards compatibility, the most common values for the
log_connections boolean are still supported (on, off, 1, 0, true, false,
yes, no). Note that previously supported substrings of on, off, true,
false, yes, and no are no longer supported.
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/flat/CAAKRu_b_smAHK0ZjrnL5GRxnAVWujEXQWpLXYzGbmpcZd3nLYw%40mail.gmail.com
Michael Paquier [Wed, 12 Mar 2025 11:37:43 +0000 (20:37 +0900)]
Remove initialization from PendingBackendStats
9a8dd2c5a6d has added an initialization to PendingBackendStats, which
has been causing compilation warnings in the buildfarm. This code does
not strictly require it as PendingBackendStats is always initialized
with memset(0), so let's remove it.
Per report from multiple buildfarm members, like ayu and batfish, via
Tom Lane.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/
1870853.
1741749264@sss.pgh.pa.us
Peter Eisentraut [Wed, 12 Mar 2025 07:49:37 +0000 (08:49 +0100)]
Prepare for Python "Limited API" in PL/Python
Using the Python Limited API would allow building PL/Python against
any Python 3.x version and using another Python 3.x version at run
time. This commit does not activate that, but it prepares the code to
only use APIs supported by the Limited API.
Implementation details:
- Convert static types to heap types
(https://docs.python.org/3/howto/isolating-extensions.html#heap-types).
- Replace PyRun_String() with component functions.
- Replace PyList_SET_ITEM() with PyList_SetItem().
This was previously committed as
c47e8df815c and then reverted because
it wasn't working under Python older than 3.8. That has been fixed in
this version. There was a Python API change/bugfix between 3.7 and
3.8 that directly affects this patch. The relevant commit is
<https://github.com/python/cpython/commit/
364f0b0f19c>. The
workarounds described there have been applied in this patch, and it
has been confirmed to work with Python 3.6 and 3.7.
Reviewed-by: Jakob Egger <jakob@eggerapps.at>
Discussion: https://www.postgresql.org/message-id/flat/
ee410de1-1e0b-4770-b125-
eeefd4726a24@eisentraut.org
Tom Lane [Wed, 12 Mar 2025 03:35:39 +0000 (23:35 -0400)]
Doc: silence A4 PDF build warnings.
Commit
0fbceae84 put a "&zwsp;" in almost but not quite the correct
place to avoid "The contents of fo:block line 1 exceed the available
area" warnings. Per buildfarm.
Heikki Linnakangas [Tue, 11 Mar 2025 21:28:38 +0000 (23:28 +0200)]
Improve snapmgr.c comment
Add more details on the different kinds of snapshots, how to use them,
and how the active snapshot stack works.
Discussion: https://www.postgresql.org/message-id/
7c56f180-b9e1-481e-8c1d-
efa63de3ecbb@iki.fi
Heikki Linnakangas [Tue, 11 Mar 2025 21:20:34 +0000 (23:20 +0200)]
Assert that a snapshot is active or registered before it's used
The comment in GetTransactionSnapshot() said that you "should call
RegisterSnapshot or PushActiveSnapshot on the returned snap if it is
to be used very long". That felt too unclear to me. Make the comment
more strongly worded.
To enforce that rule and to catch potential bugs where a snapshot
might get invalidated while it's still in use, add an assertion to
HeapTupleSatisfiesMVCC() to check that the snapshot is registered or
pushed to active stack. No new bugs were found by this, but it seems
like good future-proofing. It's not a great place for the check;
HeapTupleSatisfiesMVCC() is in fact safe to call with an unregistered
snapshot, and the assertion won't catch other unsafe uses. But it goes
a long way in practice.
Fix a few cases that were playing fast and loose with that and just
assumed that the snapshot cannot be invalidated during a scan. Those
assumptions were not wrong, but they're not performance critical, so
let's drop the excuses and just register the snapshot. These were
false positives found by the new assertion.
Discussion: https://www.postgresql.org/message-id/
7c56f180-b9e1-481e-8c1d-
efa63de3ecbb@iki.fi
Masahiko Sawada [Tue, 11 Mar 2025 16:56:40 +0000 (09:56 -0700)]
pg_logicalinspect: Fix possible crash when passing a directory path.
Previously, pg_logicalinspect functions were too trusting of their
input and blindly passed it to SnapBuildRestoreSnapshot(). If the
input pointed to a directory, the server could a PANIC error while
attempting to fsync_fname() with isdir=false on a directory.
This commit adds validation checks for input filenames and passes the
LSN extracted from the filename to SnapBuildRestoreSnapshot() instead
of the filename itself. It also adds regression tests for various
input patterns and permission checks.
Bug: #18828
Reported-by: Robins Tharakan <tharakan@gmail.com>
Co-authored-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Co-authored-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/18828-
0f4701c635064211@postgresql.org
Masahiko Sawada [Tue, 11 Mar 2025 16:30:00 +0000 (09:30 -0700)]
pg_logicalinspect: Stabilize isolation tests.
The previous isolation tests did not account for the possibility that
the background writer or the checkpointer could write a RUNNING_XACTS
record, which could cause logical decoding to produce more logical
snapshots than expected.
This commit modifies the isolation tests to verify that at least one
logical snapshot contains the expected number of committed or ongoing
catalog-change transactions.
Per buildfarm member skink.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/5qbxud4pvnvmtuoi7weiizm5hmumxaeohx4vztfhrwlfhyz6rj@buh4435mllwo
Tom Lane [Tue, 11 Mar 2025 15:19:54 +0000 (11:19 -0400)]
Improve EXPLAIN's display of window functions.
Up to now we just punted on showing the window definitions used
in a plan, with window function calls represented as "OVER (?)".
To improve that, show the window definition implemented by each
WindowAgg plan node, and reference their window names in OVER.
For nameless window clauses generated by "OVER (...)", assign
unique names w1, w2, etc.
In passing, re-order the properties shown for a WindowAgg node
so that the Run Condition (if any) appears after the Window
property and before the Filter (if any). This seems more
sensible since the Run Condition is associated with the Window
and acts before the Filter.
Thanks to David G. Johnston and Álvaro Herrera for design
suggestions.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/144530.
1741469955@sss.pgh.pa.us
Peter Geoghegan [Tue, 11 Mar 2025 14:35:56 +0000 (10:35 -0400)]
nbtree: Make BTMaxItemSize into object-like macro.
Make nbtree's "1/3 of a page limit" BTMaxItemSize function-like macro
(which accepts a "page" argument) into an object-like macro that can be
used from code that doesn't have convenient access to an nbtree page.
Preparation for an upcoming patch that adds skip scan to nbtree.
Parallel index scans that use skip scan will serialize datums (not just
SAOP array subscripts) when scheduling primitive scans. BTMaxItemSize
will be used by btestimateparallelscan to determine how much DSM to
request.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wz=H_RG5weNGeUG_TkK87tRBnH9mGCQj6WpM4V4FNWKv2g@mail.gmail.com
Peter Geoghegan [Tue, 11 Mar 2025 13:20:50 +0000 (09:20 -0400)]
Show index search count in EXPLAIN ANALYZE, take 2.
Expose the count of index searches/index descents in EXPLAIN ANALYZE's
output for index scan/index-only scan/bitmap index scan nodes. This
information is particularly useful with scans that use ScalarArrayOp
quals, where the number of index searches can be unpredictable due to
implementation details that interact with physical index characteristics
(at least with nbtree SAOP scans, since Postgres 17 commit
5bf748b8).
The information shown also provides useful context when EXPLAIN ANALYZE
runs a plan with an index scan node that successfully applied the skip
scan optimization (set to be added to nbtree by an upcoming patch).
The instrumentation works by teaching all index AMs to increment a new
nsearches counter whenever a new index search begins. The counter is
incremented at exactly the same point that index AMs already increment
the pg_stat_*_indexes.idx_scan counter (we're counting the same event,
but at the scan level rather than the relation level). Parallel queries
have workers copy their local counter struct into shared memory when an
index scan node ends -- even when it isn't a parallel aware scan node.
An earlier version of this patch that only worked with parallel aware
scans became commit
5ead85fb (though that was quickly reverted by commit
d00107cd following "debug_parallel_query=regress" buildfarm failures).
Our approach doesn't match the approach used when tracking other index
scan related costs (e.g., "Rows Removed by Filter:"). It is comparable
to the approach used in similar cases involving costs that are only
readily accessible inside an access method, not from the executor proper
(e.g., "Heap Blocks:" output for a Bitmap Heap Scan, which was recently
enhanced to show per-worker costs by commit
5a1e6df3, using essentially
the same scheme as the one used here). It is necessary for index AMs to
have direct responsibility for maintaining the new counter, since the
counter might need to be incremented multiple times per amgettuple call
(or per amgetbitmap call). But it is also necessary for the executor
proper to manage the shared memory now used to transfer each worker's
counter struct to the leader.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Reviewed-By: Tomas Vondra <tomas@vondra.me>
Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzkRqvaqR2CTNqTZP0z6FuL4-3ED6eQB0yx38XBNj1v-4Q@mail.gmail.com
Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com
Peter Eisentraut [Tue, 11 Mar 2025 12:42:03 +0000 (13:42 +0100)]
Update nls.mk for newly added file
Commit
f18231e8175 moved some code to a new file, but the new file
wasn't added to nls.mk.
Álvaro Herrera [Tue, 11 Mar 2025 11:50:35 +0000 (12:50 +0100)]
BRIN: be more strict about required support procs
With improperly defined operator classes, it's possible to get a
Postgres crash because we'd try to invoke a procedure that doesn't
exist. This is because the code is being a bit too trusting that the
opclass is correctly defined. Add some ereport(ERROR)s for cases where
mandatory support procedures are not defined, transforming the crashes
into errors.
The particular case that was reported is an incomplete opclass in
PostGIS.
Backpatch all the way down to 13.
Reported-by: Tobias Wendorff <tobias.wendorff@tu-dortmund.de>
Diagnosed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/
fb6d9a35-6c8e-4869-af80-
0a4944a793a4@tu-dortmund.de
Daniel Gustafsson [Tue, 11 Mar 2025 11:02:42 +0000 (12:02 +0100)]
Add special case fast-paths for strict functions
Many STRICT function calls will have one or two arguments, in which
case we can speed up checking for NULL input by avoiding setting up
a loop over the arguments. This adds EEOP_FUNCEXPR_STRICT_1 and the
corresponding EEOP_FUNCEXPR_STRICT_2 for functions with one and two
arguments respectively.
Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://postgr.es/m/
415721CE-7D2E-4B74-B5D9-
1950083BA03E@yesql.se
Discussion: https://postgr.es/m/
20191023163849.sosqbfs5yenocez3@alap3.anarazel.de
Daniel Gustafsson [Tue, 11 Mar 2025 11:02:38 +0000 (12:02 +0100)]
Replace EEOP_DONE with special steps for return/no return
Knowing when the side-effects of an expression is the intended result
of the execution, rather than the returnvalue, is important for being
able generate more efficient JITed code. This replaces EEOP_DONE with
two new steps: EEOP_DONE_RETURN and EEOP_DONE_NO_RETURN. Expressions
which return a value should use the former step; expressions used for
their side-effects which don't return value should use the latter.
Author: Andres Freund <andres@anarazel.de>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://postgr.es/m/
415721CE-7D2E-4B74-B5D9-
1950083BA03E@yesql.se
Discussion: https://postgr.es/m/
20191023163849.sosqbfs5yenocez3@alap3.anarazel.de
Peter Eisentraut [Tue, 11 Mar 2025 09:43:48 +0000 (10:43 +0100)]
Move RemoveInheritedConstraint() call slightly earlier
This change is harmless and does not affect the existing intended
operation. It is necessary for a subsequent patch operation (NOT
ENFORCED foreign keys), where we may need to change the child
constraint to enforced. In this case, we would create the necessary
triggers and queue the constraint for validation, so it is important
to remove any unnecessary constraints before proceeding.
This is a small change that could have been included in the previous
"split tryAttachPartitionForeignKey" refactoring patch (commit
1d26c2d2c4b), but was kept separate to highlight the changes.
Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com
Peter Eisentraut [Tue, 11 Mar 2025 08:33:36 +0000 (09:33 +0100)]
refactor: Split tryAttachPartitionForeignKey()
Split tryAttachPartitionForeignKey() into three functions:
AttachPartitionForeignKey(), RemoveInheritedConstraint(), and
DropForeignKeyConstraintTriggers(), so they can be reused in some
subsequent patches for the NOT ENFORCED feature.
Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com
Peter Eisentraut [Tue, 11 Mar 2025 07:40:42 +0000 (08:40 +0100)]
refactor: re-add ATExecAlterChildConstr()
ATExecAlterChildConstr() was removed in commit
80d7f990496, but it is
needed in some subsequent patches for the NOT ENFORCED feature, to
recurse over child constraints. This adds it back in slightly altered
form.
Author: Amul Sul <amul.sul@enterprisedb.com>
Reviewed-by: Alexandra Wang <alexandra.wang.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAAJ_b962c5AcYW9KUt_R_ER5qs3fUGbe4az-SP-vuwPS-w-AGA%40mail.gmail.com
Michael Paquier [Tue, 11 Mar 2025 00:04:11 +0000 (09:04 +0900)]
Add WAL data to backend statistics
This commit adds per-backend WAL statistics, providing the same
information as pg_stat_wal, except that it is now possible to know how
much WAL activity is happening in each backend rather than an overall
aggregate of all the activity. Like pg_stat_wal, the implementation
relies on pgWalUsage, tracking the difference of activity between two
reports to pgstats.
This data can be retrieved with a new system function called
pg_stat_get_backend_wal(), that returns one tuple based on the PID
provided in input. Like pg_stat_get_backend_io(), this is useful when
joined with pg_stat_activity to get a live picture of the WAL generated
for each running backend, showing how the activity is [un]balanced.
pgstat_flush_backend() gains a new flag value, able to control the flush
of the WAL stats.
This commit relies mostly on the infrastructure provided by
9aea73fc61d4, that has introduced backend statistics.
Bump catalog version. A bump of PGSTAT_FILE_FORMAT_ID is not required,
as backend stats do not persist on disk.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/Z3zqc4o09dM/Ezyz@ip-10-97-1-34.eu-west-3.compute.internal
Andres Freund [Mon, 10 Mar 2025 23:11:32 +0000 (19:11 -0400)]
tests: Make postmaster/002_connection_limits deal verbose logs
When log_error_verbosity=verbose is configured the test would hand (and then
fail), because of the sqlstate being added between log level and message. Make
regex cope.
Reported-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/
c7ba6bd0-3701-43d1-9087-
017777fe9cd2%40dunslane.net
Tom Lane [Mon, 10 Mar 2025 21:49:27 +0000 (17:49 -0400)]
CREATE INDEX: do update index stats if autovacuum=off.
This fixes a thinko from commit
d611f8b15. The intent was to prevent
updating the stats of the pre-existing heap if autovacuum is off,
but it also disabled updating the stats of the just-created index.
There is AFAICS no good reason to do the latter, since there could not
be any pre-existing stats to refrain from overwriting, and the zeroed
stats that are there to begin with are very unlikely to be useful.
Moreover, the change broke our cross-version upgrade tests again.
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
1116282.
1741374848@sss.pgh.pa.us
Heikki Linnakangas [Mon, 10 Mar 2025 16:54:58 +0000 (18:54 +0200)]
Fix a few more redundant calls of GetLatestSnapshot()
Commit
2367503177 fixed this in RelationFindReplTupleByIndex(), but I
missed two other similar cases.
Per report from Ranier Vilela.
Discussion: https://www.postgresql.org/message-id/CAEudQArUT1dE45WN87F-Gb7XMy_hW6x1DFd3sqdhhxP-RMDa0Q@mail.gmail.com
Backpatch-through: 13
Heikki Linnakangas [Mon, 10 Mar 2025 15:07:38 +0000 (17:07 +0200)]
Fix snapshot used in logical replication index lookup
The function calls GetLatestSnapshot() to acquire a fresh snapshot,
makes it active, and was meant to pass it to table_tuple_lock(), but
instead called GetLatestSnapshot() again to acquire yet another
snapshot. It was harmless because the heap AM and all other known
table AMs ignore the 'snapshot' argument anyway, but let's be tidy.
In the long run, this perhaps should be redesigned so that snapshot
was not needed in the first place. The table AM API uses TID +
snapshot as the unique identifier for the row version, which is
questionable when the row came from an index scan with a Dirty
snapshot. You might lock a different row version when you use a
different snapshot in the table_tuple_lock() call (a fresh MVCC
snapshot) than in the index scan (DirtySnapshot). However, in the heap
AM and other AMs where the TID alone identifies the row version, it
doesn't matter. So for now, just fix the obvious albeit harmless bug.
This has been wrong ever since the table AM API was introduced in
commit
5db6df0c01, so backpatch to all supported versions.
Discussion: https://www.postgresql.org/message-id/
83d243d6-ad8d-4307-8b51-
2ee5844f6230@iki.fi
Backpatch-through: 13
Tom Lane [Mon, 10 Mar 2025 14:22:08 +0000 (10:22 -0400)]
Doc: improve description of window function processing.
The previous wording talked about a "single pass over the data",
which can be read as promising more than intended (to wit, that only
one WindowAgg plan node will be used). What we promise is only what
the SQL spec requires, namely that the data not get re-sorted between
window functions with compatible PARTITION BY/ORDER BY clauses.
Adjust the wording in hopes of making this clearer.
Reported-by: Christopher Inokuchi <cinokuchi@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CABde6B5va2wMsnM79u_x=n9KUgfKQje_pbLROEBmA9Ru5XWidw@mail.gmail.com
Backpatch-through: 13
Alexander Korotkov [Mon, 10 Mar 2025 11:42:00 +0000 (13:42 +0200)]
Use extended stats for precise estimation of bucket size in hash join
Recognizing the real-life complexity where columns in the table often have
functional dependencies, PostgreSQL's estimation of the number of distinct
values over a set of columns can be underestimated (or much rarely,
overestimated) when dealing with multi-clause JOIN. In the case of hash
join, it can end up with a small number of predicted hash buckets and, as
a result, picking non-optimal merge join.
To improve the situation, we introduce one additional stage of bucket size
estimation - having two or more join clauses estimator lookup for extended
statistics and use it for multicolumn estimation. Clauses are grouped into
lists, each containing expressions referencing the same relation. The result
of the multicolumn estimation made over such a list is combined with others
according to the caller's logic. Clauses that are not estimated are returned
to the caller for further estimation.
Discussion: https://postgr.es/m/
52257607-57f6-850d-399a-
ec33a654457b%40postgrespro.ru
Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Andy Fan <zhihui.fan1213@gmail.com>
Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Reviewed-by: Alena Rybakina <lena.ribackina@yandex.ru>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Alexander Korotkov [Mon, 10 Mar 2025 11:38:39 +0000 (13:38 +0200)]
Teach Append to consider tuple_fraction when accumulating subpaths.
This change is dedicated to more active usage of IndexScan and parameterized
NestLoop paths in partitioned cases under an Append node, as it already works
with plain tables. As newly added regression tests demonstrate, it should
provide more smartness to the partitionwise technique.
With an indication of how many tuples are needed, it may be more meaningful
to use the 'fractional branch' subpaths of the Append path list, which are
more optimal for this specific number of tuples. Planning on a higher level,
if the optimizer needs all the tuples, it will choose non-fractional paths.
In the case when, during execution, Append needs to return fewer tuples than
declared by tuple_fraction, it would not be harmful to use the 'intermediate'
variant of paths. However, it will earn a considerable profit if a sensible
set of tuples is selected.
The change of the existing regression test demonstrates the positive outcome
of this feature: instead of scanning the whole table, the optimizer prefers
to use a parameterized scan, being aware of the only single tuple the join
has to produce to perform the query.
Discussion: https://www.postgresql.org/message-id/flat/CAN-LCVPxnWB39CUBTgOQ9O7Dd8DrA_tpT1EY3LNVnUuvAX1NjA%40mail.gmail.com
Author: Nikita Malakhov <hukutoc@gmail.com>
Author: Andrei Lepikhov <lepihov@gmail.com>
Reviewed-by: Andy Fan <zhihuifan1213@163.com>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Peter Eisentraut [Mon, 10 Mar 2025 10:29:54 +0000 (11:29 +0100)]
Remove support for temporal RESTRICT foreign keys
It isn't clear how these should behave, so let's wait to implement them
until we are sure how to do it.
This feature was initially added by commit
89f908a6d0a, so it hasn't
been released yet.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://postgr.es/m/
e773bc11-4ac1-40de-bb91-
814e02f05b6d%40eisentraut.org
David Rowley [Mon, 10 Mar 2025 00:36:04 +0000 (13:36 +1300)]
Fix incorrect #endif comment
Noticed while reading code in this area.
Heikki Linnakangas [Sun, 9 Mar 2025 18:40:45 +0000 (20:40 +0200)]
Fix incorrect assertion in libpqwalreceiver
Was supposed to check the length of the array, but was checking its
size in bytes.
Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://www.postgresql.org/message-id/CA%2BCOZaA_9afJxj9ZuO73U5P7WXP%2BZM9NGnZvTDCmBFz0FGP%2BwA@mail.gmail.com
Heikki Linnakangas [Sun, 9 Mar 2025 17:47:55 +0000 (19:47 +0200)]
Fix test name and username used in failed connection attempts
The first failed connection tests the "regular" connections limit, not
the reserved limit.
In the second failed connection, the username doesn't really matter,
but since the previous successful connections used "regress_reserved",
it seems weird to switch back to "regress_regular" for the
expected-to-fail attempt.
Discussion: https://www.postgresql.org/message-id/
fd5e9523-78d3-4270-86b2-
fd1b1eeb4fc9@iki.fi
Tom Lane [Sun, 9 Mar 2025 17:11:20 +0000 (13:11 -0400)]
Don't try to parallelize array_agg() on an anonymous record type.
This doesn't work because record_recv requires the typmod that
identifies the specific record type (in our session) and
array_agg_deserialize has no convenient way to get that information.
The result is an "input of anonymous composite types is not
implemented" error.
We could probably make this work if we had to, but it does not seem
worth the trouble, given that it took this long to get a field report.
Just shut off parallelization, as though record_recv didn't exist.
Oversight in commit
16fd03e95. Back-patch to v16 where that
came in.
Reported-by: Kirill Zdornyy <kirill@dineserve.com>
Diagnosed-by: Richard Guo <guofenglinux@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/atLI5Kce2ie1zcYjU0w_kjtVaxiYbYGTihrkLDmGZQnRDD4pnXukIATaABbnIj9pUnelC4ESvCXMm4HAyHg-v61XABaKpERj0A2IXzJZM7g=@dineserve.com
Backpatch-through: 16
Nathan Bossart [Sat, 8 Mar 2025 20:28:16 +0000 (14:28 -0600)]
doc: Adjust note about pg_upgrade's --jobs option.
Presently, this section lists a couple of parallelized parts of
pg_upgrade and suggests a starting point for setting the --jobs
option. The list of parallelized tasks is not particularly
actionable, and the phrasing for the --jobs recommendation is
confusing to some readers.
This commit attempts to improve this section by eliminating the
list of parallelized tasks and instead highlighting that --jobs is
most useful for clusters with multiple databases or tablespaces.
Additionally, the recommendation for setting --jobs is simplified
to suggest starting with the number of CPU cores.
Reported-by: Magnus Hagander <magnus@hagander.net>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://postgr.es/m/Z8dBn_5iGLNuYiPo%40nathan
Jeff Davis [Sat, 8 Mar 2025 19:25:36 +0000 (11:25 -0800)]
Don't convert to and from floats in pg_dump.
Commit
8f427187db improved performance by remembering relation stats
as native types rather than issuing a new query for each relation.
Using native types is fine for integers like relpages; but reltuples
is floating point. The commit controllled for that complexity by using
setlocale(LC_NUMERIC, "C"). After that, Alexander Lakhin found a
problem in pg_strtof(), fixed in
00d61a08c5.
While we aren't aware of any more problems with that approach, it
seems wise to just use a string the whole way for floating point
values, as Corey's original patch did, and get rid of the
setlocale(). Integers are still converted to native types to avoid
wasting memory.
Co-authored-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/
3049348.
1740855411@sss.pgh.pa.us
Discussion: https://postgr.es/m/
560cca3781740bd69881bb07e26eb8f65b09792c.camel%40j-davis.com
Tom Lane [Sat, 8 Mar 2025 16:24:22 +0000 (11:24 -0500)]
Clear errno before calling strtol() in spell.c.
Per POSIX, a caller of strtol() that wishes to check for errors must
set errno to 0 beforehand. Several places in spell.c neglected that,
so that they risked delivering a false overflow error in case errno
had been ERANGE already. Given the lack of field reports, this case
may be unreachable at present --- but it's surely trouble waiting to
happen, so fix it.
Author: Jacob Brazeal <jacob.brazeal@gmail.com>
Discussion: https://postgr.es/m/CA+COZaBhsq6EromFm+knMJfzK6nTpG23zJ+K2=nfUQQXcj_xcQ@mail.gmail.com
Backpatch-through: 13
Peter Geoghegan [Sat, 8 Mar 2025 16:10:14 +0000 (11:10 -0500)]
Make parallel nbtree index scans use an LWLock.
Teach parallel nbtree index scans to use an LWLock (not a spinlock) to
protect the scan's shared descriptor state.
Preparation for an upcoming patch that will add skip scan optimizations
to nbtree. That patch will create the need to occasionally allocate
memory while the scan descriptor is locked, while copying datums that
were serialized by another backend.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com
Peter Eisentraut [Sat, 8 Mar 2025 08:37:06 +0000 (09:37 +0100)]
Make amcanorder independent of amconsistentordering
Follow-up to commit
af4002b381d: Make amconsistentordering not depend
on amcanorder. Although they are related, they are independent
properties.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org
Peter Eisentraut [Sat, 8 Mar 2025 07:06:30 +0000 (08:06 +0100)]
Fix typo
Duplicate assignment in commit
af4002b381d should have been a
different field. (But it didn't affect the outcome.)
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org
Michael Paquier [Sat, 8 Mar 2025 04:39:57 +0000 (13:39 +0900)]
Use stricter ordering in regression test query for pg_stat_io
The query introduced in
8b532771a099 is proving to have ordering issues
under at least the locale cs_CZ. This commit updates the query to use a
stricter ordering.
Per reports from buildfarm members hippopotamus and jay.
Michael Paquier [Sat, 8 Mar 2025 03:22:41 +0000 (12:22 +0900)]
Add regression test listing all the possible tuples in pg_stat_io
pg_stat_io returns a set of tuples based on a combination of three
properties (BackendType, IOObject and IOContext) and
pgstat_tracks_io_object() to decide if a BackendType should return a
tuple based on a pair made of an IOObject and an IOContext.
This commit adds a regression test to track all the combinations
supported. This is useful to know which tuples are relevant when adding
a new BackendType to the set or when touching pgstat_tracks_io_object(),
and I have noticed while playing with this area that it is not
complicated to break it without the regression tests noticing a
difference in some cases.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z8exfAehbVbEKXW5@paquier.xyz
Michael Paquier [Sat, 8 Mar 2025 01:56:30 +0000 (10:56 +0900)]
Improve check for detection of pending data in backend statistics
The callback pgstat_backend_have_pending_cb() is used as a way for
pg_stat_report() to detect if there is any pending data for backend
statistics.
It did not include a check based on pgstat_tracks_backend_bktype(), that
discards processes whose backend types do not support backend
statistics. The logic is not a problem on HEAD, as processes that do
not support backend statistics cannot touch PendingBackendStats, so the
callback would always report that there is no pending data in this case.
However, we would run into trouble once backend statistics include
portions of pending stats that are not always zeroed, like pgWalUsage.
There is no reason for pgstat_backend_have_pending_cb() to not check
for pgstat_tracks_backend_bktype(), anyway, and this pattern is safer in
the long run, so let's update the code to do so.
While on it, this commit adds a proper initialization to
PendingBackendStats.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/Z8l6EMM4ImVoWRkg@ip-10-97-1-34.eu-west-3.compute.internal
Peter Geoghegan [Fri, 7 Mar 2025 23:35:13 +0000 (18:35 -0500)]
nbtree: refine _bt_readnextpage contract comments.
Another minor follow-up commit for commit
1bd4bc85, which changed the
_bt_readnextpage contract.
Nathan Bossart [Fri, 7 Mar 2025 21:23:09 +0000 (15:23 -0600)]
Assert that wrapper_handler()'s argument is within expected range.
pqsignal() already does a similar check, but strange Valgrind
reports have us wondering if wrapper_handler() is somehow getting
called with an invalid signal number.
Reported-by: Tomas Vondra <tomas@vondra.me>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
ace01111-f9ac-4f61-b1b1-
8e9379415444%40vondra.me
Backpatch-through: 17
Tom Lane [Fri, 7 Mar 2025 18:24:09 +0000 (13:24 -0500)]
Include column name in build_attrmap_by_position's error reports.
Formerly we only provided the column number, but it's frequently
more useful to mention the column name. The input tupdesc often
doesn't have useful column names, but the output tupdesc usually
contains user-supplied names, so report that one.
Author: Marcos Pegoraro <marcos@f10.com.br>
Co-authored-by: jian he <jian.universality@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Co-authored-by: Erik Wienhold <ewie@ewie.name>
Reviewed-by: Vladlen Popolitov <v.popolitov@postgrespro.ru>
Discussion: https://postgr.es/m/CAB-JLwanky28gjAMdnMh1CjyO1b2zLdr6UOA1-oY9G7PVL9KKQ@mail.gmail.com
Andres Freund [Fri, 7 Mar 2025 18:09:16 +0000 (13:09 -0500)]
tests: Don't fail due to high default timeout in postmaster/003_start_stop
Some BF animals use very high timeouts due to their slowness. Unfortunately
postmaster/003_start_stop fails if a high timeout is configured, due to
authentication_timeout having a fairly low max.
As this test is reasonably fast, the easiest fix seems to be to cap the
timeout to 600.
Per buildfarm animal skink.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/ggflhkciwdyotpoie323chu2c2idpjk5qimrn462encwx2io7s@thmcxl7i6dpw
Andres Freund [Fri, 7 Mar 2025 18:09:16 +0000 (13:09 -0500)]
tests: Fix race condition in postmaster/002_connection_limits
The test occasionally failed due to unexpected connection limit errors being
encountered after having waited for FATAL errors on another connection. These
spurious failures were caused by the the backend reporting FATAL errors to the
client before detaching from the PGPROC entry. Adding a sleep(1) before
proc_exit() makes it easy to reproduce that problem.
To fix the issue, add a helper function that waits for postmaster to notice
the process having exited. For now this is implemented by waiting for the
DEBUG2 message that postmaster logs in that case. That's not the prettiest
fix, but simple. If we notice this problem elsewhere, it might be worthwhile
to make this more general, e.g. by adding an injection point.
Reported-by: Tomas Vondra <tomas@vondra.me>
Diagnosed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Tested-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/ggflhkciwdyotpoie323chu2c2idpjk5qimrn462encwx2io7s@thmcxl7i6dpw
Robert Haas [Fri, 7 Mar 2025 14:00:53 +0000 (09:00 -0500)]
doc: Add missing decimal places to example rowcount.
Commit
95dbd827f2edc4d10bebd7e840a0bd6782cf69b7 updated a bunch
of similar cases in the documentation, but missed this one.
Author: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Reviewed-by: Fabrízio de Royes Mello <fabriziomello@gmail.com>
Peter Eisentraut [Fri, 7 Mar 2025 10:20:26 +0000 (11:20 +0100)]
Improve possible performance regression
Commit
ce62f2f2a0a introduced calls to GetIndexAmRoutineByAmId() in
lsyscache.c functions. This call is a bit more expensive than a
simple syscache lookup. So rearrange the nesting so that we call that
one last and do the cheaper checks first.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org
Peter Eisentraut [Fri, 7 Mar 2025 09:51:53 +0000 (10:51 +0100)]
Rename amcancrosscompare
After more discussion about commit
ce62f2f2a0a, rename the index AM
property amcancrosscompare to two separate properties
amconsistentequality and amconsistentordering. Also improve the
documentation and update some comments that were previously missed.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/E1tngY6-0000UL-2n%40gemulon.postgresql.org
Dean Rasheed [Fri, 7 Mar 2025 09:31:18 +0000 (09:31 +0000)]
Allow casting between bytea and integer types.
This allows smallint, integer, and bigint values to be cast to and
from bytea. The bytea value is the two's complement representation of
the integer, with the most significant byte first. For example:
1234::bytea -> \x000004d2
(-1234)::bytea -> \xfffffb2e
Author: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-by: Joel Jacobson <joel@compiler.org>
Reviewed-by: Yugo Nagata <nagata@sraoss.co.jp>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/CAJ7c6TPtOp6%2BkFX5QX3fH1SVr7v65uHr-7yEJ%3DGMGQi5uhGtcA%40mail.gmail.com
Jeff Davis [Fri, 7 Mar 2025 03:36:34 +0000 (19:36 -0800)]
CREATE INDEX: don't update table stats if autovacuum=off.
We previously fixed this for binary upgrade in
71b66171d0, but a
similar problem remained when dumping statistics without data.
Fix by not opportunistically updating table stats during CREATE INDEX
when autovacuum is disabled. For stats to be stable at all, the server
needs to be aware that it should not take every opportunity to update
stats. Per discussion, autovacuum=off is a signal that the user
expects stats to be stable; though if necessary, we could create
a more specific mode in the future.
Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://postgr.es/m/CAExHW5vf9D+8-a5_BEX3y=2y_xY9hiCxV1=C+FnxDvfprWvkng@mail.gmail.com
Discussion: https://postgr.es/m/
ca81cbf6e6ea2af838df972801ad4da52640a503.camel%40j-davis.com
John Naylor [Fri, 7 Mar 2025 03:35:21 +0000 (10:35 +0700)]
Revert "vacuumdb: Add option for analyzing only relations missing stats."
This reverts commit
5f8eb25706b62923c53172e453c8a4dedd877a3d, which in
my branch by mistake.
John Naylor [Fri, 7 Mar 2025 03:22:56 +0000 (10:22 +0700)]
Doc: correct aggressive vacuum threshold for multixact members storage
The threshold is two billion members, which was interpreted as 2GB
in the documentation. Fix to reflect that each member takes up five
bytes, which translates to about 10GB. This is not exact, because of
page boundaries. While at it, mention the maximum size 20GB.
This has been wrong since commit
c552e171d16e, so backpatch to
version 14.
Author: Alex Friedman <alexf01@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/CACbFw60UOk6fCC02KsyT3OfU9Dnuq5roYxdw2aFisiN_p1L0bg@mail.gmail.com
Backpatch-through: 14
Nathan Bossart [Tue, 4 Feb 2025 21:07:54 +0000 (15:07 -0600)]
vacuumdb: Add option for analyzing only relations missing stats.
This commit adds a new --missing-only option that can be used in
conjunction with --analyze-only and --analyze-in-stages. When this
option is specified, vacuumdb will generate ANALYZE commands for a
relation if it is missing any statistics it should ordinarily have.
For example, if a table has statistics for one column but not
another, we will analyze the whole table. A similar principle
applies to extended statistics, expression indexes, and table
inheritance.
Co-authored-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: TODO
Discussion: https://postgr.es/m/Z5O1bpcwDrMgyrYy%40nathan
Michael Paquier [Thu, 6 Mar 2025 23:12:45 +0000 (08:12 +0900)]
Fix race condition in TAP test 007_pre_auth
The authentication test added in
c76db55c9085 expects a backend to start
and wait at the injection point "init-pre-auth". A query is used to
retrieve the PID of the backend waiting at authentication, but its WHERE
clause was too soft, checking only for a backend in a "starting" state.
As proved by the CI, this WHERE clause is not enough. There is a small
window between the moment when the backend is reported as "starting" in
its backend entry and the moment when it waits in its injection point,
and it was possible for the test to return the PID of a backend process
not yet waiting in the injection point, causing spurious failures. This
issue is fixed by tweaking the query retrieving the PID of the backend
waiting before authentication so as we check for "init-pre-auth" in its
wait_event. An extra check based on the backend_type is added, based on
a suggestion by Jacob, to be more cautious.
Error spotted by the CI on Windows, but it could happen anywhere, as
long as the authentication path is slow enough compared to the TAP test.
Reported-by: Andres Freund <andres@anarazel.de>
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/soexrl7oeyku24bj3czupxmv27ow35u6edymp5y3oyoysbe2kb@r3tgoos2xp2x
Álvaro Herrera [Thu, 6 Mar 2025 17:14:41 +0000 (18:14 +0100)]
reindexdb: move PQfinish() calls to the right place
get_parallel_object_list() has no business closing a connection it did
not create. Make things more sensible by closing the connection at the
level where it is created, in reindex_one_database().
Extracted from a larger patch by the same author. However, the patch as
submitted not only was not described as containing this change, but in
addition it contained a fatal flaw whereby reindexdb would crash and
fail across all of its TAP test, which is why I list myself as
co-author.
Author: Ranier Vilela <ranier.vf@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CAEudQArfqr0-s0VVPSEh=0kgOgBJvFNdGW=xSL5rBcr0WDMQYQ@mail.gmail.com
Tom Lane [Thu, 6 Mar 2025 16:54:27 +0000 (11:54 -0500)]
Fix some performance issues in GIN query startup.
If a GIN index search had a lot of search keys (for example,
"jsonbcol ?| array[]" with tens of thousands of array elements),
both ginFillScanKey() and startScanKey() took O(N^2) time.
Worse, those loops were uncancelable for lack of CHECK_FOR_INTERRUPTS.
The problem in ginFillScanKey() is the brute-force search key
de-duplication done in ginFillScanEntry(). The most expedient
solution seems to be to just stop trying to de-duplicate once
there are "too many" search keys. We could imagine working harder,
say by using a sort-and-unique algorithm instead of brute force
compare-all-the-keys. But it seems unlikely to be worth the trouble.
There is no correctness issue here, since the code already allowed
duplicate keys if any extra_data is present.
The problem in startScanKey() is the loop that attempts to identify
the first non-required search key. In the submitted test case, that
vainly tests all the key positions, and each iteration takes O(N)
time. One part of that is that it's reinitializing the entryRes[]
array from scratch each time, which is entirely unnecessary given
that the triConsistentFn isn't supposed to scribble on its input.
We can easily adjust the array contents incrementally instead.
The other part of it is that the triConsistentFn may itself take
O(N) time (and does in this test case). This is all extremely
brute force: in simple cases with AND or OR semantics, we could
know without any looping whatever that all or none of the keys
are required. But GIN opclasses don't have any API for exposing
that knowledge, so at least in the short run there is little to
be done about that. Put in a CHECK_FOR_INTERRUPTS so that at
least the loop is cancelable.
These two changes together resolve the primary complaint that
the test query doesn't respond promptly to cancel interrupts.
Also, while they don't completely eliminate the O(N^2) behavior,
they do provide quite a nice speedup for mid-sized examples.
Bug: #18831
Reported-by: Niek <niek.brasa@hitachienergy.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18831-
e845ac44ebc5dd36@postgresql.org
Backpatch-through: 13
Andrew Dunstan [Thu, 6 Mar 2025 15:24:03 +0000 (10:24 -0500)]
Further fix for json_strip_nulls documentation
Oversight in commit
4603903d294.
Author: Shinoda, Noriyoshi (SXD Japan FSI) <noriyoshi.shinoda@hpe.com>
Andrew Dunstan [Thu, 6 Mar 2025 13:46:15 +0000 (08:46 -0500)]
Remove extraneous commas in json{b}_strip_nulls documentation
Oversight in commit
4603903d294.
Author: Ian Lawrence Barwick <barwick@gmail.com>
Amit Kapila [Thu, 6 Mar 2025 08:49:38 +0000 (14:19 +0530)]
Avoid invalidating all RelationSyncCache entries on publication change.
On change of publication via ALTER PUBLICATION ... SET/ADD/DROP commands,
we were invalidating all the relations present in relation sync cache
maintained by pgoutput. We need to invalidate only the relation entries
that are changed as part of publication DDL.
We have ensured that the publication DDL execution generated the
invalidations required to invalidate impacted relation sync entries in
RelationSyncCache.
This improves the performance by avoiding building the cache entries for
the cases where a publication has many tables but only one of them is
dropped.
Author: Shlok Kyal <shlok.kyal.oss@gmail.com>
Author: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Hou Zhijie <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/OSCPR01MB14966C09AA201EFFA706576A7F5C92@OSCPR01MB14966.jpnprd01.prod.outlook.com
Jeff Davis [Thu, 6 Mar 2025 08:19:22 +0000 (00:19 -0800)]
Organize and deduplicate statistics import tests.
Author: Corey Huinker <corey.huinker@gmail.com>
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_bWEqUfxhODfJ-XbZC75vq=P6DYOKK6biyey=yM1Ah3Hg@mail.gmail.com
Discussion: https://postgr.es/m/CADkLM=f1n2_Vomq0gKab7xdxDHmJGgn=DE48P8fzQOp3Mrs1Qg@mail.gmail.com
Jeff Davis [Thu, 6 Mar 2025 08:11:12 +0000 (00:11 -0800)]
Address stats export review comments.
Per discussion, did not use Jian He's patch exactly.
Reported-by: jian he <jian.universality@gmail.com>
Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://postgr.es/m/CACJufxFVq=tq9u1zrHWYSbMi1T07gS9Ff0LJScMco4HZmtZ1xw@mail.gmail.com
Discussion: https://postgr.es/m/CADkLM=f1n2_Vomq0gKab7xdxDHmJGgn=DE48P8fzQOp3Mrs1Qg@mail.gmail.com
Jeff Davis [Thu, 6 Mar 2025 07:07:25 +0000 (23:07 -0800)]
Address stats import review comments.
Reported-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxHG9MBQozbJQ4JRBcRbUO+t+sx4qLZX092rS_9b4SR_EA@mail.gmail.com
Heikki Linnakangas [Thu, 6 Mar 2025 01:10:22 +0000 (03:10 +0200)]
Fix compiler warnings about typedef redefinitions
Clang with -Wtypedef-redefinition produced warnings:
src/include/storage/latch.h:122:3: error: redefinition of typedef 'Latch' is a C11 feature [-Werror,-Wtypedef-redefinition]
Per buildfarm
Michael Paquier [Thu, 6 Mar 2025 00:39:45 +0000 (09:39 +0900)]
Add more monitoring data for WAL writes in the WAL receiver
This commit adds two improvements related to the monitoring of WAL
writes for the WAL receiver.
First, write counts and timings are now counted in pg_stat_io for the
WAL receiver. These have been discarded from pg_stat_wal in
ff99918c625a due to performance concerns, related to the fact that we
still relied on an on-disk file for the stats back then, even with
track_wal_io_timing to avoid the overhead of the timestamp calculations.
This implementation is simpler than the original proposal as it is
possible to rely on the APIs of pgstat_io.c to do the job. Like the
fsync and read data, track_wal_io_timing needs to be enabled to track
the timings.
Second, a wait event is added around the pg_pwrite() call in charge of
the writes, using the exiting WAIT_EVENT_WAL_WRITE. This is useful as
the WAL receiver data is tracked in pg_stat_activity.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z8gFnH4o3jBm5BRz@ip-10-97-1-34.eu-west-3.compute.internal
Heikki Linnakangas [Wed, 5 Mar 2025 23:26:16 +0000 (01:26 +0200)]
Split WaitEventSet functions to separate source file
latch.c now only contains the Latch related functions, which build on
the WaitEventSet abstraction. Most of the platform-dependent stuff is
now in waiteventset.c.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
8a507fb6-df28-49d3-81a5-
ede180d7f0fb@iki.fi
Heikki Linnakangas [Wed, 5 Mar 2025 23:26:12 +0000 (01:26 +0200)]
Use ModifyWaitEvent to update exit_on_postmaster_death
This is in preparation for splitting WaitEventSet related functions to
a separate source file. That will hide the details of WaitEventSet
from WaitLatch, so it must use an exposed function instead of
modifying WaitEventSet->exit_on_postmaster_death directly.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
8a507fb6-df28-49d3-81a5-
ede180d7f0fb@iki.fi
Fujii Masao [Wed, 5 Mar 2025 23:22:30 +0000 (08:22 +0900)]
ecpg: Fix compiler warning in ecpg build with Meson.
Previously, Meson could produce a warning about the use of 'deps' in ecpg:
WARNING: Project targets '>=0.54' but uses a feature introduced in '0.60.0': list.<plus>. The right-hand operand was not a list.
The right-hand operand of 'deps' should be a list. This commit fixes
the warning by wrapping it with square brackets.
This issue was introduced in commit
28f04984f0c.
Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/CAOYmi+ks8wO06Ymxduw2h_eQJ_D4_jHGeyMK0P=p5Q3psnEdMA@mail.gmail.com
Heikki Linnakangas [Wed, 5 Mar 2025 21:46:29 +0000 (23:46 +0200)]
Remove unused ShutdownLatchSupport() function
The only caller was removed in commit
80a8f95b3b. I don't foresee
needing it any time soon, and I'm working on some big changes in this
area, so let's remove it out of the way.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
8a507fb6-df28-49d3-81a5-
ede180d7f0fb@iki.fi
Daniel Gustafsson [Wed, 5 Mar 2025 21:12:20 +0000 (22:12 +0100)]
ci: Remove installation of libcurl
The CI images come with libcurl pre-installed since commit
a119426
in the pg-vm-images repository so remove the installation commands
from the Cirrus tasks. Installation of libcurl packages was added
in the OAuth patchset which introduced the dependency, a backpatch
is thus not applicable.
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
8745B9D8-D897-4302-BD4C-
FC18F291ECB7@yesql.se
Andres Freund [Wed, 5 Mar 2025 18:19:28 +0000 (13:19 -0500)]
ci: Document what makes certain tasks special
To increase coverage without drastically increasing CI resource usage, we have
different CI tasks test different things (e.g. the linux tasks use
sanitizers). Unfortunately that can create confusing situations where CI
fails on some OS, but not others, without the problem appearing to be platform
dependent.
To, partially, address that, add a comment, prefixed with SPECIAL, to each
task that we use to test in some non-default way.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/321570.
1741195755@sss.pgh.pa.us
Andres Freund [Wed, 5 Mar 2025 18:19:28 +0000 (13:19 -0500)]
ci: freebsd: Specify debug_parallel_query=regress
A lot of buildfarm animals run with debug_parallel_query=regress, while CI
didn't test that. That lead to the annoying situation of only noticing related
test instabilities after merging changes upstream.
FreeBSD was chosen because it's a relatively fast task. It also tests
debug_write_read_parse_plan_trees etc, which probably is exercised a bit more
heavily with debug_parallel_query=regress.
Discussion: https://postgr.es/m/zbuk4mlov22yfoktf5ub3lwjw2b7ezwphwolbplthepda42int@h6wpvq7orc44
Andres Freund [Wed, 5 Mar 2025 15:33:47 +0000 (10:33 -0500)]
ci: Upgrade FreeBSD image
Upgrade to the current stable version. To avoid needing commits like this in
the future, the CI image name now doesn't contain the OS version number
anymore.
Backpatch to all versions with CI support, we don't want to generate CI images
for multiple FreeBSD versions.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ3_P4JJ6tWZafjf-_XbHgG6DQGXhH-y6Yp78_bwBJjcww@mail.gmail.com
Backpatch-through: 15
Peter Geoghegan [Wed, 5 Mar 2025 15:27:31 +0000 (10:27 -0500)]
Revert "Show index search count in EXPLAIN ANALYZE."
This reverts commit
5ead85fbc81162ab1594f656b036a22e814f96b3.
This commit shows test failures with debug_parallel_query=regress. The
underlying issue needs to be debugged, so revert for now.
Andrew Dunstan [Wed, 5 Mar 2025 14:50:34 +0000 (09:50 -0500)]
Allow json{b}_strip_nulls to remove null array elements
An additional paramater ("strip_in_arrays") is added to these functions.
It defaults to false. If true, then null array elements are removed as
well as null valued object fields. JSON that just consists of a single
null is not affected.
Author: Florents Tselai <florents.tselai@gmail.com>
Discussion: https://postgr.es/m/
4BCECCD5-4F40-4313-9E98-
9E16BEB0B01D@gmail.com
Peter Geoghegan [Wed, 5 Mar 2025 14:36:48 +0000 (09:36 -0500)]
Show index search count in EXPLAIN ANALYZE.
Expose the count of index searches/index descents in EXPLAIN ANALYZE's
output for index scan nodes. This information is particularly useful
with scans that use ScalarArrayOp quals, where the number of index scans
isn't predictable in advance (at least not with optimizations like the
one added to nbtree by Postgres 17 commit
5bf748b8). It will also be
useful when EXPLAIN ANALYZE shows details of an nbtree index scan that
uses skip scan optimizations set to be introduced by an upcoming patch.
The instrumentation works by teaching index AMs to increment a new
nsearches counter whenever a new index search begins. The counter is
incremented at exactly the same point that index AMs must already
increment the index's pg_stat_*_indexes.idx_scan counter (we're counting
the same event, but at the scan level rather than the relation level).
The new counter is stored in the scan descriptor (IndexScanDescData),
which explain.c reaches by going through the scan node's PlanState.
This approach doesn't match the approach used when tracking other index
scan specific costs (e.g., "Rows Removed by Filter:"). It is similar to
the approach used in other cases where we must track costs that are only
readily accessible inside an access method, and not from the executor
(e.g., "Heap Blocks:" output for a Bitmap Heap Scan). It is inherently
necessary to maintain a counter that can be incremented multiple times
during a single amgettuple call (or amgetbitmap call), and directly
exposing PlanState.instrument to index access methods seems unappealing.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Tomas Vondra <tomas@vondra.me>
Reviewed-By: Robert Haas <robertmhaas@gmail.com>
Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=PKR6rB7qbx+Vnd7eqeB5VTcrW=iJvAsTsKbdG+kW_UA@mail.gmail.com
Discussion: https://postgr.es/m/CAH2-WzkRqvaqR2CTNqTZP0z6FuL4-3ED6eQB0yx38XBNj1v-4Q@mail.gmail.com
Heikki Linnakangas [Wed, 5 Mar 2025 14:22:26 +0000 (16:22 +0200)]
Rename some signal and interrupt handling functions for consistency
The usual pattern for handling a signal is that the signal handler
sets a flag and calls SetLatch(MyLatch), and CHECK_FOR_INTERRUPTS() or
other code that is part of a wait loop calls another function to deal
with it. The naming of the functions involved was a bit inconsistent,
however. CHECK_FOR_INTERRUPTS() calls ProcessInterrupts() to do the
heavy-lifting, but the analogous functions in aux processes were
called HandleMainLoopInterrupts(), HandleStartupProcInterrupts(),
etc. Similarly, most subroutines of ProcessInterrupts() were called
Process*(), but some were called Handle*().
To make things less confusing, rename all the functions that are part
of the overall signal/interrupt handling system but are not executed
in a signal handler to e.g. ProcessSomething(), rather than
HandleSomething(). The "Process" prefix is now consistently used in
the non-signal-handler functions, and the "Handle" prefix in functions
that are part of signal handlers, except for some completely unrelated
functions that clearly have nothing to do with signal or interrupt
handling.
Reviewed-by: Nathan Bossart <nathandbossart@gmail.com>
Discussion: https://www.postgresql.org/message-id/
8a384b26-1499-41f6-be33-
64b801fb98b8@iki.fi
Álvaro Herrera [Wed, 5 Mar 2025 12:50:22 +0000 (13:50 +0100)]
Add ALTER TABLE ... ALTER CONSTRAINT ... SET [NO] INHERIT
This allows to redefine an existing non-inheritable constraint to be
inheritable, which allows to straighten up situations with NO INHERIT
constraints so that thay can become normal constraints without having to
re-verify existing data. For existing inheritance children this may
require creating additional constraints, if they don't exist already.
It also allows to do the opposite, if only for symmetry.
Author: Suraj Kharage <suraj.kharage@enterprisedb.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CAF1DzPVfOW6Kk=7SSh7LbneQDJWh=PbJrEC_Wkzc24tHOyQWGg@mail.gmail.com
Michael Paquier [Wed, 5 Mar 2025 01:17:39 +0000 (10:17 +0900)]
Fix some gaps in pg_stat_io with WAL receiver and WAL summarizer
The WAL receiver and WAL summarizer processes gain each one a call to
pgstat_report_wal(), to make sure that they report their WAL statistics
to pgstats, gathering data for pg_stat_io.
In the WAL receiver, the stats reports are timed with status updates sent
to the primary, that depend on wal_receiver_status_interval and
wal_receiver_timeout. This is a conservative choice, but perhaps we
could be more aggressive with the frequency of the stats reports. An
interesting historical fact is that the WAL receiver does writes and
syncs of WAL, but it has never reported its statistics to pgstats in
pg_stat_wal.
In the WAL summarizer, the stats reports are done each time the process
waits for WAL.
While on it, pg_stat_io is adjusted so as these two processes do not
report any rows when IOObject is not WAL, making the view easier to use
with less rows.
Two tests are added in TAP, checking statistics for the WAL summarizer
and the WAL receiver. Status updates in the WAL receiver are currently
possible in the recovery test 001_stream_rep.pl.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/Z8UKZyVSHUUQJHNb@paquier.xyz
Michael Paquier [Tue, 4 Mar 2025 22:56:03 +0000 (07:56 +0900)]
psql: Fix memory leak with \gx used within a pipeline
While inside a pipeline, \gx is currently forbidden and will make
exec_command_g() exit early. There was a memory leak in this code path,
so let's fix it.
Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Discussion: https://postgr.es/m/CAO6_XqqFVQjLjZQiL7xdwLpzZEy1ghO_JWvCFPM_OmwF9s7XdA@mail.gmail.com
Tomas Vondra [Tue, 4 Mar 2025 19:37:55 +0000 (20:37 +0100)]
Enforce memory limit during parallel GIN builds
Index builds are expected to respect maintenance_work_mem, just like
other maintenance operations. For serial builds this is done simply by
flushing the buffer in ginBuildCallback() into the index. But with
parallel builds it's more complicated, because there are multiple places
that can allocate memory.
ginBuildCallbackParallel() does the same thing as ginBuildCallback(),
except that the accumulated items are written into tuplesort. Then the
entries with the same key get merged - first in the worker, then in the
leader - and the TID lists may get (arbitrarily) long. It's unlikely it
would exceed the memory limit, but it's possible. We address this by
evicting some of the data if the list gets too long.
We can't simply dump the whole in-memory TID list. The GIN index bulk
insert code expects to see TIDs in monotonic order; it may fail if the
TIDs go backwards. If the TID lists overlap, evicting the whole current
TID list would break this (a later entry might add "old" TID values into
the already-written part).
In the workers this is not an issue, because the lists never overlap.
But the leader may see overlapping lists produced by the workers.
We can however derive a safe "horizon" TID - the entries (for a given
key) are sorted by (key, first TID), which means no future list can add
values before the last "first TID" we've seen. This patch tracks the
"frozen" part of the TID list, which we know can't change by merging
additional TID lists. If needed, we can evict this part of the list.
We don't want to do this too often - the smaller lists we evict, the
more expensive it'll be to merge them in the next step (especially in
the leader). Therefore we only trim the list if we have at least 1024
frozen items, and if the whole list is at least 64kB large.
These thresholds are somewhat arbitrary and conservative. We might
calculate the values from maintenance_work_mem, but tests show that does
not really improve anything (time, compression ratio, ...). So we stick
to these conservative values to release memory faster.
Author: Tomas Vondra
Reviewed-by: Matthias van de Meent, Andy Fan, Kirill Reshke
Discussion: https://postgr.es/m/
6ab4003f-a8b8-4d75-a67f-
f25ad98582dc%40enterprisedb.com
Masahiko Sawada [Tue, 4 Mar 2025 19:16:12 +0000 (11:16 -0800)]
pg_upgrade: Check for the expected error message in TAP tests.
Since pg_upgrade prints its error messages on stdout, we can't use
command_fails_like() to check if it fails for the right reason. This
commit uses command_checks_all() in pg_upgrade TAP tests to check the
exit status and stdout, enabling proper verification of error
reasons.
Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Discussion: https://postgr.es/m/87tt8h1vb7.fsf@wibble.ilmari.org