David Rowley [Tue, 10 Oct 2023 01:16:54 +0000 (14:16 +1300)]
Revert "Optimize various aggregate deserialization functions"
This reverts commit
608fd198def5390c3490bfe903730207dfd8eeb4.
On 2nd thoughts, the StringInfo API requires that strings are NUL
terminated and pointing directly to the data in a bytea Datum isn't NUL
terminated.
Discussion: https://postgr.es/m/CAApHDvorfO3iBZ=xpiZvp3uHtJVLyFaPBSvcAhAq2HPLnaNSwQ@mail.gmail.com
Michael Paquier [Tue, 10 Oct 2023 00:04:28 +0000 (09:04 +0900)]
worker_spi: Fix another stability issue with BGWORKER_BYPASS_ALLOWCONN
worker_spi_launch() may report that a worker stopped when it fails to
connect on a database that does not allow connections if the worker
exits before the SQL function checks for the current status of the
worker. The test is switched to use Cluster::psql instead of
safe_psql() so as it does not fail hard when this query errors. While
on it, this removes a query that looks at pg_stat_activity to simplify
the test, as a check on the contents of the server logs achieves the
same when the worker cannot connect to the database without
datallowconn.
Per buildfarm members kestrel, mamba and serinus. Bonus thanks to Tom
Lane for providing the logs of the failure from mamba that the buildfarm
was not able to show up. Note that I have reproduced the failure with a
hardcoded stop point.
Discussion: https://postgr.es/m/
3365937.
1696801735@sss.pgh.pa.us
Tom Lane [Mon, 9 Oct 2023 15:29:21 +0000 (11:29 -0400)]
Doc: use CURRENT_USER not USER in plpgsql trigger examples.
While these two built-in functions do exactly the same thing,
CURRENT_USER seems preferable to use in documentation examples.
It's easier to look up if the reader is unsure what it is.
Also, this puts these examples in sync with an adjacent example
that already used CURRENT_USER.
Per question from Kirk Parker.
Discussion: https://postgr.es/m/CANwZ8rmN_Eb0h0hoMRS8Feftaik0z89PxVsKg+cP+PctuOq=Qg@mail.gmail.com
Heikki Linnakangas [Mon, 9 Oct 2023 08:52:09 +0000 (11:52 +0300)]
Rename StartBackgroundWorker() to BackgroundWorkerMain().
The comment claimed that it is "called from postmaster", but it is
actually called in the child process, pretty early in the process
initialization. I guess you could interpret "called from postmaster"
to mean that, but it seems wrong to me. Rename the function to be
consistent with other functions with similar role.
Reviewed-by: Thomas Munro
Discussion: https://www.postgresql.org/message-id/
4f95c1fc-ad3c-7974-3a8c-
6faa3931804c@iki.fi
Heikki Linnakangas [Mon, 9 Oct 2023 08:23:50 +0000 (11:23 +0300)]
Allocate Backend structs in PostmasterContext.
The child processes don't need them. By allocating them in
PostmasterContext, the memory gets free'd and is made available for
other stuff in the child processes.
Reviewed-by: Thomas Munro
Discussion: https://www.postgresql.org/message-id/
4f95c1fc-ad3c-7974-3a8c-
6faa3931804c@iki.fi
Heikki Linnakangas [Mon, 9 Oct 2023 08:23:47 +0000 (11:23 +0300)]
Clarify the checks in RegisterBackgroundWorker.
In EXEC_BACKEND or single-user mode, we process
shared_preload_libraries at postmaster startup as usual, but also at
backend startup. When a library calls RegisterBackgroundWorker() when
being loaded into a backend process, we go through the motions to add
the worker to BackgroundWorkerList, even though that is a
postmaster-private data structure. Make it return early when called in
a backend process, without changing BackgroundWorkerList.
You could argue that it was intentional: In non-EXEC_BACKEND mode, the
backend processes inherit BackgroundWorkerList at fork(), so it does
make some sense to initialize it to the same state in EXEC_BACKEND
mode, too. It's clearly a postmaster-private structure, though, and
all the functions that use it are clearly marked as "should only be
called in postmaster".
You could also argue that libraries should not call
RegisterBackgroundWorker() during backend startup. It's too late to
correctly register any static background workers at that stage. But
it's a common pattern in extensions, and it doesn't seem worth the
churn to require all extensions to change it.
Another sloppiness was the exception for "internal" background
workers. We checked that RegisterBackgroundWorker() was called during
shared_preload_libraries processing, or the background worker was an
internal one. That exception was made in commit
665d1fad99 to allow
postmaster to register the logical apply launcher in
ApplyLauncherRegister(). The way the check was written, it would not
complain if you registered an internal background worker in a regular
backend process. But it would complain if postmaster registered a
background worker defined in a shared library, outside
shared_preload_libraries processing. I think the correct rule is that
you can only register static background workers in the postmaster
process, and only before the bgworker shared memory array has been
initialized. Check for that more directly.
Reviewed-by: Thomas Munro
Discussion: https://www.postgresql.org/message-id/
4f95c1fc-ad3c-7974-3a8c-
6faa3931804c@iki.fi
David Rowley [Mon, 9 Oct 2023 04:25:16 +0000 (17:25 +1300)]
Optimize various aggregate deserialization functions
The serialized representation of an internal aggregate state is a bytea
value. In each deserial function, in order to "receive" the bytea value
we appended it onto a short-lived StringInfoData using
appendBinaryStringInfo. This was a little wasteful as it meant having to
palloc memory, copy a (possibly long) series of bytes then later pfree
that memory. Instead of going to this extra trouble, we can just fake up
a StringInfoData and point the data directly at the bytea's payload. This
should help increase the performance of internal aggregate
deserialization.
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/CAApHDvr=e-YOigriSHHm324a40HPqcUhSp6pWWgjz5WwegR=cQ@mail.gmail.com
Amit Kapila [Mon, 9 Oct 2023 03:48:47 +0000 (09:18 +0530)]
Remove duplicate words in docs and code comments.
Additionally, add a missing "the" in a couple of places.
Author: Vignesh C, Dagfinn Ilmari Mannsåker
Discussion: http://postgr.es/m/CALDaNm28t+wWyPfuyqEaARS810Je=dRFkaPertaLAEJYY2cWYQ@mail.gmail.com
David Rowley [Mon, 9 Oct 2023 03:37:05 +0000 (16:37 +1300)]
Strip off ORDER BY/DISTINCT aggregate pathkeys in create_agg_path
1349d2790 added code to adjust the PlannerInfo.group_pathkeys so that
ORDER BY / DISTINCT aggregate functions could obtain pre-sorted inputs
to allow faster execution. That commit forgot to adjust the pathkeys in
create_agg_path(). Some code in there assumed that it was always fine
to make the AggPath's pathkeys the same as its subpath's. That seems to
have been ok up until
1349d2790, but since that commit adds pathkeys for
columns which are within the aggregate function, those columns won't be
available above the aggregate node. This can result in "could not find
pathkey item to sort" during create_plan().
The fix here is to strip off the additional pathkeys added by
adjust_group_pathkeys_for_groupagg(). It seems that the pathkeys here
will only ever be group_pathkeys, so all we need to do is check if the
length of the pathkey list is longer than the num_groupby_pathkeys and
get rid of the additional ones only if we see extras.
Reported-by: Justin Pryzby
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/ZQhYYRhUxpW3PSf9%40telsasoft.com
Backpatch-through: 16, where
1349d2790 was introduced
David Rowley [Mon, 9 Oct 2023 02:53:16 +0000 (15:53 +1300)]
Remove debug_print_rel and replace usages with pprint
Going by
c4a1933b4,
b33ef397a and
05893712c (to name just a few), it seems
that maintaining debug_print_rel() is often forgotten. In the case of
c4a1933b4, it was several years before anyone noticed that a path type
was not handled by debug_print_rel(). (debug_print_rel() is only
compiled when building with OPTIMIZER_DEBUG).
After a quick survey on the pgsql-hackers mailing list, nobody came
forward to admit that they use OPTIMIZER_DEBUG. So to prevent any future
maintenance neglect, let's just remove debug_print_rel() and have
OPTIMIZER_DEBUG make use of pprint() instead (as suggested by Tom Lane).
If anyone wants to come forward to claim they make use of
OPTIMIZER_DEBUG in a way that they need debug_print_rel() then they have
around 10 months remaining in the v17 cycle where we could revert this.
If nobody comes forward in that time, then we can likely safely declare
debug_print_rel() as not worth keeping.
Discussion: https://postgr.es/m/CAApHDvoCdjo8Cu2zEZF4-AxWG-90S+pYXAnoDDa9J3xH-OrczQ@mail.gmail.com
Alexander Korotkov [Sat, 7 Oct 2023 17:36:47 +0000 (20:36 +0300)]
Fix another typo in
e0b1ee17dc
Reported-by: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4_kHMJDak75y1kBTirv-drS1-knT-7Mpg5LprAjqRJDVA%40mail.gmail.com
Tom Lane [Sat, 7 Oct 2023 16:08:10 +0000 (12:08 -0400)]
Restore proper linkage of pg_char_to_encoding() and friends.
Back in the 8.3 era we discovered that it was problematic if
libpq.so had encoding ID assignments different from the backend,
which is possible because on some platforms libpq.so might be
of a different major version from the calling programs.
psql should use libpq's assignments, but initdb has to use the
backend's, else it will put wrong values into pg_database.
The solution devised in commit
8468146b0 relied on giving initdb
its own copy of encnames.c rather than relying on the functions
exported by libpq. Later, that metamorphosed into ensuring that
libpgcommon got linked before libpq -- which made things OK for
initdb but broke psql. We didn't notice for lack of any changes
in enum pg_enc since then. Commit
06843df4a reversed that, fixing
the latent bug in psql but adding one in initdb. The meson build
infrastructure is also not being sufficiently careful about link
order, and trying to make it so would be equally fragile.
Hence, let's use a new scheme based on giving the libpq-exported
symbols different real names than the same functions exported from
libpgcommon.a or libpgcommon_srv.a. (We could distinguish those
two cases as well, but there seems no need to.) libpq gets the
official names to avoid an ABI break for libpq clients, while the
other cases use #define's to make the real names "xxx_private"
rather than "xxx". By controlling where the #define's are
applied, we can force any particular client program to use one
set or the other of the encnames.c functions.
We cannot back-patch this, since it'd be an ABI break for backend
loadable modules, but there seems little need to. We're just
trying to ensure that the world is safe for hypothetical future
additions to enum pg_enc.
In passing this should fix "duplicate symbol" linker warnings
that we've been seeing on AIX buildfarm members since commit
06843df4a. It's not very clear why that linker is complaining
now, when there were strictly *more* duplicates visible before,
but in any case this should remove the reason for complaint.
Patch by me; thanks to Andres Freund for review.
Discussion: https://postgr.es/m/
2385119.
1696354473@sss.pgh.pa.us
Alexander Korotkov [Sat, 7 Oct 2023 08:55:17 +0000 (11:55 +0300)]
Fix typos in
e0b1ee17dc
Reported-by: Alexander Lakhin
Peter Eisentraut [Fri, 6 Oct 2023 09:52:05 +0000 (11:52 +0200)]
Add test for checking the line length of --help output
There was some discussion what the line length should be. Most output
currently clearly targets around 80 columns, but the maximum in use
currently is 95, so we set that as the current maximum. This just
ensures that there is some guidance and there are no wild deviations.
based on patch by Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Discussion: https://www.postgresql.org/message-id/flat/
50ca8ff35a8dd8f9ec89963b503571a7@oss.nttdata.com
Peter Eisentraut [Fri, 6 Oct 2023 08:55:10 +0000 (10:55 +0200)]
Remove environment-variable-based defaults in psql --help
This seemed inconsistent with the --help output of other tools.
Depending on the values, it can cause ugly formatting. Also, we're
not getting the defaults from libpq, we're just emulating the methods
libpq uses to derive these values, so they might not be 100% correct.
Author: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Discussion: https://www.postgresql.org/message-id/flat/
50ca8ff35a8dd8f9ec89963b503571a7@oss.nttdata.com
Etsuro Fujita [Fri, 6 Oct 2023 09:30:00 +0000 (18:30 +0900)]
Remove extra parenthesis from comment.
Alexander Korotkov [Fri, 6 Oct 2023 07:40:51 +0000 (10:40 +0300)]
Skip checking of scan keys required for directional scan in B-tree
Currently, B-tree code matches every scan key to every item on the page.
Imagine the ordered B-tree scan for the query like this.
SELECT * FROM tbl WHERE col > 'a' AND col < 'b' ORDER BY col;
The (col > 'a') scan key will be always matched once we find the location to
start the scan. The (col < 'b') scan key will match every item on the page
as long as it matches the last item on the page.
This patch implements prechecking of the scan keys required for directional
scan on beginning of page scan. If precheck is successful we can skip this
scan keys check for the items on the page. That could lead to significant
acceleration especially if the comparison operator is expensive.
Idea from patch by Konstantin Knizhnik.
Discussion: https://postgr.es/m/
079c3f8e-3371-abe2-e93c-
fc8a0ae3f571%40garret.ru
Reviewed-by: Peter Geoghegan, Pavel Borisov
Heikki Linnakangas [Fri, 6 Oct 2023 07:22:02 +0000 (10:22 +0300)]
Fix crash on syslogger startup
When syslogger starts up, ListenSockets is still NULL. Don't try to
pfree it. Oversight in commit
e29c464395.
Reported-by: Michael Paquier
Discussion: https://www.postgresql.org/message-id/ZR-uNkgL7m60lWUe@paquier.xyz
Michael Paquier [Fri, 6 Oct 2023 00:56:55 +0000 (09:56 +0900)]
worker_spi: Fix test failure with BGWORKER_BYPASS_ALLOWCONN
A bgworker can spawn parallel workers of its own when executing queries,
and if the worker uses BGWORKER_BYPASS_ALLOWCONN while the database it
is connected to does not allow connections, a parallel worker would fail
to startup. In the case of this module, the step checking for the
presence of the schema to create was spawning a worker, failing the last
test introduced by
991bb0f9653c.
This issue could be reproduced with debug_parallel_query = 'regress',
for example.
Per buildfarm member crake.
Michael Paquier [Fri, 6 Oct 2023 00:01:27 +0000 (09:01 +0900)]
worker_spi: Add tests for BGWORKER_BYPASS_ALLOWCONN
This bgworker flag exists in the core code since
eed1ce72e159, but was
never tested. This relies on
4f2994647ff1, that has added a way to
start dynamic workers with this flag enabled.
Reviewed-by: Bertrand Drouvot, Bharath Rupireddy
Discussion: https://postgr.es/m/
bcc36259-7850-4882-97ef-
d6b905d2fc51@gmail.com
Peter Eisentraut [Thu, 5 Oct 2023 14:17:16 +0000 (16:17 +0200)]
Push attcompression and attstorage handling into BuildDescForRelation()
This was previously handled by the callers but it can be moved into a
common place.
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da@eisentraut.org
Peter Eisentraut [Thu, 5 Oct 2023 14:17:16 +0000 (16:17 +0200)]
Move BuildDescForRelation() from tupdesc.c to tablecmds.c
BuildDescForRelation() main job is to convert ColumnDef lists to
pg_attribute/tuple descriptor arrays, which is really mostly an
internal subroutine of DefineRelation() and some related functions,
which is more the remit of tablecmds.c and doesn't have much to do
with the basic tuple descriptor interfaces in tupdesc.c. This is also
supported by observing the header includes we can remove in tupdesc.c.
By moving it over, we can also (in the future) make
BuildDescForRelation() use more internals of tablecmds.c that are not
sensible to be exposed in tupdesc.c.
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da@eisentraut.org
Peter Eisentraut [Thu, 5 Oct 2023 14:17:16 +0000 (16:17 +0200)]
Push attidentity and attgenerated handling into BuildDescForRelation()
Previously, this was handled by the callers separately, but it can be
trivially moved into BuildDescForRelation() so that it is handled in a
central place.
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da@eisentraut.org
Heikki Linnakangas [Thu, 5 Oct 2023 12:05:25 +0000 (15:05 +0300)]
Refactor ListenSocket array.
Keep track of the used size of the array. That avoids looping through
the whole array in a few places. It doesn't matter from a performance
point of view since the array is small anyway, but this feels less
surprising and is a little less code. Now that we have an explicit
NumListenSockets variable that is statically initialized to 0, we
don't need the loop to initialize the array.
Allocate the array in PostmasterContext. The array isn't needed in
child processes, so this allows reusing that memory. We could easily
make the array resizable now, but we haven't heard any complaints
about the current 64 sockets limit.
Discussion: https://www.postgresql.org/message-id/
7bb7ad65-a018-2419-742f-
fa5fd877d338@iki.fi
Alvaro Herrera [Thu, 5 Oct 2023 08:59:08 +0000 (10:59 +0200)]
Improve JsonLexContext's freeability
Previously, the JSON code didn't have to worry too much about freeing
JsonLexContext, because it was never too long-lived. With new features
being added for SQL/JSON this is no longer the case. Add a routine
that knows how to free this struct and apply that to a few places, to
prevent this from becoming problematic.
At the same time, we change the API of makeJsonLexContextCstringLen to
make it receive a pointer to JsonLexContext for callers that want it to
be stack-allocated; it can also be passed as NULL to get the original
behavior of a palloc'ed one.
This also causes an ABI break due to the addition of flags to
JsonLexContext, so we can't easily backpatch it. AFAICS that's not much
of a problem; apparently some leaks might exist in JSON usage of
text-search, for example via json_to_tsvector, but I haven't seen any
complaints about that.
Per Coverity complaint about datum_to_jsonb_internal().
Discussion: https://postgr.es/m/
20230808174110.oq3iymllsv6amkih@alvherre.pgsql
David Rowley [Thu, 5 Oct 2023 08:03:10 +0000 (21:03 +1300)]
Consider cheap startup paths in add_paths_to_append_rel
6b94e7a6d did this for ordered append paths to allow fast startup
MergeAppends, however, nothing was done for the Append case.
Here we adjust add_paths_to_append_rel() to have it build an AppendPath
containing the cheapest startup paths from each of the child relations
when the append rel has "consider_startup" set.
Author: Andy Fan, David Rowley
Discussion: https://www.postgresql.org/message-id/CAKU4AWrXSkUV=Pt-gRxQT7EbfUeNssprGyNsB=5mJibFZ6S3ww@mail.gmail.com
David Rowley [Thu, 5 Oct 2023 07:30:47 +0000 (20:30 +1300)]
Fix memory leak in Memoize code
Ensure we switch to the per-tuple memory context to prevent any memory
leaks of detoasted Datums in MemoizeHash_hash() and MemoizeHash_equal().
Reported-by: Orlov Aleksej
Author: Orlov Aleksej, David Rowley
Discussion: https://postgr.es/m/
83281eed63c74e4f940317186372abfd%40cft.ru
Backpatch-through: 14, where Memoize was added
Peter Eisentraut [Thu, 5 Oct 2023 06:53:21 +0000 (08:53 +0200)]
Constify crc32_sz
Author: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://postgr.es/m/
e08317a0-a2e7-c60d-c14a-
ad9fc34f8f6c%40eisentraut.org
Peter Eisentraut [Thu, 5 Oct 2023 06:43:49 +0000 (08:43 +0200)]
Modernize const handling with readline
The comment
/* On some platforms, readline is declared as readline(char *) */
is obsolete. The casting away of const can be removed.
The const in the readline() prototype was added in GNU readline 4.2,
released in 2001. BSD libedit has also had const in the prototype
since at least 2001.
(The commit that introduced this comment (
187e865174) talked about
FreeBSD 4.8, which didn't have readline compatibility in libedit yet,
so it must have been talking about GNU readline in the base system.
This checks out, but already FreeBSD 5 had an updated GNU readline
with const.)
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
862fc1d4-9a0c-d2b6-5451-
ee3dc750bcab%40eisentraut.org
Michael Paquier [Thu, 5 Oct 2023 03:22:28 +0000 (12:22 +0900)]
worker_spi: Expand set of options to start workers
A couple of new options are added to this module to provide more control
on the ways bgworkers are started:
- A new GUC called worker_spi.role to control which role to use by
default when starting a worker.
- worker_spi_launch() gains three arguments: a role OID, a database OID
and flags (currently only BGWORKER_BYPASS_ALLOWCONN). By default, the
role OID and the database OID are InvalidOid, in which case the worker
would use the related GUCs.
Workers loaded by shared_preload_libraries use the default values
provided by the GUCs, with flags at 0. The options are given to the
main bgworker routine through bgw_extra. A test case is tweaked to
start two dynamic workers with databases and roles defined by the caller
of worker_spi_launch().
These additions will have the advantage of expanding the tests for
bgworkers, for at least two cases:
- BGWORKER_BYPASS_ALLOWCONN has no coverage in the core tree.
- A new bgworker flag is under discussion, and this eases the
integration of new tests.
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/
bcc36259-7850-4882-97ef-
d6b905d2fc51@gmail.com
Michael Paquier [Thu, 5 Oct 2023 01:23:22 +0000 (10:23 +0900)]
dblink: Replace WAIT_EVENT_EXTENSION with custom wait events
Two custom wait events are added here:
- "DblinkConnect", when waiting to establish a connection to a remote
server.
- "DblinkGetConnect", when waiting to establish a connection to a remote
server but it could not be found in the list of already-opened ones.
Author: Masahiro Ikeda
Discussion: https://postgr.es/m/
197bce267fa691a0ac62c86c4ab904c4@oss.nttdata.com
Michael Paquier [Thu, 5 Oct 2023 00:50:42 +0000 (09:50 +0900)]
postgres_fdw: Replace WAIT_EVENT_EXTENSION with custom wait events
Three custom wait events are added here:
- "PostgresFdwCleanupResult", waiting while cleaning up PQgetResult() on
transaction abort.
- "PostgresFdwConnect", waiting to establish a connection to a remote
server.
- "PostgresFdwGetResult", waiting to receive a result from a remote
server.
Author: Masahiro Ikeda
Discussion: https://postgr.es/m/
197bce267fa691a0ac62c86c4ab904c4@oss.nttdata.com
Nathan Bossart [Wed, 4 Oct 2023 19:40:50 +0000 (14:40 -0500)]
Document that --sync-method takes an argument.
This was missed in commit
8c16ad3b43.
Reported-by: Robert Haas
Reviewed-by: Daniel Gustafsson, Robert Haas, Alvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/CA%2BTgmoZi7pcx-ec3oJLWSr2R%3DDn2Zeiyx3EXQKc_1TTvA6Eepg%40mail.gmail.com
Peter Eisentraut [Wed, 4 Oct 2023 13:03:48 +0000 (15:03 +0200)]
doc: Clarify not-null constraints in information schema
Add a bit of clarification in various places that not-null constraints
are included under check constraints in the information schema.
Michael Paquier [Wed, 4 Oct 2023 08:12:25 +0000 (17:12 +0900)]
test_shm_mq: Replace WAIT_EVENT_EXTENSION with custom wait events
Two custom wait events are added here:
- "TestShmMqBgWorkerStartup", when setting up a set of bgworkers in
wait_for_workers_to_become_ready().
- "TestShmMqMessageQueue", when waiting for a queued message in
test_shm_mq_pipelined().
Author: Masahiro Ikeda
Discussion: https://postgr.es/m/
197bce267fa691a0ac62c86c4ab904c4@oss.nttdata.com
Michael Paquier [Wed, 4 Oct 2023 07:16:50 +0000 (16:16 +0900)]
worker_spi: Rename custom wait event to "WorkerSpiMain"
This naming is more consistent with all the other user-facing wait event
strings. Other in-core modules will use the same naming convention, so
let's be consistent here as well.
Extracted from a larger patch by the same author.
Author: Masahiro Ikeda
Discussion: https://postgr.es/m/
197bce267fa691a0ac62c86c4ab904c4@oss.nttdata.com
Tom Lane [Tue, 3 Oct 2023 18:13:53 +0000 (14:13 -0400)]
Doc: suppress "exceed the available area" warning in PDF build.
Allow a line break in example output, as we have done elsewhere.
Overlength output was added in commit
1e68e43d3.
While here, adjust some shaky grammar in an adjacent note
(from a different commit,
c9af05465).
Per buildfarm.
Peter Eisentraut [Tue, 3 Oct 2023 15:40:15 +0000 (17:40 +0200)]
Remove RelationGetIndexRawAttOptions()
There was only one caller left, for which this function was overkill.
Also, having it in relcache.c was inappropriate, since it doesn't work
with the relcache at all.
Discussion: https://www.postgresql.org/message-id/flat/
f84640e3-00d3-5abd-3f41-
e6a19d33c40b@eisentraut.org
Peter Eisentraut [Tue, 3 Oct 2023 15:39:31 +0000 (17:39 +0200)]
Remove IndexInfo.ii_OpclassOptions field
It is unnecessary to include this field in IndexInfo. It is only used
by DDL code, not during execution. It is really only used to pass
local information around between functions in index.c and indexcmds.c,
for which it is clearer to use local variables, like in similar cases.
Discussion: https://www.postgresql.org/message-id/flat/
f84640e3-00d3-5abd-3f41-
e6a19d33c40b@eisentraut.org
Tom Lane [Tue, 3 Oct 2023 15:41:42 +0000 (11:41 -0400)]
Add some notes about why "ALTER TYPE enum DROP VALUE" is hard.
In hopes of putting these where any would-be implementer is sure to
find them, make a placeholder grammar production for ALTER DROP VALUE
and put them there. This is really just a docs patch, though.
Vik Fearing, with a bit more wordsmithing by me
Discussion: https://postgr.es/m/
9fffd149-da0f-0c9c-6745-
731fb688642a@postgresfriends.org
Robert Haas [Tue, 3 Oct 2023 15:00:40 +0000 (11:00 -0400)]
In basebackup.c, refactor to create read_file_data_into_buffer.
This further reduces the length and complexity of sendFile(),
hopefully make it easier to understand and modify. In addition
to moving some logic into a new function, I took this opportunity
to make a few slight adjustments to sendFile() itself, including
renaming the 'len' variable to 'bytes_done', since we use it to represent
the number of bytes we've already handled so far, not the total
length of the file.
Patch by me, reviewed by David Steele.
Discussion: http://postgr.es/m/CA+TgmoYt5jXH4U6cu1dm9Oe2FTn1aae6hBNhZzJJjyjbE_zYig@mail.gmail.com
Robert Haas [Tue, 3 Oct 2023 14:37:20 +0000 (10:37 -0400)]
In basebackup.c, refactor to create verify_page_checksum.
If checksum verification fails for a particular page, we reread the
page and try one more time. The code that does this somewhat complex
and difficult to follow. Move some of the logic into a new function
and rearrange the code a bit to try to make it clearer. This way,
we don't need the block_retry Boolean, a couple of other variables
move from sendFile() into the new function, and some code is now less
deeply indented.
Patch by me, reviewed by David Steele.
Discussion: http://postgr.es/m/CA+TgmoYt5jXH4U6cu1dm9Oe2FTn1aae6hBNhZzJJjyjbE_zYig@mail.gmail.com
Michael Paquier [Tue, 3 Oct 2023 06:37:00 +0000 (15:37 +0900)]
Avoid memory size overflow when allocating backend activity buffer
The code in charge of copying the contents of PgBackendStatus to local
memory could fail on memory allocation because of an overflow on the
amount of memory to use. The overflow can happen when combining a high
value track_activity_query_size (max at 1MB) with a large
max_connections, when both multiplied get higher than INT32_MAX as both
parameters treated as signed integers. This could for example trigger
with the following functions, all calling pgstat_read_current_status():
- pg_stat_get_backend_subxact()
- pg_stat_get_backend_idset()
- pg_stat_get_progress_info()
- pg_stat_get_activity()
- pg_stat_get_db_numbackends()
The change to use MemoryContextAllocHuge() has been introduced in
8d0ddccec636, so backpatch down to 12.
Author: Jakub Wartak
Discussion: https://postgr.es/m/CAKZiRmw8QSNVw2qNK-dznsatQqz+9DkCquxP0GHbbv1jMkGHMA@mail.gmail.com
Backpatch-through: 12
Peter Eisentraut [Tue, 3 Oct 2023 06:30:20 +0000 (08:30 +0200)]
Fix incorrect format placeholder
David Rowley [Tue, 3 Oct 2023 04:09:52 +0000 (17:09 +1300)]
Tidy-up some appendStringInfo*() usages
Make a few newish calls to appendStringInfo() which have no special
formatting use appendStringInfoString() instead. Also, adjust usages of
appendStringInfoString() which only append a string containing a single
character to make use of appendStringInfoChar() instead.
This makes the code marginally faster, but primarily this change is so
we use the StringInfo type as it was intended to be used.
Discussion: https://postgr.es/m/CAApHDvpXKQmL+r=VDNS98upqhr9yGBhv2Jw3GBFFk_wKHcB39A@mail.gmail.com
Michael Paquier [Tue, 3 Oct 2023 01:21:44 +0000 (10:21 +0900)]
Fail hard on out-of-memory failures in xlogreader.c
This commit changes the WAL reader routines so as a FATAL for the
backend or exit(FAILURE) for the frontend is triggered if an allocation
for a WAL record decode fails in walreader.c, rather than treating this
case as bogus data, which would be equivalent to the end of WAL. The
key is to avoid palloc_extended(MCXT_ALLOC_NO_OOM) in walreader.c,
relying on plain palloc() calls.
The previous behavior could make WAL replay finish too early than it
should. For example, crash recovery finishing earlier may corrupt
clusters because not all the WAL available locally was replayed to
ensure a consistent state. Out-of-memory failures would show up
randomly depending on the memory pressure on the host, but one simple
case would be to generate a large record, then replay this record after
downsizing a host, as Ethan Mertz originally reported.
This relies on
bae868caf222, as the WAL reader routines now do the
memory allocation required for a record only once its header has been
fully read and validated, making xl_tot_len trustable. Making the WAL
reader react differently on out-of-memory or bogus record data would
require ABI changes, so this is the safest choice for stable branches.
Also, it is worth noting that
3f1ce973467a has been using a plain
palloc() in this code for some time now.
Thanks to Noah Misch and Thomas Munro for the discussion.
Like the other commit, backpatch down to 12, leaving out v11 that will
be EOL'd soon. The behavior of considering a failed allocation as bogus
data comes originally from
0ffe11abd3a0, where the record length
retrieved from its header was not entirely trustable.
Reported-by: Ethan Mertz
Discussion: https://postgr.es/m/ZRKKdI5-RRlta3aF@paquier.xyz
Backpatch-through: 12
Michael Paquier [Mon, 2 Oct 2023 23:27:34 +0000 (08:27 +0900)]
Replace use of stat()[7] by -s switch in TAP tests to retrieve file size
The list form of stat() is an inelegant API as it relies on the position
of the file size in the list returned in result. Like in any other
places of the tree, replace that with a -s switch instead.
Another suggestion from Dagfinn is File::Stat, which we've been already
using for some other fields. It really comes down to a matter of taste
to choose that over -s, and the latter is more used in the tree.
Author: Bertrand Drouvot
Reviewed-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/
b2020df7-d0fc-4ea5-b2a9-
7efc6d36b2ac@gmail.com
Tom Lane [Mon, 2 Oct 2023 17:27:51 +0000 (13:27 -0400)]
Fix omission of column-level privileges in selective pg_restore.
In a selective restore, ACLs for a table should be dumped if the
table is selected to be dumped. However, if the table has both
table-level and column-level ACLs, only the table-level ACL was
restored. This happened because _tocEntryRequired assumed that
an ACL could have only one dependency (the one on its table),
and punted if there was more than one. But since commit
ea9125304,
column-level ACLs also depend on the table-level ACL if any, to
ensure correct ordering in parallel restores. To fix, adjust the
logic in _tocEntryRequired to ignore dependencies on ACLs.
I extended a test case in 002_pg_dump.pl so that it purports to
test for this; but in fact the test passes even without the fix.
That's because this bug only manifests during a selective restore,
while the scenarios 002_pg_dump.pl tests include only selective dumps.
Perhaps somebody would like to extend the script so that it can test
scenarios including selective restore, but I'm not touching that.
Euler Taveira and Tom Lane, per report from Kong Man.
Back-patch to all supported branches.
Discussion: https://postgr.es/m/DM4PR11MB73976902DBBA10B1D652F9498B06A@DM4PR11MB7397.namprd11.prod.outlook.com
Robert Haas [Mon, 2 Oct 2023 15:40:07 +0000 (11:40 -0400)]
Remove retry loop in heap_page_prune().
The retry loop is needed because heap_page_prune() calls
HeapTupleSatisfiesVacuum() and then lazy_scan_prune() does the same
thing again, and they might get different answers due to concurrent
clog updates. But this patch makes heap_page_prune() return the
HeapTupleSatisfiesVacuum() results that it computed back to the
caller, which allows lazy_scan_prune() to avoid needing to recompute
those values in the first place. That's nice both because it eliminates
the need for a retry loop and also because it's cheaper.
Melanie Plageman, reviewed by David Geier, Andres Freund, and me.
Discussion: https://postgr.es/m/CAAKRu_br124qsGJieuYA0nGjywEukhK1dKBfRdby_4yY3E9SXA%40mail.gmail.com
Heikki Linnakangas [Mon, 2 Oct 2023 09:39:35 +0000 (12:39 +0300)]
Flush WAL stats in bgwriter
bgwriter can write out WAL, but did not flush the WAL pgstat counters,
so the writes were not seen in pg_stat_wal.
Back-patch to v14, where pg_stat_wal was introduced.
Author: Nazir Bilal Yavuz
Reviewed-by: Matthias van de Meent, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/CAN55FZ2FPYngovZstr%3D3w1KSEHe6toiZwrurbhspfkXe5UDocg%40mail.gmail.com
Heikki Linnakangas [Mon, 2 Oct 2023 09:18:57 +0000 (12:18 +0300)]
Add rmgrdesc README
In the README, briefly explain what rmgrdesc functions are, and why
they are in a separate directory. Commit
c03c2eae0a added some
guidelines on the preferred output format; move that to the README
too.
Reviewed-by: Melanie Plageman, Peter Geoghegan
Discussion: https://www.postgresql.org/message-id/
9159daf7-f42d-781b-458f-
1b2cf32cb256%40iki.fi
Heikki Linnakangas [Mon, 2 Oct 2023 08:46:25 +0000 (11:46 +0300)]
Add regression tests for psql \g piped into a program
Author: Daniel Vérité
Reviewed-by: Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/
33ce8350-8cd1-45ff-a5fe-
f9be7bc70649%40manitou-mail.org
Amit Langote [Mon, 2 Oct 2023 04:48:15 +0000 (13:48 +0900)]
Revert "Add soft error handling to some expression nodes"
This reverts commit
7fbc75b26ed8ec70c729c5e7f8233896c54c900f.
Looks like the LLVM additions may not be totally correct.
Amit Langote [Mon, 2 Oct 2023 02:52:28 +0000 (11:52 +0900)]
Add soft error handling to some expression nodes
This adjusts the expression evaluation code for CoerceViaIO and
CoerceToDomain to handle errors softly if needed.
For CoerceViaIo, this means using InputFunctionCallSafe(), which
provides the option to handle errors softly, instead of calling the
type input function directly.
For CoerceToDomain, this simply entails replacing the ereport() in
ExecEvalConstraintCheck() by errsave().
In both cases, the ErrorSaveContext to be used when evaluating the
expression is stored by ExecInitExprRec() in the expression's struct
in the expression's ExprEvalStep. The ErrorSaveContext is passed by
setting ExprState.escontext to point to it when calling
ExecInitExprRec() on the expression whose errors are to be handled
softly.
Note that no call site of ExecInitExprRec() has been changed in this
commit, so there's no functional change. This is intended for
implementing new SQL/JSON expression nodes in future commits that
will use to it suppress errors that may occur during type coercions.
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
Michael Paquier [Mon, 2 Oct 2023 02:05:05 +0000 (11:05 +0900)]
psql: Set variables from query result on failure when printing tuples
SetResultVariables() was not getting called when "printing" a result
that failed (see around PrintQueryResult), which would cause some
variables to not be set, like ROW_COUNT, SQLSTATE or ERROR. This can be
confusing as a previous result would be retained.
This state could be reached when failing to process tuples in a few
commands, like \gset when it returns no tuples, or \crosstabview. A
test is added, based on \gset.
This is arguably a bug fix, but no backpatch is done as there is a risk
of breaking scripts that rely on the previous behavior, even if they do
so accidentally.
Reported-by: amutu
Author: Japin Li
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/18134-
87126d90cb4dd049@postgresql.org
Noah Misch [Sun, 1 Oct 2023 19:20:55 +0000 (12:20 -0700)]
Correct assertion and comments about XLogRecordMaxSize.
The largest allocation, of xl_tot_len+8192, is in allocate_recordbuf().
Discussion: https://postgr.es/m/
20230812211327.GB2326466@rfd.leadboat.com
Tom Lane [Sun, 1 Oct 2023 17:16:47 +0000 (13:16 -0400)]
Fix datalen calculation in tsvectorrecv().
After receiving position data for a lexeme, tsvectorrecv()
advanced its "datalen" value by (npos+1)*sizeof(WordEntry)
where the correct calculation is (npos+1)*sizeof(WordEntryPos).
This accidentally failed to render the constructed tsvector
invalid, but it did result in leaving some wasted space
approximately equal to the space consumed by the position data.
That could have several bad effects:
* Disk space is wasted if the received tsvector is stored into a
table as-is.
* A legal tsvector could get rejected with "maximum total lexeme
length exceeded" if the extra space pushes it over the MAXSTRPOS
limit.
* In edge cases, the finished tsvector could be assigned a length
larger than the allocated size of its palloc chunk, conceivably
leading to SIGSEGV when the tsvector gets copied somewhere else.
The odds of a field failure of this sort seem low, though valgrind
testing could probably have found this.
While we're here, let's express the calculation as
"sizeof(uint16) + npos * sizeof(WordEntryPos)" to avoid the type
pun implicit in the "npos + 1" formulation. It's not wrong
given that WordEntryPos had better be 2 bytes to avoid padding
problems, but it seems clearer this way.
Report and patch by Denis Erokhin. Back-patch to all supported
versions.
Discussion: https://postgr.es/m/
009801d9f2d9$
f29730c0$
d7c59240$@datagile.ru
Tom Lane [Sun, 1 Oct 2023 16:09:26 +0000 (12:09 -0400)]
In COPY FROM, fail cleanly when unsupported encoding conversion is needed.
In recent releases, such cases fail with "cache lookup failed for
function 0" rather than complaining that the conversion function
doesn't exist as prior versions did. Seems to be a consequence of
sloppy refactoring in commit
f82de5c46. Add the missing error check.
Per report from Pierre Fortin. Back-patch to v14 where the
oversight crept in.
Discussion: https://postgr.es/m/
20230929163739.
3bea46e5.pfortin@pfortin.com
Andrew Dunstan [Sun, 1 Oct 2023 14:18:41 +0000 (10:18 -0400)]
Only evaluate default values as required when doing COPY FROM
Commit
9f8377f7a2 was a little too eager in fetching default values.
Normally this would not matter, but if the default value is not valid
for the type (e.g. a varchar that's too long) it caused an unnecessary
error.
Complaint and fix from Laurenz Albe
Backpatch to release 16.
Discussion: https://postgr.es/m/
75a7b7483aeb331aa017328d606d568fc715b90d.camel@cybertec.at
Andres Freund [Sat, 30 Sep 2023 19:10:15 +0000 (12:10 -0700)]
meson: macos: Correct -exported_symbols_list syntax for Sonoma compat
-exported_symbols_list=... works on Ventura and earlier, but not on
Sonoma. The easiest way to fix it is to -Wl,-exported_symbols_list,@0@ which
actually seems more appropriate anyway, it's obviously a linker argument. It
is easier to use the -Wl,, syntax than passing multiple arguments, due to the
way the export_fmt is used (a single string that's formatted), but if it turns
out to be necessary, we can go for multiple arguments as well.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
20230928222248.jw6s7yktpfsfczha@alap3.anarazel.de
Backpatch: 16-, where the meson based buildsystem was added
Andrew Dunstan [Sat, 30 Sep 2023 16:34:41 +0000 (12:34 -0400)]
Provide FORCE_NULL * and FORCE_NOT_NULL * options for COPY FROM
These options already exist, but you need to specify a column list for
them, which can be cumbersome. We already have the possibility of all
columns for FORCE QUOTE, so this is simply extending that facility to
FORCE_NULL and FORCE_NOT_NULL.
Author: Zhang Mingli
Reviewed-By: Richard Guo, Kyatoro Horiguchi, Michael Paquier.
Discussion: https://postgr.es/m/CACJufxEnVqzOFtqhexF2+AwOKFrV8zHOY3y=p+gPK6eB14pn_w@mail.gmail.com
Heikki Linnakangas [Sat, 30 Sep 2023 14:03:50 +0000 (17:03 +0300)]
Fix briefly showing old progress stats for ANALYZE on inherited tables.
ANALYZE on a table with inheritance children analyzes all the child
tables in a loop. When stepping to next child table, it updated the
child rel ID value in the command progress stats, but did not reset
the 'sample_blks_total' and 'sample_blks_scanned' counters.
acquire_sample_rows() updates 'sample_blks_total' as soon as the scan
starts and 'sample_blks_scanned' after processing the first block, but
until then, pg_stat_progress_analyze would display a bogus combination
of the new child table relid with old counter values from the
previously processed child table. Fix by resetting 'sample_blks_total'
and 'sample_blks_scanned' to zero at the same time that
'current_child_table_relid' is updated.
Backpatch to v13, where pg_stat_progress_analyze view was introduced.
Reported-by: Justin Pryzby
Discussion: https://www.postgresql.org/message-id/
20230122162345.GP13860%40telsasoft.com
Dean Rasheed [Sat, 30 Sep 2023 09:52:21 +0000 (10:52 +0100)]
Fix EvalPlanQual rechecking during MERGE.
Under some circumstances, concurrent MERGE operations could lead to
inconsistent results, that varied according the plan chosen. This was
caused by a lack of rowmarks on the source relation, which meant that
EvalPlanQual rechecking was not guaranteed to return the same source
tuples when re-running the join query.
Fix by ensuring that preprocess_rowmarks() sets up PlanRowMarks for
all non-target relations used in MERGE, in the same way that it does
for UPDATE and DELETE.
Per bug #18103. Back-patch to v15, where MERGE was introduced.
Dean Rasheed, reviewed by Richard Guo.
Discussion: https://postgr.es/m/18103-
c4386baab8e355e3%40postgresql.org
Tom Lane [Sat, 30 Sep 2023 00:20:57 +0000 (20:20 -0400)]
Remove environment sensitivity in pl/tcl regression test.
Add "-gmt 1" to our test invocations of the Tcl "clock" command,
so that they do not consult the timezone environment. While it
doesn't really matter which timezone is used here, it does
matter that the command not fall over entirely. We've now
discovered that at least on FreeBSD, "clock scan" will fail if
/etc/localtime is missing. It seems worth making the test
insensitive to that.
Per Tomas Vondras' buildfarm animal dikkop. Thanks to
Thomas Munro for the diagnosis.
Discussion: https://postgr.es/m/
316d304a-1dcd-cea1-3d6c-
27f794727a06@enterprisedb.com
Bruce Momjian [Fri, 29 Sep 2023 22:33:03 +0000 (18:33 -0400)]
doc: remove PG version mention in EXPLAIN output
Reported-by: Daniel Westermann
Discussion: https://postgr.es/m/GV0P278MB0419DF1A8673E8D17A6287FAD2FA9@GV0P278MB0419.CHEP278.PROD.OUTLOOK.COM
Backpatch-through: master
Bruce Momjian [Fri, 29 Sep 2023 18:25:59 +0000 (14:25 -0400)]
C comment: add optimizer function reference
Reported-by: James Coleman
Discussion: https://postgr.es/m/CAAaqYe9F6uoMhAr+8rMLwvGzaKaSknPA0Wi3Ehtv8pbSYmJq-Q@mail.gmail.com
Backpatch-through: master
Tom Lane [Fri, 29 Sep 2023 18:07:30 +0000 (14:07 -0400)]
Suppress macOS warnings about duplicate libraries in link commands.
As of Xcode 15 (macOS Sonoma), the linker complains about duplicate
references to the same library. We see warnings about libpgport and
libpgcommon being duplicated in many client executables. This is a
consequence of the hack introduced in commit
6b7ef076b to list
libpgport before libpq while not removing it from $(LIBS).
(Commit
8396447cd later applied the same rule to libpgcommon.)
The concern in
6b7ef076b was to ensure that the client executable
wouldn't unintentionally depend on pgport functions from libpq.
That concern is obsolete on any platform for which we can do symbol
export control, because if we can then the pgport functions in libpq
won't be exposed anyway. Hence, we can fix this problem by just
removing libpgport and libpgcommon from $(libpq_pgport), and letting
clients depend on the occurrences in $(LIBS).
In the back branches, do that only on macOS (which we know has
symbol export control). In HEAD, let's be more aggressive and
remove the extra libraries everywhere. The only still-supported
platforms that lack export control are MinGW/Cygwin, and it
doesn't seem worth sweating over ABI stability details for those
(or if somebody does care, it'd probably be possible to perform
symbol export control for those too). As well as being simpler,
this might give some microscopic improvement in build time.
The meson build system is not changed here, as it doesn't have
this particular disease, though it does have some related issues
that we'll fix separately.
Discussion: https://postgr.es/m/467042.
1695766998@sss.pgh.pa.us
Tom Lane [Fri, 29 Sep 2023 17:13:54 +0000 (13:13 -0400)]
Doc: improve description of dump/restore's --clean and --if-exists.
Try to make these option descriptions a little clearer for novices.
Per gripe from Attila Gulyás.
Discussion: https://postgr.es/m/
169590536647.
3727336.
11070254203649648453@wrigleys.postgresql.org
Daniel Gustafsson [Fri, 29 Sep 2023 13:55:37 +0000 (15:55 +0200)]
doc: Change statistics function xref to the right target
Commit
7d3b7011b added a link to the statistics functions, which at the
time were anchored under the section for statistics views.
aebe989477a
added a separate section for statistics functions, but the link was not
updated to point to the new anchor. Fix by changing the xref.
Backpatch to all supported branches.
Author: Peter Smith <peter.b.smith@fujitsu.com>
Discussion: https://postgr.es/m/CAHut+Ptr0jKzNNtWnssLq+3jNhbyaBseqf6NPrWHk08mQFRoTg@mail.gmail.com
Backpatch-through: 11
Peter Eisentraut [Fri, 29 Sep 2023 08:59:46 +0000 (10:59 +0200)]
Revert "pg_resetwal: Improve error with wrong/missing data directory"
This reverts commit
1d863c250461410e60c9ed5d3180f32336f4c3e2.
This broke specifying the data directory as a relative path.
Reported-by: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Discussion: https://www.postgresql.org/message-id/flat/TYAPR01MB58664AD301F511B1EA5B72B4F5C0A%40TYAPR01MB5866.jpnprd01.prod.outlook.com
David Rowley [Fri, 29 Sep 2023 03:58:32 +0000 (16:58 +1300)]
Robustify find_base_rel and find_base_rel_ignore_join
Improve find_base_rel() and find_base_rel_ignore_join() so that they
raise an ERROR if they ever receive a negative relid value in
non-cassert builds. If either of these functions had ever received a
negative relid then they'd have attempted to access memory that does not
belong to simple_rel_array.
Because no evidence has been presented of actual cases where bugs have
caused this to happen, here we take a lightweight approach to checking
for negative values and simply cast both values to uint32 before
performing the comparison. This will cause any negative relids to be
seen as greater than simple_rel_array_size which will ERROR rather than
attempt to access a negative simple_rel_array element. Obviously, the
run-time error is better than a crash, so it makes sense to protect
against this, especially when it can be done without adding any
additional run-time overhead.
There is a slight change here if the functions are ever called with a
relid of 0. This will pass the bounds check, but that array entry
should be NULL (along with the corresponding simple_rte_array entry), so
won't pass the "if (rel)" condition and still fall through and raise an
ERROR.
Author: Ranier Vilela
Reviewed-by: Ashutosh Bapat, David Rowley
Discussion: https://postgr.es/m/CAEudQArQSghBu2gLojg4o_tnHj_x2HcS%3D%2BwewL3NJS8z0VnK%2Bg%40mail.gmail.com
Michael Paquier [Fri, 29 Sep 2023 01:34:04 +0000 (10:34 +0900)]
doc: Fix descriptions related to the handling of non-ASCII characters
Since
45b1a67a0fcb, non-printable ASCII characters do not show up in
various configuration paths as question marks, but as hexadecimal
escapes. The documentation was not updated to reflect that.
Author: Hayato Kuroda
Reviewed-by: Jian He, Tom Lane, Karl O. Pinc, Peter Smith
Discussion: https://postgr.es/m/TYAPR01MB586631D0961BF9C44893FAB1F523A@TYAPR01MB5866.jpnprd01.prod.outlook.com
Backpatch-through: 16
Peter Geoghegan [Thu, 28 Sep 2023 23:29:37 +0000 (16:29 -0700)]
Fix btmarkpos/btrestrpos array key wraparound bug.
nbtree's mark/restore processing failed to correctly handle an edge case
involving array key advancement and related search-type scan key state.
Scans with ScalarArrayScalarArrayOpExpr quals requiring mark/restore
processing (for a merge join) could incorrectly conclude that an
affected array/scan key must not have advanced during the time between
marking and restoring the scan's position.
As a result of all this, array key handling within btrestrpos could skip
a required call to _bt_preprocess_keys(). This confusion allowed later
primitive index scans to overlook tuples matching the true current array
keys. The scan's search-type scan keys would still have spurious values
corresponding to the final array element(s) -- not values matching the
first/now-current array element(s).
To fix, remember that "array key wraparound" has taken place during the
ongoing btrescan in a flag variable stored in the scan's state, and use
that information at the point where btrestrpos decides if another call
to _bt_preprocess_keys is required.
Oversight in commit
70bc5833, which taught nbtree to handle array keys
during mark/restore processing, but missed this subtlety. That commit
was itself a bug fix for an issue in commit
9e8da0f7, which taught
nbtree to handle ScalarArrayOpExpr quals natively.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-WzkgP3DDRJxw6DgjCxo-cu-DKrvjEv_ArkP2ctBJatDCYg@mail.gmail.com
Backpatch: 11- (all supported branches).
Tom Lane [Thu, 28 Sep 2023 18:05:25 +0000 (14:05 -0400)]
Fix checking of index expressions in CompareIndexInfo().
This code was sloppy about comparison of index columns that
are expressions. It didn't reliably reject cases where one
index has an expression where the other has a plain column,
and it could index off the start of the attmap array, leading
to a Valgrind complaint (though an actual crash seems unlikely).
I'm not sure that the expression-vs-column sloppiness leads
to any visible problem in practice, because the subsequent
comparison of the two expression lists would reject cases
where the indexes have different numbers of expressions
overall. Maybe we could falsely match indexes having the
same expressions in different column positions, but it'd
require unlucky contents of the word before the attmap array.
It's not too surprising that no problem has been reported
from the field. Nonetheless, this code is clearly wrong.
Per bug #18135 from Alexander Lakhin. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/18135-
532f4a755e71e4d2@postgresql.org
Robert Haas [Thu, 28 Sep 2023 14:36:34 +0000 (10:36 -0400)]
Return data from heap_page_prune via a struct.
Previously, one of the values in the struct was returned as the return
value, and another was returned via an output parameter. In
preparation for returning more stuff, consolidate both values into a
struct returned via an output parameter.
Melanie Plageman, reviewed by Andres Freund and by me.
Discussion: https://postgr.es/m/CAAKRu_br124qsGJieuYA0nGjywEukhK1dKBfRdby_4yY3E9SXA%40mail.gmail.com
Daniel Gustafsson [Thu, 28 Sep 2023 13:33:37 +0000 (15:33 +0200)]
doc: Clarify where ereport severity levels are defined
For a reader unfamiliar with the postgres code it might take some
grepping to find where elevels are defined. This adds a reference
to elog.h in the text like how SQLSTATE errorcodes are referenced
to errcodes.h on the same page.
Author: Kuwamura Masaki <kuwamura@db.is.i.nagoya-u.ac.jp>
Discussion: https://postgr.es/m/CAMyC8qqp1UDA9zothnJ9CbUYByytwpALS3LkdZ6bs1w5kZw5Xg@mail.gmail.com
David Rowley [Thu, 28 Sep 2023 11:02:22 +0000 (00:02 +1300)]
Add missing TidRangePath handling in print_path()
Tid Range scans were added back in
bb437f995. That commit forgot to add
handling for TidRangePaths in print_path().
Only people building with OPTIMIZER_DEBUG might have noticed this, which
likely is the reason it's taken 4 years for anyone to notice.
Author: Andrey Lepikhov
Reported-by: Andrey Lepikhov
Discussion: https://postgr.es/m/
379082d6-1b6a-4cd6-9ecf-
7157d8c08635@postgrespro.ru
Backpatch-through: 14, where
bb437f995 was introduced
Etsuro Fujita [Thu, 28 Sep 2023 10:45:00 +0000 (19:45 +0900)]
Fix typo in src/backend/access/transam/README.
Peter Eisentraut [Thu, 28 Sep 2023 10:08:54 +0000 (12:08 +0200)]
doc: Improve documentation about pg_resetwal -f option
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
0f3ab4a1-ae80-56e8-3426-
6b4a02507687@eisentraut.org
Peter Eisentraut [Thu, 28 Sep 2023 09:58:36 +0000 (11:58 +0200)]
pg_resetwal: Use frontend logging API
This now causes error messages related to the lack of the -f option to
appear on standard error rather than standard output.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
0f3ab4a1-ae80-56e8-3426-
6b4a02507687@eisentraut.org
Peter Eisentraut [Thu, 28 Sep 2023 09:49:20 +0000 (11:49 +0200)]
pg_resetwal: Regroup --help output
Put the options to modify the control values into a separate group.
This matches the outline of the man page.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
0f3ab4a1-ae80-56e8-3426-
6b4a02507687@eisentraut.org
Peter Eisentraut [Thu, 28 Sep 2023 09:40:00 +0000 (11:40 +0200)]
pg_resetwal: Improve error with wrong/missing data directory
Run chdir() before permission check to get a less confusing error
message if the specified data directory does not exist.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
0f3ab4a1-ae80-56e8-3426-
6b4a02507687@eisentraut.org
Peter Eisentraut [Thu, 28 Sep 2023 09:27:22 +0000 (11:27 +0200)]
pg_resetwal: Update an obsolete comment
The comment claimed that pg_resetwal updates the pg_control file if it
is of an old version. This has apparently never been true. Also, in
c3c09be34b, another comment was added elsewhere that this currently
does not happen. So this comment is wrong and redundant and can be
removed.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
0f3ab4a1-ae80-56e8-3426-
6b4a02507687@eisentraut.org
Michael Paquier [Thu, 28 Sep 2023 06:17:55 +0000 (15:17 +0900)]
Show parameters of CALL as constants in pg_stat_statements
This commit changes the query jumbling of CallStmt so as its IN/OUT
parameters are able to show up as constants with a parameter symbol in
pg_stat_statements, like:
CALL proc1($1, $2);
CALL proc2($1, $2, $3);
The transformed FuncExpr is used in the query ID computation instead of
the FuncCall generated by the parser, so as it is sensitive to the OID
of the procedure and its list of input arguments. The output arguments
are handled in a separate list in CallStmt, which is also included in
the computation.
Tests are added to pg_stat_statements to show how this affects CALL with
IN/OUT parameters as well as overloaded functions.
Like
638d42a3c520 or
31de7e60da34, this improves the monitoring of
workloads with a lot of CALL statements, preventing unnecessary bloat
when these use different input (or event output) values.
Author: Sami Imseih
Discussion: https://postgr.es/m/
B44FA29D-EBD0-4DD9-ABC2-
16F1CB087074@amazon.com
Amit Langote [Thu, 28 Sep 2023 00:44:39 +0000 (09:44 +0900)]
Remove obsolete executor cleanup code
This commit removes unnecessary ExecExprFreeContext() calls in
ExecEnd* routines because the actual cleanup is managed by
FreeExecutorState(). With no callers remaining for
ExecExprFreeContext(), this commit also removes the function.
This commit also drops redundant ExecClearTuple() calls, because
ExecResetTupleTable() in ExecEndPlan() already takes care of
resetting and dropping all TupleTableSlots initialized with
ExecInitScanTupleSlot() and ExecInitExtraTupleSlot().
After these modifications, the ExecEnd*() routines for ValuesScan,
NamedTuplestoreScan, and WorkTableScan became redundant. So, this
commit removes them.
Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com
Michael Paquier [Thu, 28 Sep 2023 00:33:51 +0000 (09:33 +0900)]
Move tracking of in_streaming to PGOutputData
"in_streaming" is a flag used to track if an instance of pgoutput is
streaming changes. When pgoutput is started, the flag was always reset,
switched it back and forth in the stream start/stop callbacks.
Before this commit, it was a global variable, which is confusing as it
is actually attached to a state of PGOutputData. Per my analysis, using
a global variable did not lead to an active bug like in
54ccfd65868c,
but it makes the code more consistent. Note that we cannot backpatch
this change anyway as it requires the addition of a new field to
PGOutputData, exposed in pgoutput.h.
Author: Hou Zhijie
Reviewed-by: Amit Kapila, Michael Paquier, Peter Smith
Discussion: https://postgr.es/m/OS0PR01MB571690EF24F51F51EFFCBB0E94FAA@OS0PR01MB5716.jpnprd01.prod.outlook.com
Peter Eisentraut [Wed, 27 Sep 2023 17:52:40 +0000 (18:52 +0100)]
Add TupleDescGetDefault()
This unifies some repetitive code.
Note: I didn't push the "not found" error message into the new
function, even though all existing callers would be able to make use
of it. Using the existing error handling as-is would probably require
exposing the Relation type via tupdesc.h, which doesn't seem
desirable. (Or even if we changed it to just report the OID, it would
inject the concept of a relation containing the tuple descriptor into
tupdesc.h, which might be a layering violation. Perhaps some further
improvements could be considered here separately.)
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da%40eisentraut.org
Daniel Gustafsson [Wed, 27 Sep 2023 11:02:21 +0000 (13:02 +0200)]
llvmjit: Use explicit LLVMContextRef for inlining
When performing inlining LLVM unfortunately "leaks" types (the
types survive and are usable, but a new round of inlining will
recreate new structurally equivalent types). This accumulation
will over time amount to a memory leak which for some queries
can be large enough to trigger the OOM process killer.
To avoid accumulation of types, all IR related data is stored
in an LLVMContextRef which is dropped and recreated in order
to release all types. Dropping and recreating incurs overhead,
so it will be done only after 100 queries. This is a heuristic
which might be revisited, but until we can get the size of the
context from LLVM we are flying a bit blind.
This issue has been reported several times, there may be more
references to it in the archives on top of the threads linked
below.
Backpatching of this fix will be handled once it has matured
in master for a bit.
Reported-By: Justin Pryzby <pryzby@telsasoft.com>
Reported-By: Kurt Roeckx <kurt@roeckx.be>
Reported-By: Jaime Casanova <jcasanov@systemguards.com.ec>
Reported-By: Lauri Laanmets <pcspets@gmail.com>
Author: Andres Freund and Daniel Gustafsson
Discussion: https://postgr.es/m/
7acc8678-df5f-4923-9cf6-
e843131ae89d@www.fastmail.com
Discussion: https://postgr.es/m/
20201218235607.GC30237@telsasoft.com
Discussion: https://postgr.es/m/CAPH-tTxLf44s3CvUUtQpkDr1D8Hxqc2NGDzGXS1ODsfiJ6WSqA@mail.gmail.com
Daniel Gustafsson [Wed, 27 Sep 2023 11:02:14 +0000 (13:02 +0200)]
llvmjit: Make llvm_types_module variable static
Commit
b059d2f45685a introduced llvm_types_module and accidentally
exported it. As there is no usecase for accessing this variable
externally, this makes it static.
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/
20221101055132.pjjsvlkeo4stbjkq@awork3.anarazel.de
Daniel Gustafsson [Wed, 27 Sep 2023 11:02:01 +0000 (13:02 +0200)]
llvmjit: Remove unnecessary types
These types were added in
fb46ac26fe but hasn't been used, so
remove until there is a need for them.
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/
20221101055132.pjjsvlkeo4stbjkq@awork3.anarazel.de
Amit Kapila [Wed, 27 Sep 2023 09:02:51 +0000 (14:32 +0530)]
Fix the misuse of origin filter across multiple pg_logical_slot_get_changes() calls.
The pgoutput module uses a global variable (publish_no_origin) to cache
the action for the origin filter, but we didn't reset the flag when
shutting down the output plugin, so subsequent retries may access the
previous publish_no_origin value.
We fix this by storing the flag in the output plugin's private data.
Additionally, the patch removes the currently unused origin string from the
structure.
For the back branch, to avoid changing the exposed structure, we eliminated the
global variable and instead directly used the origin string for change
filtering.
Author: Hou Zhijie
Reviewed-by: Amit Kapila, Michael Paquier
Backpatch-through: 16
Discussion: http://postgr.es/m/OS0PR01MB571690EF24F51F51EFFCBB0E94FAA@OS0PR01MB5716.jpnprd01.prod.outlook.com
Michael Paquier [Wed, 27 Sep 2023 05:40:23 +0000 (14:40 +0900)]
unaccent: Tweak value of PYTHON when building without Python support
As coded, the module's Makefile would fail to set a value for PYTHON as
it checked if the variable is defined. When compiling without
--with-python, PYTHON is defined and set to an empty value, so the
existing check is not able to do its work.
This commit switches the rule to check if the value is empty rather than
defined, allowing the generation of unaccent.rules even if --with-python
is not used as long as "python" exists. BISON and FLEX do the same in
pgxs.mk, for instance.
Thinko in
f85a485f89e2.
Author: Japin Li
Discussion: https://postgr.es/m/MEYP282MB1669F86C0DC7B4DC48489CB0B6C3A@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Backpatch-through: 13
Tom Lane [Wed, 27 Sep 2023 01:06:21 +0000 (21:06 -0400)]
Stop using "-multiply_defined suppress" on macOS.
We started to use this linker switch in commit
9df308697 of
2004-07-13, which was in the OS X 10.3 era. Apparently it's been a
no-op since around OS X 10.9. Apple's most recent toolchain version
actively complains about it, so it's time to get rid of it.
Discussion: https://postgr.es/m/467042.
1695766998@sss.pgh.pa.us
Bruce Momjian [Tue, 26 Sep 2023 23:44:22 +0000 (19:44 -0400)]
doc: clarify the effect of concurrent work_mem allocations
Reported-by: Sami Imseih
Discussion: https://postgr.es/m/
66590882-F48C-4A25-83E3-
73792CF8C51F@amazon.com
Backpatch-through: 11
Bruce Momjian [Tue, 26 Sep 2023 23:23:59 +0000 (19:23 -0400)]
doc: clarify handling of time zones with "time with time zone"
Reported-by: davecramer@postgres.rocks
Discussion: https://postgr.es/m/
168451942371.714.
9173574930845904336@wrigleys.postgresql.org
Backpatch-through: 11
Bruce Momjian [Tue, 26 Sep 2023 23:02:18 +0000 (19:02 -0400)]
doc: clarify the behavior of unopenable listen_addresses
Reported-by: Gurjeet Singh
Discussion: https://postgr.es/m/CABwTF4WYPD9ov-kcSq1+J+ZJ5wYDQLXquY6Lu2cvb-Y7pTpSGA@mail.gmail.com
Backpatch-through: 11
Bruce Momjian [Tue, 26 Sep 2023 22:54:10 +0000 (18:54 -0400)]
doc: pg_upgrade, clarify standby servers must remain running
Also mention that mismatching primary/standby LSNs should never
happen.
Reported-by: Nikolay Samokhvalov
Discussion: https://postgr.es/m/CAM527d8heqkjG5VrvjU3Xjsqxg41ufUyabD9QZccdAxnpbRH-Q@mail.gmail.com
Backpatch-through: 11
Bruce Momjian [Tue, 26 Sep 2023 21:41:48 +0000 (17:41 -0400)]
pgrowlocks: change lock mode output labels for consistency
Change "Share" to "For Share" and "Key Share" to "For Key Share" for
consistency with other lock mode labels.
BACKWARD COMPATIBILITY BREAK
Reported-by: David Cook
Discussion: https://postgr.es/m/CA+dNBPNBf+FCEwohe7SH1tSks0R_G4F=tuvM=hnPs4qWiAH8vg@mail.gmail.com
Backpatch-through: master
Bruce Momjian [Tue, 26 Sep 2023 21:31:06 +0000 (17:31 -0400)]
doc: mention GROUP BY columns can reference target col numbers
Reported-by: hape <postgres-hape@gmx.de>
Discussion: https://postgr.es/m/
168871536004.379168.
9352636188330923805@wrigleys.postgresql.org
Backpatch-through: 11
Peter Eisentraut [Tue, 26 Sep 2023 21:08:58 +0000 (22:08 +0100)]
pgbench: Improve help output of -I option
Add a description of the step letters to the --help output.
Author: Gurjeet Singh <gurjeet@singh.im>
Reviewed-by: Tristen Raab <tristen.raab@highgo.ca>
Discussion: https://www.postgresql.org/message-id/flat/CABwTF4Xbc=K4tFj5Znc8jx0GCufQa577GCDsWD7=71qDnUEOyQ@mail.gmail.com