Andrew Dunstan [Sat, 24 Apr 2021 14:13:07 +0000 (10:13 -0400)]
Provide pg_amcheck with an --install-missing option
This will install amcheck in the database if not present. The default
schema is for the extension is pg_catalog, but this can be overridden by
providing a value for the option.
Mark Dilger, slightly editorialized by me.
(rather divergent)
Discussion: https://postgr.es/m/
bdc0f7c2-09e3-ee57-8471-
569dfb509234@dunslane.net
Andrew Dunstan [Sat, 24 Apr 2021 13:37:20 +0000 (09:37 -0400)]
Teach PostgresVersion all the ways to mark non-release code
As well as 'devel' version_stamp.pl provides for 'alphaN'
'betaN' and 'rcN', so teach PostgresVersion about those.
Also stash the version string instead of trying to reconstruct it during
stringification.
Discussion: https://postgr.es/m/YIHlw5nSgAHs4dK1@paquier.xyz
Michael Paquier [Sat, 24 Apr 2021 06:07:04 +0000 (15:07 +0900)]
Fix come comments in execMain.c
1375422 has refactored this area of the executor code, and some comments
went out-of-sync.
Author: Yukun Wang
Reviewed-by: Amul Sul
Discussion: https://postgr.es/m/OS0PR01MB60033394FCAEF79B98F078F5B4459@OS0PR01MB6003.jpnprd01.prod.outlook.com
Michael Paquier [Sat, 24 Apr 2021 01:44:13 +0000 (10:44 +0900)]
Doc: Remove extraneous whitespaces with some tags
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20210423184338.GL7256@telsasoft.com
Michael Paquier [Sat, 24 Apr 2021 00:09:02 +0000 (09:09 +0900)]
Add some forgotten LSN_FORMAT_ARGS() in xlogreader.c
6f6f284 has introduced a specific macro to make printf()-ing of LSNs
easier. This takes care of what looks like the remaining code paths
that did not get the call.
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, Tom Lane
Discussion: https://postgr.es/m/YIJS9x6K8ruizN7j@paquier.xyz
Peter Geoghegan [Fri, 23 Apr 2021 22:37:03 +0000 (15:37 -0700)]
amcheck: MAXALIGN() nbtree special area offset.
This isn't strictly necessary, but in theory it might matter if in the
future the width of the nbtree special area changes -- its total size
might not be an even number of MAXALIGN() quantums, even with padding.
PageInit() MAXALIGN()s all special area offsets, but amcheck uses the
offset to perform initial basic validation of line pointers, so we don't
rely on the offset from the page header.
The real reason to do this is to set a good example for new code that
adds amcheck coverage for other index AMs.
Reported-By: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CALj2ACUMqTR9nErh99FbOBmzCXE9=gXNqhBiwYOhejJJS1LXqQ@mail.gmail.com
Peter Eisentraut [Fri, 23 Apr 2021 12:18:11 +0000 (14:18 +0200)]
Factor out system call names from error messages
Instead, put them in via a format placeholder. This reduces the
number of distinct translatable messages and also reduces the chances
of typos during translation. We already did this for the system call
arguments in a number of cases, so this is just the same thing taken a
bit further.
Discussion: https://www.postgresql.org/message-id/flat/
92d6f545-5102-65d8-3c87-
489f71ea0a37%40enterprisedb.com
Peter Eisentraut [Fri, 23 Apr 2021 11:27:01 +0000 (13:27 +0200)]
Use correct format placeholder for WSAGetLastError()
Some code thought this was unsigned, but it's signed int.
Alexander Korotkov [Fri, 23 Apr 2021 09:57:33 +0000 (12:57 +0300)]
Mark multirange_constructor0() and multirange_constructor2() strict
These functions shouldn't receive null arguments: multirange_constructor0()
doesn't have any arguments while multirange_constructor2() has a single array
argument, which is never null.
But mark them strict anyway for the sake of uniformity.
Also, make checks for null arguments use elog() instead of ereport() as these
errors should normally be never thrown. And adjust corresponding comments.
Catversion is bumped.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/
0f783a96-8d67-9e71-996b-
f34a7352eeef%40enterprisedb.com
Fujii Masao [Fri, 23 Apr 2021 10:10:24 +0000 (19:10 +0900)]
Reorder COMPRESSION option in gram.y and parsenodes.h into alphabetical order.
Commit
bbe0a81db6 introduced "INCLUDING COMPRESSION" option
in CREATE TABLE command, but previously TableLikeOption in gram.y and
parsenodes.h didn't classify this new option in alphabetical order
with the rest.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/YHerAixOhfR1ryXa@paquier.xyz
Magnus Hagander [Fri, 23 Apr 2021 09:41:36 +0000 (11:41 +0200)]
Mention that toplevel is part of pg_stat_statements key.
While at it, also document that toplevel is always true if
pg_stat_statements.track is set to top.
Author: Julien Rouhaud
Reported-By: Fujii Masao
Discussion: https://postgr.es/m/
a878d5ea-64a7-485e-5d2f-
177618ebc52d@oss.nttdata.com
Peter Eisentraut [Fri, 23 Apr 2021 07:55:23 +0000 (09:55 +0200)]
pg_amcheck: Use logging functions
This was already mostly done, but some error messages were printed the
long way.
Peter Eisentraut [Fri, 23 Apr 2021 07:28:44 +0000 (09:28 +0200)]
doc: Fix typos
Author: Justin Pryzby <pryzby@telsasoft.com>
Fujii Masao [Fri, 23 Apr 2021 06:45:46 +0000 (15:45 +0900)]
doc: Fix obsolete description about pg_basebackup.
Previously it was documented that if using "-X none" option there was
no guarantee that all required WAL files were archived at the end of
pg_basebackup when taking a backup from the standby. But this limitation
was removed by commit
52f8a59dd9. Now, even when taking a backup
from the standby, pg_basebackup can wait for all required WAL files
to be archived. Therefore this commit removes such obsolete
description from the docs.
Also this commit adds new description about the limitation when
taking a backup from the standby, into the docs. The limitation is that
pg_basebackup cannot force the standbfy to switch to a new WAL file
at the end of backup, which may cause pg_basebackup to wait a long
time for the last required WAL file to be switched and archived,
especially when write activity on the primary is low.
Back-patch to v10 where the issue was introduced.
Reported-by: Kyotaro Horiguchi
Author: Kyotaro Horiguchi, Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/
20210420.133235.
1342729068750553399.horikyota.ntt@gmail.com
Peter Eisentraut [Fri, 23 Apr 2021 05:21:13 +0000 (07:21 +0200)]
Fix incorrect format placeholder
Michael Paquier [Fri, 23 Apr 2021 04:34:02 +0000 (13:34 +0900)]
Fix some comments in fmgr.c
Oversight in
2a0faed.
Author: Hou Zhijie
Discussion: https://postgr.es/m/OS0PR01MB5716405E2464D85E6DB6DC0794469@OS0PR01MB5716.jpnprd01.prod.outlook.com
Michael Paquier [Fri, 23 Apr 2021 04:25:49 +0000 (13:25 +0900)]
Remove use of [U]INT64_FORMAT in some translatable strings
%lld with (long long), or %llu with (unsigned long long) are more
adapted. This is similar to
3286065.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/
20210421.200000.
1462448394029407895.horikyota.ntt@gmail.com
Etsuro Fujita [Fri, 23 Apr 2021 03:00:00 +0000 (12:00 +0900)]
Minor code cleanup in asynchronous execution support.
This is cleanup for commit
27e1f1456:
* ExecAppendAsyncEventWait(), which was modified a bit further by commit
a8af856d3, duplicated the same nevents calculation. Simplify the code
a little bit to avoid the duplication. Update comments there.
* Add an assertion to ExecAppendAsyncRequest().
* Update a comment about merging the async_capable options from input
relations in merge_fdw_options(), per complaint from Kyotaro Horiguchi.
* Add a comment for fetch_more_data_begin().
Author: Etsuro Fujita
Discussion: https://postgr.es/m/CAPmGK1637W30Wx3MnrReewhafn6F_0J76mrJGoFXFnpPq4QfvA%40mail.gmail.com
Tom Lane [Thu, 22 Apr 2021 21:30:42 +0000 (17:30 -0400)]
Don't crash on reference to an un-available system column.
Adopt a more consistent policy about what slot-type-specific
getsysattr functions should do when system attributes are not
available. To wit, they should all throw the same user-oriented
error, rather than variously crashing or emitting developer-oriented
messages.
This closes a identifiable problem in commits
a71cfc56b and
3fb93103a (in v13 and v12), so back-patch into those branches,
along with a test case to try to ensure we don't break it again.
It is not known that any of the former crash cases are reachable
in HEAD, but this seems like a good safety improvement in any case.
Discussion: https://postgr.es/m/
141051591267657@mail.yandex.ru
Peter Eisentraut [Thu, 22 Apr 2021 20:47:57 +0000 (22:47 +0200)]
Fix some trailing whitespace in documentation files
Alvaro Herrera [Thu, 22 Apr 2021 20:04:48 +0000 (16:04 -0400)]
Fix uninitialized memory bug
Have interested callers of find_inheritance_children set the
detached_exist value to false prior to calling it, so that that routine
only has to set it true in the rare cases where it is necessary. Don't
touch it otherwise.
Per buildfarm member thorntail (which reported a UBSan failure here).
Bruce Momjian [Thu, 22 Apr 2021 20:01:17 +0000 (16:01 -0400)]
doc: mention <link> can be inside of <command>, but not <xref>
This was discussed in commit
9081bddbd.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/flat/87o8pco34z.fsf@wibble.ilmari.org
Andrew Dunstan [Thu, 22 Apr 2021 19:25:37 +0000 (15:25 -0400)]
Make PostgresVersion code a bit more robust and simple.
per gripe from Alvaro Herrera.
Alvaro Herrera [Thu, 22 Apr 2021 19:13:25 +0000 (15:13 -0400)]
Fix relcache inconsistency hazard in partition detach
During queries coming from ri_triggers.c, we need to omit partitions
that are marked pending detach -- otherwise, the RI query is tricked
into allowing a row into the referencing table whose corresponding row
is in the detached partition. Which is bogus: once the detach operation
completes, the row becomes an orphan.
However, the code was not doing that in repeatable-read transactions,
because relcache kept a copy of the partition descriptor that included
the partition, and used it in the RI query. This commit changes the
partdesc cache code to only keep descriptors that aren't dependent on
a snapshot (namely: those where no detached partition exist, and those
where detached partitions are included). When a partdesc-without-
detached-partitions is requested, we create one afresh each time; also,
those partdescs are stored in PortalContext instead of
CacheMemoryContext.
find_inheritance_children gets a new output *detached_exist boolean,
which indicates whether any partition marked pending-detach is found.
Its "include_detached" input flag is changed to "omit_detached", because
that name captures desired the semantics more naturally.
CreatePartitionDirectory() and RelationGetPartitionDesc() arguments are
identically renamed.
This was noticed because a buildfarm member that runs with relcache
clobbering, which would not keep the improperly cached partdesc, broke
one test, which led us to realize that the expected output of that test
was bogus. This commit also corrects that expected output.
Author: Amit Langote <amitlangote09@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/
3269784.
1617215412@sss.pgh.pa.us
Tom Lane [Thu, 22 Apr 2021 18:47:26 +0000 (14:47 -0400)]
Doc: document the tie-breaking behavior of the round() function.
Back-patch to v13; the table layout in older branches is unfriendly
to adding such details.
Laurenz Albe
Discussion: https://postgr.es/m/
161881920775.685.
12293798764864559341@wrigleys.postgresql.org
Andrew Dunstan [Thu, 22 Apr 2021 14:56:28 +0000 (10:56 -0400)]
Make PostgresNode version aware
A new PostgresVersion object type is created and this is used in
PostgresNode using the output of `pg_config --version` and the result
stored in the PostgresNode object. This object can be compared to other
PostgresVersion objects, or to a number or string.
PostgresNode is currently believed to be compatible with versions down
to release 12, so PostgresNode will issue a warning if used with a
version prior to that.
No attempt has been made to deal with incompatibilities in older
versions - that remains work to be undertaken in a subsequent
development cycle.
Based on code from Mark Dilger and Jehan-Guillaume de Rorthais.
Discussion: https://postgr.es/m/
a80421c0-3d7e-def1-bcfe-
24777f15e344@dunslane.net
Michael Paquier [Thu, 22 Apr 2021 03:48:54 +0000 (12:48 +0900)]
Fix relation leak for subscribers firing triggers in logical replication
Creating a trigger on a relation to which an apply operation is
triggered would cause a relation leak once the change gets committed,
as the executor would miss that the relation needs to be closed
beforehand. This issue got introduced with the refactoring done in
1375422c, where it becomes necessary to track relations within
es_opened_result_relations to make sure that they are closed.
We have discussed using ExecInitResultRelation() coupled with
ExecCloseResultRelations() for the relations in need of tracking by the
apply operations in the subscribers, which would simplify greatly the
opening and closing of indexes, but this requires a larger rework and
reorganization of the worker code, particularly for the tuple routing
part. And that's not really welcome post feature freeze. So, for now,
settle down to the same solution as TRUNCATE which is to fill in
es_opened_result_relations with the relation opened, to make sure that
ExecGetTriggerResultRel() finds them and that they get closed.
The code is lightly refactored so as a relation is not registered three
times for each DML code path, making the whole a bit easier to follow.
Reported-by: Tang Haiying, Shi Yu, Hou Zhijie
Author: Amit Langote, Masahiko Sawada, Hou Zhijie
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/OS0PR01MB611383FA0FE92EB9DE21946AFB769@OS0PR01MB6113.jpnprd01.prod.outlook.com
Michael Paquier [Thu, 22 Apr 2021 00:47:43 +0000 (09:47 +0900)]
doc: Move parallel_leader_participation to its correct category
parallel_leader_participation got introduced in
e5253fd, where it was
listed under RESOURCES_ASYNCHRONOUS in guc.c, but the documentation
did not reflect that and listed it with the other planner-related
options. This commit fixes this inconsistency as the parameter is
intended to be an asynchronous one.
While on it, reorganize a bit the section dedicated to asynchronous
parameters, backend_flush_after being moved first to do better in terms
of alphabetical order of the options listed.
Reported-by: Yanliang Lei
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/16972-
42d4b0c15aa1d5f5@postgresql.org
Alvaro Herrera [Wed, 21 Apr 2021 22:36:12 +0000 (18:36 -0400)]
Add comment about extract_autovac_opts not holding lock
Per observation from Tom Lane.
Discussion: https://postgr.es/m/
1901125.
1617904665@sss.pgh.pa.us
Alvaro Herrera [Wed, 21 Apr 2021 22:12:05 +0000 (18:12 -0400)]
Don't add a redundant constraint when detaching a partition
On ALTER TABLE .. DETACH CONCURRENTLY, we add a new table constraint
that duplicates the partition constraint. But if the partition already
has another constraint that implies that one, then that's unnecessary.
We were already avoiding the addition of a duplicate constraint if there
was an exact 'equal' match -- this just improves the quality of the check.
Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/
20210410184226.GY6592@telsasoft.com
Andrew Dunstan [Wed, 21 Apr 2021 15:12:04 +0000 (11:12 -0400)]
fix silly perl error in commit
d064afc720
Peter Eisentraut [Wed, 21 Apr 2021 14:02:03 +0000 (16:02 +0200)]
Update config.guess and config.sub
Andrew Dunstan [Wed, 21 Apr 2021 14:21:22 +0000 (10:21 -0400)]
Only ever test for non-127.0.0.1 addresses on Windows in PostgresNode
This has been found to cause hangs where tcp usage is forced.
Alexey Kodratov
Discussion: https://postgr.es/m/
82e271a9a11928337fcb5b5e57b423c0@postgrespro.ru
Backpatch to all live branches
Peter Eisentraut [Wed, 21 Apr 2021 09:54:47 +0000 (11:54 +0200)]
Add DISTINCT to information schema usage views
Since pg_depend can contain duplicate entries, we need to eliminate
those in information schema views that build on pg_depend, using
DISTINCT. Some of the older views already did that correctly, but
some of the more recently added ones didn't. (In some of these views,
it might not be possible to reproduce the issue because of how the
implementation happens to deduplicate dependencies while recording
them, but it seems better to keep this consistent in all cases.)
Peter Eisentraut [Wed, 21 Apr 2021 06:26:18 +0000 (08:26 +0200)]
Use correct format placeholder for timeline IDs
Should be %u rather than %d.
Peter Eisentraut [Wed, 21 Apr 2021 06:14:43 +0000 (08:14 +0200)]
doc: Improve hyphenation consistency
Peter Eisentraut [Mon, 19 Apr 2021 20:48:13 +0000 (22:48 +0200)]
Don't use INT64_FORMAT inside message strings
Use %lld and cast to long long int instead.
Peter Eisentraut [Mon, 19 Apr 2021 20:47:43 +0000 (22:47 +0200)]
Fix typo
Michael Paquier [Wed, 21 Apr 2021 03:02:41 +0000 (12:02 +0900)]
doc: List compute_query_id in required config for pg_stat_statements
Not enabling compute_query_id would disable pg_stat_statements even if
the module is listed in shared_preload_libraries, so add it to the
minimum configuration set as listed in its documentation.
Author: Greg Nancarrow
Reviewed-by: Julien Rouhaud, Bharath Rupireddy
Discussion: https://postgr.es/m/CAJcOf-fXyb2QiDbwftD813UF70w-+BsK-03bFp1GrijXU9GQYQ@mail.gmail.com
Michael Paquier [Wed, 21 Apr 2021 01:34:43 +0000 (10:34 +0900)]
Add CURRENT_ROLE to list of roles for tab completion of GRANT in psql
This compatibility has been added in
45b9805, but psql forgot the call.
Author: Wei Wang
Reviewed-by: Aleksander Alekseev
Discussion: https://postgr.es/m/OS3PR01MB6275935F62E161BCD393D6559E489@OS3PR01MB6275.jpnprd01.prod.outlook.com
Tom Lane [Tue, 20 Apr 2021 21:01:43 +0000 (17:01 -0400)]
Improve WAL record descriptions for SP-GiST records.
While tracking down the bug fixed in the preceding commit, I got quite
annoyed by the low quality of spg_desc's output. Add missing fields,
try to make the formatting consistent.
Tom Lane [Tue, 20 Apr 2021 20:58:30 +0000 (16:58 -0400)]
Fix under-parenthesized XLogRecHasBlockRef() macro.
Commit
f003d9f87 left this macro with inadequate (or, one could say,
too much) parenthesization. Which was catastrophic to the correctness
of calls such as "if (!XLogRecHasBlockRef(record, 1)) ...". There
are only a few of those, which perhaps explains why we didn't notice
immediately (with our general weakness of WAL replay testing being
another factor). I found it by debugging intermittent replay failures
like
2021-04-08 14:33:30.191 EDT [29463] PANIC: failed to locate backup block with ID 1
2021-04-08 14:33:30.191 EDT [29463] CONTEXT: WAL redo at 0/
95D3438 for SPGist/ADD_NODE: off 1; blkref #0: rel 1663/16384/25998, blk 1
Bruce Momjian [Tue, 20 Apr 2021 16:57:59 +0000 (12:57 -0400)]
Fix interaction of log_line_prefix's query_id and log_statement
log_statement is issued before query_id can be computed, so properly
clear the value, and document the interaction.
Reported-by: Fujii Masao, Michael Paquier
Discussion: https://postgr.es/m/YHPkU8hFi4no4NSw@paquier.xyz
Author: Julien Rouhaud
Bruce Momjian [Tue, 20 Apr 2021 16:22:26 +0000 (12:22 -0400)]
adjust query id feature to use pg_stat_activity.query_id
Previously, it was pg_stat_activity.queryid to match the
pg_stat_statements queryid column. This is an adjustment to patch
4f0b0966c8. This also adjusts some of the internal function calls to
match. Catversion bumped.
Reported-by: Álvaro Herrera, Julien Rouhaud
Discussion: https://postgr.es/m/
20210408032704.GA7498@alvherre.pgsql
Tom Lane [Tue, 20 Apr 2021 15:37:36 +0000 (11:37 -0400)]
Rename find_em_expr_usable_for_sorting_rel.
I didn't particularly like this function name, as it fails to
express what's going on. Also, returning the sort expression
alone isn't too helpful --- typically, a caller would also
need some other fields of the EquivalenceMember. But the
sole caller really only needs a bool result, so let's make
it "bool relation_can_be_sorted_early()".
Discussion: https://postgr.es/m/
91f3ec99-85a4-fa55-ea74-
33f85a5c651f@swarm64.com
Tom Lane [Tue, 20 Apr 2021 15:32:02 +0000 (11:32 -0400)]
Fix planner failure in some cases of sorting by an aggregate.
An oversight introduced by the incremental-sort patches caused
"could not find pathkey item to sort" errors in some situations
where a sort key involves an aggregate or window function.
The basic problem here is that find_em_expr_usable_for_sorting_rel
isn't properly modeling what prepare_sort_from_pathkeys will do
later. Rather than hoping we can keep those functions in sync,
let's refactor so that they actually share the code for
identifying a suitable sort expression.
With this refactoring, tlist.c's tlist_member_ignore_relabel
is unused. I removed it in HEAD but left it in place in v13,
in case any extensions are using it.
Per report from Luc Vlaming. Back-patch to v13 where the
problem arose.
James Coleman and Tom Lane
Discussion: https://postgr.es/m/
91f3ec99-85a4-fa55-ea74-
33f85a5c651f@swarm64.com
Andrew Dunstan [Tue, 20 Apr 2021 14:14:16 +0000 (10:14 -0400)]
Avoid unfortunate IPC::Run path caching in PostgresNode
Commit
b34ca595ab provided for installation-aware instances of
PostgresNode. However, it turns out that IPC::Run works against this by
caching the path to a binary and not consulting the path again, even if
it has changed. We work around this by calling Postgres binaries with
the installed path rather than just a bare name to be looked up in the
environment path, if there is an installed path. For the common case
where there is no installed path we continue to use the bare command
name.
Diagnosis and solution from Mark Dilger
Discussion: https://postgr.es/m/
E8F512F8-B4D6-4514-BA8D-
2E671439DA92@enterprisedb.com
Magnus Hagander [Tue, 20 Apr 2021 12:35:16 +0000 (14:35 +0200)]
Fix typo in comment
Author: Julien Rouhaud
Backpatch-through: 11
Discussion: https://postgr.es/m/
20210420121659.odjueyd4rpilorn5@nol
Peter Geoghegan [Tue, 20 Apr 2021 01:55:31 +0000 (18:55 -0700)]
Document LP_DEAD accounting issues in VACUUM.
Document VACUUM's soft assumption that any LP_DEAD items encountered
during pruning will become LP_UNUSED items before VACUUM finishes up.
This is integral to the accounting used by VACUUM to generate its final
report on the table to the stats collector. It also affects how VACUUM
determines which heap pages are truncatable. In both cases VACUUM is
concerned with the likely contents of the page in the near future, not
the current contents of the page.
This state of affairs created the false impression that VACUUM's dead
tuple accounting had significant difference with similar accounting used
during ANALYZE. There were and are no substantive differences, at least
when the soft assumption completely works out. This is far clearer now.
Also document cases where things don't quite work out for VACUUM's dead
tuple accounting. It's possible that a significant number of LP_DEAD
items will be left behind by VACUUM, and won't be recorded as remaining
dead tuples in VACUUM's statistics collector report. This behavior
dates back to commit
a96c41fe, which taught VACUUM to run without index
and heap vacuuming at the user's request. The failsafe mechanism added
to VACUUM more recently by commit
1e55e7d1 takes the same approach to
dead tuple accounting.
Reported-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=Jmtu18PrsYq3EvvZJGOmZqSO2u3bvKpx9xJa5uhNp=Q@mail.gmail.com
Peter Eisentraut [Mon, 19 Apr 2021 08:43:18 +0000 (10:43 +0200)]
Use correct format placeholder for pids
Should be signed, not unsigned.
Amit Kapila [Mon, 19 Apr 2021 03:32:47 +0000 (09:02 +0530)]
Fix test case added by commit
f5fc2f5b23.
In the new test after resetting the stats, we were not waiting for the
stats message to be delivered. Also, we need to decode the results for
the new test, otherwise, it will show the old stats.
In passing,
a. Change docs added by commit
f5fc2f5b23 as per suggestion by
Justin Pryzby.
b. Bump the PGSTAT_FILE_FORMAT_ID as commit
f5fc2f5b23 changes the file
format of stats.
Reported-by: Tom Lane based on buildfarm reports
Author: Vignesh C, Justin Pryzby
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/
20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de
Michael Paquier [Mon, 19 Apr 2021 02:32:30 +0000 (11:32 +0900)]
Fix typos and grammar in comments and docs
Author: Justin Pryzby
Discussion: https://postgr.es/m/
20210416070310.GG3315@telsasoft.com
Michael Paquier [Mon, 19 Apr 2021 01:15:35 +0000 (10:15 +0900)]
Replace magic constants for seek() calls in perl scripts
A couple of tests have been using 0 as magic constant while SEEK_SET can
be used instead. This makes the code easier to understand, and more
consistent with the changes done in
3c5b068.
Per discussion with Andrew Dunstan.
Discussion: https://postgr.es/m/YHrc24AgJQ6tQ1q0@paquier.xyz
Thomas Munro [Sun, 18 Apr 2021 22:22:31 +0000 (10:22 +1200)]
Explain postmaster's treatment of SIGURG.
Add a few words of comment to explain why SIGURG doesn't follow the
dummy_handler pattern used for SIGUSR2, since that might otherwise
appear to be a bug.
Discussion: https://postgr.es/m/
4006115.
1618577212%40sss.pgh.pa.us
Peter Eisentraut [Sun, 18 Apr 2021 14:11:58 +0000 (16:11 +0200)]
Add missing source files to nls.mk
Peter Eisentraut [Sat, 17 Apr 2021 12:14:26 +0000 (14:14 +0200)]
doc: Fix up spacing around verbatim DocBook elements
Peter Eisentraut [Sat, 17 Apr 2021 07:40:50 +0000 (09:40 +0200)]
Use correct format placeholder for block numbers
Should be %u rather than %d.
Tom Lane [Sat, 17 Apr 2021 02:23:46 +0000 (22:23 -0400)]
Rethink extraction of collation dependencies.
As it stands, find_expr_references_walker() pays attention to leaf-node
collation fields while ignoring the input collations of actual function
and operator nodes. That seems exactly backwards from a semantic
standpoint, and it leads to reporting dependencies on collations that
really have nothing to do with the expression's behavior.
Hence, rewrite to look at function input collations instead. This
isn't completely perfect either; it fails to account for the behavior
of record_eq and its siblings. (The previous coding at least gave an
approximation of that, though I think it could be fooled pretty easily
into considering the columns of irrelevant composite types.) We may
be able to improve on this later, but for now this should satisfy the
buildfarm members that didn't like
ef387bed8.
In passing fix some oversights in GetTypeCollations(), and get
rid of its duplicative de-duplications. (I'm worried that it's
still potentially O(N^2) or worse, but this makes it a little
better.)
Discussion: https://postgr.es/m/
3564817.
1618420687@sss.pgh.pa.us
Tom Lane [Fri, 16 Apr 2021 23:01:22 +0000 (19:01 -0400)]
Update dummy prosrc values.
Ooops, forgot to s/system_views.sql/system_functions.sql/g
in this part of
767982e36.
No need for an additional catversion bump, I think, since
these strings are gone by the time initdb finishes.
Discussion: https://postgr.es/m/
3956760.
1618529139@sss.pgh.pa.us
Tom Lane [Fri, 16 Apr 2021 22:36:45 +0000 (18:36 -0400)]
Convert built-in SQL-language functions to SQL-standard-body style.
Adopt the new pre-parsed representation for all built-in and
information_schema SQL-language functions, except for a small
number that can't presently be converted because they have
polymorphic arguments.
This eliminates residual hazards around search-path safety of
these functions, and might provide some small performance benefits
by reducing parsing costs. It seems useful also to provide more
test coverage for the SQL-standard-body feature.
Discussion: https://postgr.es/m/
3956760.
1618529139@sss.pgh.pa.us
Tom Lane [Fri, 16 Apr 2021 22:20:42 +0000 (18:20 -0400)]
Split function definitions out of system_views.sql into a new file.
Invent system_functions.sql to carry the function definitions that
were formerly in system_views.sql. The function definitions were
already a quarter of the file and are about to be more, so it seems
appropriate to give them their own home.
In passing, fix an oversight in
dfb75e478: it neglected to call
check_input() for system_constraints.sql.
Discussion: https://postgr.es/m/
3956760.
1618529139@sss.pgh.pa.us
Andrew Dunstan [Fri, 16 Apr 2021 20:54:04 +0000 (16:54 -0400)]
Allow TestLib::slurp_file to skip contents, and use as needed
In order to avoid getting old logfile contents certain functions in
PostgresNode were doing one of two things. On Windows it rotated the
logfile and restarted the server, while elsewhere it truncated the log
file. Both of these are unnecessary. We borrow from the buildfarm which
does this instead: note the size of the logfile before we start, and
then when fetching the logfile skip to that position before accumulating
contents. This is spelled differently on Windows but the effect is the
same. This is largely centralized in TestLib's slurp_file function,
which has a new optional parameter, the offset to skip to before
starting to reading the file. Code in the client becomes much neater.
Backpatch to all live branches.
Michael Paquier, slightly modified by me.
Discussion: https://postgr.es/m/YHajnhcMAI3++pJL@paquier.xyz
Tom Lane [Fri, 16 Apr 2021 16:26:50 +0000 (12:26 -0400)]
Fix bogus collation-version-recording logic.
recordMultipleDependencies had the wrong scope for its "version"
variable, allowing a version label to leak from the collation entry it
was meant for to subsequent non-collation entries. This is relatively
hard to trigger because of the OID-descending order that the inputs
will normally arrive in: subsequent non-collation items will tend to
be pinned. But it can be exhibited easily with a custom collation.
Also, don't special-case the default collation, but instead ignore
pinned-ness of a collation when we've found a version for it. This
avoids creating useless pg_depend entries, and removes a not-very-
future-proof assumption that C, POSIX, and DEFAULT are the only
pinned collations.
A small problem is that, because the default collation may or may
not have a version, the regression tests can't assume anything about
whether dependency entries will be made for it. This seems OK though
since it's now handled just the same as other collations, and we have
test cases for both versioned and unversioned collations.
Fixes oversights in commit
257836a75. Thanks to Julien Rouhaud
for review.
Discussion: https://postgr.es/m/
3564817.
1618420687@sss.pgh.pa.us
Tom Lane [Fri, 16 Apr 2021 15:30:27 +0000 (11:30 -0400)]
Fix wrong units in two ExplainPropertyFloat calls.
This is only a latent bug, since these calls are only reached for
non-text output formats, and currently none of those will print
the units. Still, we should get it right in case that ever changes.
Justin Pryzby
Discussion: https://postgr.es/m/
20210415163846.GA3315@telsasoft.com
Peter Eisentraut [Fri, 16 Apr 2021 09:46:01 +0000 (11:46 +0200)]
psql: Refine lexing of BEGIN...END blocks in CREATE FUNCTION statements
Only track BEGIN...END blocks if they are in a CREATE [OR REPLACE]
{FUNCTION|PROCEDURE} statement. Ignore if in parentheses.
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://www.postgresql.org/message-id/
cee01d26fe55bc086b3bcf10bfe4e8d450e2f608.camel@cybertec.at
Peter Eisentraut [Fri, 16 Apr 2021 09:04:04 +0000 (11:04 +0200)]
psql: Small fixes for better translatability
Michael Paquier [Fri, 16 Apr 2021 07:56:12 +0000 (16:56 +0900)]
doc: Fix typo in example query of SQL/JSON
Author: Erik Rijkers
Discussion: https://postgr.es/m/
1219476687.20432.
1617452918468@webmailclassic.xs4all.nl
Backpatch-through: 12
Amit Kapila [Fri, 16 Apr 2021 02:04:43 +0000 (07:34 +0530)]
Add information of total data processed to replication slot stats.
This adds the statistics about total transactions count and total
transaction data logically sent to the decoding output plugin from
ReorderBuffer. Users can query the pg_stat_replication_slots view to check
these stats.
Suggested-by: Andres Freund
Author: Vignesh C and Amit Kapila
Reviewed-by: Sawada Masahiko, Amit Kapila
Discussion: https://postgr.es/m/
20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de
Thomas Munro [Fri, 16 Apr 2021 01:20:58 +0000 (13:20 +1200)]
Doc: Document known problem with Windows collation versions.
Warn users that locales with traditional Windows NLS names like
"English_United States.1252" won't provide version information, and that
something like initdb --lc-collate=en-US would be needed to fix that
problem for the initial template databases.
Discussion: https://postgr.es/m/CA%2BhUKGJ_hk3rU%3D%3Dg2FpAMChb_4i%2BTJacpjjqFsinY-tRM3FBmA%40mail.gmail.com
Tom Lane [Thu, 15 Apr 2021 21:24:12 +0000 (17:24 -0400)]
Provide query source text when parsing a SQL-standard function body.
Without this, we lose error cursor positions, as shown in the
modified regression test result.
Discussion: https://postgr.es/m/
2197698.
1617984583@sss.pgh.pa.us
Tom Lane [Thu, 15 Apr 2021 21:17:45 +0000 (17:17 -0400)]
Revert "Cope with NULL query string in ExecInitParallelPlan()."
This reverts commit
b3ee4c503872f3d0a5d6a7cbde48815f555af15b.
We don't need it in the wake of the preceding commit, which
added an upstream check that the querystring isn't null.
Discussion: https://postgr.es/m/
2197698.
1617984583@sss.pgh.pa.us
Tom Lane [Thu, 15 Apr 2021 21:17:20 +0000 (17:17 -0400)]
Undo decision to allow pg_proc.prosrc to be NULL.
Commit
e717a9a18 changed the longstanding rule that prosrc is NOT NULL
because when a SQL-language function is written in SQL-standard style,
we don't currently have anything useful to put there. This seems a poor
decision though, as it could easily have negative impacts on external
PLs (opening them to crashes they didn't use to have, for instance).
SQL-function-related code can just as easily test "is prosqlbody not
null" as "is prosrc null", so there's no real gain there either.
Hence, revert the NOT NULL marking removal and adjust related logic.
For now, we just put an empty string into prosrc for SQL-standard
functions. Maybe we'll have a better idea later, although the
history of things like pg_attrdef.adsrc suggests that it's not
easy to maintain a string equivalent of a node tree.
This also adds an assertion that queryDesc->sourceText != NULL
to standard_ExecutorStart. We'd been silently relying on that
for awhile, so let's make it less silent.
Also fix some overlooked documentation and test cases.
Discussion: https://postgr.es/m/
2197698.
1617984583@sss.pgh.pa.us
Tom Lane [Thu, 15 Apr 2021 20:31:36 +0000 (16:31 -0400)]
Stabilize recently-added information_schema test queries.
These queries could show unexpected entries if the core system,
or concurrently-running test scripts, created any functions that
would appear in the information_schema views. Restrict them
to showing functions belonging to this test's schema, as the
far-older nearby test case does.
Per experimentation with conversion of some built-in functions
to SQL-function-body style.
Peter Eisentraut [Thu, 15 Apr 2021 17:41:42 +0000 (19:41 +0200)]
Revert "psql: Show all query results by default"
This reverts commit
3a5130672296ed4e682403a77a9a3ad3d21cef75.
Per discussion, this patch had too many issues to resolve at this
point of the development cycle. We'll try again in the future.
Discussion: https://www.postgresql.org/message-id/flat/alpine.DEB.2.21.
1904132231510.8961@lancre
Fujii Masao [Thu, 15 Apr 2021 14:15:19 +0000 (23:15 +0900)]
doc: Add missing COMPRESSION into CREATE TABLE synopsis.
Commit
bbe0a81db6 introduced "INCLUDING COMPRESSION" option
in CREATE TABLE command, but forgot to mention it in the
CREATE TABLE syntax synopsis.
Author: Fujii Masao
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
54d30e66-dbd6-5485-aaf6-
a291ed55919d@oss.nttdata.com
Michael Paquier [Thu, 15 Apr 2021 07:45:34 +0000 (16:45 +0900)]
doc: Simplify example of HISTFILE for psql
e4c7619 has added a space to the example used for HISTFILE in the docs
of psql before the variable DBNAME, as a workaround because variables
were not parsed the same way back then. This behavior has changed in
9.2, causing the example in the psql docs to result in the same history
file created with or without a space added before the DBNAME variable.
Let's just remove this space in the example, to reduce any confusion, as
the point of it is to prove that a per-database history file is easy to
set up, and that's easier to read this way.
Per discussion with Tom Lane.
Reported-by: Ludovic Kuty
Discussion: https://postgr.es/m/
161830067409.691.
16198363670687811485@wrigleys.postgresql.org
Peter Eisentraut [Thu, 15 Apr 2021 07:08:18 +0000 (09:08 +0200)]
pg_upgrade: Small fix for better translatability of help output
Peter Eisentraut [Thu, 15 Apr 2021 06:58:03 +0000 (08:58 +0200)]
amcheck: Use correct format placeholder for TOAST chunk numbers
Several of these were already fixed in passing in
9acaf1a62197205b06a85afbfcaa7ffaac939ef3, but one was remaining
inconsistent.
Michael Paquier [Thu, 15 Apr 2021 01:03:46 +0000 (10:03 +0900)]
Tweak behavior of pg_dump --extension with configuration tables
6568cef, that introduced the option, had an inconsistent behavior when
it comes to configuration tables set up by pg_extension_config_dump, as
the data of all configuration tables would included in a dump even for
extensions not listed by a set of --extension switches.
The contents dumped changed depending on the schema where an extension
was installed when an extension was not listed. For example, an
extension installed under the public schema would have its configuration
data not dumped even when not listed with --extension, which was
inconsistent with the case of an extension installed on a non-public
schema, where the configuration would be dumped.
Per discussion with Noah, we have settled down to the simple rule of
dumping configuration data of an extension if it is listed in
--extension (default is unchanged and backward-compatible, to dump
everything on sight if there are no extensions directly listed). This
avoids some weird cases where the dumps depended on a --schema for one.
More tests are added to cover the gap, where we cross-check more
behaviors depending on --schema when an extension is not listed.
Reported-by: Noah Misch
Reviewed-by: Noah Misch
Discussion: https://postgr.es/m/
20210404220802.GA728316@rfd.leadboat.com
Tom Lane [Wed, 14 Apr 2021 18:28:17 +0000 (14:28 -0400)]
Fix obsolete comments referencing JoinPathExtraData.extra_lateral_rels.
That field went away in commit
edca44b15, but it seems that
commit
45be99f8c re-introduced some comments mentioning it.
Noted by James Coleman, though this isn't exactly his
proposed new wording. Also thanks to Justin Pryzby for
software archaeology.
Discussion: https://postgr.es/m/CAAaqYe8fxZjq3na+XkNx4C78gDqykH-7dbnzygm9Qa9nuDTePg@mail.gmail.com
Robert Haas [Wed, 14 Apr 2021 16:46:31 +0000 (12:46 -0400)]
amcheck: Reword some messages and fix an alignment problem.
We don't need to mention the attribute number in these messages, because
there's a dedicated column for that, but we should mention the toast
value ID, because that's really useful for any follow-up troubleshooting
the user wants to do. This also rewords some of the messages to hopefully
read a little better.
Also, use VARATT_EXTERNAL_GET_POINTER in case we're accessing a TOAST
pointer that isn't aligned on a platform that's fussy about alignment,
so that we don't crash while corruption-checking the user's data.
Mark Dilger, reviewed by me.
Discussion: http://postgr.es/m/
7D3B9BF6-50D0-4C30-8506-
1C1851C7F96F@enterprisedb.com
Peter Eisentraut [Wed, 14 Apr 2021 07:10:57 +0000 (09:10 +0200)]
Improve quoting in some error messages
Michael Paquier [Wed, 14 Apr 2021 06:55:55 +0000 (15:55 +0900)]
doc: Move force_parallel_mode to section for developer options
This GUC has always been classified as a planner option since its
introduction in
7c944bd, and was listed in postgresql.conf.sample. As
this parameter exists for testing purposes, move it to the section
dedicated to developer parameters and hence remove it from
postgresql.conf.sample. This will avoid any temptation to play with it
on production servers for users that should never really have to touch
this parameter.
The general description used for developer options is reworded a bit, to
take into account the inclusion of force_parallel_mode, per a suggestion
from Tom Lane.
Per discussion between Tom Lane, Bruce Momjian, Justin Pryzby, Bharath
Rupireddy and me.
Author: Justin Pryzby, Tom Lane
Discussion: https://postgr.es/m/
20210403152402.GA8049@momjian.us
Michael Paquier [Wed, 14 Apr 2021 05:23:53 +0000 (14:23 +0900)]
Simplify tests of postgres_fdw terminating connections
The tests introduced in
32a9c0b for connections broken and
re-established rely on pg_terminate_backend() for their logic. When
these were introduced, this function simply sent a signal to a backend
without waiting for the operation to complete, and the tests repeatedly
looked at pg_stat_activity to check if the operation was completed or
not. Since
aaf0432, it is possible to define a timeout to make
pg_terminate_backend() wait for a certain duration, so make use of it,
with a timeout reasonably large enough (3min) to give enough room for
the tests to pass even on slow machines.
Some measurements show that the tests of postgres_fdw are much faster
with this change. For example, on my laptop, they now take 4s instead
of 6s.
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACXGY_EfGrMTjKjHy2zi-u1u9rdeioU_fro0T6Jo8t56KQ@mail.gmail.com
Amit Kapila [Wed, 14 Apr 2021 03:25:03 +0000 (08:55 +0530)]
Use NameData datatype for slotname in stats.
This will make it consistent with the other usage of slotname in the code.
In the passing, change pgstat_report_replslot signature to use a structure
rather than multiple parameters.
Reported-by: Andres Freund
Author: Vignesh C
Reviewed-by: Sawada Masahiko, Amit Kapila
Discussion: https://postgr.es/m/
20210319185247.ldebgpdaxsowiflw@alap3.anarazel.de
Tomas Vondra [Tue, 13 Apr 2021 22:46:12 +0000 (00:46 +0200)]
Initialize t_self and t_tableOid in statext_expressions_load
The function is building a fake heap tuple, but left some of the header
fields (tid and table OID) uninitialized. Per Coverity report.
Reported-by: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQApj6h8tZ0-eP5Af5PKc5NG1YUc7=SdN_99YoHS51fKa0Q@mail.gmail.com
Peter Geoghegan [Tue, 13 Apr 2021 19:58:31 +0000 (12:58 -0700)]
Don't truncate heap when VACUUM's failsafe is in effect.
It seems like a good idea to bypass heap truncation when the wraparound
failsafe mechanism (which was added in commit
1e55e7d1) is in effect.
Deliberately don't bypass heap truncation in the INDEX_CLEANUP=off case,
even though it is similar to the failsafe case. There is already a
separate reloption (and related VACUUM parameter) for that.
Reported-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoDWRh6oTN5T8wa+cpZUVpHXET8BJ8Da7WHVHpwkPP6KLg@mail.gmail.com
Tom Lane [Tue, 13 Apr 2021 19:39:33 +0000 (15:39 -0400)]
Allow table-qualified variable names in ON CONFLICT ... WHERE.
Previously you could only use unqualified variable names here.
While that's not a functional deficiency, since only the target
table can be referenced, it's a surprising inconsistency with the
rules for partial-index predicates, on which this syntax is
supposedly modeled.
The fix for that is no harder than passing addToRelNameSpace = true
to addNSItemToQuery. However, it's really pretty bogus for
transformOnConflictArbiter and transformOnConflictClause to be
messing with the namespace item for the target table at all.
It's not theirs to manage, it results in duplicative creations of
namespace items, and transformOnConflictClause wasn't even doing
it quite correctly (that coding resulted in two nsitems for the
target table, since it hadn't cleaned out the existing one).
Hence, make transformInsertStmt responsible for setting up the
target nsitem once for both these clauses and RETURNING.
Also, arrange for ON CONFLICT ... UPDATE's "excluded" pseudo-relation
to be added to the rangetable before we run transformOnConflictArbiter.
This produces a more helpful HINT if someone writes "excluded.col"
in the arbiter expression.
Per bug #16958 from Lukas Eder. Although I agree this is a bug,
the consequences are hardly severe, so no back-patch.
Discussion: https://postgr.es/m/16958-
963f638020de271c@postgresql.org
Robert Haas [Tue, 13 Apr 2021 18:56:12 +0000 (14:56 -0400)]
docs: Update TOAST storage docs for configurable compression.
Mention that there are multiple TOAST compression methods and that the
compression method used is stored in a TOAST pointer along with the
other information that was stored there previously. Add a reference to
the documentation for default_toast_compression, where the supported
methods are listed, instead of duplicating that here.
I haven't tried to preserve the text claiming that pglz is "fairly
simple and very fast." I have no view on the veracity of the former
claim, but LZ4 seems to be faster (and to compress better) so it
seems better not to muddy the waters by talking about compression
speed as a strong point of PGLZ.
Patch by me, reviewed by Justin Pryzby.
Discussion: http://postgr.es/m/CA+Tgmoaw_YBwQhOS_hhEPPwFhfAnu+VCLs18EfGr9gQw1z4H-w@mail.gmail.com
Tom Lane [Tue, 13 Apr 2021 19:10:18 +0000 (15:10 -0400)]
Fix some inappropriately-disallowed uses of ALTER ROLE/DATABASE SET.
Most GUC check hooks that inspect database state have special checks
that prevent them from throwing hard errors for state-dependent issues
when source == PGC_S_TEST. This allows, for example,
"ALTER DATABASE d SET default_text_search_config = foo" when the "foo"
configuration hasn't been created yet. Without this, we have problems
during dump/reload or pg_upgrade, because pg_dump has no idea about
possible dependencies of GUC values and can't ensure a safe restore
ordering.
However, check_role() and check_session_authorization() hadn't gotten
the memo about that, and would throw hard errors anyway. It's not
entirely clear what is the use-case for "ALTER ROLE x SET role = y",
but we've now heard two independent complaints about that bollixing
an upgrade, so apparently some people are doing it.
Hence, fix these two functions to act more like other check hooks
with similar needs. (But I did not change their insistence on
being inside a transaction, as it's still not apparent that setting
either GUC from the configuration file would be wise.)
Also fix check_temp_buffers, which had a different form of the disease
of making state-dependent checks without any exception for PGC_S_TEST.
A cursory survey of other GUC check hooks did not find any more issues
of this ilk. (There are a lot of interdependencies among
PGC_POSTMASTER and PGC_SIGHUP GUCs, which may be a bad idea, but
they're not relevant to the immediate concern because they can't be
set via ALTER ROLE/DATABASE.)
Per reports from Charlie Hornsby and Nathan Bossart. Back-patch
to all supported branches.
Discussion: https://postgr.es/m/HE1P189MB0523B31598B0C772C908088DB7709@HE1P189MB0523.EURP189.PROD.OUTLOOK.COM
Discussion: https://postgr.es/m/
20160711223641.1426.86096@wrigleys.postgresql.org
Tom Lane [Tue, 13 Apr 2021 17:37:07 +0000 (13:37 -0400)]
Redesign the caching done by get_cached_rowtype().
Previously, get_cached_rowtype() cached a pointer to a reference-counted
tuple descriptor from the typcache, relying on the ExprContextCallback
mechanism to release the tupdesc refcount when the expression tree
using the tupdesc was destroyed. This worked fine when it was designed,
but the introduction of within-DO-block COMMITs broke it. The refcount
is logged in a transaction-lifespan resource owner, but plpgsql won't
destroy simple expressions made within the DO block (before its first
commit) until the DO block is exited. That results in a warning about
a leaked tupdesc refcount when the COMMIT destroys the original resource
owner, and then an error about the active resource owner not holding a
matching refcount when the expression is destroyed.
To fix, get rid of the need to have a shutdown callback at all, by
instead caching a pointer to the relevant typcache entry. Those
survive for the life of the backend, so we needn't worry about the
pointer becoming stale. (For registered RECORD types, we can still
cache a pointer to the tupdesc, knowing that it won't change for the
life of the backend.) This mechanism has been in use in plpgsql
and expandedrecord.c since commit
4b93f5799, and seems to work well.
This change requires modifying the ExprEvalStep structs used by the
relevant expression step types, which is slightly worrisome for
back-patching. However, there seems no good reason for extensions
to be familiar with the details of these particular sub-structs.
Per report from Rohit Bhogate. Back-patch to v11 where within-DO-block
COMMITs became a thing.
Discussion: https://postgr.es/m/CAAV6ZkQRCVBh8qAY+SZiHnz+U+FqAGBBDaDTjF2yiKa2nJSLKg@mail.gmail.com
Tom Lane [Tue, 13 Apr 2021 16:17:24 +0000 (12:17 -0400)]
Avoid improbable PANIC during heap_update.
heap_update needs to clear any existing "all visible" flag on
the old tuple's page (and on the new page too, if different).
Per coding rules, to do this it must acquire pin on the appropriate
visibility-map page while not holding exclusive buffer lock;
which creates a race condition since someone else could set the
flag whenever we're not holding the buffer lock. The code is
supposed to handle that by re-checking the flag after acquiring
buffer lock and retrying if it became set. However, one code
path through heap_update itself, as well as one in its subroutine
RelationGetBufferForTuple, failed to do this. The end result,
in the unlikely event that a concurrent VACUUM did set the flag
while we're transiently not holding lock, is a non-recurring
"PANIC: wrong buffer passed to visibilitymap_clear" failure.
This has been seen a few times in the buildfarm since recent VACUUM
changes that added code paths that could set the all-visible flag
while holding only exclusive buffer lock. Previously, the flag
was (usually?) set only after doing LockBufferForCleanup, which
would insist on buffer pin count zero, thus preventing the flag
from becoming set partway through heap_update. However, it's
clear that it's heap_update not VACUUM that's at fault here.
What's less clear is whether there is any hazard from these bugs
in released branches. heap_update is certainly violating API
expectations, but if there is no code path that can set all-visible
without a cleanup lock then it's only a latent bug. That's not
100% certain though, besides which we should worry about extensions
or future back-patch fixes that could introduce such code paths.
I chose to back-patch to v12. Fixing RelationGetBufferForTuple
before that would require also back-patching portions of older
fixes (notably
0d1fe9f74), which is more code churn than seems
prudent to fix a hypothetical issue.
Discussion: https://postgr.es/m/
2247102.
1618008027@sss.pgh.pa.us
Fujii Masao [Tue, 13 Apr 2021 05:21:51 +0000 (14:21 +0900)]
doc: Fix typo in logicaldecoding.sgml.
Introduced in commit
0aa8a01d04.
Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Ps8rkVHKWyjg09Fb1PaVG5-EhoFTFJ9OZMF4HPzDSXfew@mail.gmail.com
Noah Misch [Tue, 13 Apr 2021 02:24:41 +0000 (19:24 -0700)]
Use "-I." in directories holding Bison parsers, for Oracle compilers.
With the Oracle Developer Studio 12.6 compiler, #line directives alter
the current source file location for purposes of #include "..."
directives. Hence, a VPATH build failed with 'cannot find include file:
"specscanner.c"'. With two exceptions, parser-containing directories
already add "-I. -I$(srcdir)"; eliminate the exceptions. Back-patch to
9.6 (all supported versions).
Noah Misch [Tue, 13 Apr 2021 02:24:21 +0000 (19:24 -0700)]
Port regress-python3-mangle.mk to Solaris "sed".
It doesn't support "\(foo\)*" like a POSIX "sed" implementation does;
see the Autoconf manual. Back-patch to 9.6 (all supported versions).
Thomas Munro [Tue, 13 Apr 2021 00:34:25 +0000 (12:34 +1200)]
Fix potential SSI hazard in heap_update().
Commit
6f38d4dac38 failed to heed a warning about the stability of the
value pointed to by "otid". The caller is allowed to pass in a pointer to
newtup->t_self, which will be updated during the execution of the
function. Instead, the SSI check should use the value we copy into
oldtup.t_self near the top of the function.
Not a live bug, because newtup->t_self doesn't really get updated until
a bit later, but it was confusing and broke the rule established by the
comment.
Back-patch to 13.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
2689164.
1618160085%40sss.pgh.pa.us
Michael Paquier [Tue, 13 Apr 2021 00:42:01 +0000 (09:42 +0900)]
Remove duplicated --no-sync switches in new tests of test_pg_dump
These got introduced in
6568cef.
Reported-by: Noah Misch
Discussion: https://postgr.es/m/
20210404220802.GA728316@rfd.leadboat.com
Tom Lane [Mon, 12 Apr 2021 22:58:20 +0000 (18:58 -0400)]
Remove no-longer-relevant test case.
collate.icu.utf8.sql was exercising the recording of a collation
dependency for an enum comparison expression, but such an expression
should never have had any collation dependency in the first place.
After I fixed that in commit
c402b02b9, the test started failing.
We don't need to test that scenario anymore, so just remove the
now-useless test steps.
(This test case is new in HEAD, so no need to back-patch.)
Discussion: https://postgr.es/m/
3044030.
1618261159@sss.pgh.pa.us
Discussion: https://postgr.es/m/HK0PR01MB22744393C474D503E16C8509F4709@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
Tom Lane [Mon, 12 Apr 2021 18:37:22 +0000 (14:37 -0400)]
Fix old bug with coercing the result of a COLLATE expression.
There are hacks in parse_coerce.c to push down a requested coercion
to below any CollateExpr that may appear. However, we did that even
if the requested data type is non-collatable, leading to an invalid
expression tree in which CollateExpr is applied to a non-collatable
type. The fix is just to drop the CollateExpr altogether, reasoning
that it's useless.
This bug is ten years old, dating to the original addition of
COLLATE support. The lack of field complaints suggests that there
aren't a lot of user-visible consequences. We noticed the problem
because it would trigger an assertion in DefineVirtualRelation if
the invalid structure appears as an output column of a view; however,
in a non-assert build, you don't see a crash just a (subtly incorrect)
complaint about applying collation to a non-collatable type. I found
that by putting the incorrect structure further down in a view, I could
make a view definition that would fail dump/reload, per the added
regression test case. But CollateExpr doesn't do anything at run-time,
so this likely doesn't lead to any really exciting consequences.
Per report from Yulin Pei. Back-patch to all supported branches.
Discussion: https://postgr.es/m/HK0PR01MB22744393C474D503E16C8509F4709@HK0PR01MB2274.apcprd01.prod.exchangelabs.com
Peter Eisentraut [Mon, 12 Apr 2021 18:29:24 +0000 (20:29 +0200)]
pg_upgrade: Print OID using %u instead of %d
This could write wrong output into the cluster deletion script if a
database OID exceeds the signed 32-bit range.