Tom Lane [Sun, 24 Jan 2021 19:59:33 +0000 (14:59 -0500)]
Add a simple test for contrib/auto_explain.
This module formerly had zero test coverage.
Discussion: https://postgr.es/m/
1445881.
1611441692@sss.pgh.pa.us
Magnus Hagander [Sun, 24 Jan 2021 13:19:00 +0000 (14:19 +0100)]
Remove make_diff set of tools
These are mostly obsoleted by the switch to git, and it's easier to
remove them than to update the incorrect documentation.
Discussion: https://postgr.es/m/CABUevEwmASMn4WRJ6RagBx43sj10ctfMHcMA_-7KA3pDYmwpJw@mail.gmail.com
Tom Lane [Sun, 24 Jan 2021 03:40:46 +0000 (22:40 -0500)]
Doc: clean up contrib/pageinspect's GIST function documentation.
I came to fix the overwidth-PDF-page warnings seen in the buildfarm,
but stayed long enough to copy-edit some nearby text.
Tomas Vondra [Sat, 23 Jan 2021 23:24:50 +0000 (00:24 +0100)]
Fix COPY FREEZE with CLOBBER_CACHE_ALWAYS
This adds code omitted from commit
7db0cd2145 by accident, which had
two consequences. Firstly, only rows inserted by heap_multi_insert were
frozen as expected when running COPY FREEZE, while heap_insert left
rows unfrozen. That however includes rows in TOAST tables, so a lot of
data might have been left unfrozen. Secondly, page might have been left
partially empty after relcache invalidation.
This addresses both of those issues.
Discussion: https://postgr.es/m/CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com
Tom Lane [Sat, 23 Jan 2021 20:50:51 +0000 (15:50 -0500)]
Doc: update example connection-failure messages in the documentation.
Now that the dust has more or less settled on
52a10224e and follow-ons,
make sure the examples in the documentation are up-to-date.
Tom Lane [Sat, 23 Jan 2021 20:08:39 +0000 (15:08 -0500)]
Update ecpg's connect-test1 for connection-failure message changes.
I should have updated this in commits
52a10224e and follow-ons,
but I missed it because it's not run by default, and none of the
buildfarm runs it either. Maybe we should try to improve that
situation.
Discussion: https://postgr.es/m/CAH2-Wz=j9SRW=s5BV4-3k+=tr4N3A03in+gTuVA09vNF+-iHjA@mail.gmail.com
Michael Paquier [Sat, 23 Jan 2021 02:33:04 +0000 (11:33 +0900)]
Introduce SHA1 implementations in the cryptohash infrastructure
With this commit, SHA1 goes through the implementation provided by
OpenSSL via EVP when building the backend with it, and uses as fallback
implementation KAME which was located in pgcrypto and already shaped for
an integration with a set of init, update and final routines.
Structures and routines have been renamed to make things consistent with
the fallback implementations of MD5 and SHA2.
uuid-ossp has used for ages a shortcut with pgcrypto to fetch a copy of
SHA1 if needed. This was built depending on the build options within
./configure, so this cleans up some code and removes the build
dependency between pgcrypto and uuid-ossp.
Note that this will help with the refactoring of HMAC, as pgcrypto
offers the option to use MD5, SHA1 or SHA2, so only the second option
was missing to make that possible.
Author: Michael Paquier
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/X9HXKTgrvJvYO7Oh@paquier.xyz
Tom Lane [Sat, 23 Jan 2021 00:25:39 +0000 (19:25 -0500)]
Suppress bison warning in ecpg grammar.
opt_distinct_clause is only used in PLpgSQL_Expr, which ecpg
ignores, so it needs to ignore opt_distinct_clause too.
My oversight in
7cd9765f9; reported by Bruce Momjian.
Discussion: https://postgr.es/m/E1l33wr-0005sJ-9n@gemulon.postgresql.org
Tom Lane [Fri, 22 Jan 2021 23:58:25 +0000 (18:58 -0500)]
Doc: improve directions for building on macOS.
In light of recent discussions, we should instruct people to
install Apple's command line tools; installing Xcode is secondary.
Also, fix sample command for finding out the default sysroot,
as we now know that the command originally recommended can give
a result that doesn't match your OS version.
Also document the workaround to use if you really don't want
configure to select a sysroot at all.
Discussion: https://postgr.es/m/
20210119111625.20435-1-james.hilliard1@gmail.com
Tom Lane [Fri, 22 Jan 2021 21:52:31 +0000 (16:52 -0500)]
Avoid redundantly prefixing PQerrorMessage for a connection failure.
libpq's error messages for connection failures pretty well stand on
their own, especially since commits
52a10224e/
27a48e5a1. Prefixing
them with 'could not connect to database "foo"' or the like is just
redundant, and perhaps even misleading if the specific database name
isn't relevant to the failure. (When it is, we trust that the
backend's error message will include the DB name.) Indeed, psql
hasn't used any such prefix in a long time. So, make all our other
programs and documentation examples agree with psql's practice.
Discussion: https://postgr.es/m/
1094524.
1611266589@sss.pgh.pa.us
Tom Lane [Fri, 22 Jan 2021 21:26:22 +0000 (16:26 -0500)]
Re-allow DISTINCT in pl/pgsql expressions.
I'd omitted this from the grammar in commit
c9d529848, figuring that
it wasn't worth supporting. However we already have one complaint,
so it seems that judgment was wrong. It doesn't require a huge
amount of code, so add it back. (I'm still drawing the line at
UNION/INTERSECT/EXCEPT though: those'd require an unreasonable
amount of grammar refactoring, and the single-result-row restriction
makes them near useless anyway.)
Also rethink the documentation: this behavior is a property of
all pl/pgsql expressions, not just assignments.
Discussion: https://postgr.es/m/
20210122134106.
e94c5cd7@mail.verfriemelt.org
Tom Lane [Fri, 22 Jan 2021 16:29:43 +0000 (11:29 -0500)]
Doc: remove misleading claim in documentation of PQreset().
This text claimed that the reconnection would occur "to the same
server", but there is no such guarantee in the code, nor would
insisting on that be an improvement.
Back-patch to v10 where multi-host connection strings were added.
Discussion: https://postgr.es/m/
1095901.
1611268376@sss.pgh.pa.us
Magnus Hagander [Fri, 22 Jan 2021 11:49:53 +0000 (12:49 +0100)]
Remove reference to ftp servers from documentation
It's been a long time since we used ftp, but there was a single
reference left in the docs.
Author: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/
6880D602-7286-46EC-8A03-
14E3248FEC7A@yesql.se
Peter Eisentraut [Fri, 22 Jan 2021 10:58:21 +0000 (11:58 +0100)]
Remove bogus tracepoint
Calls to LWLockWaitForVar() fired the TRACE_POSTGRESQL_LWLOCK_ACQUIRE
tracepoint, but LWLockWaitForVar() never actually acquires the LWLock.
(Probably a copy/paste bug in
68a2e52bbaf.) Remove it.
Author: Craig Ringer <craig.ringer@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CAGRY4nxJo+-HCC2i5H93ttSZ4gZO-FSddCwvkb-qAfQ1zdXd1w@mail.gmail.com
Heikki Linnakangas [Fri, 22 Jan 2021 09:10:42 +0000 (11:10 +0200)]
doc: Copy-edit the "Overview of PostgreSQL Internals" chapter
Rephrase a few sentences to be more concise.
Refer to the postmaster process as "postmaster", not "postgres". This
originally said "postmaster process", but was changed to "postgres
process" in commit
5266f221a2, when we merged the "postmaster" and
"postgres" commands, and "postmaster" became just a symlink. That was a
case of overzealous search & replace, because the process is still called
"postmaster".
Author: Erik Rijkers and Jürgen Purtz
Discussion: https://www.postgresql.org/message-id/
aa31f359-1168-ded5-53d0-
0ed228bfe097%40iki.fi
Michael Paquier [Fri, 22 Jan 2021 00:26:27 +0000 (09:26 +0900)]
Move SSL information callback earlier to capture more information
The callback for retrieving state change information during connection
setup was only installed when the connection was mostly set up, and
thus didn't provide much information and missed all the details related
to the handshake.
This also extends the callback with SSL_state_string_long() to print
more information about the state change within the SSL object handled.
While there, fix some comments which were incorrectly referring to the
callback and its previous location in fe-secure.c.
Author: Daniel Gustafsson
Discussion: https://postgr.es/m/
232CF476-94E1-42F1-9408-
719E2AEC5491@yesql.se
Tom Lane [Thu, 21 Jan 2021 21:10:18 +0000 (16:10 -0500)]
Improve new wording of libpq's connection failure messages.
"connection to server so-and-so failed:" seems clearer than the
previous wording "could not connect to so-and-so:" (introduced by
52a10224e), because the latter suggests a network-level connection
failure. We're now prefixing this string to all types of connection
failures, for instance authentication failures; so we need wording
that doesn't imply a low-level error.
Per discussion with Robert Haas.
Discussion: https://postgr.es/m/CA+TgmobssJ6rS22dspWnu-oDxXevGmhMD8VcRBjmj-b9UDqRjw@mail.gmail.com
Tom Lane [Thu, 21 Jan 2021 20:37:23 +0000 (15:37 -0500)]
Fix pull_varnos' miscomputation of relids set for a PlaceHolderVar.
Previously, pull_varnos() took the relids of a PlaceHolderVar as being
equal to the relids in its contents, but that fails to account for the
possibility that we have to postpone evaluation of the PHV due to outer
joins. This could result in a malformed plan. The known cases end up
triggering the "failed to assign all NestLoopParams to plan nodes"
sanity check in createplan.c, but other symptoms may be possible.
The right value to use is the join level we actually intend to evaluate
the PHV at. We can get that from the ph_eval_at field of the associated
PlaceHolderInfo. However, there are some places that call pull_varnos()
before the PlaceHolderInfos have been created; in that case, fall back
to the conservative assumption that the PHV will be evaluated at its
syntactic level. (In principle this might result in missing some legal
optimization, but I'm not aware of any cases where it's an issue in
practice.) Things are also a bit ticklish for calls occurring during
deconstruct_jointree(), but AFAICS the ph_eval_at fields should have
reached their final values by the time we need them.
The main problem in making this work is that pull_varnos() has no
way to get at the PlaceHolderInfos. We can fix that easily, if a
bit tediously, in HEAD by passing it the planner "root" pointer.
In the back branches that'd cause an unacceptable API/ABI break for
extensions, so leave the existing entry points alone and add new ones
with the additional parameter. (If an old entry point is called and
encounters a PHV, it'll fall back to using the syntactic level,
again possibly missing some valid optimization.)
Back-patch to v12. The computation is surely also wrong before that,
but it appears that we cannot reach a bad plan thanks to join order
restrictions imposed on the subquery that the PlaceHolderVar came from.
The error only became reachable when commit
4be058fe9 allowed trivial
subqueries to be collapsed out completely, eliminating their join order
restrictions.
Per report from Stephan Springl.
Discussion: https://postgr.es/m/171041.
1610849523@sss.pgh.pa.us
Tomas Vondra [Thu, 21 Jan 2021 02:23:24 +0000 (03:23 +0100)]
Fix initialization of FDW batching in ExecInitModifyTable
ExecInitModifyTable has to initialize batching for all result relations,
not just the first one. Furthermore, when junk filters were necessary,
the pointer pointed past the mtstate->resultRelInfo array.
Per reports from multiple non-x86 animals (florican, locust, ...).
Discussion: https://postgr.es/m/
20200628151002.7x5laxwpgvkyiu3q@development
Michael Paquier [Thu, 21 Jan 2021 01:56:03 +0000 (10:56 +0900)]
Switch "cl /?" to "cl /help" in MSVC scripts for platform detection
"cl /?" produces a different output if run on a real or a virtual drive
(this can be set with a simple subst command), causing an error in the
MSVC scripts if building on a virtual drive because the platform to use
cannot be detected.
"cl /help", on the contrary, produces a consistent output if used on a
real or virtual drive. Changing to "/help" allows the compilation to
work with a virtual drive as long as the top of the code repository is
part of the drive, without impacting the build on real drives.
Reported-by: Robert Grange
Author: Juan José Santamaría Flecha
Discussion: https://postgr.es/m/16825-
c4f104bcebc67034@postgresql.org
Tomas Vondra [Wed, 20 Jan 2021 22:05:46 +0000 (23:05 +0100)]
Implement support for bulk inserts in postgres_fdw
Extends the FDW API to allow batching inserts into foreign tables. That
is usually much more efficient than inserting individual rows, due to
high latency for each round-trip to the foreign server.
It was possible to implement something similar in the regular FDW API,
but it was inconvenient and there were issues with reporting the number
of actually inserted rows etc. This extends the FDW API with two new
functions:
* GetForeignModifyBatchSize - allows the FDW picking optimal batch size
* ExecForeignBatchInsert - inserts a batch of rows at once
Currently, only INSERT queries support batching. Support for DELETE and
UPDATE may be added in the future.
This also implements batching for postgres_fdw. The batch size may be
specified using "batch_size" option both at the server and table level.
The initial patch version was written by me, but it was rewritten and
improved in many ways by Takayuki Tsunakawa.
Author: Takayuki Tsunakawa
Reviewed-by: Tomas Vondra, Amit Langote
Discussion: https://postgr.es/m/
20200628151002.7x5laxwpgvkyiu3q@development
Tomas Vondra [Wed, 20 Jan 2021 21:56:06 +0000 (22:56 +0100)]
psql \dX: list extended statistics objects
The new command lists extended statistics objects. All past releases
with extended statistics are supported.
This is a simplified version of commit
891a1d0bca, which had to be
reverted due to not considering pg_statistic_ext_data is not accessible
by regular users. Fields requiring access to this catalog were removed.
It's possible to add them, but it'll require changes to core.
Author: Tatsuro Yamada
Reviewed-by: Julien Rouhaud, Alvaro Herrera, Tomas Vondra, Noriyoshi Shinoda
Discussion: https://postgr.es/m/
c027a541-5856-75a5-0868-
341301e1624b%40nttcom.co.jp_1
Tom Lane [Wed, 20 Jan 2021 17:07:23 +0000 (12:07 -0500)]
Further tweaking of PG_SYSROOT heuristics for macOS.
It emerges that in some phases of the moon (perhaps to do with
directory entry order?), xcrun will report that the SDK path is
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk
which is normally a symlink to a version-numbered sibling directory.
Our heuristic to skip non-version-numbered pathnames was rejecting
that, which is the wrong thing to do. We'd still like to end up
with a version-numbered PG_SYSROOT value, but we can have that by
dereferencing the symlink.
Like the previous fix, back-patch to all supported versions.
Discussion: https://postgr.es/m/522433.
1611089678@sss.pgh.pa.us
Tom Lane [Wed, 20 Jan 2021 16:49:29 +0000 (11:49 -0500)]
Disable vacuum page skipping in selected test cases.
By default VACUUM will skip pages that it can't immediately get
exclusive access to, which means that even activities as harmless
and unpredictable as checkpoint buffer writes might prevent a page
from being processed. Ordinarily this is no big deal, but we have
a small number of test cases that examine the results of VACUUM's
processing and therefore will fail if the page of interest is skipped.
This seems to be the explanation for some rare buildfarm failures.
To fix, add the DISABLE_PAGE_SKIPPING option to the VACUUM commands
in tests where this could be an issue.
In passing, remove a duplicated query in pageinspect/sql/page.sql.
Back-patch as necessary (some of these cases are as old as v10).
Discussion: https://postgr.es/m/413923.
1611006484@sss.pgh.pa.us
Heikki Linnakangas [Wed, 20 Jan 2021 09:58:03 +0000 (11:58 +0200)]
Fix bug in detecting concurrent page splits in GiST insert
In commit
9eb5607e699, I got the condition on checking for split or
deleted page wrong: I used && instead of ||. The comment correctly said
"concurrent split _or_ deletion".
As a result, GiST insertion could miss a concurrent split, and insert to
wrong page. Duncan Sands demonstrated this with a test script that did a
lot of concurrent inserts.
Backpatch to v12, where this was introduced. REINDEX is required to fix
indexes that were affected by this bug.
Backpatch-through: 12
Reported-by: Duncan Sands
Discussion: https://www.postgresql.org/message-id/
a9690483-6c6c-3c82-c8ba-
dc1a40848f11%40deepbluecap.com
Thomas Munro [Wed, 20 Jan 2021 09:31:26 +0000 (22:31 +1300)]
Fix sample output of EXPLAIN ANALYZE.
Since commit
f0f13a3a08b2757997410f3a1c38bdc22973c525, we estimate
ModifyTable paths without a RETURNING clause differently. Update an
example from the manual that showed the old behavior.
Author: Takayuki Tsunakawa <tsunakawa.takay@fujitsu.com>
Reviewed-by: Laurenz Albe <laurenz.albe@cybertec.at>
Discussion: https://postgr.es/m/TYAPR01MB29905674F41693BBA9DA28CAFEA20%40TYAPR01MB2990.jpnprd01.prod.outlook.com
Michael Paquier [Wed, 20 Jan 2021 04:28:10 +0000 (13:28 +0900)]
Add regression test for DROP OWNED BY with default ACLs
DROP OWNED BY has a specific code path to remove ACLs stored in
pg_default_acl when cleaning up shared dependencies that had no
coverage with the existing tests. This issue has been found while
digging into the bug fixed by
21378e1.
As ALTER DEFAULT PRIVILEGES impacts the ACLs of all objects created
while the default permissions are visible, the test uses a transaction
rollback to isolate the test and avoid any impact with other sessions
running in parallel.
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/YAbQ1OD+3ip4lRv8@paquier.xyz
Michael Paquier [Wed, 20 Jan 2021 02:38:17 +0000 (11:38 +0900)]
Fix ALTER DEFAULT PRIVILEGES with duplicated objects
Specifying duplicated objects in this command would lead to unique
constraint violations in pg_default_acl or "tuple already updated by
self" errors. Similarly to GRANT/REVOKE, increment the command ID after
each subcommand processing to allow this case to work transparently.
A regression test is added by tweaking one of the existing queries of
privileges.sql to stress this case.
Reported-by: Andrus
Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/
ae2a7dc1-9d71-8cba-3bb9-
e4cb7eb1f44e@hot.ee
Backpatch-through: 9.5
Tom Lane [Tue, 19 Jan 2021 18:25:33 +0000 (13:25 -0500)]
Remove faulty support for MergeAppend plan with WHERE CURRENT OF.
Somebody extended search_plan_tree() to treat MergeAppend exactly
like Append, which is 100% wrong, because unlike Append we can't
assume that only one input node is actively returning tuples.
Hence a cursor using a MergeAppend across a UNION ALL or inheritance
tree could falsely match a WHERE CURRENT OF query at a row that
isn't actually the cursor's current output row, but coincidentally
has the same TID (in a different table) as the current output row.
Delete the faulty code; this means that such a case will now return
an error like 'cursor "foo" is not a simply updatable scan of table
"bar"', instead of silently misbehaving. Users should not find that
surprising though, as the same cursor query could have failed that way
already depending on the chosen plan. (It would fail like that if the
sort were done with an explicit Sort node instead of MergeAppend.)
Expand the clearly-inadequate commentary to be more explicit about
what this code is doing, in hopes of forestalling future mistakes.
It's been like this for awhile, so back-patch to all supported
branches.
Discussion: https://postgr.es/m/482865.
1611075182@sss.pgh.pa.us
Peter Eisentraut [Tue, 19 Jan 2021 09:28:05 +0000 (10:28 +0100)]
pageinspect: Change block number arguments to bigint
Block numbers are 32-bit unsigned integers. Therefore, the smallest
SQL integer type that they can fit in is bigint. However, in the
pageinspect module, most input and output parameters dealing with
block numbers were declared as int. The behavior with block numbers
larger than a signed 32-bit integer was therefore dubious. Change
these arguments to type bigint and add some more explicit error
checking on the block range.
(Other contrib modules appear to do this correctly already.)
Since we are changing argument types of existing functions, in order
to not misbehave if the binary is updated before the extension is
updated, we need to create new C symbols for the entry points, similar
to how it's done in other extensions as well.
Reported-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/flat/
d8f6bdd536df403b9b33816e9f7e0b9d@G08CNEXMBPEKD05.g08.fujitsu.local
Fujii Masao [Mon, 18 Jan 2021 15:56:10 +0000 (00:56 +0900)]
doc: Add note about the server name of postgres_fdw_get_connections() returns.
Previously the document didn't mention the case where
postgres_fdw_get_connections() returns NULL in server_name column.
Users might be confused about why NULL was returned.
This commit adds the note that, in postgres_fdw_get_connections(),
the server name of an invalid connection will be NULL if the server is dropped.
Suggested-by: Zhijie Hou
Author: Bharath Rupireddy
Reviewed-by: Zhijie Hou, Fujii Masao
Discussion: https://postgr.es/m/
e7ddd14e96444fce88e47a709c196537@G08CNEXMBPEKD05.g08.fujitsu.local
Amit Kapila [Tue, 19 Jan 2021 02:40:13 +0000 (08:10 +0530)]
pgindent worker.c.
This is a leftover from commit
0926e96c49. Changing this separately
because this file is being modified for upcoming patch logical replication
of 2PC.
Author: Peter Smith
Discussion: https://postgr.es/m/CAHut+Ps+EgG8KzcmAyAgBUi_vuTps6o9ZA8DG6SdnO0-YuOhPQ@mail.gmail.com
Bruce Momjian [Mon, 18 Jan 2021 23:48:25 +0000 (18:48 -0500)]
doc: adjust alignment of doc file list for "pg_waldump.sgml"
Backpatch-through: 10
Tom Lane [Mon, 18 Jan 2021 23:32:30 +0000 (18:32 -0500)]
Avoid crash with WHERE CURRENT OF and a custom scan plan.
execCurrent.c's search_plan_tree() assumed that ForeignScanStates
and CustomScanStates necessarily have a valid ss_currentRelation.
This is demonstrably untrue for postgres_fdw's remote join and
remote aggregation plans, and non-leaf custom scans might not have
an identifiable scan relation either. Avoid crashing by ignoring
such nodes when the field is null.
This solution will lead to errors like 'cursor "foo" is not a
simply updatable scan of table "bar"' in cases where maybe we
could have allowed WHERE CURRENT OF to work. That's not an issue
for postgres_fdw's usages, since joins or aggregations would render
WHERE CURRENT OF invalid anyway. But an otherwise-transparent
upper level custom scan node might find this annoying. When and if
someone cares to expend work on such a scenario, we could invent a
custom-scan-provider callback to determine what's safe.
Report and patch by David Geier, commentary by me. It's been like
this for awhile, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
0253344d-9bdd-11c4-7f0d-
d88c02cd7991@swarm64.com
Tom Lane [Mon, 18 Jan 2021 20:55:01 +0000 (15:55 -0500)]
Narrow the scope of a local variable.
This is better style and more symmetrical with the other if-branch.
This likely should have been included in
9de77b545 (which created
the opportunity), but it was overlooked.
Japin Li
Discussion: https://postgr.es/m/MEYP282MB16699FA4A7CD57EB250E871FB6A40@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Tom Lane [Mon, 18 Jan 2021 20:11:32 +0000 (15:11 -0500)]
Add bytea equivalents of ltrim() and rtrim().
We had bytea btrim() already, but for some reason not the other two.
Joel Jacobson
Discussion: https://postgr.es/m/
d10cd5cd-a901-42f1-b832-
763ac6f7ff3a@www.fastmail.com
Robert Haas [Mon, 18 Jan 2021 17:09:52 +0000 (12:09 -0500)]
Allow for error or refusal while absorbing a ProcSignalBarrier.
Previously, the per-barrier-type functions tasked with absorbing
them were expected to always succeed and never throw an error.
However, that's a bit inconvenient. Further study has revealed that
there are realistic cases where it might not be possible to absorb
a ProcSignalBarrier without terminating the transaction, or even
the whole backend. Similarly, for some barrier types, there might
be other reasons where it's not reasonably possible to absorb the
barrier at certain points in the code, so provide a way for a
per-barrier-type function to reject absorbing the barrier.
Unfortunately, there's still no committed code making use of this
infrastructure; hopefully, we'll get there. :-(
Patch by me, reviewed by Andres Freund and Amul Sul.
Discussion: http://postgr.es/m/
20200908182005.xya7wetdh3pndzim@alap3.anarazel.de
Discussion: http://postgr.es/m/CA+Tgmob56Pk1-5aTJdVPCWFHon7me4M96ENpGe9n_R4JUjjhZA@mail.gmail.com
Magnus Hagander [Mon, 18 Jan 2021 16:51:49 +0000 (17:51 +0100)]
Bump PGSTAT_FILE_FORMAT_ID
This was missed in
960869da08
Reported-By: Laurenz Albe
Discussion: https://postgr.es/m/
4f0aacc5fe1b4bfafa32b36ecd97469fae526a75.camel@cybertec.at
Heikki Linnakangas [Mon, 18 Jan 2021 12:48:43 +0000 (14:48 +0200)]
Check for BuildIndexValueDescription returning NULL in gist_page_items
Per Coverity. BuildIndexValueDescription() cannot actually return NULL in
this instance, because it only returns NULL if the user doesn't have the
required privileges, and this function can only be used by superuser. But
better safe than sorry.
Peter Eisentraut [Mon, 18 Jan 2021 07:49:10 +0000 (08:49 +0100)]
Pause recovery for insufficient parameter settings
When certain parameters are changed on a physical replication primary,
this is communicated to standbys using the XLOG_PARAMETER_CHANGE WAL
record. The standby then checks whether its own settings are at least
as big as the ones on the primary. If not, the standby shuts down
with a fatal error.
This patch changes this behavior for hot standbys to pause recovery at
that point instead. That allows read traffic on the standby to
continue while database administrators figure out next steps. When
recovery is unpaused, the server shuts down (as before). The idea is
to fix the parameters while recovery is paused and then restart when
there is a maintenance window.
Reviewed-by: Sergei Kornilov <sk@zsrv.org>
Discussion: https://www.postgresql.org/message-id/flat/
4ad69a4c-cc9b-0dfe-0352-
8b1b0cd36c7b@2ndquadrant.com
Fujii Masao [Mon, 18 Jan 2021 06:11:08 +0000 (15:11 +0900)]
postgres_fdw: Add function to list cached connections to foreign servers.
This commit adds function postgres_fdw_get_connections() to return
the foreign server names of all the open connections that postgres_fdw
established from the local session to the foreign servers. This function
also returns whether each connection is valid or not.
This function is useful when checking all the open foreign server connections.
If we found some connection to drop, from the result of function, probably
we can explicitly close them by the function that upcoming commit will add.
This commit bumps the version of postgres_fdw to 1.1 since it adds
new function.
Author: Bharath Rupireddy, tweaked by Fujii Masao
Reviewed-by: Zhijie Hou, Alexey Kondratov, Zhihong Yu, Fujii Masao
Discussion: https://postgr.es/m/
2d5cb0b3-a6e8-9bbb-953f-
879f47128faa@oss.nttdata.com
Michael Paquier [Mon, 18 Jan 2021 05:03:10 +0000 (14:03 +0900)]
Refactor option handling of CLUSTER, REINDEX and VACUUM
This continues the work done in
b5913f6. All the options of those
commands are changed to use hex values rather than enums to reduce the
risk of compatibility bugs when introducing new options. Each option
set is moved into a new structure that can be extended with more
non-boolean options (this was already the case of VACUUM). The code of
REINDEX is restructured so as manual REINDEX commands go through a
single routine from utility.c, like VACUUM, to ease the allocation
handling of option parameters when a command needs to go through
multiple transactions.
This can be used as a base infrastructure for future patches related to
those commands, including reindex filtering and tablespace support.
Per discussion with people mentioned below, as well as Alvaro Herrera
and Peter Eisentraut.
Author: Michael Paquier, Justin Pryzby
Reviewed-by: Alexey Kondratov, Justin Pryzby
Discussion: https://postgr.es/m/X8riynBLwxAD9uKk@paquier.xyz
Heikki Linnakangas [Sun, 17 Jan 2021 22:46:03 +0000 (00:46 +0200)]
pageinspect: Fix relcache leak in gist_page_items().
The gist_page_items() function opened the index relation on first call and
closed it on the last call. But there's no guarantee that the function is
run to completion, leading to a relcache leak and warning at the end of
the transaction. To fix, refactor the function to return all the rows in
one call, as a tuplestore.
Reported-by: Tom Lane
Discussion: https://www.postgresql.org/message-id/234863.
1610916631%40sss.pgh.pa.us
Tomas Vondra [Sun, 17 Jan 2021 21:11:39 +0000 (22:11 +0100)]
Set PD_ALL_VISIBLE and visibility map bits in COPY FREEZE
Make sure COPY FREEZE marks the pages as PD_ALL_VISIBLE and updates the
visibility map. Until now we only marked individual tuples as frozen,
but page-level flags were not updated, so the first VACUUM after the
COPY FREEZE had to rewrite the whole table.
This is a fairly old patch, and multiple people worked on it. The first
version was written by Jeff Janes, and then reworked by Pavan Deolasee
and Anastasia Lubennikova.
Author: Anastasia Lubennikova, Pavan Deolasee, Jeff Janes
Reviewed-by: Kuntal Ghosh, Jeff Janes, Tomas Vondra, Masahiko Sawada,
Andres Freund, Ibrar Ahmed, Robert Haas, Tatsuro Ishii,
Darafei Praliaskouski
Discussion: https://postgr.es/m/CABOikdN-ptGv0mZntrK2Q8OtfUuAjqaYMGmkdU1dCKFtUxVLrg@mail.gmail.com
Discussion: https://postgr.es/m/CAMkU%3D1w3osJJ2FneELhhNRLxfZitDgp9FPHee08NT2FQFmz_pQ%40mail.gmail.com
Tom Lane [Sun, 17 Jan 2021 17:53:48 +0000 (12:53 -0500)]
Add missing array-enlargement logic to test_regex.c.
The stanza to report a "partial" match could overrun the initially
allocated output array, so it needs its own copy of the array-resizing
logic that's in the main loop. I overlooked the need for this in
ca8217c10.
Per report from Alexander Lakhin.
Discussion: https://postgr.es/m/
3206aace-50db-e02a-bbea-
76d5cdaa2cb6@gmail.com
Magnus Hagander [Sun, 17 Jan 2021 14:31:23 +0000 (15:31 +0100)]
Add documentation chapter about checksums
Data checksums did not have a longer discussion in the docs,
this adds a short section with an overview.
Extracted from the larger patch for on-line enabling of checksums, which
has many more authors and reviewers.
Author: Daniel Gustafsson
Reviewed-By: Magnus Hagander, Michael Banck (and others through the big patch)
Discussion: https://postgr.es/m/
5ff49fa4.
1c69fb81.658f3.04ac@mx.google.com
Tomas Vondra [Sun, 17 Jan 2021 14:11:14 +0000 (15:11 +0100)]
Revert "psql \dX: list extended statistics objects"
Reverts
891a1d0bca, because the new psql command \dX only worked for
users users who can read pg_statistic_ext_data catalog, and most regular
users lack that privilege (the catalog may contain sensitive user data).
Reported-by: Noriyoshi Shinoda
Discussion: https://postgr.es/m/
c027a541-5856-75a5-0868-
341301e1624b%40nttcom.co.jp_1
Magnus Hagander [Sun, 17 Jan 2021 13:28:17 +0000 (14:28 +0100)]
Add --no-instructions parameter to initdb
Specifying this parameter removes the informational messages about how
to start the server. This is intended for use by wrappers in different
packaging systems, where those instructions would most likely be wrong
anyway, but the other output from initdb would still be useful (and thus
just redirecting everything to /dev/null would be bad).
Author: Magnus Hagander
Reviewed-By: Peter Eisentraut
Discusion: https://postgr.es/m/CABUevEzo4t5bmTXF0_B9WzmuWpVbMpkNZZiGvzV8NZa-=fPqeQ@mail.gmail.com
Magnus Hagander [Sun, 17 Jan 2021 12:34:09 +0000 (13:34 +0100)]
Add pg_stat_database counters for sessions and session time
This add counters for number of sessions, the different kind of session
termination types, and timers for how much time is spent in active vs
idle in a database to pg_stat_database.
Internally this also renames the parameter "force" to disconnect. This
was the only use-case for the parameter before, so repurposing it to
this mroe narrow usecase makes things cleaner than inventing something
new.
Author: Laurenz Albe
Reviewed-By: Magnus Hagander, Soumyadeep Chakraborty, Masahiro Ikeda
Discussion: https://postgr.es/m/
b07e1f9953701b90c66ed368656f2aef40cac4fb.camel@cybertec.at
Tomas Vondra [Sat, 16 Jan 2021 23:16:25 +0000 (00:16 +0100)]
psql \dX: list extended statistics objects
The new command lists extended statistics objects, possibly with their
sizes. All past releases with extended statistics are supported.
Author: Tatsuro Yamada
Reviewed-by: Julien Rouhaud, Alvaro Herrera, Tomas Vondra
Discussion: https://postgr.es/m/
c027a541-5856-75a5-0868-
341301e1624b%40nttcom.co.jp_1
Jeff Davis [Sat, 16 Jan 2021 22:40:12 +0000 (14:40 -0800)]
Documenation fixups for replication protocol.
There is no CopyResponse message; it should be CopyOutResponse.
Also, if there is no WAL to stream, the server does not immediately
send a CommandComplete; it's a historical timeline, so it will send a
response tuple first.
Discussion: https://postgr.es/m/
0a2c985ebcaa1acd385350aeba561b6509187394.camel@j-davis.com
Noah Misch [Sat, 16 Jan 2021 20:21:35 +0000 (12:21 -0800)]
Fix pg_dump for GRANT OPTION among initial privileges.
The context is an object that no longer bears some aclitem that it bore
initially. (A user issued REVOKE or GRANT statements upon the object.)
pg_dump is forming SQL to reproduce the object ACL. Since initdb
creates no ACL bearing GRANT OPTION, reaching this bug requires an
extension where the creation script establishes such an ACL. No PGXN
extension does that. If an installation did reach the bug, pg_dump
would have omitted a semicolon, causing a REVOKE and the next SQL
statement to fail. Separately, since the affected code exists to
eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT
OPTION FOR. Back-patch to 9.6, where commit
23f34fa4ba358671adab16773e79c17c92cbc870 first appeared.
Discussion: https://postgr.es/m/
20210109102423.GA160022@rfd.leadboat.com
Noah Misch [Sat, 16 Jan 2021 20:21:35 +0000 (12:21 -0800)]
Prevent excess SimpleLruTruncate() deletion.
Every core SLRU wraps around. With the exception of pg_notify, the wrap
point can fall in the middle of a page. Account for this in the
PagePrecedes callback specification and in SimpleLruTruncate()'s use of
said callback. Update each callback implementation to fit the new
specification. This changes SerialPagePrecedesLogically() from the
style of asyncQueuePagePrecedes() to the style of CLOGPagePrecedes().
(Whereas pg_clog and pg_serial share a key space, pg_serial is nothing
like pg_notify.) The bug fixed here has the same symptoms and user
followup steps as
592a589a04bd456410b853d86bd05faa9432cbbb. Back-patch
to 9.5 (all supported versions).
Reviewed by Andrey Borodin and (in earlier versions) by Tom Lane.
Discussion: https://postgr.es/m/
20190202083822.GC32531@gust.leadboat.com
Amit Kapila [Sat, 16 Jan 2021 04:45:32 +0000 (10:15 +0530)]
Remove unnecessary pstrdup in fetch_table_list.
The result of TextDatumGetCString is already palloc'ed so we don't need to
allocate memory for it again. We decide not to backpatch it as there
doesn't seem to be any case where it can create a meaningful leak.
Author: Zhijie Hou
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/
229fed2eb8c54c71a96ccb99e516eb12@G08CNEXMBPEKD05.g08.fujitsu.local
Tomas Vondra [Fri, 15 Jan 2021 22:24:19 +0000 (23:24 +0100)]
Disallow CREATE STATISTICS on system catalogs
Add a check that CREATE STATISTICS does not add extended statistics on
system catalogs, similarly to indexes etc. It can be overriden using
the allow_system_table_mods GUC.
This bug exists since
7b504eb282c, adding the extended statistics, so
backpatch all the way back to PostgreSQL 10.
Author: Tomas Vondra
Reported-by: Dean Rasheed
Backpatch-through: 10
Discussion: https://postgr.es/m/CAEZATCXAPrrOKwEsyZKQ4uzzJQWBCt6QAvOcgqRGdWwT1zb%2BrQ%40mail.gmail.com
Tom Lane [Fri, 15 Jan 2021 16:28:51 +0000 (11:28 -0500)]
Improve our heuristic for selecting PG_SYSROOT on macOS.
In cases where Xcode is newer than the underlying macOS version,
asking xcodebuild for the SDK path will produce a pointer to the
SDK shipped with Xcode, which may end up building code that does
not work on the underlying macOS version. It appears that in
such cases, xcodebuild's answer also fails to match the default
behavior of Apple's compiler: assuming one has installed Xcode's
"command line tools", there will be an SDK for the OS's own version
in /Library/Developer/CommandLineTools, and the compiler will
default to using that. This is all pretty poorly documented,
but experimentation suggests that "xcrun --show-sdk-path" gives
the sysroot path that the compiler is actually using, at least
in some cases. Hence, try that first, but revert to xcodebuild
if xcrun fails (in very old Xcode, it is missing or lacks the
--show-sdk-path switch).
Also, "xcrun --show-sdk-path" may give a path that is valid but lacks
any OS version identifier. We don't really want that, since most
of the motivation for wiring -isysroot into the build flags at all
is to ensure that all parts of a PG installation are built against
the same SDK, even when considering extensions built later and/or on
a different machine. Insist on finding "N.N" in the directory name
before accepting the result. (Adding "--sdk macosx" to the xcrun
call seems to produce the same answer as xcodebuild, but usually
more quickly because it's cached, so we also try that as a fallback.)
The core reason why we don't want to use Xcode's default SDK in cases
like this is that Apple's technology for introducing new syscalls
does not play nice with Autoconf: for example, configure will think
that preadv/pwritev exist when using a Big Sur SDK, even when building
on an older macOS version where they don't exist. It'd be nice to
have a better solution to that problem, but this patch doesn't attempt
to fix that.
Per report from Sergey Shinderuk. Back-patch to all supported versions.
Discussion: https://postgr.es/m/
ed3b8e5d-0da8-6ebd-fd1c-
e0ac80a4b204@postgrespro.ru
Alvaro Herrera [Fri, 15 Jan 2021 13:31:42 +0000 (10:31 -0300)]
Avoid spurious wait in concurrent reindex
This is like commit
c98763bf51bf, but for REINDEX CONCURRENTLY. To wit:
this flags indicates that the current process is safe to ignore for the
purposes of waiting for other snapshots, when doing CREATE INDEX
CONCURRENTLY and REINDEX CONCURRENTLY. This helps two processes doing
either of those things not deadlock, and also avoids spurious waits.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Hamid Akhtar <hamid.akhtar@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/
20201130195439.GA24598@alvherre.pgsql
Fujii Masao [Fri, 15 Jan 2021 03:44:17 +0000 (12:44 +0900)]
Fix calculation of how much shared memory is required to store a TOC.
Commit
ac883ac453 refactored shm_toc_estimate() but changed its calculation
of shared memory size for TOC incorrectly. Previously this could cause too
large memory to be allocated.
Back-patch to v11 where the bug was introduced.
Author: Takayuki Tsunakawa
Discussion: https://postgr.es/m/TYAPR01MB2990BFB73170E2C4921E2C4DFEA80@TYAPR01MB2990.jpnprd01.prod.outlook.com
Michael Paquier [Fri, 15 Jan 2021 02:46:34 +0000 (11:46 +0900)]
Remove PG_SHA*_DIGEST_STRING_LENGTH from sha2.h
The last reference to those variables has been removed in
aef8948, so
this cleans up a bit the code.
Discussion: https://postgr.es/m/X//ggAqmTtt+3t7X@paquier.xyz
Michael Paquier [Fri, 15 Jan 2021 01:33:13 +0000 (10:33 +0900)]
Fix O(N^2) stat() calls when recycling WAL segments
The counter tracking the last segment number recycled was getting
initialized when recycling one single segment, while it should be used
across a full cycle of segments recycled to prevent useless checks
related to entries already recycled.
This performance issue has been introduced by
b2a5545, and it was first
implemented in
61b86142.
No backpatch is done per the lack of field complaints.
Reported-by: Andres Freund, Thomas Munro
Author: Michael Paquier
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/
20170621211016.eln6cxxp3jrv7m4m@alap3.anarazel.de
Discussion: https://postgr.es/m/CA+hUKG+DRiF9z1_MU4fWq+RfJMxP7zjoptfcmuCFPeO4JM2iVg@mail.gmail.com
Fujii Masao [Fri, 15 Jan 2021 01:30:19 +0000 (10:30 +0900)]
postgres_fdw: Save foreign server OID in connection cache entry.
The foreign server OID stored in the connection cache entry is used as
a lookup key to directly get the server name.
Previously since the connection cache entry did not have the server OID,
postgres_fdw had to get the server OID at first from user mapping before
getting the server name. So if the corresponding user mapping was dropped,
postgres_fdw could raise the error "cache lookup failed for user mapping"
while looking up user mapping and fail to get the server name even though
the server had not been dropped yet.
Author: Bharath Rupireddy
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/CALj2ACVRZPUB7ZwqLn-6DY8C_UmPs6084gSpHA92YBv++1AJXA@mail.gmail.com
Tom Lane [Thu, 14 Jan 2021 21:19:38 +0000 (16:19 -0500)]
pg_dump: label PUBLICATION TABLE ArchiveEntries with an owner.
This is the same fix as commit
9eabfe300 applied to INDEX ATTACH
entries, but for table-to-publication attachments. As in that
case, even though the backend doesn't record "ownership" of the
attachment, we still ought to label it in the dump archive with
the role name that should run the ALTER PUBLICATION command.
The existing behavior causes the ALTER to be done by the original
role that started the restore; that will usually work fine, but
there may be corner cases where it fails.
The bulk of the patch is concerned with changing struct
PublicationRelInfo to include a pointer to the associated
PublicationInfo object, so that we can get the owner's name
out of that when the time comes. While at it, I rewrote
getPublicationTables() to do just one query of pg_publication_rel,
not one per table.
Back-patch to v10 where this code was introduced.
Discussion: https://postgr.es/m/
1165710.
1610473242@sss.pgh.pa.us
Alvaro Herrera [Thu, 14 Jan 2021 18:32:14 +0000 (15:32 -0300)]
Prevent drop of tablespaces used by partitioned relations
When a tablespace is used in a partitioned relation (per commits
ca4103025dfe in pg12 for tables and
33e6c34c3267 in pg11 for indexes),
it is possible to drop the tablespace, potentially causing various
problems. One such was reported in bug #16577, where a rewriting ALTER
TABLE causes a server crash.
Protect against this by using pg_shdepend to keep track of tablespaces
when used for relations that don't keep physical files; we now abort a
tablespace if we see that the tablespace is referenced from any
partitioned relations.
Backpatch this to 11, where this problem has been latent all along. We
don't try to create pg_shdepend entries for existing partitioned
indexes/tables, but any ones that are modified going forward will be
protected.
Note slight behavior change: when trying to drop a tablespace that
contains both regular tables as well as partitioned ones, you'd
previously get ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE and now you'll
get ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST. Arguably, the latter is more
correct.
It is possible to add protecting pg_shdepend entries for existing
tables/indexes, by doing
ALTER TABLE ONLY some_partitioned_table SET TABLESPACE pg_default;
ALTER TABLE ONLY some_partitioned_table SET TABLESPACE original_tablespace;
for each partitioned table/index that is not in the database default
tablespace. Because these partitioned objects do not have storage, no
file needs to be actually moved, so it shouldn't take more time than
what's required to acquire locks.
This query can be used to search for such relations:
SELECT ... FROM pg_class WHERE relkind IN ('p', 'I') AND reltablespace <> 0
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/16577-
881633a9f9894fd5@postgresql.org
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Fujii Masao [Thu, 14 Jan 2021 05:37:01 +0000 (14:37 +0900)]
Stabilize timeline switch regression test.
Commit
fef5b47f6b added the regression test to check whether a standby is
able to follow a primary on a newer timeline when WAL archiving is enabled.
But the buildfarm member florican reported that this test failed because
the requested WAL segment was removed and replication failed. This is a
timing issue. Since neither replication slot is used nor wal_keep_size is set
in the test, checkpoint could remove the WAL segment that's still necessary
for replication.
This commit stabilizes the test by setting wal_keep_size.
Back-patch to v13 where the regression test that this commit stabilizes
was added.
Author: Fujii Masao
Discussion: https://postgr.es/m/X//PsenxcC50jDzX@paquier.xyz
Fujii Masao [Thu, 14 Jan 2021 06:41:22 +0000 (15:41 +0900)]
Improve tab-completion for CLOSE, DECLARE, FETCH and MOVE.
This commit makes CLOSE, FETCH and MOVE commands tab-complete the list of
cursors. Also this commit makes DECLARE command tab-complete the options.
Author: Shinya Kato, Sawada Masahiko, tweaked by Fujii Masao
Reviewed-by: Shinya Kato, Sawada Masahiko, Fujii Masao
Discussion: https://postgr.es/m/
b0e4c5c53ef84c5395524f5056fc71f0@MP-MSGSS-MBX001.msg.nttdata.co.jp
Thomas Munro [Thu, 14 Jan 2021 05:09:32 +0000 (18:09 +1300)]
Minor header cleanup for the new iovec code.
Remove redundant function declaration and improve header comment in
pg_iovec.h. Move the new declaration in fd.h next to a group of more
similar functions.
Fujii Masao [Thu, 14 Jan 2021 03:27:11 +0000 (12:27 +0900)]
Ensure that a standby is able to follow a primary on a newer timeline.
Commit
709d003fbd refactored WAL-reading code, but accidentally caused
WalSndSegmentOpen() to fail to follow a timeline switch while reading from
a historic timeline. This issue caused a standby to fail to follow a primary
on a newer timeline when WAL archiving is enabled.
If there is a timeline switch within the segment, WalSndSegmentOpen() should
read from the WAL segment belonging to the new timeline. But previously
since it failed to follow a timeline switch, it tried to read the WAL segment
with old timeline. When WAL archiving is enabled, that WAL segment with
old timeline doesn't exist because it's renamed to .partial. This leads
a primary to have tried to read non-existent WAL segment, and which caused
replication to faill with the error "ERROR: requested WAL segment ... has
already been removed".
This commit fixes WalSndSegmentOpen() so that it's able to follow a timeline
switch, to ensure that a standby is able to follow a primary on a newer
timeline even when WAL archiving is enabled.
This commit also adds the regression test to check whether a standby is
able to follow a primary on a newer timeline when WAL archiving is enabled.
Back-patch to v13 where the bug was introduced.
Reported-by: Kyotaro Horiguchi
Author: Kyotaro Horiguchi, tweaked by Fujii Masao
Reviewed-by: Alvaro Herrera, Fujii Masao
Discussion: https://postgr.es/m/
20201209.174314.
282492377848029776.horikyota.ntt@gmail.com
Michael Paquier [Thu, 14 Jan 2021 02:13:24 +0000 (11:13 +0900)]
Rework refactoring of hex and encoding routines
This commit addresses some issues with
c3826f83 that moved the hex
decoding routine to src/common/:
- The decoding function lacked overflow checks, so when used for
security-related features it was an open door to out-of-bound writes if
not carefully used that could remain undetected. Like the base64
routines already in src/common/ used by SCRAM, this routine is reworked
to check for overflows by having the size of the destination buffer
passed as argument, with overflows checked before doing any writes.
- The encoding routine was missing. This is moved to src/common/ and
it gains the same overflow checks as the decoding part.
On failure, the hex routines of src/common/ issue an error as per the
discussion done to make them usable by frontend tools, but not by shared
libraries. Note that this is why ECPG is left out of this commit, and
it still includes a duplicated logic doing hex encoding and decoding.
While on it, this commit uses better variable names for the source and
destination buffers in the existing escape and base64 routines in
encode.c and it makes them more robust to overflow detection. The
previous core code issued a FATAL after doing out-of-bound writes if
going through the SQL functions, which would be enough to detect
problems when working on changes that impacted this area of the
code. Instead, an error is issued before doing an out-of-bound write.
The hex routines were being directly called for bytea conversions and
backup manifests without such sanity checks. The current calls happen
to not have any problems, but careless uses of such APIs could easily
lead to CVE-class bugs.
Author: Bruce Momjian, Michael Paquier
Reviewed-by: Sehrope Sarkuni
Discussion: https://postgr.es/m/
20201231003557.GB22199@momjian.us
Thomas Munro [Wed, 13 Jan 2021 22:10:24 +0000 (11:10 +1300)]
Move our p{read,write}v replacements into their own files.
macOS's ranlib issued a warning about an empty pread.o file with the
previous arrangement, on systems new enough to require no replacement
functions. Let's go back to using configure's AC_REPLACE_FUNCS system
to build and include each .o in the library only if it's needed, which
requires moving the *v() functions to their own files.
Also move the _with_retry() wrapper to a more permanent home.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
1283127.
1610554395%40sss.pgh.pa.us
Tom Lane [Wed, 13 Jan 2021 21:23:15 +0000 (16:23 -0500)]
Mark inet_server_addr() and inet_server_port() as parallel-restricted.
These need to be PR because they access the MyProcPort data structure,
which doesn't get copied to parallel workers. The very similar
functions inet_client_addr() and inet_client_port() are already
marked PR, but somebody missed these.
Although this is a pre-existing bug, we can't readily fix it in the back
branches since we can't force initdb. Given the small usage of these
two functions, and the even smaller likelihood that they'd get pushed to
a parallel worker anyway, it doesn't seem worth the trouble to suggest
that DBAs should fix it manually.
Masahiko Sawada
Discussion: https://postgr.es/m/CAD21AoAT4aHP0Uxq91qpD7NL009tnUYQe-b14R3MnSVOjtE71g@mail.gmail.com
Tom Lane [Wed, 13 Jan 2021 21:14:38 +0000 (16:14 -0500)]
Run reformat-dat-files to declutter the catalog data files.
Things had gotten pretty messy here, apparently mostly but not
entirely the fault of the multirange patch. No functional changes.
Tom Lane [Wed, 13 Jan 2021 20:59:57 +0000 (15:59 -0500)]
Doc, more or less: uncomment tutorial example that was fixed long ago.
Reverts a portion of commit
344190b7e. Apparently, back in the
twentieth century we had some issues with multi-statement SQL
functions, but they've worked fine for a long time.
Daniel Westermann
Discussion: https://postgr.es/m/GVAP278MB04242DCBF5E31F528D53FA18D2A90@GVAP278MB0424.CHEP278.PROD.OUTLOOK.COM
Alvaro Herrera [Wed, 13 Jan 2021 20:55:41 +0000 (17:55 -0300)]
Call out vacuum considerations in create index docs
Backpatch to pg12, which is as far as it goes without conflicts.
Author: James Coleman <jtc331@gmail.com>
Reviewed-by: "David G. Johnston" <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAAaqYe9oEfbz7AxXq7OX+FFVi5w5p1e_Of8ON8ZnKO9QqBfmjg@mail.gmail.com
Tom Lane [Wed, 13 Jan 2021 19:52:49 +0000 (14:52 -0500)]
Disallow a digit as the first character of a variable name in pgbench.
The point of this restriction is to avoid trying to substitute variables
into timestamp literal values, which may contain strings like '12:34'.
There is a good deal more that should be done to reduce pgbench's
tendency to substitute where it shouldn't. But this is sufficient to
solve the case complained of by Jaime Soler, and it's simple enough
to back-patch.
Back-patch to v11; before commit
9d36a3866, pgbench had a slightly
different definition of what a variable name is, and anyway it seems
unwise to change long-stable branches for this.
Fabien Coelho
Discussion: https://postgr.es/m/alpine.DEB.2.22.394.
2006291740420.805678@pseudo
Heikki Linnakangas [Wed, 13 Jan 2021 18:57:21 +0000 (20:57 +0200)]
Fix test failure with wal_level=minimal.
The newly-added gist pageinspect test prints the LSNs of GiST pages,
expecting them all to be 1 (GistBuildLSN). But with wal_level=minimal,
they got updated by the whole-relation WAL-logging at commit. Fix by
wrapping the problematic tests in the same transaction with the CREATE
INDEX.
Per buildfarm failure on thorntail.
Discussion: https://www.postgresql.org/message-id/
3B4F97E5-40FB-4142-8CAA-
B301CDFBF982%40iki.fi
Tom Lane [Wed, 13 Jan 2021 18:30:04 +0000 (13:30 -0500)]
Doc: clarify behavior of back-half options in pg_dump.
Options that change how the archive data is converted to SQL text
are ignored when dumping to archive formats. The documentation
previously said "not meaningful", which is not helpful.
Discussion: https://postgr.es/m/
161052021249.12228.
9598689907884726185@wrigleys.postgresql.org
Peter Geoghegan [Wed, 13 Jan 2021 17:21:32 +0000 (09:21 -0800)]
Enhance nbtree index tuple deletion.
Teach nbtree and heapam to cooperate in order to eagerly remove
duplicate tuples representing dead MVCC versions. This is "bottom-up
deletion". Each bottom-up deletion pass is triggered lazily in response
to a flood of versions on an nbtree leaf page. This usually involves a
"logically unchanged index" hint (these are produced by the executor
mechanism added by commit
9dc718bd).
The immediate goal of bottom-up index deletion is to avoid "unnecessary"
page splits caused entirely by version duplicates. It naturally has an
even more useful effect, though: it acts as a backstop against
accumulating an excessive number of index tuple versions for any given
_logical row_. Bottom-up index deletion complements what we might now
call "top-down index deletion": index vacuuming performed by VACUUM.
Bottom-up index deletion responds to the immediate local needs of
queries, while leaving it up to autovacuum to perform infrequent clean
sweeps of the index. The overall effect is to avoid certain
pathological performance issues related to "version churn" from UPDATEs.
The previous tableam interface used by index AMs to perform tuple
deletion (the table_compute_xid_horizon_for_tuples() function) has been
replaced with a new interface that supports certain new requirements.
Many (perhaps all) of the capabilities added to nbtree by this commit
could also be extended to other index AMs. That is left as work for a
later commit.
Extend deletion of LP_DEAD-marked index tuples in nbtree by adding logic
to consider extra index tuples (that are not LP_DEAD-marked) for
deletion in passing. This increases the number of index tuples deleted
significantly in many cases. The LP_DEAD deletion process (which is now
called "simple deletion" to clearly distinguish it from bottom-up
deletion) won't usually need to visit any extra table blocks to check
these extra tuples. We have to visit the same table blocks anyway to
generate a latestRemovedXid value (at least in the common case where the
index deletion operation's WAL record needs such a value).
Testing has shown that the "extra tuples" simple deletion enhancement
increases the number of index tuples deleted with almost any workload
that has LP_DEAD bits set in leaf pages. That is, it almost never fails
to delete at least a few extra index tuples. It helps most of all in
cases that happen to naturally have a lot of delete-safe tuples. It's
not uncommon for an individual deletion operation to end up deleting an
order of magnitude more index tuples compared to the old naive approach
(e.g., custom instrumentation of the patch shows that this happens
fairly often when the regression tests are run).
Add a further enhancement that augments simple deletion and bottom-up
deletion in indexes that make use of deduplication: Teach nbtree's
_bt_delitems_delete() function to support granular TID deletion in
posting list tuples. It is now possible to delete individual TIDs from
posting list tuples provided the TIDs have a tableam block number of a
table block that gets visited as part of the deletion process (visiting
the table block can be triggered directly or indirectly). Setting the
LP_DEAD bit of a posting list tuple is still an all-or-nothing thing,
but that matters much less now that deletion only needs to start out
with the right _general_ idea about which index tuples are deletable.
Bump XLOG_PAGE_MAGIC because xl_btree_delete changed.
No bump in BTREE_VERSION, since there are no changes to the on-disk
representation of nbtree indexes. Indexes built on PostgreSQL 12 or
PostgreSQL 13 will automatically benefit from bottom-up index deletion
(i.e. no reindexing required) following a pg_upgrade. The enhancement
to simple deletion is available with all B-Tree indexes following a
pg_upgrade, no matter what PostgreSQL version the user upgrades from.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-By: Victor Yegorov <vyegorov@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wzm+maE3apHB8NOtmM=p-DO65j2V5GzAWCOEEuy3JZgb2g@mail.gmail.com
Peter Geoghegan [Wed, 13 Jan 2021 16:11:00 +0000 (08:11 -0800)]
Pass down "logically unchanged index" hint.
Add an executor aminsert() hint mechanism that informs index AMs that
the incoming index tuple (the tuple that accompanies the hint) is not
being inserted by execution of an SQL statement that logically modifies
any of the index's key columns.
The hint is received by indexes when an UPDATE takes place that does not
apply an optimization like heapam's HOT (though only for indexes where
all key columns are logically unchanged). Any index tuple that receives
the hint on insert is expected to be a duplicate of at least one
existing older version that is needed for the same logical row. Related
versions will typically be stored on the same index page, at least
within index AMs that apply the hint.
Recognizing the difference between MVCC version churn duplicates and
true logical row duplicates at the index AM level can help with cleanup
of garbage index tuples. Cleanup can intelligently target tuples that
are likely to be garbage, without wasting too many cycles on less
promising tuples/pages (index pages with little or no version churn).
This is infrastructure for an upcoming commit that will teach nbtree to
perform bottom-up index deletion. No index AM actually applies the hint
just yet.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Victor Yegorov <vyegorov@gmail.com>
Discussion: https://postgr.es/m/CAH2-Wz=CEKFa74EScx_hFVshCOn6AA5T-ajFASTdzipdkLTNQQ@mail.gmail.com
Fujii Masao [Wed, 13 Jan 2021 13:59:17 +0000 (22:59 +0900)]
Log long wait time on recovery conflict when it's resolved.
This is a follow-up of the work done in commit
0650ff2303. This commit
extends log_recovery_conflict_waits so that a log message is produced
also when recovery conflict has already been resolved after deadlock_timeout
passes, i.e., when the startup process finishes waiting for recovery
conflict after deadlock_timeout. This is useful in investigating how long
recovery conflicts prevented the recovery from applying WAL.
Author: Fujii Masao
Reviewed-by: Kyotaro Horiguchi, Bertrand Drouvot
Discussion: https://postgr.es/m/
9a60178c-a853-1440-2cdc-
c3af916cff59@amazon.com
Heikki Linnakangas [Wed, 13 Jan 2021 10:32:54 +0000 (12:32 +0200)]
Fix portability issues in the new gist pageinspect test.
1. The raw bytea representation of the point-type keys used in the test
depends on endianess. Remove the raw key_data column from the test.
2. The items stored on non-leftmost gist page depends on how many items
git on the other pages. This showed up as a failure on 32-bit i386
systems. To fix, only test the gist_page_items() function on the
leftmost leaf page.
Per Andrey Borodin and the buildfarm.
Discussion: https://www.postgresql.org/message-id/
9FCEC1DC-86FB-4A57-88EF-
DD13663B36AF%40yandex-team.ru
Magnus Hagander [Wed, 13 Jan 2021 10:07:37 +0000 (11:07 +0100)]
Remove incorrect markup
Seems
737d69ffc3c made a copy/paste or automation error resulting in two
extra right-parenthesis.
Reported-By: Michael Vastola
Backpatch-through: 13
Discussion: https://postgr.es/m/
161051035421.12224.
1741822783166533529@wrigleys.postgresql.org
Heikki Linnakangas [Wed, 13 Jan 2021 08:33:33 +0000 (10:33 +0200)]
Add functions to 'pageinspect' to inspect GiST indexes.
Author: Andrey Borodin and me
Discussion: https://www.postgresql.org/message-id/
3E4F9093-A1B5-4DF8-A292-
0B48692E3954%40yandex-team.ru
Thomas Munro [Wed, 13 Jan 2021 06:11:09 +0000 (19:11 +1300)]
Don't use elog() in src/port/pwrite.c.
Nothing broke because of this oversight yet, but it would fail to link
if we tried to use pg_pwrite() in frontend code on a system that lacks
pwrite(). Use an assertion instead. Also pgindent while here.
Discussion: https://postgr.es/m/CA%2BhUKGL57RvoQsS35TVPnQoPYqbtBixsdRhynB8NpcUKpHTTtg%40mail.gmail.com
Amit Kapila [Wed, 13 Jan 2021 02:49:50 +0000 (08:19 +0530)]
Fix memory leak in SnapBuildSerialize.
The memory for the snapshot was leaked while serializing it to disk during
logical decoding. This memory will be freed only once walsender stops
streaming the changes. This can lead to a huge memory increase when master
logs Standby Snapshot too frequently say when the user is trying to create
many replication slots.
Reported-by: funnyxj.fxj@alibaba-inc.com
Diagnosed-by: funnyxj.fxj@alibaba-inc.com
Author: Amit Kapila
Backpatch-through: 9.5
Discussion: https://postgr.es/m/
033ab54c-6393-42ee-8ec9-
2b399b5d8cde.funnyxj.fxj@alibaba-inc.com
Amit Kapila [Wed, 13 Jan 2021 02:16:11 +0000 (07:46 +0530)]
Optimize DropRelFileNodesAllBuffers() for recovery.
Similar to commit
d6ad34f341, this patch optimizes
DropRelFileNodesAllBuffers() by avoiding the complete buffer pool scan and
instead find the buffers to be invalidated by doing lookups in the
BufMapping table.
This optimization helps operations where the relation files need to be
removed like Truncate, Drop, Abort of Create Table, etc.
Author: Kirk Jamison
Reviewed-by: Kyotaro Horiguchi, Takayuki Tsunakawa, and Amit Kapila
Tested-By: Haiying Tang
Discussion: https://postgr.es/m/OSBPR01MB3207DCA7EC725FDD661B3EDAEF660@OSBPR01MB3207.jpnprd01.prod.outlook.com
Michael Paquier [Wed, 13 Jan 2021 01:32:21 +0000 (10:32 +0900)]
Fix routine name in comment of catcache.c
Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACUDXLAkf_XxQO9tAUtnTNGi3Lmd8fANd+vBJbcHn1HoWA@mail.gmail.com
Alvaro Herrera [Tue, 12 Jan 2021 20:04:49 +0000 (17:04 -0300)]
Invent struct ReindexIndexInfo
This struct is used by ReindexRelationConcurrently to keep track of the
relations to process. This saves having to obtain some data repeatedly,
and has future uses as well.
Reviewed-by: Dmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: Hamid Akhtar <hamid.akhtar@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/
20201130195439.GA24598@alvherre.pgsql
Tom Lane [Tue, 12 Jan 2021 18:37:38 +0000 (13:37 -0500)]
pg_dump: label INDEX ATTACH ArchiveEntries with an owner.
Although a partitioned index's attachment to its parent doesn't
have separate ownership, the ArchiveEntry for it needs to be
marked with an owner anyway, to ensure that the ALTER command
is run by the appropriate role when restoring with
--use-set-session-authorization. Without this, the ALTER will
be run by the role that started the restore session, which will
usually work but it's formally the wrong thing.
Back-patch to v11 where this type of ArchiveEntry was added.
In HEAD, add equivalent commentary to the just-added TABLE ATTACH
case, which I'd made do the right thing already.
Discussion: https://postgr.es/m/
1094034.
1610418498@sss.pgh.pa.us
Tom Lane [Tue, 12 Jan 2021 17:52:14 +0000 (12:52 -0500)]
Doc: fix description of privileges needed for ALTER PUBLICATION.
Adding a table to a publication requires ownership of the table
(in addition to ownership of the publication). This was mentioned
nowhere.
Alvaro Herrera [Tue, 12 Jan 2021 14:48:45 +0000 (11:48 -0300)]
Fix thinko in comment
This comment has been wrong since its introduction in commit
2c03216d8311.
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoAzz6qipFJBbGEaHmyWxvvNDp8httbwLR9tUQWaTjUs2Q@mail.gmail.com
Amit Kapila [Tue, 12 Jan 2021 02:49:39 +0000 (08:19 +0530)]
Fix relation descriptor leak.
We missed closing the relation descriptor while sending changes via the
root of partitioned relations during logical replication.
Author: Amit Langote and Mark Zhao
Reviewed-by: Amit Kapila and Ashutosh Bapat
Backpatch-through: 13, where it was introduced
Discussion: https://postgr.es/m/tencent_41FEA657C206F19AB4F406BE9252A0F69C06@qq.com
Discussion: https://postgr.es/m/tencent_6E296D2F7D70AFC90D83353B69187C3AA507@qq.com
Amit Kapila [Tue, 12 Jan 2021 02:15:40 +0000 (07:45 +0530)]
Optimize DropRelFileNodeBuffers() for recovery.
The recovery path of DropRelFileNodeBuffers() is optimized so that
scanning of the whole buffer pool can be avoided when the number of
blocks to be truncated in a relation is below a certain threshold. For
such cases, we find the buffers by doing lookups in BufMapping table.
This improves the performance by more than 100 times in many cases
when several small tables (tested with 1000 relations) are truncated
and where the server is configured with a large value of shared
buffers (greater than equal to 100GB).
This optimization helps cases (a) when vacuum or autovacuum truncated off
any of the empty pages at the end of a relation, or (b) when the relation is
truncated in the same transaction in which it was created.
This commit introduces a new API smgrnblocks_cached which returns a cached
value for the number of blocks in a relation fork. This helps us to determine
the exact size of relation which is required to apply this optimization. The
exact size is required to ensure that we don't leave any buffer for the
relation being dropped as otherwise the background writer or checkpointer
can lead to a PANIC error while flushing buffers corresponding to files that
don't exist.
Author: Kirk Jamison based on ideas by Amit Kapila
Reviewed-by: Kyotaro Horiguchi, Takayuki Tsunakawa, and Amit Kapila
Tested-By: Haiying Tang
Discussion: https://postgr.es/m/OSBPR01MB3207DCA7EC725FDD661B3EDAEF660@OSBPR01MB3207.jpnprd01.prod.outlook.com
Tom Lane [Tue, 12 Jan 2021 02:09:03 +0000 (21:09 -0500)]
Dump ALTER TABLE ... ATTACH PARTITION as a separate ArchiveEntry.
Previously, we emitted the ATTACH PARTITION command as part of
the child table's ArchiveEntry. This was a poor choice since it
complicates restoring the partition as a standalone table; you have
to ignore the error from the ATTACH, which isn't even an option when
restoring direct-to-database with pg_restore. (pg_restore will issue
the whole ArchiveEntry as one PQexec, so that any error rolls back
the table creation as well.) Hence, separate it out as its own
ArchiveEntry, as indeed we already did for index ATTACH PARTITION
commands.
Justin Pryzby
Discussion: https://postgr.es/m/
20201023052940.GE9241@telsasoft.com
Tom Lane [Tue, 12 Jan 2021 00:58:07 +0000 (19:58 -0500)]
Make pg_dump's table of object-type priorities more maintainable.
Wedging a new object type into this table has historically required
manually renumbering a lot of existing entries. (Although it appears
that some people got lazy and re-used the priority level of an
existing object type, even if it wasn't particularly related.)
We can let the compiler do the counting by inventing an enum type that
lists the desired priority levels in order. Now, if you want to add
or remove a priority level, that's a one-liner.
This patch is not purely cosmetic, because I split apart the priorities
of DO_COLLATION and DO_TRANSFORM, as well as those of DO_ACCESS_METHOD
and DO_OPERATOR, which look to me to have been merged out of expediency
rather than because it was a good idea. Shell types continue to be
sorted interchangeably with full types, and opclasses interchangeably
with opfamilies.
Thomas Munro [Mon, 11 Jan 2021 21:55:35 +0000 (10:55 +1300)]
Fix function prototypes in dependency.h.
Commit
257836a7 accidentally deleted a couple of
redundant-but-conventional "extern" keywords on function prototypes.
Put them back.
Reported-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Tom Lane [Mon, 11 Jan 2021 19:53:42 +0000 (14:53 -0500)]
Rethink SQLSTATE code for ERRCODE_IDLE_SESSION_TIMEOUT.
Move it to class 57 (Operator Intervention), which seems like a
better choice given that from the client's standpoint it behaves
a heck of a lot like, e.g., ERRCODE_ADMIN_SHUTDOWN.
In a green field I'd put ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT
here as well. But that's been around for a few years, so it's
probably too late to change its SQLSTATE code.
Discussion: https://postgr.es/m/
763A0689-F189-459E-946F-
F0EC4458980B@hotmail.com
Tom Lane [Mon, 11 Jan 2021 19:12:31 +0000 (14:12 -0500)]
Try next host after a "cannot connect now" failure.
If a server returns ERRCODE_CANNOT_CONNECT_NOW, try the next host,
if multiple host names have been provided. This allows dealing
gracefully with standby servers that might not be in hot standby mode
yet.
In the wake of the preceding commit, it might be plausible to retry
many more error cases than we do now, but I (tgl) am hesitant to
move too aggressively on that --- it's not clear it'd be desirable
for cases such as bad-password, for example. But this case seems
safe enough.
Hubert Zhang, reviewed by Takayuki Tsunakawa
Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
Tom Lane [Mon, 11 Jan 2021 19:03:39 +0000 (14:03 -0500)]
Uniformly identify the target host in libpq connection failure reports.
Prefix "could not connect to host-or-socket-path:" to all connection
failure cases that occur after the socket() call, and remove the
ad-hoc server identity data that was appended to a few of these
messages. This should produce much more intelligible error reports
in multiple-target-host situations, especially for error cases that
are off the beaten track to any degree (because none of those provided
any server identity info).
As an example of the change, formerly a connection attempt with a bad
port number such as "psql -p 12345 -h localhost,/tmp" might produce
psql: error: could not connect to server: Connection refused
Is the server running on host "localhost" (::1) and accepting
TCP/IP connections on port 12345?
could not connect to server: Connection refused
Is the server running on host "localhost" (127.0.0.1) and accepting
TCP/IP connections on port 12345?
could not connect to server: No such file or directory
Is the server running locally and accepting
connections on Unix domain socket "/tmp/.s.PGSQL.12345"?
Now it looks like
psql: error: could not connect to host "localhost" (::1), port 12345: Connection refused
Is the server running on that host and accepting TCP/IP connections?
could not connect to host "localhost" (127.0.0.1), port 12345: Connection refused
Is the server running on that host and accepting TCP/IP connections?
could not connect to socket "/tmp/.s.PGSQL.12345": No such file or directory
Is the server running locally and accepting connections on that socket?
This requires adjusting a couple of regression tests to allow for
variation in the contents of a connection failure message.
Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
Tom Lane [Mon, 11 Jan 2021 18:43:19 +0000 (13:43 -0500)]
Allow pg_regress.c wrappers to postprocess test result files.
Add an optional callback to regression_main() that, if provided,
is invoked on each test output file before we try to compare it
to the expected-result file.
The main and isolation test programs don't need this (yet).
In pg_regress_ecpg, add a filter that eliminates target-host
details from "could not connect" error reports. This filter
doesn't do anything as of this commit, but it will be needed
by the next one.
In the long run we might want to provide some more general,
perhaps pattern-based, filtering mechanism for test output.
For now, this will solve the immediate problem.
Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
Tom Lane [Mon, 11 Jan 2021 18:12:09 +0000 (13:12 -0500)]
In libpq, always append new error messages to conn->errorMessage.
Previously, we had an undisciplined mish-mash of printfPQExpBuffer and
appendPQExpBuffer calls to report errors within libpq. This commit
establishes a uniform rule that appendPQExpBuffer[Str] should be used.
conn->errorMessage is reset only at the start of an application request,
and then accumulates messages till we're done. We can remove no less
than three different ad-hoc mechanisms that were used to get the effect
of concatenation of error messages within a sequence of operations.
Although this makes things quite a bit cleaner conceptually, the main
reason to do it is to make the world safer for the multiple-target-host
feature that was added awhile back. Previously, there were many cases
in which an error occurring during an individual host connection attempt
would wipe out the record of what had happened during previous attempts.
(The reporting is still inadequate, in that it can be hard to tell which
host got the failure, but that seems like a matter for a separate commit.)
Currently, lo_import and lo_export contain exceptions to the "never
use printfPQExpBuffer" rule. If we changed them, we'd risk reporting
an incidental lo_close failure before the actual read or write
failure, which would be confusing, not least because lo_close happened
after the main failure. We could improve this by inventing an
internal version of lo_close that doesn't reset the errorMessage; but
we'd also need a version of PQfn() that does that, and it didn't quite
seem worth the trouble for now.
Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com