Heikki Linnakangas [Wed, 14 Jul 2021 10:08:28 +0000 (13:08 +0300)]
In psql \copy from, send data to server in larger chunks.
Previously, we would send each line as a separate CopyData message.
That's pretty wasteful if the table is narrow, as each CopyData message
has 5 bytes of overhead. For efficiency, buffer up and pack 8 kB of
input data into each CopyData message.
The server also sends each line as a separate CopyData message in COPY TO
STDOUT, and that's similarly wasteful. But that's documented in the FE/BE
protocol description, so changing that would be a wire protocol break.
Reviewed-by: Aleksander Alekseev
Discussion: https://www.postgresql.org/message-id/
40b2cec0-d0fb-3191-2ae1-
9a3fe16a7e48%40iki.fi
Magnus Hagander [Wed, 14 Jul 2021 09:04:15 +0000 (11:04 +0200)]
Clarify description of pg_stat_statements columns
Reported-By: Peter Eisentraut
Backpatch-through: 14
Discussion: https://postgr.es/m/
8f5e63b8-e8ed-0f80-d8c4-
68222624c200@enterprisedb.com
Peter Eisentraut [Wed, 14 Jul 2021 07:15:14 +0000 (09:15 +0200)]
Fix lack of message pluralization
Amit Kapila [Wed, 14 Jul 2021 02:03:50 +0000 (07:33 +0530)]
Add support for prepared transactions to built-in logical replication.
To add support for streaming transactions at prepare time into the
built-in logical replication, we need to do the following things:
* Modify the output plugin (pgoutput) to implement the new two-phase API
callbacks, by leveraging the extended replication protocol.
* Modify the replication apply worker, to properly handle two-phase
transactions by replaying them on prepare.
* Add a new SUBSCRIPTION option "two_phase" to allow users to enable
two-phase transactions. We enable the two_phase once the initial data sync
is over.
We however must explicitly disable replication of two-phase transactions
during replication slot creation, even if the plugin supports it. We
don't need to replicate the changes accumulated during this phase,
and moreover, we don't have a replication connection open so we don't know
where to send the data anyway.
The streaming option is not allowed with this new two_phase option. This
can be done as a separate patch.
We don't allow to toggle two_phase option of a subscription because it can
lead to an inconsistent replica. For the same reason, we don't allow to
refresh the publication once the two_phase is enabled for a subscription
unless copy_data option is false.
Author: Peter Smith, Ajin Cherian and Amit Kapila based on previous work by Nikhil Sontakke and Stas Kelvich
Reviewed-by: Amit Kapila, Sawada Masahiko, Vignesh C, Dilip Kumar, Takamichi Osumi, Greg Nancarrow
Tested-By: Haiying Tang
Discussion: https://postgr.es/m/
02DA5F5E-CECE-4D9C-8B4B-
418077E2C010@postgrespro.ru
Discussion: https://postgr.es/m/CAA4eK1+opiV4aFTmWWUF9h_32=HfPOW9vZASHarT0UA5oBrtGw@mail.gmail.com
Michael Paquier [Wed, 14 Jul 2021 01:37:26 +0000 (10:37 +0900)]
Install properly fe-auth-sasl.h
The internals of the frontend-side callbacks for SASL are visible in
libpq-int.h, but the header was not getting installed. This would cause
compilation failures for applications playing with the internals of
libpq.
Issue introduced in
9fd8557.
Author: Mikhail Kulagin
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/
05ce01d777cb$
40f31d60$
c2d95820$@postgrespro.ru
David Rowley [Wed, 14 Jul 2021 00:43:58 +0000 (12:43 +1200)]
Change the name of the Result Cache node to Memoize
"Result Cache" was never a great name for this node, but nobody managed
to come up with another name that anyone liked enough. That was until
David Johnston mentioned "Node Memoization", which Tom Lane revised to
just "Memoize". People seem to like "Memoize", so let's do the rename.
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/
20210708165145.GG1176@momjian.us
Backpatch-through: 14, where Result Cache was introduced
Tom Lane [Tue, 13 Jul 2021 19:01:01 +0000 (15:01 -0400)]
Rename debug_invalidate_system_caches_always to debug_discard_caches.
The name introduced by commit
4656e3d66 was agreed to be unreasonably
long. To match this change, rename initdb's recently-added
--clobber-cache option to --discard-caches.
Discussion: https://postgr.es/m/
1374320.
1625430433@sss.pgh.pa.us
David Rowley [Tue, 13 Jul 2021 01:56:59 +0000 (13:56 +1200)]
Remove useless range checks on INT8 sequences
There's no point in checking if an INT8 sequence has a seqmin and seqmax
value is outside the range of the minimum and maximum values for an int64
type. These both use the same underlying types so an INT8 certainly
cannot be outside the minimum and maximum values supported by int64.
This code is fairly harmless and it seems likely that most compilers
would optimize it out anyway, never-the-less, let's remove it replacing
it with a small comment to mention why the check is not needed.
Author: Greg Nancarrow, with the comment revised by David Rowley
Discussion: https://postgr.es/m/CAJcOf-c9KBUZ8ow_6e%3DWSfbbEyTKfqV%3DVwoFuODQVYMySHtusw%40mail.gmail.com
David Rowley [Tue, 13 Jul 2021 01:27:05 +0000 (13:27 +1200)]
Robustify tuplesort's free_sort_tuple function
41469253e went to the trouble of removing a theoretical bug from
free_sort_tuple by checking if the tuple was NULL before freeing it. Let's
make this a little more robust by also setting the tuple to NULL so that
should we be called again we won't end up doing a pfree on the already
pfree'd tuple. Per advice from Tom Lane.
Discussion: https://postgr.es/m/
3188192.
1626136953@sss.pgh.pa.us
Backpatch-through: 9.6, same as
41469253e
David Rowley [Tue, 13 Jul 2021 00:40:16 +0000 (12:40 +1200)]
Fix theoretical bug in tuplesort
This fixes a theoretical bug in tuplesort.c which, if a bounded sort was
used in combination with a byval Datum sort (tuplesort_begin_datum), when
switching the sort to a bounded heap in make_bounded_heap(), we'd call
free_sort_tuple(). The problem was that when sorting Datums of a byval
type, the tuple is NULL and free_sort_tuple() would free the memory for it
regardless of that. This would result in a crash.
Here we fix that simply by adding a check to see if the tuple is NULL
before trying to disassociate and free any memory belonging to it.
The reason this bug is only theoretical is that nowhere in the current
code base do we do tuplesort_set_bound() when performing a Datum sort.
However, let's backpatch a fix for this as if any extension uses the code
in this way then it's likely to cause problems.
Author: Ronan Dunklau
Discussion: https://postgr.es/m/CAApHDvpdoqNC5FjDb3KUTSMs5dg6f+XxH4Bg_dVcLi8UYAG3EQ@mail.gmail.com
Backpatch-through: 9.6, oldest supported version
Thomas Munro [Mon, 12 Jul 2021 23:13:48 +0000 (11:13 +1200)]
Add PSQL_WATCH_PAGER for psql's \watch command.
Allow a pager to be used by the \watch command. This works but isn't
very useful with traditional pagers like "less", so use a different
environment variable. The popular open source tool "pspg" (also by
Pavel) knows how to display the output if you set PSQL_WATCH_PAGER="pspg
--stream".
To make \watch react quickly when the user quits the pager or presses
^C, and also to increase the accuracy of its timing and decrease the
rate of useless context switches, change the main loop of the \watch
command to use sigwait() rather than a sleeping/polling loop, on Unix.
Supported on Unix only for now (like pspg).
Author: Pavel Stehule <pavel.stehule@gmail.com>
Author: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRBfzUUPz-3gN5oAzto9SDuRSq-TQPfXU_P6h0L7hO%2BEhg%40mail.gmail.com
Tom Lane [Mon, 12 Jul 2021 23:17:35 +0000 (19:17 -0400)]
Probe for preadv/pwritev in a more macOS-friendly way.
Apple's mechanism for dealing with functions that are available
in only some OS versions confuses AC_CHECK_FUNCS, and therefore
AC_REPLACE_FUNCS. We can use AC_CHECK_DECLS instead, so long as
we enable -Werror=unguarded-availability-new. This allows people
compiling for macOS to control whether or not preadv/pwritev are
used by setting MACOSX_DEPLOYMENT_TARGET, rather than supplying
a back-rev SDK. (Of course, the latter still works, too.)
James Hilliard
Discussion: https://postgr.es/m/
20210122193230.25295-1-james.hilliard1@gmail.com
Tom Lane [Mon, 12 Jul 2021 21:01:29 +0000 (17:01 -0400)]
Replace RelationOpenSmgr() with RelationGetSmgr().
The idea behind this patch is to design out bugs like the one fixed
by commit
9d523119f. Previously, once one did RelationOpenSmgr(rel),
it was considered okay to access rel->rd_smgr directly for some
not-very-clear interval. But since that pointer will be cleared by
relcache flushes, we had bugs arising from overreliance on a previous
RelationOpenSmgr call still being effective.
Now, very little code except that in rel.h and relcache.c should ever
touch the rd_smgr field directly. The normal coding rule is to use
RelationGetSmgr(rel) and not expect the result to be valid for longer
than one smgr function call. There are a couple of places where using
the function every single time seemed like overkill, but they are now
annotated with large warning comments.
Amul Sul, after an idea of mine.
Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
Peter Eisentraut [Mon, 12 Jul 2021 20:07:35 +0000 (22:07 +0200)]
doc: Fix typo in function prototype
Heikki Linnakangas [Mon, 12 Jul 2021 08:13:33 +0000 (11:13 +0300)]
Remove dead assignment to local variable.
This should have been removed in commit
7e30c186da, which split the loop
into two. Only the first loop uses the 'from' variable; updating it in
the second loop is bogus. It was never read after the first loop, so this
was harmless and surely optimized away by the compiler, but let's be tidy.
Backpatch to all supported versions.
Author: Ranier Vilela
Discussion: https://www.postgresql.org/message-id/CAEudQAoWq%2BAL3BnELHu7gms2GN07k-np6yLbukGaxJ1vY-zeiQ%40mail.gmail.com
Michael Paquier [Mon, 12 Jul 2021 05:46:08 +0000 (14:46 +0900)]
Revert "Fix issues with Windows' stat() for files pending on deletion"
This reverts commit
54fb8c7, as per the issues reported by fairywren
when it comes to MinGW because of the lack of microsoft_native_stat()
there. Using just stat() for MSVC is not sufficient to take care of the
concurrency problems with files pending on deletion. It may be possible
to paint some __MINGW64__ in the code to switch to a different
implementation of stat() in this build context, but I am not sure either
if relying on the implementation of stat() in MinGW to take care of the
problems we are trying to fix is enough or not. So this needs more
study.
Discussion: https://postgr.es/m/YOvOlfRrIO0yGtgw@paquier.xyz
Backpatch-through: 14
Michael Paquier [Mon, 12 Jul 2021 04:02:31 +0000 (13:02 +0900)]
Fix issues with Windows' stat() for files pending on deletion
The code introduced by
bed9075 to enhance the stat() implementation on
Windows for file sizes larger than 4GB fails to properly detect files
pending for deletion with its method based on NtQueryInformationFile()
or GetFileInformationByHandleEx(), as proved by Alexander Lakhin in a
custom TAP test of his own.
The method used in the implementation of open() to sleep and loop when
when failing on ERROR_ACCESS_DENIED (EACCES) is showing much more
stability, so switch to this method. This could still lead to issues if
the permission problem stays around for much longer than the timeout of
1 second used, but that should (hopefully) never happen in
performance-critical paths. Still, there could be a point in increasing
the timeouts for the sake of machines that handle heavy loads.
Note that WIN32's open() now uses microsoft_native_stat() as it should
be similar to stat() when working around issues with concurrent file
deletions.
I have spent some time testing this patch with pgbench in combination
of the SQL functions from genfile.c, as well as running the TAP test
provided on the thread with MSVC builds, and this looks much more
stable than the previous method.
Author: Alexander Lakhin
Reviewed-by: Tom Lane, Michael Paquier, Justin Pryzby
Discussion: https://postgr.es/m/
c3427edf-d7c0-ff57-90f6-
b5de3bb62709@gmail.com
Backpatch-through: 14
Michael Paquier [Mon, 12 Jul 2021 02:05:27 +0000 (11:05 +0900)]
pageinspect: Improve page_header() for pages of 32kB
ld_upper, ld_lower, pd_special and the page size have been using
smallint as return type, which could cause those fields to return
negative values in certain cases for builds configures with a page size
of 32kB.
Bump pageinspect to 1.10. page_header() is able to handle the correct
return type of those fields at runtime when using an older version of
the extension, with some tests are added to cover that.
Author: Quan Zongliang
Reviewed-by: Michael Paquier, Bharath Rupireddy
Discussion: https://postgr.es/m/
8b8ec36e-61fe-14f9-005d-
07bc85aa4eed@yeah.net
Tom Lane [Sun, 11 Jul 2021 16:54:24 +0000 (12:54 -0400)]
Lock the extension during ALTER EXTENSION ADD/DROP.
Although we were careful to lock the object being added or dropped,
we failed to get any sort of lock on the extension itself. This
allowed the ALTER to proceed in parallel with a DROP EXTENSION,
which is problematic for a couple of reasons. If both commands
succeeded we'd be left with a dangling link in pg_depend, which
would cause problems later. Also, if the ALTER failed for some
reason, it might try to print the extension's name, and that could
result in a crash or (in older branches) a silly error message
complaining about extension "(null)".
Per bug #17098 from Alexander Lakhin. Back-patch to all
supported branches.
Discussion: https://postgr.es/m/17098-
b960f3616c861f83@postgresql.org
Thomas Munro [Sun, 11 Jul 2021 07:50:55 +0000 (19:50 +1200)]
Fix pgbench timestamp bugs.
Commit
547f04e changed pgbench to use microsecond accounting, but
introduced a couple of logging and aggregation bugs:
1. We print Unix epoch timestamps so that you can correlate them with
other logs, but these were inadvertently changed to use a
system-dependent reference epoch. Compute Unix timestamps, and begin
aggregation intervals on the boundaries of whole Unix epoch seconds, as
before.
2. The user-supplied aggregation interval needed to be scaled.
Back-patch to 14.
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Author: Yugo NAGATA <nagata@sraoss.co.jp>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reported-by: YoungHwan Joo <rulyox@gmail.com>
Reported-by: Gregory Smith <gregsmithpgsql@gmail.com>
Discussion: https://postgr.es/m/CAF7igB1r6wRfSCEAB-iZBKxkowWY6%2BdFF2jObSdd9%2BiVK%2BvHJg%40mail.gmail.com
Discussion: https://postgr.es/m/CAHLJuCW_8Vpcr0%3Dt6O_gozrg3wXXWXZXDioYJd3NhvKriqgpfQ%40mail.gmail.com
Jeff Davis [Fri, 9 Jul 2021 21:15:48 +0000 (14:15 -0700)]
Fix assign_record_type_typmod().
If an error occurred in the wrong place, it was possible to leave an
unintialized entry in the hash table, leading to a crash. Fixed.
Also, be more careful about the order of operations so that an
allocation error doesn't leak memory in CacheMemoryContext or
unnecessarily advance NextRecordTypmod.
Backpatch through version 11. Earlier versions (prior to
35ea75632a5)
do not exhibit the problem, because an uninitialized hash entry
contains a valid empty list.
Author: Sait Talha Nisanci <Sait.Nisanci@microsoft.com>
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/HE1PR8303MB009069D476225B9A9E194B8891779@HE1PR8303MB0090.EURPRD83.prod.outlook.com
Backpatch-through: 11
Tom Lane [Sat, 10 Jul 2021 17:19:30 +0000 (13:19 -0400)]
Fix busted test for ldap_initialize.
Sigh ... I was expecting AC_CHECK_LIB to do something it didn't,
namely update LIBS. This led to not finding ldap_initialize.
Fix by moving the probe for ldap_initialize. In some sense this
is more correct anyway, since (at least for now) we care about
whether ldap_initialize exists in libldap not libldap_r.
Per buildfarm member elver and local testing.
Discussion: https://postgr.es/m/17083-
a19190d9591946a7@postgresql.org
Michael Paquier [Sat, 10 Jul 2021 12:45:28 +0000 (21:45 +0900)]
Add more sanity checks in SASL exchanges
The following checks are added, to make the SASL infrastructure more
aware of defects when implementing new mechanisms:
- Detect that no output is generated by a mechanism if an exchange fails
in the backend, failing if there is a message waiting to be sent.
- Handle zero-length messages in the frontend. The backend handles that
already, and SCRAM would complain if sending empty messages as this is
not authorized for this mechanism, but other mechanisms may want this
capability (the SASL specification allows that).
- Make sure that a mechanism generates a message in the middle of the
exchange in the frontend.
SCRAM, as implemented, respects all these requirements already, and the
recent refactoring of SASL done in
9fd8557 helps in documenting that in
a cleaner way.
Analyzed-by: Jacob Champion
Author: Michael Paquier
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/
3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com
Dean Rasheed [Sat, 10 Jul 2021 11:42:59 +0000 (12:42 +0100)]
Fix numeric_mul() overflow due to too many digits after decimal point.
This fixes an overflow error when using the numeric * operator if the
result has more than 16383 digits after the decimal point by rounding
the result. Overflow errors should only occur if the result has too
many digits *before* the decimal point.
Discussion: https://postgr.es/m/CAEZATCUmeFWCrq2dNzZpRj5+6LfN85jYiDoqm+ucSXhb9U2TbA@mail.gmail.com
Tom Lane [Fri, 9 Jul 2021 20:59:07 +0000 (16:59 -0400)]
Un-break AIX build, take 2.
I incorrectly diagnosed the reason why hoverfly is unhappy.
Looking closer, it appears that it fails to link libldap
unless libssl is also present; so the problem was my
idea of clearing LIBS before making the check. Revert
to essentially the original coding, except that instead
of failing when libldap_r isn't there, use libldap.
Per buildfarm member hoverfly.
Discussion: https://postgr.es/m/17083-
a19190d9591946a7@postgresql.org
Alvaro Herrera [Fri, 9 Jul 2021 19:57:59 +0000 (15:57 -0400)]
libpq: Fix sending queries in pipeline aborted state
When sending queries in pipeline mode, we were careless about leaving
the connection in the right state so that PQgetResult would behave
correctly; trying to read further results after sending a query after
having read a result with an error would sometimes hang. Fix by
ensuring internal libpq state is changed properly. All the state
changes were being done by the callers of pqAppendCmdQueueEntry(); it
would have become too repetitious to have this logic in each of them, so
instead put it all in that function and relieve callers of the
responsibility.
Add a test to verify this case. Without the code fix, this new test
hangs sometimes.
Also, document that PQisBusy() would return false when no queries are
pending result. This is not intuitively obvious, and NULL would be
obtained by calling PQgetResult() at that point, which is confusing.
Wording by Boris Kolpackov.
In passing, fix bogus use of "false" to mean "0", per Ranier Vilela.
Backpatch to 14.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Boris Kolpackov <boris@codesynthesis.com>
Discussion: https://postgr.es/m/boris.
20210624103805@codesynthesis.com
Jeff Davis [Fri, 9 Jul 2021 15:48:19 +0000 (08:48 -0700)]
Eliminate replication protocol error related to IDENTIFY_SYSTEM.
The requirement that IDENTIFY_SYSTEM be run before START_REPLICATION
was both undocumented and unnecessary. Remove the error and ensure
that ThisTimeLineID is initialized in START_REPLICATION.
Elect not to backport because this requirement was expected behavior
(even if inconsistently enforced), and is not likely to cause any
major problem.
Author: Jeff Davis
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/
de4bbf05b7cd94227841c433ea6ff71d2130c713.camel%40j-davis.com
Tom Lane [Fri, 9 Jul 2021 18:15:41 +0000 (14:15 -0400)]
Un-break AIX build.
In commit
d0a02bdb8, I'd supposed that uniformly probing for
ldap_bind would make the intent clearer. However, that seems
not to work on AIX, for obscure reasons (maybe it's a macro
there?). Revert to the former behavior of probing
ldap_simple_bind for thread-safe cases and ldap_bind otherwise.
Per buildfarm member hoverfly.
Discussion: https://postgr.es/m/17083-
a19190d9591946a7@postgresql.org
Tom Lane [Fri, 9 Jul 2021 17:38:24 +0000 (13:38 -0400)]
Avoid creating a RESULT RTE that's marked LATERAL.
Commit
7266d0997 added code to pull up simple constant function
results, converting the RTE_FUNCTION RTE to a dummy RTE_RESULT
RTE since it no longer need be scanned. But I forgot to clear
the LATERAL flag if the RTE has it set. If the function reduced
to a constant, it surely contains no lateral references so this
simplification is logically OK. It's needed because various other
places will Assert that RESULT RTEs aren't LATERAL.
Per bug #17097 from Yaoguang Chen. Back-patch to v13 where the
faulty code came in.
Discussion: https://postgr.es/m/17097-
3372ef9f798fc94f@postgresql.org
Tom Lane [Fri, 9 Jul 2021 16:38:55 +0000 (12:38 -0400)]
Update configure's probe for libldap to work with OpenLDAP 2.5.
The separate libldap_r is gone and libldap itself is now always
thread-safe. Unfortunately there seems no easy way to tell by
inspection whether libldap is thread-safe, so we have to take
it on faith that libldap is thread-safe if there's no libldap_r.
That should be okay, as it appears that libldap_r was a standard
part of the installation going back at least 20 years.
Report and patch by Adrian Ho. Back-patch to all supported
branches, since people might try to build any of them with
a newer OpenLDAP.
Discussion: https://postgr.es/m/17083-
a19190d9591946a7@postgresql.org
Tom Lane [Fri, 9 Jul 2021 15:02:26 +0000 (11:02 -0400)]
Reject cases where a query in WITH rewrites to just NOTIFY.
Since the executor can't cope with a utility statement appearing
as a node of a plan tree, we can't support cases where a rewrite
rule inserts a NOTIFY into an INSERT/UPDATE/DELETE command appearing
in a WITH clause of a larger query. (One can imagine ways around
that, but it'd be a new feature not a bug fix, and so far there's
been no demand for it.) RewriteQuery checked for this, but it
missed the case where the DML command rewrites to *only* a NOTIFY.
That'd lead to crashes later on in planning. Add the missed check,
and improve the level of testing of this area.
Per bug #17094 from Yaoguang Chen. It's been busted since WITH
was introduced, so back-patch to all supported branches.
Discussion: https://postgr.es/m/17094-
bf15dff55eaf2e28@postgresql.org
David Rowley [Fri, 9 Jul 2021 06:56:00 +0000 (18:56 +1200)]
Teach pg_size_pretty and pg_size_bytes about petabytes
There was talk about adding units all the way up to yottabytes but it
seems quite far-fetched that anyone would need those. Since such large
units are not exactly commonplace, it seems unlikely that having
pg_size_pretty outputting unit any larger than petabytes would actually be
helpful to anyone.
Since petabytes are on the horizon, let's just add those only. Maybe one
day we'll get to add additional units, but it will likely be a while
before we'll need to think beyond petabytes in regards to the size of a
database.
Author: David Christensen
Discussion: https://postgr.es/m/CAOxo6XKmHc_WZip-x5QwaOqFEiCq_SVD0B7sbTZQk+qqcn2qaw@mail.gmail.com
Michael Paquier [Fri, 9 Jul 2021 06:27:36 +0000 (15:27 +0900)]
Add forgotten LSN_FORMAT_ARGS() in xlogreader.c
These should have been part of
4035cd5, that introduced LZ4 support for
wal_compression.
Thomas Munro [Fri, 9 Jul 2021 05:51:48 +0000 (17:51 +1200)]
Remove more obsolete comments about semaphores.
Commit
6753333f stopped using semaphores as the sleep/wake mechanism for
heavyweight locks, but some obsolete references to that scheme remained
in comments. As with similar commit
25b93a29, back-patch all the way.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/CA%2BhUKGLafjB1uzXcy%3D%3D2L3cy7rjHkqOVn7qRYGBjk%3D%3DtMJE7Yg%40mail.gmail.com
David Rowley [Fri, 9 Jul 2021 04:29:02 +0000 (16:29 +1200)]
Use a lookup table for units in pg_size_pretty and pg_size_bytes
We've grown 2 versions of pg_size_pretty over the years, one for BIGINT
and one for NUMERIC. Both should output the same, but keeping them in
sync is harder than needed due to neither function sharing a source of
truth about which units to use and how to transition to the next largest
unit.
Here we add a static array which defines the units that we recognize and
have both pg_size_pretty and pg_size_pretty_numeric use it. This will
make adding any units in the future a very simple task.
The table contains all information required to allow us to also modify
pg_size_bytes to use the lookup table, so adjust that too.
There are no behavioral changes here.
Author: David Rowley
Reviewed-by: Dean Rasheed, Tom Lane, David Christensen
Discussion: https://postgr.es/m/CAApHDvru1F7qsEVL-iOHeezJ+5WVxXnyD_Jo9nht+Eh85ekK-Q@mail.gmail.com
David Rowley [Fri, 9 Jul 2021 02:04:30 +0000 (14:04 +1200)]
Fix incorrect return value in pg_size_pretty(bigint)
Due to how pg_size_pretty(bigint) was implemented, it's possible that when
given a negative number of bytes that the returning value would not match
the equivalent positive return value when given the equivalent positive
number of bytes. This was due to two separate issues.
1. The function used bit shifting to convert the number of bytes into
larger units. The rounding performed by bit shifting is not the same as
dividing. For example -3 >> 1 = -2, but -3 / 2 = -1. These two
operations are only equivalent with positive numbers.
2. The half_rounded() macro rounded towards positive infinity. This meant
that negative numbers rounded towards zero and positive numbers rounded
away from zero.
Here we fix #1 by dividing the values instead of bit shifting. We fix #2
by adjusting the half_rounded macro always to round away from zero.
Additionally, adjust the pg_size_pretty(numeric) function to be more
explicit that it's using division rather than bit shifting. A casual
observer might have believed bit shifting was used due to a static
function being named numeric_shift_right. However, that function was
calculating the divisor from the number of bits and performed division.
Here we make that more clear. This change is just cosmetic and does not
affect the return value of the numeric version of the function.
Here we also add a set of regression tests both versions of
pg_size_pretty() which test the values directly before and after the
function switches to the next unit.
This bug was introduced in
8a1fab36a. Prior to that negative values were
always displayed in bytes.
Author: Dean Rasheed, David Rowley
Discussion: https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.com
Backpatch-through: 9.6, where the bug was introduced.
Daniel Gustafsson [Thu, 8 Jul 2021 10:45:09 +0000 (12:45 +0200)]
Fix typos in pgstat.c, reorderbuffer.c and pathnodes.h
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
50250765-5B87-4AD7-9770-
7FCED42A6175@yesql.se
Peter Eisentraut [Thu, 8 Jul 2021 07:38:52 +0000 (09:38 +0200)]
Improve error messages about mismatching relkind
Most error messages about a relkind that was not supported or
appropriate for the command was of the pattern
"relation \"%s\" is not a table, foreign table, or materialized view"
This style can become verbose and tedious to maintain. Moreover, it's
not very helpful: If I'm trying to create a comment on a TOAST table,
which is not supported, then the information that I could have created
a comment on a materialized view is pointless.
Instead, write the primary error message shorter and saying more
directly that what was attempted is not possible. Then, in the detail
message, explain that the operation is not supported for the relkind
the object was. To simplify that, add a new function
errdetail_relkind_not_supported() that does this.
In passing, make use of RELKIND_HAS_STORAGE() where appropriate,
instead of listing out the relkinds individually.
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/
dc35a398-37d0-75ce-07ea-
1dd71d98f8ec@2ndquadrant.com
Tom Lane [Wed, 7 Jul 2021 19:21:25 +0000 (15:21 -0400)]
Fix crash in postgres_fdw for provably-empty remote UPDATE/DELETE.
In
86dc90056, I'd written find_modifytable_subplan with the assumption
that if the immediate child of a ModifyTable is a Result, it must be
a projecting Result with a subplan. However, if the UPDATE or DELETE
has a provably-constant-false WHERE clause, that's not so: we'll
generate a dummy subplan with a childless Result. Add the missing
null-check so we don't crash on such cases.
Per report from Alexander Pyhalov.
Discussion: https://postgr.es/m/
b9a6f53549456b2f3e2fd150dcd79d72@postgrespro.ru
Fujii Masao [Wed, 7 Jul 2021 12:54:47 +0000 (21:54 +0900)]
doc: Fix description about pg_stat_statements.track_planning.
This commit fixes wrong wording like "a fewer kinds"
in the description about track_planning option.
Back-patch to v13 where pg_stat_statements.track_planning was added.
Author: Justin Pryzby
Reviewed-by: Julien Rouhaud, Fujii Masao
Discussion: https://postgr.es/m/
20210418233615.GB7256@telsasoft.com
David Rowley [Wed, 7 Jul 2021 04:29:17 +0000 (16:29 +1200)]
Use a hash table to speed up NOT IN(values)
Similar to
50e17ad28, which allowed hash tables to be used for IN clauses
with a set of constants, here we add the same feature for NOT IN clauses.
NOT IN evaluates the same as: WHERE a <> v1 AND a <> v2 AND a <> v3.
Obviously, if we're using a hash table we must be exactly equivalent to
that and return the same result taking into account that either side of
the condition could contain a NULL. This requires a little bit of
special handling to make work with the hash table version.
When processing NOT IN, the ScalarArrayOpExpr's operator will be the <>
operator. To be able to build and lookup a hash table we must use the
<>'s negator operator. The planner checks if that exists and is hashable
and sets the relevant fields in ScalarArrayOpExpr to instruct the executor
to use hashing.
Author: David Rowley, James Coleman
Reviewed-by: James Coleman, Zhihong Yu
Discussion: https://postgr.es/m/CAApHDvoF1mum_FRk6D621edcB6KSHBi2+GAgWmioj5AhOu2vwQ@mail.gmail.com
Fujii Masao [Wed, 7 Jul 2021 02:13:40 +0000 (11:13 +0900)]
postgres_fdw: Tighten up allowed values for batch_size, fetch_size options.
Previously the values such as '100$%$#$#', '9,223,372,' were accepted and
treated as valid integers for postgres_fdw options batch_size and fetch_size.
Whereas this is not the case with fdw_startup_cost and fdw_tuple_cost options
for which an error is thrown. This was because endptr was not used
while converting strings to integers using strtol.
This commit changes the logic so that it uses parse_int function
instead of strtol as it serves the purpose by returning false in case
if it is unable to convert the string to integer. Note that
this function also rounds off the values such as '100.456' to 100 and
'100.567' or '100.678' to 101.
While on this, use parse_real for fdw_startup_cost and fdw_tuple_cost options.
Since parse_int and parse_real are being used for reloptions and GUCs,
it is more appropriate to use in postgres_fdw rather than using strtol
and strtod directly.
Back-patch to v14.
Author: Bharath Rupireddy
Reviewed-by: Ashutosh Bapat, Tom Lane, Kyotaro Horiguchi, Fujii Masao
Discussion: https://postgr.es/m/CALj2ACVMO6wY5Pc4oe1OCgUOAtdjHuFsBDw8R5uoYR86eWFQDA@mail.gmail.com
Michael Paquier [Wed, 7 Jul 2021 01:55:15 +0000 (10:55 +0900)]
Refactor SASL code with a generic interface for its mechanisms
The code of SCRAM and SASL have been tightly linked together since SCRAM
exists in the core code, making hard to apprehend the addition of new
SASL mechanisms, but these are by design different facilities, with
SCRAM being an option for SASL. This refactors the code related to both
so as the backend and the frontend use a set of callbacks for SASL
mechanisms, documenting while on it what is expected by anybody adding a
new SASL mechanism.
The separation between both layers is neat, using two sets of callbacks
for the frontend and the backend to mark the frontier between both
facilities. The shape of the callbacks is now directly inspired from
the routines used by SCRAM, so the code change is straight-forward, and
the SASL code is moved into its own set of files. These will likely
change depending on how and if new SASL mechanisms get added in the
future.
Author: Jacob Champion
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/
3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com
Tom Lane [Tue, 6 Jul 2021 22:10:11 +0000 (18:10 -0400)]
Allow CustomScan providers to say whether they support projections.
Previously, all CustomScan providers had to support projections,
but there may be cases where this is inconvenient. Add a flag
bit to say if it's supported.
Important item for the release notes: this is non-backwards-compatible
since the default is now to assume that CustomScan providers can't
project, instead of assuming that they can. It's fail-soft, but could
result in visible performance penalties due to adding unnecessary
Result nodes.
Sven Klemm, reviewed by Aleksander Alekseev; some cosmetic fiddling
by me.
Discussion: https://postgr.es/m/CAMCrgp1kyakOz6c8aKhNDJXjhQ1dEjEnp+6KNT3KxPrjNtsrDg@mail.gmail.com
Alvaro Herrera [Tue, 6 Jul 2021 21:48:41 +0000 (17:48 -0400)]
Improve TestLib::system_or_bail error reporting
The original coding was not quoting the complete failing command, and it
wasn't printing the reason for the failure either. Do both.
This is cosmetic only, so no backpatch.
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/
202106301524.eq5pblzstapj@alvherre.pgsql
Tom Lane [Tue, 6 Jul 2021 18:23:09 +0000 (14:23 -0400)]
Reduce the cost of planning deeply-nested views.
Joel Jacobson reported that deep nesting of trivial (flattenable)
views results in O(N^3) growth of planning time for N-deep nesting.
It turns out that a large chunk of this cost comes from copying around
the "subquery" sub-tree of each view's RTE_SUBQUERY RTE. But once we
have successfully flattened the subquery, we don't need that anymore,
because the planner isn't going to do anything else interesting with
that RTE. We already zap the subquery pointer during setrefs.c (cf.
add_rte_to_flat_rtable), but it's useless baggage earlier than that
too. Clearing the pointer as soon as pull_up_simple_subquery is done
with the RTE reduces the cost from O(N^3) to O(N^2); which is still
not great, but it's quite a lot better. Further improvements will
require rethinking of the RTE data structure, which is being considered
in another thread.
Patch by me; thanks to Dean Rasheed for review.
Discussion: https://postgr.es/m/
797aff54-b49b-4914-9ff9-
aa42564a4d7d@www.fastmail.com
Tom Lane [Tue, 6 Jul 2021 16:36:12 +0000 (12:36 -0400)]
Avoid doing catalog lookups in postgres_fdw's conversion_error_callback.
As in
50371df26, this is a bad idea since the callback can't really
know what error is being thrown and thus whether or not it is safe
to attempt catalog accesses. Rather than pushing said accesses into
the mainline code where they'd usually be a waste of cycles, we can
look at the query's rangetable instead.
This change does mean that we'll be printing query aliases (if any
were used) rather than the table or column's true name. But that
doesn't seem like a bad thing: it's certainly a more useful definition
in self-join cases, for instance. In any case, it seems unlikely that
any applications would be depending on this detail, so it seems safe
to change.
Patch by me. Original complaint by Andres Freund; Bharath Rupireddy
noted the connection to conversion_error_callback.
Discussion: https://postgr.es/m/
20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de
Tom Lane [Tue, 6 Jul 2021 14:34:51 +0000 (10:34 -0400)]
Doc: add info about timestamps with fractional-minute UTC offsets.
Our code has supported fractional-minute UTC offsets for ages, but
there was no mention of the possibility in the main docs, and only
a very indirect reference in Appendix B. Improve that.
Discussion: https://postgr.es/m/
162543102827.697.
5755498651217979813@wrigleys.postgresql.org
Amit Kapila [Tue, 6 Jul 2021 02:16:50 +0000 (07:46 +0530)]
Refactor function parse_subscription_options.
Instead of using multiple parameters in parse_subscription_options
function signature, use the struct SubOpts that encapsulate all the
subscription options and their values. It will be useful for future work
where we need to add other options in the subscription. Also, use bitmaps
to pass the supported and retrieve the specified options much like the way
it is done in the commit
a3dc926009.
Author: Bharath Rupireddy
Reviewed-By: Peter Smith, Amit Kapila, Alvaro Herrera
Discussion: https://postgr.es/m/CALj2ACXtoQczfNsDQWobypVvHbX2DtgEHn8DawS0eGFwuo72kw@mail.gmail.com
David Rowley [Tue, 6 Jul 2021 00:38:50 +0000 (12:38 +1200)]
Fix typo in comment
Author: James Coleman
Discussion: https://postgr.es/m/CAAaqYe8f8ENA0i1PdBtUNWDd2sxHSMgscNYbjhaXMuAdfBrZcg@mail.gmail.com
David Rowley [Tue, 6 Jul 2021 00:24:43 +0000 (12:24 +1200)]
Reduce the number of pallocs when building partition bounds
In each of the create_*_bound() functions for LIST, RANGE and HASH
partitioning, there were a large number of palloc calls which could be
reduced down to a much smaller number.
In each of these functions, an array was built so that we could qsort it
before making the PartitionBoundInfo. For LIST and HASH partitioning, an
array of pointers was allocated then each element was allocated within
that array. Since the number of items of each dimension is known
beforehand, we can just allocate a single chunk of memory for this.
Similarly, with all partition strategies, we're able to reduce the number
of allocations to build the ->datums field. This is an array of Datum
pointers, but there's no need for the Datums that each element points to
to be singly allocated. One big chunk will do. For RANGE partitioning,
the PartitionBoundInfo->kind field can get the same treatment.
We can apply the same optimizations to partition_bounds_copy(). Doing
this might have a small effect on cache performance when searching for the
correct partition during partition pruning or DML on a partitioned table.
However, that's likely to be small and this is mostly about reducing
palloc overhead.
Author: Nitin Jadhav, Justin Pryzby, David Rowley
Reviewed-by: Justin Pryzby, Zhihong Yu
Discussion: https://postgr.es/m/flat/CAMm1aWYFTqEio3bURzZh47jveiHRwgQTiSDvBORczNEz2duZ1Q@mail.gmail.com
Michael Paquier [Mon, 5 Jul 2021 23:10:59 +0000 (08:10 +0900)]
Use WaitLatch() instead of pg_usleep() at the end of backups
This concerns pg_stop_backup() and BASE_BACKUP, when waiting for the
WAL segments required for a backup to be archived. This simplifies a
bit the handling of the wait event used in this code path.
Author: Bharath Rupireddy
Reviewed-by: Michael Paquier, Stephen Frost
Discussion: https://postgr.es/m/CALj2ACU4AdPCq6NLfcA-ZGwX7pPCK5FgEj-CAU0xCKzkASSy_A@mail.gmail.com
Tom Lane [Mon, 5 Jul 2021 20:51:57 +0000 (16:51 -0400)]
Reduce overhead of cache-clobber testing in LookupOpclassInfo().
Commit
03ffc4d6d added logic to bypass all caching behavior in
LookupOpclassInfo when CLOBBER_CACHE_ALWAYS is enabled. It doesn't
look like I stopped to think much about what that would cost, but
recent investigation shows that the cost is enormous: it roughly
doubles the time needed for cache-clobber test runs.
There does seem to be value in this behavior when trying to test
the opclass-cache loading logic itself, but for other purposes the
cost is excessive. Hence, let's back off to doing this only when
debug_invalidate_system_caches_always is at least 3; or in older
branches, when CLOBBER_CACHE_RECURSIVELY is defined.
While here, clean up some other minor issues in LookupOpclassInfo.
Re-order the code so we aren't left with broken cache entries (leading
to later core dumps) in the unlikely case that we suffer OOM while
trying to allocate space for a new entry. (That seems to be my
oversight in
03ffc4d6d.) Also, in >= v13, stop allocating one array
entry too many. That's evidently left over from sloppy reversion in
851b14b0c.
Back-patch to all supported branches, mainly to reduce the runtime
of cache-clobbering buildfarm animals.
Discussion: https://postgr.es/m/
1370856.
1625428625@sss.pgh.pa.us
Tom Lane [Mon, 5 Jul 2021 18:34:47 +0000 (14:34 -0400)]
Rethink blocking annotations in detach-partition-concurrently-[34].
In
741d7f104, I tried to make the reports from canceled steps come out
after the pg_cancel_backend() steps, since that was the most common
ordering before. However, that doesn't ensure that a canceled step
doesn't report even later, as shown in a recent failure on buildfarm
member idiacanthus. Rather than complicating things even more with
additional annotations, let's just force the cancel's effect to be
reported first. It's not *that* unnatural-looking.
Back-patch to v14 where these test cases appeared.
Report: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=idiacanthus&dt=2021-07-02%2001%3A40%3A04
Dean Rasheed [Mon, 5 Jul 2021 09:16:42 +0000 (10:16 +0100)]
Prevent numeric overflows in parallel numeric aggregates.
Formerly various numeric aggregate functions supported parallel
aggregation by having each worker convert partial aggregate values to
Numeric and use numeric_send() as part of serializing their state.
That's problematic, since the range of Numeric is smaller than that of
NumericVar, so it's possible for it to overflow (on either side of the
decimal point) in cases that would succeed in non-parallel mode.
Fix by serializing NumericVars instead, to avoid the overflow risk and
ensure that parallel and non-parallel modes work the same.
A side benefit is that this improves the efficiency of the
serialization/deserialization code, which can make a noticeable
difference to performance with large numbers of parallel workers.
No back-patch due to risk from changing the binary format of the
aggregate serialization states, as well as lack of prior field
complaints and low probability of such overflows in practice.
Patch by me. Thanks to David Rowley for review and performance
testing, and Ranier Vilela for an additional suggestion.
Discussion: https://postgr.es/m/CAEZATCUmeFWCrq2dNzZpRj5+6LfN85jYiDoqm+ucSXhb9U2TbA@mail.gmail.com
Peter Eisentraut [Mon, 5 Jul 2021 06:26:00 +0000 (08:26 +0200)]
doc: Fix quoting markup
Amit Kapila [Mon, 5 Jul 2021 04:06:11 +0000 (09:36 +0530)]
Doc: Hash Indexes.
A new chapter for Hash Indexes, designed to help users understand how
they work and when to use them.
Backpatch-through 10 where we have made hash indexes durable.
Author: Simon Riggs
Reviewed-By: Justin Pryzby, Amit Kapila
Discussion: https://postgr.es/m/CANbhV-HRjNPYgHo--P1ewBrFJ-GpZPb9_25P7=Wgu7s7hy_sLQ@mail.gmail.com
Michael Paquier [Sun, 4 Jul 2021 11:59:04 +0000 (20:59 +0900)]
doc: Mention requirement to --enable-tap-tests on section for TAP tests
Author: Greg Sabino Mullane
Discussion: https://postgr.es/m/CAKAnmmJYH2FBn_+Vwd2FD5SaKn8hjhAXOCHpZc6n4wXaUaW_SA@mail.gmail.com
Backpatch-through: 9.6
David Rowley [Sun, 4 Jul 2021 10:28:38 +0000 (22:28 +1200)]
Doc: mention that VACUUM can't utilize over 1GB of RAM
Document that setting maintenance_work_mem to values over 1GB has no
effect on VACUUM.
Reported-by: Martín Marqués
Author: Laurenz Albe
Discussion: https://postgr.es/m/CABeG9LsZ2ozUMcqtqWu_-GiFKB17ih3p8wBHXcpfnHqhCnsc7A%40mail.gmail.com
Backpatch-through: 9.6, oldest supported release
David Rowley [Sun, 4 Jul 2021 06:47:31 +0000 (18:47 +1200)]
Cleanup some aggregate code in the executor
Here we alter the code that calls build_pertrans_for_aggref() so that the
function no longer needs to special-case whether it's dealing with an
aggtransfn or an aggcombinefn. This allows us to reuse the
build_aggregate_transfn_expr() function and just get rid of the
build_aggregate_combinefn_expr() completely.
All of the special case code that was in build_pertrans_for_aggref() has
been moved up to the calling functions.
This saves about a dozen lines of code in nodeAgg.c and a few dozen more
in parse_agg.c
Also, rename a few variables in nodeAgg.c to try to make it more clear
that we're working with either a aggtransfn or an aggcombinefn. Some of
the old names would have you believe that we were always working with an
aggtransfn.
Discussion: https://postgr.es/m/CAApHDvptMQ9FmF0D67zC_w88yVnoNVR2+kkOQGUrCmdxWxLULQ@mail.gmail.com
Tom Lane [Sat, 3 Jul 2021 15:21:40 +0000 (11:21 -0400)]
Further restrict the scope of no-exit()-in-libpq test.
Disable this check altogether in --enable-coverage builds,
because newer versions of gcc insert exit() as well as abort()
calls for that. Also disable it on AIX and Solaris, because
those platforms tend to provide facilities such as libldap
as static libraries, which then get included in libpq's shlib.
We can't expect such libraries to honor our coding rules.
(That platform list might need additional tweaking, but I think
this is enough to keep the buildfarm happy.)
Per reports from Jacob Champion and Noah Misch.
Discussion: https://postgr.es/m/
3128896.
1624742969@sss.pgh.pa.us
Bruce Momjian [Sat, 3 Jul 2021 00:42:46 +0000 (20:42 -0400)]
doc: adjust "cities" example to be consistent with other SQL
Reported-by: tom@crystae.net
Discussion: https://postgr.es/m/
162345756191.14472.
9754568432103008703@wrigleys.postgresql.org
Backpatch-through: 9.6
Bruce Momjian [Fri, 2 Jul 2021 22:00:30 +0000 (18:00 -0400)]
docs: clarify new aggressive vacuum mode for multi-xacts
Reported-by: eric.mutta@gmail.com
Discussion: https://postgr.es/m/
162395467510.686.
11947486273299446208@wrigleys.postgresql.org
Backpatch-through: 14
Tom Lane [Fri, 2 Jul 2021 20:04:54 +0000 (16:04 -0400)]
Don't try to print data type names in slot_store_error_callback().
The existing code tried to do syscache lookups in an already-failed
transaction, which is problematic to say the least. After some
consideration of alternatives, the best fix seems to be to just drop
type names from the error message altogether. The table and column
names seem like sufficient localization. If the user is unsure what
types are involved, she can check the local and remote table
definitions.
Having done that, we can also discard the LogicalRepTypMap hash
table, which had no other use. Arguably, LOGICAL_REP_MSG_TYPE
replication messages are now obsolete as well; but we should
probably keep them in case some other use emerges. (The complexity
of removing something from the replication protocol would likely
outweigh any savings anyhow.)
Masahiko Sawada and Bharath Rupireddy, per complaint from Andres
Freund. Back-patch to v10 where this code originated.
Discussion: https://postgr.es/m/
20210106020229.ne5xnuu6wlondjpe@alap3.anarazel.de
Peter Eisentraut [Fri, 2 Jul 2021 09:59:55 +0000 (11:59 +0200)]
Use InvalidBucket instead of -1 where appropriate
Reported-by: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAEudQAp%3DZwKjrP4L%2BCzqV7SmWiaQidPPRqj4tqdjDG4KBx5yrg%40mail.gmail.com
Michael Paquier [Fri, 2 Jul 2021 03:58:34 +0000 (12:58 +0900)]
Use WaitLatch() instead of pg_usleep() at end-of-vacuum truncation
This has the advantage to make a process more responsive when the
postmaster dies, even if the wait time was rather limited as there was
only a 50ms timeout here. Another advantage of this change is for
monitoring, as we gain a new wait event for the end-of-vacuum
truncation.
Author: Bharath Rupireddy
Reviewed-by: Aleksander Alekseev, Thomas Munro, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACU4AdPCq6NLfcA-ZGwX7pPCK5FgEj-CAU0xCKzkASSy_A@mail.gmail.com
Thomas Munro [Fri, 2 Jul 2021 01:07:16 +0000 (13:07 +1200)]
Remove some dead stores.
Remove redundant local variable assignments left behind by commit
2fc7af5e966.
Author: Quan Zongliang <quanzongliang@yeah.net>
Reviewed-by: Jacob Champion <pchampion@vmware.com>
Discussion: https://postgr.es/m/
de141d14-4fd6-3148-99bf-
856b71aa948a%40yeah.net
Michael Paquier [Fri, 2 Jul 2021 00:35:12 +0000 (09:35 +0900)]
Simplify error handing of jsonapi.c for the frontend
This commit removes a dependency to the central logging facilities in
the JSON parsing routines of src/common/, which existed to log errors
when seeing error codes that do not match any existing values in
JsonParseErrorType, which is not something that should never happen.
The routine providing a detailed error message based on the error code
is made backend-only, the existing code being unsafe to use in the
frontend as the error message may finish by being palloc'd or point to a
static string, so there is no way to know if the memory of the message
should be pfree'd or not. The only user of this routine in the frontend
was pg_verifybackup, that is changed to use a more generic error message
on parsing failure.
Note that making this code more resilient to OOM failures if used in
shared libraries would require much more work as a lot of code paths
still rely on palloc() & friends, but we are not sure yet if we need to
go down to that. Still, removing the dependency to logging is a step
toward more portability.
This cleans up the handling of check_stack_depth() while on it, as it
exists only in the backend.
Per discussion with Jacob Champion and Tom Lane.
Discussion: https://postgr.es/m/YNwL7kXwn3Cckbd6@paquier.xyz
Peter Eisentraut [Thu, 1 Jul 2021 20:23:37 +0000 (22:23 +0200)]
doc: Remove inappropriate <acronym> tags
Andrew Dunstan [Thu, 1 Jul 2021 19:38:06 +0000 (15:38 -0400)]
add missing tag from commit
b8c4261e5e
Peter Eisentraut [Thu, 1 Jul 2021 19:27:39 +0000 (21:27 +0200)]
doc: Clean up title case use
Andrew Dunstan [Thu, 1 Jul 2021 18:21:09 +0000 (14:21 -0400)]
Add new make targets world-bin and install-world-bin
These are the same as world and install-world respectively, but without
building or installing the documentation. There are many reasons for
wanting to be able to do this, including speed, lack of documentation
building tools, and wanting to build other formats of the documentation.
Plans for simplifying the buildfarm client code include using these
targets.
Backpatch to all live branches.
Discussion: https://postgr.es/m/
6a421136-d462-b043-a8eb-
e75b2861f3df@dunslane.net
Tom Lane [Thu, 1 Jul 2021 17:33:05 +0000 (13:33 -0400)]
Add --clobber-cache option to initdb, for CCA testing.
Commit
4656e3d66 replaced the "#define CLOBBER_CACHE_ALWAYS"
testing mechanism with a GUC, which has been a great help for
doing cache-clobber testing in more efficient ways; but there
is a gap in the implementation. The only way to do cache-clobber
testing during an initdb run is to use the old method with #define,
because one can't set the GUC from outside. Improve this by
adding a switch to initdb for the purpose.
(Perhaps someday we should let initdb pass through arbitrary
"-c NAME=VALUE" switches. Quoting difficulties dissuaded me
from attempting that right now, though.)
Back-patch to v14 where
4656e3d66 came in.
Discussion: https://postgr.es/m/
1582507.
1624227029@sss.pgh.pa.us
Alvaro Herrera [Thu, 1 Jul 2021 16:56:30 +0000 (12:56 -0400)]
Don't reset relhasindex for partitioned tables on ANALYZE
Commit
0e69f705cc1a introduced code to analyze partitioned table;
however, that code fails to preserve pg_class.relhasindex correctly.
Fix by observing whether any indexes exist rather than accidentally
falling through to assuming none do.
Backpatch to 14.
Author: Alexander Pyhalov <a.pyhalov@postgrespro.ru>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/CALNJ-vS1R3Qoe5t4tbzxrkpBtzRbPq1dDcW4RmA_a+oqweF30w@mail.gmail.com
Tom Lane [Thu, 1 Jul 2021 14:45:12 +0000 (10:45 -0400)]
Improve build-time check that libpq doesn't call exit().
Further fixes for commit
dc227eb82. Per suggestion from
Peter Eisentraut, use a stamp-file to control when the check
is run, avoiding repeated executions during "make all".
Also, remove "-g" switch for nm: it's useless and some versions
of nm consider it to conflict with "-u". (Thanks to Noah Misch
for running down that portability issue.)
Discussion: https://postgr.es/m/
3128896.
1624742969@sss.pgh.pa.us
Andrew Dunstan [Thu, 1 Jul 2021 12:29:10 +0000 (08:29 -0400)]
Fix prove_installcheck to use correct paths when used with PGXS
The prove_installcheck recipe in src/Makefile.global.in was emitting
bogus paths for a couple of elements when used with PGXS. Here we create
a separate recipe for the PGXS case that does it correctly. We also take
the opportunity to make the make the file more readable by breaking up
the prove_installcheck and prove_check recipes across several lines, and
to remove the setting for REGRESS_SHLIB to src/test/recovery/Makefile,
which is the only set of tests that actually need it.
Backpatch to all live branches
Discussion: https://postgr.es/m/
f2401388-936b-f4ef-a07c-
a0bcc49b3300@dunslane.net
Heikki Linnakangas [Thu, 1 Jul 2021 12:32:57 +0000 (15:32 +0300)]
Allow specifying pg_waldump --rmgr option multiple times.
Before, if you specified multiple --rmgr options, only the last one took
effect. It seems more sensible to select all the specified resource
managers.
Reviewed-By: Daniel Gustafsson, Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/
98344bc2-e222-02ad-a75b-
81ffc614c155%40iki.fi
Peter Eisentraut [Thu, 1 Jul 2021 07:17:44 +0000 (09:17 +0200)]
Add tests for UNBOUNDED syntax ambiguity
There is a syntactic ambiguity in the SQL standard. Since UNBOUNDED
is a non-reserved word, it could be the name of a function parameter
and be used as an expression. There is a grammar hack to resolve such
cases as the keyword. Add some tests to record this behavior.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/
b2a09a77-3c8f-7c68-c9b7-
824054f87d98%40enterprisedb.com
David Rowley [Thu, 1 Jul 2021 03:29:06 +0000 (15:29 +1200)]
Improve various places that double the size of a buffer
Several places were performing a tight loop to determine the first power
of 2 number that's > or >= the required memory. Instead of using a loop
for that, we can use pg_nextpower2_32 or pg_nextpower2_64. When we need a
power of 2 number equal to or greater than a given amount, we just pass
the amount to the nextpower2 function. When we need a power of 2 greater
than the amount, we just pass the amount + 1.
Additionally, in tsearch there were a couple of locations that were
performing a while loop when a simple "if" would have done. In both of
these locations only 1 item is being added, so the loop could only have
ever iterated once. Changing the loop into an if statement makes the code
very slightly more optimal as the condition is checked once rather than
twice.
There are quite a few remaining locations that increase the size of the
buffer in the following form:
while (reqsize >= buflen)
{
buflen *= 2;
buf = repalloc(buf, buflen);
}
These are not touched in this commit. repalloc will error out for sizes
larger than MaxAllocSize. Changing these to use pg_nextpower2_32 would
remove the chance of that error being raised. It's unclear from the code
if the sizes could ever become that large, so err on the side of caution.
Discussion: https://postgr.es/m/CAApHDvp=tns7RL4PH0ZR0M+M-YFLquK7218x=0B_zO+DbOma+w@mail.gmail.com
Reviewed-by: Zhihong Yu
Tom Lane [Wed, 30 Jun 2021 14:52:20 +0000 (10:52 -0400)]
Fix portability fallout from commit
dc227eb82.
Give up on trying to mechanically forbid abort() within libpq.
Even though there are no such calls in the source code, we've now
seen three different scenarios where build toolchains silently
insert such calls: gcc does it for profiling, some platforms
implement assert() using it, and icc does so for no visible reason.
Checking for accidental use of exit() seems considerably more
important than checking for abort(), so we'll settle for doing
that for now.
Also, filter out __cxa_atexit() to avoid a false match. It seems
that OpenBSD inserts a call to that despite the fact that libpq
contains no C++ code.
Discussion: https://postgr.es/m/
3128896.
1624742969@sss.pgh.pa.us
Fujii Masao [Wed, 30 Jun 2021 11:57:07 +0000 (20:57 +0900)]
doc: Improve descriptions of tup_returned and tup_fetched in pg_stat_database
Previously the descriptions of tup_returned and tup_fetched columns
in pg_stat_database view were confusing. This commit improves them
so that they represent the following formulas of those columns
more accurately.
* pg_stat_database.tup_returned
= sum(pg_stat_all_tables.seq_tup_read)
+ sum(pg_stat_all_indexes.idx_tup_read)
* pg_stat_database.tup_fetched
= sum(pg_stat_all_tables.idx_tup_fetch)
In these formulas, note that the counters for some system catalogs
like pg_database shared across all databases of a cluster are excluded
from the calculations of sum.
Author: Masahiro Ikeda
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/
9eeeccdb-5dd7-90f9-2807-
a4b5d2b76ca3@oss.nttdata.com
Fujii Masao [Wed, 30 Jun 2021 10:33:18 +0000 (19:33 +0900)]
doc: Add type information for postgres_fdw parameters.
Author: Shinya Kato
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/TYAPR01MB2896DEB25C3B0D57F6139768C40F9@TYAPR01MB2896.jpnprd01.prod.outlook.com
Peter Eisentraut [Wed, 30 Jun 2021 06:29:03 +0000 (08:29 +0200)]
genbki stricter error handling
Instead of just writing warnings for invalid cross-catalog lookups,
count the errors and error out at the end.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
ca8ee41d-241b-1bf3-71f0-
aaf1add6d3c5%40enterprisedb.com
Amit Kapila [Wed, 30 Jun 2021 05:59:53 +0000 (11:29 +0530)]
Replace magic constants used in pg_stat_get_replication_slot().
A few variables have been using 10 as a magic constant while
PG_STAT_GET_REPLICATION_SLOT_COLS can be used instead.
Author: Masahiko Sawada
Reviewed-By: Amit Kapila
Backpatch-through: 14, where it was introduced
Discussion: https://postgr.es/m/CAD21AoBvqODDfmD17DkEuPCvV2KbruukXQ2Vwrv5Xi-TsAsTJA@mail.gmail.com
Amit Kapila [Wed, 30 Jun 2021 04:07:59 +0000 (09:37 +0530)]
Allow streaming the changes after speculative aborts.
Until now, we didn't allow to stream the changes in logical replication
till we receive speculative confirm or the next DML change record after
speculative inserts. The reason was that we never use to process
speculative aborts but after commit
4daa140a2f it is possible to process
them so we can allow streaming once we receive speculative abort after
speculative insertion.
We decided to backpatch to 14 where the feature for streaming in progress
transactions have been introduced as this is a minor change and makes that
functionality better.
Author: Amit Kapila
Reviewed-By: Dilip Kumar
Backpatch-through: 14
Discussion: https://postgr.es/m/CAA4eK1KdqmTCtrBR6oFfGELrLLbDLDedL6zACcsUOQuTJBj1vw@mail.gmail.com
Amit Kapila [Wed, 30 Jun 2021 03:15:47 +0000 (08:45 +0530)]
Allow enabling two-phase option via replication protocol.
Extend the replication command CREATE_REPLICATION_SLOT to support the
TWO_PHASE option. This will allow decoding commands like PREPARE
TRANSACTION, COMMIT PREPARED and ROLLBACK PREPARED for slots created with
this option. The decoding of the transaction happens at prepare command.
This patch also adds support of two-phase in pg_recvlogical via a new
option --two-phase.
This option will also be used by future patches that allow streaming of
transactions at prepare time for built-in logical replication. With this,
the out-of-core logical replication solutions can enable replication of
two-phase transactions via replication protocol.
Author: Ajin Cherian
Reviewed-By: Jeff Davis, Vignesh C, Amit Kapila
Discussion:
https://postgr.es/m/
02DA5F5E-CECE-4D9C-8B4B-
418077E2C010@postgrespro.ru
https://postgr.es/m/
64b9f783c6e125f18f88fbc0c0234e34e71d8639.camel@j-davis.com
Michael Paquier [Wed, 30 Jun 2021 02:48:53 +0000 (11:48 +0900)]
Fix incorrect PITR message for transaction ROLLBACK PREPARED
Reaching PITR on such a transaction would cause the generation of a LOG
message mentioning a transaction committed, not aborted.
Oversight in
4f1b890.
Author: Simon Riggs
Discussion: https://postgr.es/m/CANbhV-GJ6KijeCgdOrxqMCQ+C8QiK657EMhCy4csjrPcEUFv_Q@mail.gmail.com
Backpatch-through: 9.6
Michael Paquier [Wed, 30 Jun 2021 00:58:59 +0000 (09:58 +0900)]
Optimize pg_checksums --enable where checksum is already set
This commit prevents pg_checksums to do a rewrite of a block if it has
no need to, in the case where the computed checksum matches with what's
already stored in the block read. This is helpful to accelerate
successive runs of the tool when the previous ones got interrupted, for
example.
The number of blocks and files written is tracked and reported by the
tool once finished. Note that the final flush of the data folder
happens even if no blocks are written, as it could be possible that a
previous interrupted run got stopped while doing a flush.
Author: Greg Sabino Mullane
Reviewed-by: Paquier Michael, Julien Rouhaud
Discussion: https://postgr.es/m/CAKAnmmL+k6goxmVzQJB+0bAR0PN1sgo6GDUXJhyhUmVMze1QAw@mail.gmail.com
Alexander Korotkov [Tue, 29 Jun 2021 20:18:09 +0000 (23:18 +0300)]
Fixes for multirange selectivity estimation
* Fix enumeration of the multirange operators in calc_multirangesel() and
calc_multirangesel() switches.
* Add more regression tests for matching to empty ranges/multiranges.
Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/
c5269c65-f967-77c5-ff7c-
15e621c47f6a%40gmail.com
Author: Alexander Korotkov
Backpatch-through: 14, where multiranges were introduced
Alvaro Herrera [Tue, 29 Jun 2021 19:01:29 +0000 (15:01 -0400)]
Fix libpq state machine in pipeline mode
The original coding required that PQpipelineSync had been called before
the first call to PQgetResult, and failure to do that would result in an
unexpected NULL result being returned. Fix by setting the right state
when a query is sent, rather than leaving it unchanged and having
PQpipelineSync apply the necessary state change.
A new test case to verify the behavior is added, which relies on the new
PQsendFlushRequest() function added by commit
a7192326c74d.
Backpatch to 14, where pipeline mode was added.
Reported-by: Boris Kolpackov <boris@codesynthesis.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/boris.
20210616110321@codesynthesis.com
Alvaro Herrera [Tue, 29 Jun 2021 18:37:39 +0000 (14:37 -0400)]
Add PQsendFlushRequest to libpq
This new libpq function allows the application to send an 'H' message,
which instructs the server to flush its outgoing buffer.
This hasn't been needed so far because the Sync message already requests
a buffer; and I failed to realize that this was needed in pipeline mode
because PQpipelineSync also causes the buffer to be flushed. However,
sometimes it is useful to request a flush without establishing a
synchronization point.
Backpatch to 14, where pipeline mode was introduced in libpq.
Reported-by: Boris Kolpackov <boris@codesynthesis.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/
202106252350.t76x73nt643j@alvherre.pgsql
Tom Lane [Tue, 29 Jun 2021 18:34:31 +0000 (14:34 -0400)]
Fix bogus logic for reporting which hash partition conflicts.
Commit
efbfb6424 added logic for reporting exactly which existing
partition conflicts when complaining that a new hash partition's
modulus isn't compatible with the existing ones. However, it
misunderstood the partitioning data structure, and would select
the wrong partition in some cases, or crash outright due to fetching
a bogus table OID in other cases.
Per bug #17076 from Alexander Lakhin. Fix by Amit Langote;
some further work on the code comments by me.
Discussion: https://postgr.es/m/17076-
89a16ae835d329b9@postgresql.org
Tom Lane [Tue, 29 Jun 2021 15:46:17 +0000 (11:46 -0400)]
Add a build-time check that libpq doesn't call exit() or abort().
Directly exiting or aborting seems like poor form for a general-purpose
library. Now that libpq liberally uses bits out of src/common/,
it's very easy to accidentally include code that would do something
unwanted like calling exit(1) after OOM --- see for example
8ec00dc5c.
Hence, add a simple cross-check that no such calls have made it into
libpq.so.
The cross-check depends on nm(1) being available and being able to
work on a shared library, which probably isn't true everywhere.
But we can just make the test silently do nothing if nm fails.
As long as the check is effective on common platforms, that should
be good enough. (By the same logic, I've not worried about providing
an equivalent test in MSVC builds.)
Discussion: https://postgr.es/m/
3128896.
1624742969@sss.pgh.pa.us
Tom Lane [Tue, 29 Jun 2021 15:31:08 +0000 (11:31 -0400)]
Remove libpq's use of abort(3) to handle mutex failure cases.
Doing an abort() seems all right in development builds, but not in
production builds of general-purpose libraries. However, the functions
that were doing this lack any way to report a failure back up to their
callers. It seems like we can just get away with ignoring failures in
production builds, since (a) no such failures have been reported in the
dozen years that the code's been like this, and (b) failure to enforce
mutual exclusion during fe-auth.c operations would likely not cause any
problems anyway in most cases. (The OpenSSL callbacks that use this
macro are obsolete, so even less likely to cause interesting problems.)
Possibly a better answer would be to break compatibility of the
pgthreadlock_t callback API, but in the absence of field problem
reports, it doesn't really seem worth the trouble.
Discussion: https://postgr.es/m/
3131385.
1624746109@sss.pgh.pa.us
Noah Misch [Tue, 29 Jun 2021 07:44:57 +0000 (00:44 -0700)]
Remove literal backslash from Perl \Q ... \E.
The behavior changed sometime after Perl 5.8.9, and "man perlre" says it
"may lead to confusing results." Per buildfarm member gaur. This
repairs commit
a7a7be1f2fa6b9f0f48e69f12256d8f588af729b.
Discussion: https://postgr.es/m/
20210629053627.GA2061079@rfd.leadboat.com
Peter Eisentraut [Tue, 29 Jun 2021 05:57:16 +0000 (07:57 +0200)]
Add index OID macro argument to DECLARE_INDEX
Instead of defining symbols such as AmOidIndexId explicitly, include
them as an argument of DECLARE_INDEX() and have genbki.pl generate the
way as the table OID symbols from the CATALOG() declaration.
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/
ccef1e46-a404-25b1-9b4c-
85f2c08e1f28%40enterprisedb.com
Michael Paquier [Tue, 29 Jun 2021 04:57:45 +0000 (13:57 +0900)]
Bump XLOG_PAGE_MAGIC for format changes related to FPW compression
Oversight in
4035cd5, spotted by Tom Lane.
Discussion: https://postgr.es/m/365778.
1624941613@sss.pgh.pa.us
Michael Paquier [Tue, 29 Jun 2021 02:54:11 +0000 (11:54 +0900)]
Fix compilation warning in xloginsert.c
This is reproducible with gcc using at least -O0. The last checks
validating the compression of a block could not be reached with this
variable not set, but let's be clean.
Oversight in
4035cd5, per buildfarm member lapwing.
Michael Paquier [Tue, 29 Jun 2021 02:17:55 +0000 (11:17 +0900)]
Add support for LZ4 with compression of full-page writes in WAL
The logic is implemented so as there can be a choice in the compression
used when building a WAL record, and an extra per-record bit is used to
track down if a block is compressed with PGLZ, LZ4 or nothing.
wal_compression, the existing parameter, is changed to an enum with
support for the following backward-compatible values:
- "off", the default, to not use compression.
- "pglz" or "on", to compress FPWs with PGLZ.
- "lz4", the new mode, to compress FPWs with LZ4.
Benchmarking has showed that LZ4 outclasses easily PGLZ. ZSTD would be
also an interesting choice, but going just with LZ4 for now makes the
patch minimalistic as toast compression is already able to use LZ4, so
there is no need to worry about any build-related needs for this
implementation.
Author: Andrey Borodin, Justin Pryzby
Reviewed-by: Dilip Kumar, Michael Paquier
Discussion: https://postgr.es/m/
3037310D-ECB7-4BF1-AF20-
01C10BB33A33@yandex-team.ru
Noah Misch [Tue, 29 Jun 2021 01:34:56 +0000 (18:34 -0700)]
Skip WAL recycling and preallocation during archive recovery.
The previous commit addressed the chief consequences of a race condition
between InstallXLogFileSegment() and KeepFileRestoredFromArchive(). Fix
three lesser consequences. A spurious durable_rename_excl() LOG message
remained possible. KeepFileRestoredFromArchive() wasted the proceeds of
WAL recycling and preallocation. Finally, XLogFileInitInternal() could
return a descriptor for a file that KeepFileRestoredFromArchive() had
already unlinked. That felt like a recipe for future bugs.
Discussion: https://postgr.es/m/
20210202151416.GB3304930@rfd.leadboat.com