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/
[email protected]
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 <[email protected]>
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[email protected]
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 <
[email protected]>
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 <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
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/
[email protected]
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[email protected]
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[email protected]
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[email protected]
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[email protected]
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 <[email protected]>
Discussion: https://postgr.es/m/
20230928222248[email protected]
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 <
[email protected]>
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) <[email protected]>
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 <
[email protected]>
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 <
[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Reported-By: Kurt Roeckx <[email protected]>
Reported-By: Jaime Casanova <[email protected]>
Reported-By: Lauri Laanmets <[email protected]>
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[email protected]
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 <
[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/
20221101055132[email protected]
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 <
[email protected]>
Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/
20221101055132[email protected]
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: [email protected]
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 <[email protected]>
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 <
[email protected]>
Reviewed-by: Tristen Raab <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CABwTF4Xbc=K4tFj5Znc8jx0GCufQa577GCDsWD7=71qDnUEOyQ@mail.gmail.com
Bruce Momjian [Tue, 26 Sep 2023 21:07:14 +0000 (17:07 -0400)]
doc: correct reference to pg_relation in comment
Reported-by: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/
[email protected]
Backpatch-through: master
Peter Eisentraut [Tue, 26 Sep 2023 15:08:35 +0000 (16:08 +0100)]
MergeAttributes() and related variable renaming
Mainly, rename "schema" to "columns" and related changes. The
previous naming has long been confusing.
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da%40eisentraut.org
Peter Eisentraut [Tue, 26 Sep 2023 13:01:53 +0000 (14:01 +0100)]
Clean up MergeCheckConstraint()
If the constraint is not already in the list, add it ourselves,
instead of making the caller do it. This makes the interface more
consistent with other "merge" functions in this file.
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da%40eisentraut.org
Heikki Linnakangas [Tue, 26 Sep 2023 11:14:49 +0000 (14:14 +0300)]
Fix another bug in parent page splitting during GiST index build.
Yet another bug in the ilk of commits
a7ee7c851 and
741b88435. In
741b88435, we took care to clear the memorized location of the
downlink when we split the parent page, because splitting the parent
page can move the downlink. But we missed that even *updating* a tuple
on the parent can move it, because updating a tuple on a gist page is
implemented as a delete+insert, so the updated tuple gets moved to the
end of the page.
This commit fixes the bug in two different ways (belt and suspenders):
1. Clear the downlink when we update a tuple on the parent page, even
if it's not split. This the same approach as in commits
a7ee7c851
and
741b88435.
I also noticed that gistFindCorrectParent did not clear the
'downlinkoffnum' when it stepped to the right sibling. Fix that
too, as it seems like a clear bug even though I haven't been able
to find a test case to hit that.
2. Change gistFindCorrectParent so that it treats 'downlinkoffnum'
merely as a hint. It now always first checks if the downlink is
still at that location, and if not, it scans the page like before.
That's more robust if there are still more cases where we fail to
clear 'downlinkoffnum' that we haven't yet uncovered. With this,
it's no longer necessary to meticulously clear 'downlinkoffnum',
so this makes the previous fixes unnecessary, but I didn't revert
them because it still seems nice to clear it when we know that the
downlink has moved.
Also add the test case using the same test data that Alexander
posted. I tried to reduce it to a smaller test, and I also tried to
reproduce this with different test data, but I was not able to, so
let's just include what we have.
Backpatch to v12, like the previous fixes.
Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/18129-
caca016eaf0c3702@postgresql.org
Peter Eisentraut [Tue, 26 Sep 2023 10:28:57 +0000 (11:28 +0100)]
Add some const qualifiers
There was a mismatch between the const qualifiers for
excludeDirContents in src/backend/backup/basebackup.c and
src/bin/pg_rewind/filemap.c, which led to a quick search for similar
cases. We should make excludeDirContents match, but the rest of the
changes seem like a good idea as well.
Author: David Steele <
[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/
669a035c-d23d-2f38-7ff0-
0cb93e01d610@pgmasters.net
Peter Eisentraut [Tue, 26 Sep 2023 08:09:36 +0000 (09:09 +0100)]
Clean up MergeAttributesIntoExisting()
Make variable naming clearer and more consistent. Move some variables
to smaller scope. Remove some unnecessary intermediate variables.
Try to save some vertical space.
Apply analogous changes to nearby MergeConstraintsIntoExisting() and
RemoveInheritance() for consistency.
Discussion: https://www.postgresql.org/message-id/flat/
52a125e4-ff9a-95f5-9f61-
b87cf447e4da%40eisentraut.org
Peter Eisentraut [Thu, 24 Aug 2023 06:26:13 +0000 (08:26 +0200)]
Remove unused include
This was added in
add5cf28d4 but was apparently never used.
Discussion: https://www.postgresql.org/message-id/flat/
f84640e3-00d3-5abd-3f41-
e6a19d33c40b@eisentraut.org
Michael Paquier [Tue, 26 Sep 2023 00:29:47 +0000 (09:29 +0900)]
Fix behavior of "force" in pgstat_report_wal()
As implemented in
5891c7a8ed8f, setting "force" to true in
pgstat_report_wal() causes the routine to not wait for the pgstat
shmem lock if it cannot be acquired, in which case the WAL and I/O
statistics finish by not being flushed. The origin of the confusion
comes from pgstat_flush_wal() and pgstat_flush_io(), that use "nowait"
as sole argument. The I/O stats are new in v16.
This is the opposite behavior of what has been used in
pgstat_report_stat(), where "force" is the opposite of "nowait". In
this case, when "force" is true, the routine sets "nowait" to false,
which would cause the routine to wait for the pgstat shmem lock,
ensuring that the stats are always flushed. When "force" is false,
"nowait" is set to true, and the stats would only not be flushed if the
pgstat shmem lock can be acquired, returning immediately without
flushing the stats if the lock cannot be acquired.
This commit changes pgstat_report_wal() so as "force" has the same
behavior as in pgstat_report_stat(). There are currently three callers
of pgstat_report_wal():
- Two in the checkpointer where force=true during a shutdown and the
main checkpointer loop. Now the code behaves so as the stats are always
flushed.
- One in the main loop of the bgwriter, where force=false. Now the code
behaves so as the stats would not be flushed if the pgstat shmem lock
could not be acquired.
Before this commit, some stats on WAL and I/O could have been lost after
a shutdown, for example.
Reported-by: Ryoga Yoshida
Author: Ryoga Yoshida, Michael Paquier
Discussion: https://postgr.es/m/
f87a4d7be70530606b864fd1df91718c@oss.nttdata.com
Backpatch-through: 15
Michael Paquier [Mon, 25 Sep 2023 23:16:12 +0000 (08:16 +0900)]
doc: Tell about "vcregress taptest" for regression tests on Windows
There was no mention of this command in the documentation, and it is
useful to run the TAP tests of a target source directory.
Author: Yugo Nagata
Discussion: https://postgr.es/m/
20230925153204.
926d685d347ee1c8f527090c@sraoss.co.jp
Backpatch-through: 11
Thomas Munro [Mon, 25 Sep 2023 20:07:26 +0000 (09:07 +1300)]
Fix edge-case for xl_tot_len broken by
bae868ca.
bae868ca removed a check that was still needed. If you had an
xl_tot_len at the end of a page that was too small for a record header,
but not big enough to span onto the next page, we'd immediately perform
the CRC check using a bogus large length. Because of arbitrary coding
differences between the CRC implementations on different platforms,
nothing very bad happened on common modern systems. On systems using
the _sb8.c fallback we could segfault.
Restore that check, add a new assertion and supply a test for that case.
Back-patch to 12, like
bae868ca.
Tested-by: Tom Lane <[email protected]>
Tested-by: Alexander Lakhin <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKGLCkTT7zYjzOxuLGahBdQ%3DMcF%3Dz5ZvrjSOnW4EDhVjT-g%40mail.gmail.com
Nathan Bossart [Mon, 25 Sep 2023 21:12:43 +0000 (14:12 -0700)]
Add worker type to pg_stat_subscription.
Thanks to commit
2a8b40e368, the logical replication worker type is
easily determined. The worker type could already be deduced via
other columns such as leader_pid and relid, but that is unnecessary
complexity for users.
Bumps catversion.
Author: Peter Smith
Reviewed-by: Michael Paquier, Maxim Orlov, Amit Kapila
Discussion: https://postgr.es/m/CAHut%2BPtmbSMfErSk0S7xxVdZJ9XVE3xVLhqBTmT91kf57BeKDQ%40mail.gmail.com
Andres Freund [Mon, 25 Sep 2023 18:50:02 +0000 (11:50 -0700)]
pg_dump: tests: Correct test condition for invalid databases
For some reason I used not_like = { pg_dumpall_dbprivs => 1, } in the test
condition of one of the tests added in in
c66a7d75e65. That doesn't make sense
for two reasons: 1) not_like isn't a valid test condition 2) the database
should not be dumped in any of the tests. Due to 1), the test achieved its
goal, but clearly the formulation is confusing. Instead use like => {}, with
a comment explaining why.
Reported-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/
3ddf79f2-8b7b-a093-11d2-
5c739bc64f86@eisentraut.org
Backpatch: 11-, like
c66a7d75e65
Tom Lane [Mon, 25 Sep 2023 18:41:57 +0000 (14:41 -0400)]
Collect dependency information for parsed CallStmts.
Parse analysis of a CallStmt will inject mutable information,
for instance the OID of the called procedure, so that subsequent
DDL may create a need to re-parse the CALL. We failed to detect
this for CALLs in plpgsql routines, because no dependency information
was collected when putting a CallStmt into the plan cache. That
could lead to misbehavior or strange errors such as "cache lookup
failed".
Before commit
ee895a655, the issue would only manifest for CALLs
appearing in atomic contexts, because we re-planned non-atomic
CALLs every time through anyway.
It is now apparent that extract_query_dependencies() probably
needs a special case for every utility statement type for which
stmt_requires_parse_analysis() returns true. I wanted to add
something like Assert(!stmt_requires_parse_analysis(...)) when
falling out of extract_query_dependencies_walker without doing
anything, but there are API issues as well as a more fundamental
point: stmt_requires_parse_analysis is supposed to be applied to
raw parser output, so it'd be cheating to assume it will give the
correct answer for post-parse-analysis trees. I contented myself
with adding a comment.
Per bug #18131 from Christian Stork. Back-patch to all supported
branches.
Discussion: https://postgr.es/m/18131-
576854e79c5cd264@postgresql.org
Andres Freund [Mon, 25 Sep 2023 17:36:04 +0000 (10:36 -0700)]
docs: Clarify --with-segsize-blocks documentation
Without the added "relation" it's not immediately clear that the option
relates to the relation segment size and not e.g. the WAL segment size.
The option was added in
d3b111e32.
Reported-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/837536.
1695348498@sss.pgh.pa.us
Backpatch: 16-
Tom Lane [Mon, 25 Sep 2023 16:07:32 +0000 (12:07 -0400)]
Pack struct ParsedWord more tightly.
In a 64-bit build there's an awful lot of useless pad space in
ParsedWords. Since we may allocate large arrays of these,
it's worth some effort to reduce their size.
Here we reduce the alen field from uint32 to uint16, and then re-order
the fields to avoid unnecessary padding. alen is only used to
remember the allocated size of the apos[] array, which is not allowed
to exceed MAXNUMPOS (256) elements, so uint16 is plenty of space for
it. That gets us from 40 bytes to 24 on 64-bit builds, and from 20
bytes to 16 on 32-bit builds.
Per discussion of bug #18080. Unfortunately this is an ABI break
so we can't back-patch.
Discussion: https://postgr.es/m/
1146921.
1695411070@sss.pgh.pa.us
Tom Lane [Mon, 25 Sep 2023 15:50:28 +0000 (11:50 -0400)]
Limit to_tsvector_byid's initial array allocation to something sane.
The initial estimate of the number of distinct ParsedWords is just
that: an estimate. Don't let it exceed what palloc is willing to
allocate. If in fact we need more entries, we'll eventually fail
trying to enlarge the array. But if we don't, this allows success on
inputs that currently draw "invalid memory alloc request size".
Per bug #18080 from Uwe Binder. Back-patch to all supported branches.
Discussion: https://postgr.es/m/18080-
d5c5e58fef8c99b7@postgresql.org