users/gsingh/postgres.git
2 years agoMore -Wshadow=compatible-local warning fixes
David Rowley [Thu, 25 Aug 2022 14:35:40 +0000 (02:35 +1200)]
More -Wshadow=compatible-local warning fixes

In a similar effort to f01592f91, here we're targetting fixing the
warnings where we've deemed the shadowing variable to serve a close enough
purpose to the shadowed variable just to reuse the shadowed version and
not declare the shadowing variable at all.

By my count, this takes the warning count from 106 down to 71.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220825020839.GT2342@telsasoft.com

2 years agoAllow grant-level control of role inheritance behavior.
Robert Haas [Thu, 25 Aug 2022 14:06:02 +0000 (10:06 -0400)]
Allow grant-level control of role inheritance behavior.

The GRANT statement can now specify WITH INHERIT TRUE or WITH
INHERIT FALSE to control whether the member inherits the granted
role's permissions. For symmetry, you can now likewise write
WITH ADMIN TRUE or WITH ADMIN FALSE to turn ADMIN OPTION on or off.

If a GRANT does not specify WITH INHERIT, the behavior based on
whether the member role is marked INHERIT or NOINHERIT. This means
that if all roles are marked INHERIT or NOINHERIT before any role
grants are performed, the behavior is identical to what we had before;
otherwise, it's different, because ALTER ROLE [NO]INHERIT now only
changes the default behavior of future grants, and has no effect on
existing ones.

Patch by me. Reviewed and testing by Nathan Bossart and Tushar Ahuja,
with design-level comments from various others.

Discussion: http://postgr.es/m/CA+Tgmoa5Sf4PiWrfxA=sGzDKg0Ojo3dADw=wAHOhR9dggV=RmQ@mail.gmail.com

2 years agoMove NON_EXEC_STATIC from c.h to postgres.h
Peter Eisentraut [Thu, 25 Aug 2022 13:07:03 +0000 (15:07 +0200)]
Move NON_EXEC_STATIC from c.h to postgres.h

It is not needed at the scope of c.h, only in backend code.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/a6a6b48e-ca0a-b58d-18de-98e40d94b842%40enterprisedb.com

2 years agoUpdate another comment still referring to pg_start/stop_backup()
Peter Eisentraut [Thu, 25 Aug 2022 13:04:38 +0000 (15:04 +0200)]
Update another comment still referring to pg_start/stop_backup()

2 years agodoc: Fix typo in GRANT docs
Daniel Gustafsson [Thu, 25 Aug 2022 08:47:02 +0000 (10:47 +0200)]
doc: Fix typo in GRANT docs

Commit ce6b672e445 accidentally introduced a trivial typo in the
documentation for GRANT.

2 years agoFix typo in MVCC test comment
Daniel Gustafsson [Thu, 25 Aug 2022 08:31:20 +0000 (10:31 +0200)]
Fix typo in MVCC test comment

The optimization is named kill_prior_tuple but was accidentally
spelled kill_prio_tuple in the test.

Author: Mingli Zhang <avamingli@gmail.com>
Discussion: https://postgr.es/m/82d3e66a-d8ae-4bfa-943e-29c5add0743f@Spark

2 years agoRemove unused symbol __aarch64
John Naylor [Thu, 25 Aug 2022 06:37:40 +0000 (13:37 +0700)]
Remove unused symbol __aarch64

This was added as a possible variant of __aarch64__ back when 64-bit
ARM was vaporware. It hasn't shown up in the wild since then, so remove.

Nathan Bossart

Discussion: https://www.postgresql.org/message-id/CAFBsxsEN5nW3uRh%3Djrs-QexDrC1btu0ZfriD3FFfb%3D3J6tAngg%40mail.gmail.com

2 years agopg_dump: Fix new ICU tests
Peter Eisentraut [Thu, 25 Aug 2022 04:35:16 +0000 (06:35 +0200)]
pg_dump: Fix new ICU tests

ICU doesn't support some server encodings, so we need to exclude them
if a non-supported encoding was set up.

2 years agoaix: Fix SHLIB_EXPORTS reference in VPATH builds
Andres Freund [Thu, 25 Aug 2022 03:38:28 +0000 (20:38 -0700)]
aix: Fix SHLIB_EXPORTS reference in VPATH builds

The dependencies here aren't quite right independent of vpath builds or not,
but this at least makes vpath builds succeed. And it's pretty rare to change
the exports.txt file anyway... The referenced thread has a patch that will
clean that up further.

Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de

2 years agoRemove SUBSYS.o rule in common.mk, hasn't been used in a long time
Andres Freund [Thu, 25 Aug 2022 03:38:14 +0000 (20:38 -0700)]
Remove SUBSYS.o rule in common.mk, hasn't been used in a long time

Apparently I missed that this SUBSYS.o rule isn't needed anymore in
a4ebbd27527, likely because there still is a reference to it due to AIX - but
that's self contained in src/backend/Makefile

Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de

2 years agoRemove rule to generate postgres.o, not needed for 20+ years
Andres Freund [Thu, 25 Aug 2022 03:37:54 +0000 (20:37 -0700)]
Remove rule to generate postgres.o, not needed for 20+ years

Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de

2 years agosolaris: Use versioning scripts instead of -Bsymbolic
Andres Freund [Thu, 25 Aug 2022 03:37:54 +0000 (20:37 -0700)]
solaris: Use versioning scripts instead of -Bsymbolic

-Bsymbolic causes a lot of "ld: warning: symbol referencing errors"
warnings. It's quite easy to add the symbol versioning script, we just need a
slightly different parameter name.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20220823083436.whtntk3bn3qpnvmb@awork3.anarazel.de
Discussion: https://postgr.es/m/7dae5979-c6c0-cec5-7a36-76a85aa8053d@enterprisedb.com

2 years agoInclude RelFileLocator fields individually in BufferTag.
Robert Haas [Wed, 24 Aug 2022 19:50:48 +0000 (15:50 -0400)]
Include RelFileLocator fields individually in BufferTag.

This is preparatory work for a project to increase the number of bits
in a RelFileNumber from 32 to 56.

Along the way, introduce static inline accessor functions for a couple
of BufferTag fields.

Dilip Kumar, reviewed by me. The overall patch series has also had
review at various times from Andres Freund, Ashutosh Sharma, Hannu
Krosing, Vignesh C, Álvaro Herrera, and Tom Lane.

Discussion: http://postgr.es/m/CAFiTN-trubju5YbWAq-BSpZ90-Z6xCVBQE8BVqXqANOZAF1Znw@mail.gmail.com

2 years agopg_dump: Dump colliculocale
Peter Eisentraut [Wed, 24 Aug 2022 18:13:52 +0000 (20:13 +0200)]
pg_dump: Dump colliculocale

This was forgotten when the new column was introduced.

Author: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/7ad26354e75259f59c4a6c6997b8ee32%40postgrespro.ru

2 years agoDefend against stack overrun in a few more places.
Tom Lane [Wed, 24 Aug 2022 17:01:40 +0000 (13:01 -0400)]
Defend against stack overrun in a few more places.

SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c,
and regex_selectivity_sub() in selectivity estimation could recurse
until stack overflow; fix by adding check_stack_depth() calls.
So could next() in the regex compiler, but that case is better fixed by
converting its tail recursion to a loop.  (We probably get better code
that way too, since next() can now be inlined into its sole caller.)

There remains a reachable stack overrun in the Turkish stemmer, but
we'll need some advice from the Snowball people about how to fix that.

Per report from Egor Chindyaskin and Alexander Lakhin.  These mistakes
are old, so back-patch to all supported branches.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru

2 years agoDoc: remove duplicate "a" from func.sgml
David Rowley [Wed, 24 Aug 2022 11:45:57 +0000 (23:45 +1200)]
Doc: remove duplicate "a" from func.sgml

Author: Shinya Kato
Discussion: https://postgr.es/m/76c01275776749a167f49379ebec57f1@oss.nttdata.com
Backpatch-through: 15, where that change was introduced

2 years agoFix ICU locale option handling in CREATE DATABASE
Peter Eisentraut [Wed, 24 Aug 2022 11:27:34 +0000 (13:27 +0200)]
Fix ICU locale option handling in CREATE DATABASE

The code took the LOCALE option as the default/fallback for
ICU_LOCALE, but this was neither documented nor intended, so remove
it.  (It was probably left in from an earlier patch version.)

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e

2 years agoRemove initialization of MyClientConnectionInfo at backend startup
Michael Paquier [Wed, 24 Aug 2022 10:19:00 +0000 (19:19 +0900)]
Remove initialization of MyClientConnectionInfo at backend startup

This stuff should be already initialized at process startup, so adding
this extra step is confusing for no gain.

Per gripe from Tom Lane and Jacob Champion.

Discussion: https://postgr.es/m/bbf2b922-4ff7-5c30-e3ef-2a8bdcdd1116@timescale.com

2 years agoFurther -Wshadow=compatible-local warning fixes
David Rowley [Wed, 24 Aug 2022 10:04:28 +0000 (22:04 +1200)]
Further -Wshadow=compatible-local warning fixes

These should have been included in 421892a19 as these shadowed variable
warnings can also be fixed by adjusting the scope of the shadowed variable
to put the declaration for it in an inner scope.

This is part of the same effort as f01592f91.

By my count, this takes the warning count from 114 down to 106.

Author: David Rowley and Justin Pryzby
Discussion: https://postgr.es/m/CAApHDvrwLGBP%2BYw9vriayyf%3DXR4uPWP5jr6cQhP9au_kaDUhbA%40mail.gmail.com

2 years agoChange shared library installation naming on macOS
Peter Eisentraut [Wed, 24 Aug 2022 06:23:49 +0000 (08:23 +0200)]
Change shared library installation naming on macOS

It is not customary to install a shared library with a minor version
number (libpq.5.16.dylib) on macOS.  We just need the file with the
major version number (libpq.5.dylib) and the one without version
number (libpq.dylib).  This also matches the installation layout used
by Meson.

Discussion: https://www.postgresql.org/message-id/e0c44fb2-8b66-a4b9-b274-7ed3a1a0ab74@enterprisedb.com

2 years agoAllow parallel workers to retrieve some data from Port
Michael Paquier [Wed, 24 Aug 2022 03:57:13 +0000 (12:57 +0900)]
Allow parallel workers to retrieve some data from Port

This commit moves authn_id into a new global structure called
ClientConnectionInfo (mapping to a MyClientConnectionInfo for each
backend) which is intended to hold all the client information that
should be shared between the backend and any of its parallel workers,
access for extensions and triggers being the primary use case.  There is
no need to push all the data of Port to the workers, and authn_id is
quite a generic concept so using a separate structure provides the best
balance (the name of the structure has been suggested by Robert Haas).

While on it, and per discussion as this would be useful for a potential
SYSTEM_USER that can be accessed through parallel workers, a second
field is added for the authentication method, copied directly from
Port.

ClientConnectionInfo is serialized and restored using a new parallel
key and a structure tracks the length of the authn_id, making the
addition of more fields straight-forward.

Author: Jacob Champion
Reviewed-by: Bertrand Drouvot, Stephen Frost, Robert Haas, Tom Lane,
Michael Paquier, Julien Rouhaud
Discussion: https://postgr.es/m/793d990837ae5c06a558d58d62de9378ab525d83.camel@vmware.com

2 years agoFurther reduce warnings with -Wshadow=compatible-local
David Rowley [Wed, 24 Aug 2022 00:27:12 +0000 (12:27 +1200)]
Further reduce warnings with -Wshadow=compatible-local

In a similar effort to f01592f91, here we're targetting fixing the
warnings that -Wshadow=compatible-local produces that we can fix by moving
a variable to an inner scope to stop that variable from being shadowed by
another variable declared somewhere later in the function.

All of the warnings being fixed here are changing the scope of variables
which are being used as an iterator for a "for" loop.  In each instance,
the fix happens to be changing the for loop to use the C99 type
initialization.  Much of this code likely pre-dates our use of C99.

Reducing the scope of the outer scoped variable seems like the safest way
to fix these.  Renaming seems more likely to risk patches using the wrong
variable.  Reducing the scope is more likely to result in a compilation
failure after applying some future patch rather than introducing bugs with
it.

By my count, this takes the warning count from 129 down to 114.

Author: Justin Pryzby
Discussion: https://postgr.es/m/CAApHDvrwLGBP%2BYw9vriayyf%3DXR4uPWP5jr6cQhP9au_kaDUhbA%40mail.gmail.com

2 years agoMessage style adjustment
Peter Eisentraut [Tue, 23 Aug 2022 19:50:12 +0000 (21:50 +0200)]
Message style adjustment

2 years agoRemove further unwanted linker flags from perl_embed_ldflags
Peter Eisentraut [Tue, 23 Aug 2022 14:00:38 +0000 (16:00 +0200)]
Remove further unwanted linker flags from perl_embed_ldflags

Remove the contents of $Config{ldflags} from ExtUtils::Embed's ldopts,
like we already do with $Config{ccdlflags}.  Those flags are the
choices of those who built the Perl installation, which are not
necessarily appropriate for building PostgreSQL.  What we really want
from ldopts are the options identifying the location and name of the
libperl library, but unfortunately it doesn't appear possible to get
that separately from the other stuff.

The motivation for this was to strip -mmacosx-version-min options.  We
already did something similar for the -arch option.  Both of those are
now covered by this more general approach.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/8c4fcb72-2574-ff7c-4c25-1f032d4a2a57%40enterprisedb.com

2 years agoRemove our artificial PG_SOMAXCONN limit on listen queue length.
Tom Lane [Tue, 23 Aug 2022 14:14:45 +0000 (10:14 -0400)]
Remove our artificial PG_SOMAXCONN limit on listen queue length.

I added this in commit 153f40067, out of paranoia about kernels
possibly rejecting very large listen backlog requests.  However,
POSIX has said for decades that the kernel must silently reduce
any value it considers too large, and there's no evidence that
any current system doesn't obey that.  Let's just drop this limit
and save some complication.

While we're here, compute the request as twice MaxConnections not
twice MaxBackends; the latter no longer means what it did in 2001.

Per discussion of a report from Kevin McKibbin.

Discussion: https://postgr.es/m/CADc_NKg2d+oZY9mg4DdQdoUcGzN2kOYXBu-3--RW_hEe0tUV=g@mail.gmail.com

2 years agoDoc: document possible need to raise kernel's somaxconn limit.
Tom Lane [Tue, 23 Aug 2022 13:55:37 +0000 (09:55 -0400)]
Doc: document possible need to raise kernel's somaxconn limit.

On fast machines, it's possible for applications such as pgbench
to issue connection requests so quickly that the postmaster's
listen queue overflows in the kernel, resulting in unexpected
failures (with not-very-helpful error messages).  Most modern OSes
allow the queue size to be increased, so document how to do that.

Per report from Kevin McKibbin.

Discussion: https://postgr.es/m/CADc_NKg2d+oZY9mg4DdQdoUcGzN2kOYXBu-3--RW_hEe0tUV=g@mail.gmail.com

2 years agoDoc: prefer sysctl to /proc/sys in docs and comments.
Tom Lane [Tue, 23 Aug 2022 13:41:37 +0000 (09:41 -0400)]
Doc: prefer sysctl to /proc/sys in docs and comments.

sysctl is more portable than Linux's /proc/sys file tree, and
often easier to use too.  That's why most of our docs refer to
sysctl when talking about how to adjust kernel parameters.
Bring the few stragglers into line.

Discussion: https://postgr.es/m/361175.1661187463@sss.pgh.pa.us

2 years agoRemove offsetof definition
Peter Eisentraut [Tue, 23 Aug 2022 13:39:36 +0000 (15:39 +0200)]
Remove offsetof definition

This was only needed to deal with some ancient and no longer supported
systems.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/9a5223a2-3e25-d4fb-f092-01ec17428584%40enterprisedb.com

2 years agoDon't bother to set sockaddr_un.sun_len.
Thomas Munro [Tue, 23 Aug 2022 11:52:17 +0000 (23:52 +1200)]
Don't bother to set sockaddr_un.sun_len.

It's not necessary to fill in sun_len when calling bind() or connect(),
on all known systems that have it.

Discussion: https://postgr.es/m/2781112.1644819528%40sss.pgh.pa.us

2 years agoAdd CHECK_FOR_INTERRUPTS while decoding changes.
Amit Kapila [Tue, 23 Aug 2022 04:50:02 +0000 (10:20 +0530)]
Add CHECK_FOR_INTERRUPTS while decoding changes.

While decoding changes in a loop, if we skip all the changes there is no
CFI making the loop uninterruptible.

Reported-by: Whale Song and Andrey Borodin
Bug: 17580
Author: Masahiko Sawada
Reviwed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/17580-849c1d5b6d7eb422@postgresql.org
Discussion: https://postgr.es/m/B319ECD6-9A28-4CDF-A8F4-3591E0BF2369@yandex-team.ru

2 years agoDon't define FRONTEND for libpq
Andres Freund [Tue, 23 Aug 2022 03:39:30 +0000 (20:39 -0700)]
Don't define FRONTEND for libpq

Not needed anymore after 7143b3e8213.

Discussion: https://postgr.es/m/20220820194550.725755r6fj2ro3rx@awork3.anarazel.de

2 years agoDon't define FRONTEND for ecpg libraries
Andres Freund [Tue, 23 Aug 2022 03:39:30 +0000 (20:39 -0700)]
Don't define FRONTEND for ecpg libraries

Not needed anymore after 7143b3e8213.

Discussion: https://postgr.es/m/20220820194550.725755r6fj2ro3rx@awork3.anarazel.de

2 years agoDon't define FRONTEND for initdb
Andres Freund [Tue, 23 Aug 2022 03:39:30 +0000 (20:39 -0700)]
Don't define FRONTEND for initdb

No headers requiring FRONTED to be defined are included as of af1a949109d.

Since this is the last user of (contrib|frontend)_defines in Mkvcbuild.pm,
remove them.

Discussion: https://postgr.es/m/20220820194550.725755r6fj2ro3rx@awork3.anarazel.de

2 years agoRemove redundant call to pgstat_report_wal()
Andres Freund [Tue, 23 Aug 2022 03:25:42 +0000 (20:25 -0700)]
Remove redundant call to pgstat_report_wal()

pgstat_report_stat() will be called before shutdown so an explicit call to
pgstat_report_wal() just before shutdown is redundant.

This likely was not redundant before 5891c7a8ed8, but now it clearly is.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU%2BECH4tNwGFNERkZA%40mail.gmail.com

2 years agoAdd BackendType for standalone backends
Andres Freund [Tue, 23 Aug 2022 03:22:50 +0000 (20:22 -0700)]
Add BackendType for standalone backends

All backends should have a BackendType to enable statistics reporting
per BackendType.

Add a new BackendType for standalone backends, B_STANDALONE_BACKEND (and
alphabetize the BackendTypes). Both the bootstrap backend and single
user mode backends will have BackendType B_STANDALONE_BACKEND.

Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/CAAKRu_aaq33UnG4TXq3S-OSXGWj1QGf0sU%2BECH4tNwGFNERkZA%40mail.gmail.com

2 years agopgstat: Acquire lock when reading variable-numbered stats
Andres Freund [Tue, 23 Aug 2022 03:16:50 +0000 (20:16 -0700)]
pgstat: Acquire lock when reading variable-numbered stats

Somewhere during the development of the patch acquiring a lock during read
access to variable-numbered stats got lost. The missing lock acquisition won't
cause corruption, but can lead to reading torn values when accessing
stats. Add the missing lock acquisitions.

Reported-by: Greg Stark <stark@mit.edu>
Reviewed-by: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com
Backpatch: 15-

2 years agoSwitch format specifier for replication origins to %d
John Naylor [Tue, 23 Aug 2022 02:55:05 +0000 (09:55 +0700)]
Switch format specifier for replication origins to %d

Using %u with uint16 causes warnings with -Wformat-signedness. There are many
other warnings, but for now change only these since c920fe4818 already changed
the message string for most of them.

Per report from Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/31e63649-0355-7088-831e-b07d5f908a8c%40enterprisedb.com

2 years agoRemove empty statement
John Naylor [Tue, 23 Aug 2022 02:24:32 +0000 (09:24 +0700)]
Remove empty statement

Peter Smith

Discussion: https://www.postgresql.org/message-id/CAHut%2BPtRGVuj8Q_GpHHxZyk7fGwdYDG8_s4GSfKoc_4Yd9vR-w%40mail.gmail.com

2 years agoMake role grant system more consistent with other privileges.
Robert Haas [Mon, 22 Aug 2022 15:35:17 +0000 (11:35 -0400)]
Make role grant system more consistent with other privileges.

Previously, membership of role A in role B could be recorded in the
catalog tables only once. This meant that a new grant of role A to
role B would overwrite the previous grant. For other object types, a
new grant of permission on an object - in this case role A - exists
along side the existing grant provided that the grantor is different.
Either grant can be revoked independently of the other, and
permissions remain so long as at least one grant remains. Make role
grants work similarly.

Previously, when granting membership in a role, the superuser could
specify any role whatsoever as the grantor, but for other object types,
the grantor of record must be either the owner of the object, or a
role that currently has privileges to perform a similar GRANT.
Implement the same scheme for role grants, treating the bootstrap
superuser as the role owner since roles do not have owners. This means
that attempting to revoke a grant, or admin option on a grant, can now
fail if there are dependent privileges, and that CASCADE can be used
to revoke these. It also means that you can't grant ADMIN OPTION on
a role back to a user who granted it directly or indirectly to you,
similar to how you can't give WITH GRANT OPTION on a privilege back
to a role which granted it directly or indirectly to you.

Previously, only the superuser could specify GRANTED BY with a user
other than the current user. Relax that rule to allow the grantor
to be any role whose privileges the current user posseses. This
doesn't improve compatibility with what we do for other object types,
where support for GRANTED BY is entirely vestigial, but it makes this
feature more usable and seems to make sense to change at the same time
we're changing related behaviors.

Along the way, fix "ALTER GROUP group_name ADD USER user_name" to
require the same privileges as "GRANT group_name TO user_name".
Previously, CREATEROLE privileges were sufficient for either, but
only the former form was permissible with ADMIN OPTION on the role.
Now, either CREATEROLE or ADMIN OPTION on the role suffices for
either spelling.

Patch by me, reviewed by Stephen Frost.

Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com

2 years agoFix assertion failure in CREATE DATABASE
Peter Eisentraut [Mon, 22 Aug 2022 13:31:50 +0000 (15:31 +0200)]
Fix assertion failure in CREATE DATABASE

An assertion would fail when creating a database with libc locale
provider from a template database with icu locale provider.

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e

2 years agodoc: Minor wordsmithing to COPY docs
Daniel Gustafsson [Mon, 22 Aug 2022 13:08:45 +0000 (15:08 +0200)]
doc: Minor wordsmithing to COPY docs

Perform some minor wordsmithing on two sentences in the COPY documentation
to make them clearer.

While there, also ensure to wrap a few occurrences of CSV in <literal>
which were missing this.

Reported-by: Eric Mutta <eric.mutta@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/166104548566.654.11680826843612576896@wrigleys.postgresql.org

2 years agopg_upgrade: Fix thinko in database info acquisition routine
Peter Eisentraut [Mon, 22 Aug 2022 11:26:52 +0000 (13:26 +0200)]
pg_upgrade: Fix thinko in database info acquisition routine

When checking whether the major version supports per-database locale
providers, it was always looking at the version of the old cluster
instead of the cluster that was passed in.  This would lead to
failures to detect locale provider mismatches.

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e

2 years agoRemove configure probes for sockaddr_storage members.
Thomas Munro [Mon, 22 Aug 2022 04:47:17 +0000 (16:47 +1200)]
Remove configure probes for sockaddr_storage members.

Remove four probes for members of sockaddr_storage.  Keep only the probe
for sockaddr's sa_len, which is enough for our two remaining places that
know about _len fields:

1.  ifaddr.c needs to know if sockaddr has sa_len to understand the
result of ioctl(SIOCGIFCONF).  Only AIX is still using the relevant code
today, but it seems like a good idea to keep it compilable on Linux.

2.  ip.c was testing for presence of ss_len to decide whether to fill in
sun_len in our getaddrinfo_unix() function.  It's just as good to test
for sa_len.  If you have one, you have them all.

(The code in #2 isn't actually needed at all on several OSes I checked
since modern versions ignore sa_len on input to system calls.  Proving
that's the case for all relevant OSes is left for another day, but
wouldn't get rid of that last probe anyway if we still want it for #1.)

Discussion: https://postgr.es/m/CA%2BhUKGJJjF2AqdU_Aug5n2MAc1gr%3DGykNjVBZq%2Bd6Jrcp3Dyvg%40mail.gmail.com

2 years agoUse logical operator && instead of & in vacuumparallel.c.
Amit Kapila [Mon, 22 Aug 2022 03:23:58 +0000 (08:53 +0530)]
Use logical operator && instead of & in vacuumparallel.c.

As such the current usage of & won't produce incorrect results but it
would be better to use && to short-circuit the evaluation of second
condition when the same is not required.

Author: Ranier Vilela
Reviewed-by: Tom Lane, Bharath Rupireddy
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAEudQApL8QcoYwQuutkWKY_h7gBY8F0Xs34YKfc7-G0i83K_pw@mail.gmail.com

2 years agoFix comment in walsender_private.h
Michael Paquier [Mon, 22 Aug 2022 01:02:53 +0000 (10:02 +0900)]
Fix comment in walsender_private.h

All the members of the stucture are protected by the spinlock WalSnd,
but a comment referred to "replyTime" and "latch" as not being in the
set of what gets protected, contrary to what walsender.c does.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACWE_7srye4_GZ=N=-rD=qr2WHL9GZrMnhWJOJ5RdnNS2A@mail.gmail.com

2 years agoRemove dummyret definition
Peter Eisentraut [Sat, 20 Aug 2022 18:48:47 +0000 (20:48 +0200)]
Remove dummyret definition

This hasn't been used in a while (last use removed by 50d22de932, and
before that 84b6d5f359), and since we are now preferring inline
functions over complex macros, it's unlikely to be needed again.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/7110ab37-8ddd-437f-905c-6aa6205c6185%40enterprisedb.com

2 years agoregress: allow to specify directory containing expected files, for ecpg
Andres Freund [Sat, 20 Aug 2022 17:59:01 +0000 (10:59 -0700)]
regress: allow to specify directory containing expected files, for ecpg

The ecpg tests have their input directory in the build directory, as the tests
need to be built. Until now that required copying the expected/ directory to
the build directory in VPATH builds. To avoid needing to implement the same
for the meson build, add support for specifying the location of the expected
directory.

Now that that's not needed anymore, remove the copying of ecpg's expected
directory to the build directory in VPATH builds.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/20220718202327.pspcqz5mwbi2yb7w@awork3.anarazel.de

2 years agoRemove remaining mentions of UNSAFE_STAT_OK
Peter Eisentraut [Sat, 20 Aug 2022 11:53:21 +0000 (13:53 +0200)]
Remove remaining mentions of UNSAFE_STAT_OK

The last use was removed by bed90759fcbcd72d4d06969eebab81e47326f9a2.

Discussion: https://www.postgresql.org/message-id/flat/01229f9a-b358-d71e-31ae-4c0855d73cbc%40enterprisedb.com

2 years agoReduce warnings with -Wshadow=compatible-local builds
David Rowley [Sat, 20 Aug 2022 03:16:51 +0000 (15:16 +1200)]
Reduce warnings with -Wshadow=compatible-local builds

In a similar effort to f01592f91, here we further reduce the warnings we
get about local variables being shadowed when building with
-Wshadow=compatible-local.  This small change reduces the overall number
of warnings by 36.

Discussion: https://postgr.es/m/CAApHDvqBBqF=wmV5azrO7h3VwpwQo+JFBQ+g=E6wVUhKcqR8gA@mail.gmail.com

2 years agoRemove shadowed local variables that are new in v15
David Rowley [Fri, 19 Aug 2022 23:40:44 +0000 (11:40 +1200)]
Remove shadowed local variables that are new in v15

Compiling with -Wshadow=compatible-local yields quite a few warnings about
local variables being shadowed by compatible local variables in an inner
scope.  Of course, this is perfectly valid in C, but we have had bugs in
the past as a result of developers failing to notice this.  af7d270dd is a
recent example.

Here we do a cleanup of warnings we receive from -Wshadow=compatible-local
for code which is new to PostgreSQL 15.  We've yet to have the discussion
about if we actually ever want to run that as a standard compilation flag.
We'll need to at least get the number of warnings down to something easier
to manage before we can realistically consider if we want this or not.
This commit is the first step towards reducing the warnings.

The changes being made here are all fairly trivial.  Because of that, and
the fact that v15 is still in beta, this is being back-patched into 15.
It seems more risky not to do this as the risk of future bugs is increased
by the additional conflicts that this commit could cause for any future
bug fixes touching the same areas as this commit.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220817145434.GC26426%40telsasoft.com
Backpatch-through: 15

2 years agoAvoid reltuples distortion in very small tables.
Peter Geoghegan [Fri, 19 Aug 2022 16:26:08 +0000 (09:26 -0700)]
Avoid reltuples distortion in very small tables.

Consistently avoid trusting a sample of only one page at the point that
VACUUM determines a new reltuples for the target table (though only when
the table is larger than a single page).  This is follow-up work to
commit 74388a1a, which added a heuristic to prevent reltuples from
becoming distorted by successive VACUUM operations that each scan only a
single heap page (which was itself more or less a bugfix for an issue in
commit 44fa8488, which simplified VACUUM's handling of scanned pages).

The original bugfix commit did not account for certain remaining cases
that where not affected by its "2% of total relpages" heuristic.  This
happened with relations that are small enough that just one of its pages
exceeded the 2% threshold, yet still big enough for VACUUM to deem
skipping most of its pages via the visibility map worthwhile.  reltuples
could still become distorted over time with such a table, at least in
scenarios where the VACUUM command is run repeatedly and without the
table itself ever changing.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzk7d4m3oEbEWkWQKd+gz-eD_peBvdXVk1a_KBygXadFeg@mail.gmail.com
Backpatch: 15-, where the rules for scanned pages changed.

2 years agoMove a definition inside a header file
Peter Eisentraut [Fri, 19 Aug 2022 09:20:09 +0000 (11:20 +0200)]
Move a definition inside a header file

Over time, this has ended up in a slightly inappropriate place
relative to the comments around it.

2 years agodoc: Improve some markups and some wording around archiving modules
Michael Paquier [Fri, 19 Aug 2022 01:00:12 +0000 (10:00 +0900)]
doc: Improve some markups and some wording around archiving modules

This commit adds or fixes markups used in a couple of places in the docs
(for <command>, <systemitem> and <literal>).  While on it, this
clarifies some of the documentation added recently for archiving modules
with archive_command, that would still be used as default choice if no
external module is defined (though an archive module could as well use
an archive_command).

Author: Maxim Yablokov
Discussion: https://postgr.es/m/b47ec4e8-6f6a-2aba-038e-d5db150b245e@postgrespro.ru
Backpatch-through: 15

2 years agoInitialize index stats during parallel VACUUM.
Peter Geoghegan [Fri, 19 Aug 2022 00:34:14 +0000 (17:34 -0700)]
Initialize index stats during parallel VACUUM.

Initialize shared memory allocated for index stats to avoid a hard
crash.  This was possible when parallel VACUUM became confused about the
current phase of index processing.

Oversight in commit 8e1fae1938, which refactored parallel VACUUM.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-By: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220818133406.GL26426@telsasoft.com
Backpatch: 15-, the first version with the refactoring commit.

2 years agoUse correct LSN for error reporting in pg_walinspect
Jeff Davis [Thu, 18 Aug 2022 21:23:30 +0000 (14:23 -0700)]
Use correct LSN for error reporting in pg_walinspect

Usage of ReadNextXLogRecord()'s first_record parameter for error
reporting isn't always correct. For instance, in GetWALRecordsInfo()
and GetWalStats(), we're reading multiple records, and first_record
is always passed as the LSN of the first record which is then used
for error reporting for later WAL record read failures. This isn't
correct.

The correct parameter to use for error reports in case of WAL
reading failures is xlogreader->EndRecPtr. This change fixes it.

While on it, removed an unnecessary Assert in pg_walinspect code.

Reported-by: Robert Haas
Author: Bharath Rupireddy
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/CA%2BTgmoZAOGzPUifrcZRjFZ2vbtcw3mp-mN6UgEoEcQg6bY3OVg%40mail.gmail.com
Backpatch-through: 15

2 years agoBump catversion for 6566133c5f52771198aca07ed18f84519fac1be7
Robert Haas [Thu, 18 Aug 2022 19:10:06 +0000 (15:10 -0400)]
Bump catversion for 6566133c5f52771198aca07ed18f84519fac1be7

Omission noted by Tom Lane.

2 years agoDon't add HAVE_LDAP_H HAVE_WINLDAP_H to pg_config.h
Andres Freund [Thu, 18 Aug 2022 17:41:42 +0000 (10:41 -0700)]
Don't add HAVE_LDAP_H HAVE_WINLDAP_H to pg_config.h

They're not referenced, so we don't need them in in pg_config.h.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: http://postgr.es/m/e0c44fb2-8b66-a4b9-b274-7ed3a1a0ab74@enterprisedb.com

2 years agoEnsure that pg_auth_members.grantor is always valid.
Robert Haas [Thu, 18 Aug 2022 17:13:02 +0000 (13:13 -0400)]
Ensure that pg_auth_members.grantor is always valid.

Previously, "GRANT foo TO bar" or "GRANT foo TO bar GRANTED BY baz"
would record the OID of the grantor in pg_auth_members.grantor, but
that role could later be dropped without modifying or removing the
pg_auth_members record. That's not great, because we typically try
to avoid dangling references in catalog data.

Now, a role grant depends on the grantor, and the grantor can't be
dropped without removing the grant or changing the grantor.  "DROP
OWNED BY" will remove the grant, just as it does for other kinds of
privileges. "REASSIGN OWNED BY" will not, again just like what we do
in other cases involving privileges.

pg_auth_members now has an OID column, because that is needed in order
for dependencies to work. It also now has an index on the grantor
column, because otherwise dropping a role would require a sequential
scan of the entire table to see whether the role's OID is in use as
a grantor. That probably wouldn't be too large a problem in practice,
but it seems better to have an index just in case.

A follow-on patch is planned with the goal of more thoroughly
rationalizing the behavior of role grants. This patch is just trying
to do enough to make sure that the data we store in the catalogs is at
some basic level valid.

Patch by me, reviewed by Stephen Frost

Discussion: http://postgr.es/m/CA+TgmoaFr-RZeQ+WoQ5nKPv97oT9+aDgK_a5+qWHSgbDsMp1Vg@mail.gmail.com

2 years agoImprove performance of adjust_appendrel_attrs_multilevel.
Tom Lane [Thu, 18 Aug 2022 16:36:06 +0000 (12:36 -0400)]
Improve performance of adjust_appendrel_attrs_multilevel.

The present implementations of adjust_appendrel_attrs_multilevel and
its sibling adjust_child_relids_multilevel are very messy, because
they work by reconstructing the relids of the child's immediate
parent and then seeing if that's bms_equal to the relids of the
target parent.  Aside from being quite inefficient, this will not
work with planned future changes to make joinrels' relid sets
contain outer-join relids in addition to baserels.

The whole thing can be solved at a stroke by adding explicit parent
and top_parent links to child RelOptInfos, and making these functions
work with RelOptInfo pointers instead of relids.  Doing that is
simpler for most callers, too.

In my original version of this patch, I got rid of
RelOptInfo.top_parent_relids on the grounds that it was now redundant.
However, that adds a lot of code churn in places that otherwise would
not need changing, and arguably the extra indirection needed to fetch
top_parent->relids in those places costs something.  So this version
leaves that field in place.

Discussion: https://postgr.es/m/553080.1657481916@sss.pgh.pa.us

2 years agoAdjust assertion in XLogDecodeNextRecord.
Robert Haas [Thu, 18 Aug 2022 16:15:55 +0000 (12:15 -0400)]
Adjust assertion in XLogDecodeNextRecord.

As written, if you use XLogBeginRead() to position an xlogreader at
the beginning of a WAL page and then try to read WAL, this assertion
will fail. However, the header comment for XLogBeginRead() claims
that positioning an xlogreader at the beginning of a page is valid,
and the code here is perfectly able to cope with it. It's only the
assertion that causes trouble. So relax it.

This is formally a bug in all supported branches, but as it doesn't
seem to have any consequences for current uses of the xlogreader
facility, no back-patch, at least for now.

Dilip Kumar and Robert Haas

Discussion: http://postgr.es/m/CA+TgmoaJSs2_7WHW2GzFYe9+zfPtxBKvT3GW47+x=ptUE=cULw@mail.gmail.com

2 years agoFix subtly-incorrect matching of parent and child partitioned indexes.
Tom Lane [Thu, 18 Aug 2022 16:11:47 +0000 (12:11 -0400)]
Fix subtly-incorrect matching of parent and child partitioned indexes.

When creating a partitioned index, DefineIndex tries to identify
any existing indexes on the partitions that match the partitioned
index, so that it can absorb those as child indexes instead of
building new ones.  Part of the matching is to compare IndexInfo
structs --- but that wasn't done quite right.  We're comparing
the IndexInfo built within DefineIndex itself to one made from
existing catalog contents by BuildIndexInfo.  Notably, while
BuildIndexInfo will run index expressions and predicates through
expression preprocessing, that has not happened to DefineIndex's
struct.  The result is failure to match and subsequent creation
of duplicate indexes.

The easiest and most bulletproof fix is to build a new IndexInfo
using BuildIndexInfo, thereby guaranteeing that the processing done
is identical.

While here, let's also extract the opfamily and collation data
from the new partitioned index, removing ad-hoc logic that
duplicated knowledge about how those are constructed.

Per report from Christophe Pettus.  Back-patch to v11 where
we invented partitioned indexes.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/8864BFAA-81FD-4BF9-8E06-7DEB8D4164ED@thebuild.com

2 years agoWhen using the WAL-logged CREATE DATABASE strategy, bulk extend.
Robert Haas [Thu, 18 Aug 2022 15:09:39 +0000 (11:09 -0400)]
When using the WAL-logged CREATE DATABASE strategy, bulk extend.

This should improve performance, and was suggested by Andres Freund.
Back-patch to v15 to keep the code consistent across branches.

Dilip Kumar

Discussion: http://postgr.es/m/C3458199-FEDD-4356-865A-08DFAA5D4065@anarazel.de
Discussion: http://postgr.es/m/CAFiTN-sJ0vVpJrZ=R5M+g7Tr8=NN4wKOtrqOcDEsfFfnZgivVA@mail.gmail.com

2 years agoRemove unused configure variable.
Tom Lane [Thu, 18 Aug 2022 15:22:13 +0000 (11:22 -0400)]
Remove unused configure variable.

configure extracts TCL_SHLIB_LD_LIBS from tclConfig.sh, and puts the
value into Makefile.global, but then we never use it anywhere.  It
looks like I removed the only usage in cd75f94da, but didn't notice
that it was the only usage.  Might as well mop this up while we're
trying to get rid of unnecessary configure steps.

Discussion: https://postgr.es/m/2442359.1660835043@sss.pgh.pa.us

2 years agoSimplify and clarify an error message
Peter Eisentraut [Thu, 18 Aug 2022 09:33:53 +0000 (11:33 +0200)]
Simplify and clarify an error message

2 years agomstcpip.h is not missing on MinGW.
Thomas Munro [Thu, 18 Aug 2022 04:21:24 +0000 (16:21 +1200)]
mstcpip.h is not missing on MinGW.

Remove a small difference between MinGW and MSVC builds which isn't
needed for modern MinGW, noticed in passing.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com

2 years agoRemove configure probe for netinet/tcp.h.
Thomas Munro [Thu, 18 Aug 2022 04:17:13 +0000 (16:17 +1200)]
Remove configure probe for netinet/tcp.h.

<netinet/tcp.h> is in SUSv3 and all targeted Unix systems have it.
For Windows, we can provide a stub include file, to avoid some #ifdef
noise.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com

2 years agoFix macro problem with gai_strerror on Windows.
Thomas Munro [Thu, 18 Aug 2022 04:16:00 +0000 (16:16 +1200)]
Fix macro problem with gai_strerror on Windows.

Commit 5579388d was confused about why gai_strerror() didn't work, and
used gai_strerrorA().  It turns out that we had explicitly undefined
Windows' own macro for that somewhere else.  Get rid of all that, and
use the system headers' definition of gai_sterror() directly as
intended.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com

2 years agoRemove configure probe for sys/sockio.h.
Thomas Munro [Thu, 18 Aug 2022 04:12:20 +0000 (16:12 +1200)]
Remove configure probe for sys/sockio.h.

On BSD-family systems, header <sys/sockio.h> defines socket ioctl
numbers like SIOCGIFCONF.  Only AIX is using those now, but it defines
them in <net/if.h> anyway.

Supposing some PostgreSQL hacker wants to test that AIX-only code path
on a more common development system by pretending not to have
getifaddrs().  It's enough to include <sys/ioctl.h>, at least on macOS,
FreeBSD and Linux, and we're already doing that.

2 years agoRemove configure probe for net/if.h.
Thomas Munro [Thu, 18 Aug 2022 04:11:58 +0000 (16:11 +1200)]
Remove configure probe for net/if.h.

<net/if.h> is in SUSv3 and all targeted Unixes have it.  It's used in a
region that is already ifdef'd out for Windows.  We're not using it for
any standard definitions, but it's where AIX defines conventional socket
ioctl numbers.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com

2 years agoRemove dead ifaddr.c fallback code.
Thomas Munro [Thu, 18 Aug 2022 04:08:02 +0000 (16:08 +1200)]
Remove dead ifaddr.c fallback code.

We carried a special implementation of pg_foreach_ifaddr() using
Solaris's ioctl(SIOCGLIFCONF), but Solaris 11 and illumos adopted
getifaddrs() more than a decade ago, and we prefer to use that.  Solaris
10 is EOL'd.  Remove the dead code.

Adjust comment about which OSes have getifaddrs(), which also
incorrectly listed AIX.  AIX is in fact the only Unix in the build farm
that *doesn't* have it today, so the implementation based on
ioctl(SIOCGIFCONF) (note, no 'L') is still live.  All the others have
had it for at least one but mostly two decades.

The last-stop fallback at the bottom of the file is dead code in
practice, but it's hard to justify removing it because the better
options are all non-standard.

Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com

2 years agoUpdate comment in gramparse.h
John Naylor [Thu, 18 Aug 2022 02:45:05 +0000 (09:45 +0700)]
Update comment in gramparse.h

src/common/keywords.c hasn't included this header since afb0d0712.

2 years agoRefer to replication origin roident as "ID" in user facing messages and docs
John Naylor [Thu, 18 Aug 2022 01:57:13 +0000 (08:57 +0700)]
Refer to replication origin roident as "ID" in user facing messages and docs

The table column that stores this is of type oid, but is actually limited
to uint16 and has a different path for creating new values. Some of
the documentation already referred to it as an ID, so let's standardize
on that.

While at it, most format strings already use %u, so for consintency
change the remaining stragglers using %d.

Per suggestions from Tom Lane and Justin Pryzby
Discussion: https://www.postgresql.org/message-id/3437166.1659620465%40sss.pgh.pa.us
Backpatch to v15

2 years agoFix hypothetical problem passing the wrong GROUP BY pathkeys
David Rowley [Wed, 17 Aug 2022 23:32:55 +0000 (11:32 +1200)]
Fix hypothetical problem passing the wrong GROUP BY pathkeys

1349d2790 changed things to make the planner request that the
query_pathkeys contain pathkeys for any ORDER BY / DISTINCT aggregates.
Some code added prior to that commit in db0d67db2 made it so the order
that the pathkeys appear in the group_pathkeys could be changed so that
the GROUP BY could be executed in a more optimal order which minimized
sort comparisons.  1349d2790 had to make sure that the pathkeys for any
ORDER BY / DISTINCT aggregates remained at the end of the groupby_pathkeys
and wasn't reordered, so some code was added to
add_paths_to_grouping_rel() to first strip off any pathkeys belonging to
ORDER BY / DISTINCT aggregates before passing to the function to optimize
the order of the group_pathkeys.

It seems I dropped the ball in 1349d2790 and mistakenly used the untouched
PlannerInfo.group_pathkeys to pass to get_useful_group_keys_orderings()
instead of the version that had the aggregate pathkeys removed.  It was
only the code path that was handling creating paths for
partially_grouped_rel which made this mistake.  In practice, we'll never
have any extra pathkeys to strip off when processing
partially_grouped_rel as that's only used when considering partial
paths, which we never do when there are ORDER BY / DISTINCT aggregates.
So this is just a hypothetical bug, not a live bug.  We already have the
correct pathkeys determined, so it's of no extra cost to pass the
correct variable.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20220817015755.GB26426@telsasoft.com

2 years agodoc: Add a note on PO editors
Daniel Gustafsson [Wed, 17 Aug 2022 21:41:56 +0000 (23:41 +0200)]
doc: Add a note on PO editors

While PO files can be edited in any text editor, specialized tools for
translation editing can be quite helpful with automating tasks etc. Add
a small note about PO editors to encourage new translators to research
which tool works best for them.

Reviewed-by: Bruce Momjian <bruce@momjian.us>
Discussion: https://postgr.es/m/163490116698.684.10398197970578456928@wrigleys.postgresql.org

2 years agoRefactor addition of PlaceHolderVars to joinrel targetlists.
Tom Lane [Wed, 17 Aug 2022 20:12:23 +0000 (16:12 -0400)]
Refactor addition of PlaceHolderVars to joinrel targetlists.

Make build_joinrel_tlist() responsible for adding PHVs that were
already computed in one or the other input relation, and therefore
change add_placeholders_to_joinrel() to only add PHVs that will be
newly computed in this joinrel's output.  This makes the handling
of PHVs in build_joinrel_tlist() more like its handling of plain
Vars, which seems like a good thing on intelligibility grounds
and will simplify planned future changes.  There is a purely
cosmetic side-effect that the order of entries in the joinrel's
tlist may change; but since it becomes more like the order of
entries in the input tlists, that's not bad.

The reason it wasn't done like this originally was the potential
cost of looking up PlaceHolderInfo entries to consult ph_needed.
Now that that's O(1) it shouldn't hurt.

Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us

2 years agoUse an explicit state flag to control PlaceHolderInfo creation.
Tom Lane [Wed, 17 Aug 2022 19:52:53 +0000 (15:52 -0400)]
Use an explicit state flag to control PlaceHolderInfo creation.

Up to now, callers of find_placeholder_info() were required to pass
a flag indicating if it's OK to make a new PlaceHolderInfo.  That'd
be fine if the callers had free choice, but they do not.  Once we
begin deconstruct_jointree() it's no longer OK to make more PHIs;
while callers before that always want to create a PHI if it's not
there already.  So there's no freedom of action, only the opportunity
to cause bugs by creating PHIs too late.  Let's get rid of that in
favor of adding a state flag PlannerInfo.placeholdersFrozen, which
we can set at the point where it's no longer OK to make more PHIs.

This patch also simplifies a couple of call sites that were using
complicated logic to avoid calling find_placeholder_info() as much
as possible.  Now that that lookup is O(1) thanks to the previous
commit, the extra bitmap manipulations are probably a net negative.

Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us

2 years agoMake PlaceHolderInfo lookup O(1).
Tom Lane [Wed, 17 Aug 2022 19:35:51 +0000 (15:35 -0400)]
Make PlaceHolderInfo lookup O(1).

Up to now we've just searched the placeholder_list when we want to
find the PlaceHolderInfo with a given ID.  While there's no evidence
of that being a problem in the field, an upcoming patch will add
find_placeholder_info() calls in build_joinrel_tlist(), which seems
likely to make it more of an issue: a joinrel emitting lots of
PlaceHolderVars would incur O(N^2) cost, and we might be building
a lot of joinrels in complex queries.  Hence, add an array that
can be indexed directly by phid to make the lookups constant-time.

Discussion: https://postgr.es/m/1405792.1660677844@sss.pgh.pa.us

2 years agoAvoid using list_length() to test for empty list.
Tom Lane [Wed, 17 Aug 2022 15:12:35 +0000 (11:12 -0400)]
Avoid using list_length() to test for empty list.

The standard way to check for list emptiness is to compare the
List pointer to NIL; our list code goes out of its way to ensure
that that is the only representation of an empty list.  (An
acceptable alternative is a plain boolean test for non-null
pointer, but explicit mention of NIL is usually preferable.)

Various places didn't get that memo and expressed the condition
with list_length(), which might not be so bad except that there
were such a variety of ways to check it exactly: equal to zero,
less than or equal to zero, less than one, yadda yadda.  In the
name of code readability, let's standardize all those spellings
as "list == NIL" or "list != NIL".  (There's probably some
microscopic efficiency gain too, though few of these look to be
at all performance-critical.)

A very small number of cases were left as-is because they seemed
more consistent with other adjacent list_length tests that way.

Peter Smith, with bikeshedding from a number of us

Discussion: https://postgr.es/m/CAHut+PtQYe+ENX5KrONMfugf0q6NHg4hR5dAhqEXEc2eefFeig@mail.gmail.com

2 years agodoc: Consistently spell case-insensitive
Daniel Gustafsson [Wed, 17 Aug 2022 08:05:03 +0000 (10:05 +0200)]
doc: Consistently spell case-insensitive

While almost all occurrences of "case-insensitive{ly}" were spelled with
a dash, a few were using "case insensitive{ly}" with a space instead. Fix
by changing these to use a dash to be consistent.

Discussion: https://postgr.es/m/7657EDEE-5EE2-4AAB-BA95-47B4F71653E1@yesql.se

2 years agodoc: Document pg_trgm being case-insensitive by default
Daniel Gustafsson [Wed, 17 Aug 2022 07:56:02 +0000 (09:56 +0200)]
doc: Document pg_trgm being case-insensitive by default

pg_trgm is by default operating case-insensitively but the docs didn't
mention that at all.

Author: Erik Rijkers <er@xs4all.nl>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reported-by: marcmaiwald@googlemail.com
Discussion: https://postgr.es/m/166064504415.652.12724576876807446945@wrigleys.postgresql.org

2 years agoUse SetInstallXLogFileSegmentActive() in more places in xlog.c
Michael Paquier [Wed, 17 Aug 2022 06:28:45 +0000 (15:28 +0900)]
Use SetInstallXLogFileSegmentActive() in more places in xlog.c

This reduces the code paths where XLogCtl->InstallXLogFileSegmentActive
is directly touched, and this wrapper function does the same thing as
the original code replaced by the function call.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/CALj2ACVhkf-bC5CX-=6iBUfkO5GqmBntQH+m=HpY0iQ=-g1pRg@mail.gmail.com

2 years agoAllow event trigger table_rewrite for ALTER MATERIALIZED VIEW
Michael Paquier [Wed, 17 Aug 2022 05:55:20 +0000 (14:55 +0900)]
Allow event trigger table_rewrite for ALTER MATERIALIZED VIEW

This event can happen when using SET ACCESS METHOD, as the data files of
the materialized need a full refresh but this command tag was not
updated to reflect that.  The documentation is updated to track this
behavior.

Author: Onder Kalaci
Discussion: https://postgr.es/m/CACawEhXwHN3X34FiwoYG8vXR-oyUdrp7qcfRWSzS+NPahS5gSw@mail.gmail.com
Backpatch-through: 15

2 years agoFix assert in logicalmsg_desc
Tomas Vondra [Tue, 16 Aug 2022 21:52:10 +0000 (23:52 +0200)]
Fix assert in logicalmsg_desc

The assert, introduced by 9f1cf97bb5, is intended to check if the prefix
is terminated by a \0 byte, but it has two flaws. Firstly, prefix_size
includes the \0 byte, so prefix[prefix_size] points to the byte after
the null byte. Secondly, the check ensures the byte is not equal \0,
while it should be checking the opposite.

Backpatch-through: 14
Discussion: https://postgr.es/m/b99b6101-2f14-3796-3dfa-4a6cd7d4326d@enterprisedb.com

2 years agodoc: Remove reference to tty libpq connstring param
Daniel Gustafsson [Tue, 16 Aug 2022 20:54:43 +0000 (22:54 +0200)]
doc: Remove reference to tty libpq connstring param

The tty connection string parameter was removed in commit 14d9b3760
but the reference to it in the docs was mistakenly kept.  Fix by
removing it from the libpq documentation.  Backpatch through v14
where the parameter was removed.

Author: Noriyoshi Shinoda <noriyoshi.shinoda@hpe.com>
Discussion: https://postgr.es/m/DM4PR84MB173433216FCC2A3961879000EE6B9@DM4PR84MB1734.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 14

2 years agodoc: Add missing parenthesis to keycombo
Daniel Gustafsson [Tue, 16 Aug 2022 10:44:24 +0000 (12:44 +0200)]
doc: Add missing parenthesis to keycombo

The SIGINT keycombo for the pg_waldump stats emission was lacking a
closing parenthesis.  Backpatch to 15 where this feature was added.

Discussion: https://postgr.es/m/EC39E60E-C8B6-4CDF-8BFA-E4D140446B41@yesql.se
Backpatch-through: 15

2 years agoFix replica identity check for a partitioned table.
Amit Kapila [Tue, 16 Aug 2022 09:55:41 +0000 (15:25 +0530)]
Fix replica identity check for a partitioned table.

The current publisher code checks if UPDATE or DELETE can be executed with
the replica identity of the table even if it's a partitioned table. We can
skip checking the replica identity for partitioned tables because the
operations are actually performed on the leaf partitions (not the
partitioned table).

Reported-by: Brad Nicholson
Author: Hou Zhijie
Reviewed-by: Peter Smith, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAMMnM%3D8i5DohH%3DYKzV0_wYuYSYvuOJoL9F5nzXTc%2ByzsG1f6rg%40mail.gmail.com

2 years agodoc: fix wrong tag used in create sequence manual.
Tatsuo Ishii [Tue, 16 Aug 2022 00:20:14 +0000 (09:20 +0900)]
doc: fix wrong tag used in create sequence manual.

In ref/create_sequence.sgml <literal> tag was used for nextval function name.
This should have been <function> tag.

Author: Noboru Saito
Discussion: https://postgr.es/m/CAAM3qnJTDFFfRf5JHJ4AYrNcqXgMmj0pbH0%2Bvm%3DYva%2BpJyGymA%40mail.gmail.com
Backpatch-through: 10

2 years agoFix headerscheck and cpluspluscheck's exit codes.
Thomas Munro [Mon, 15 Aug 2022 22:53:29 +0000 (10:53 +1200)]
Fix headerscheck and cpluspluscheck's exit codes.

For the benefit of CI, which started running these header check scripts
in its CompilerWarnings task in commit 81b9f23c9c8, they should report
failure if any individual header failed to compile.

Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGKtDwPo9wzKgbStDwfOhEpywMc6PQofio8fAHR7yUjgxw%40mail.gmail.com

2 years agodoc: Add unit to pg_shmem_allocations attributes
Daniel Gustafsson [Mon, 15 Aug 2022 20:30:00 +0000 (22:30 +0200)]
doc: Add unit to pg_shmem_allocations attributes

The unit of size and allocated_size was not documented.  Speciyfing the
unit is in line with how many other (but not all) system view attributes
are documented so fixing by adding the unit which is "bytes".

Author: Euler Taveira <euler@eulerto.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-by: coleman.rik@gmail.com
Discussion: https://postgr.es/m/166033703458.653.1583077816076994614@wrigleys.postgresql.org

2 years agoAdd missing bad-PGconn guards in libpq entry points.
Tom Lane [Mon, 15 Aug 2022 19:40:07 +0000 (15:40 -0400)]
Add missing bad-PGconn guards in libpq entry points.

There's a convention that externally-visible libpq functions should
check for a NULL PGconn pointer, and fail gracefully instead of
crashing.  PQflush() and PQisnonblocking() didn't get that memo
though.  Also add a similar check to PQdefaultSSLKeyPassHook_OpenSSL;
while it's not clear that ordinary usage could reach that with a
null conn pointer, it's cheap enough to check, so let's be consistent.

Daniele Varrazzo and Tom Lane

Discussion: https://postgr.es/m/CA+mi_8Zm_mVVyW1iNFgyMd9Oh0Nv8-F+7Y3-BqwMgTMHuo_h2Q@mail.gmail.com

2 years agoRemove redundant spaces in _outA_Expr() output
Peter Eisentraut [Mon, 15 Aug 2022 10:43:52 +0000 (12:43 +0200)]
Remove redundant spaces in _outA_Expr() output

Since WRITE_NODE_FIELD() output always starts with a space, we don't
need to go out of our way to print another space right before it.

This change is only for visual appearance; the tokenizer on the
reading side would read it the same way (but there is no read support
for A_Expr at this time anyway).

2 years agoImprove tab completion of ALTER TYPE in psql
Michael Paquier [Mon, 15 Aug 2022 05:08:59 +0000 (14:08 +0900)]
Improve tab completion of ALTER TYPE in psql

This commit adds support for more tab completion in this command as of
"ALTER TYPE .. SET".  The completion of "RENAME VALUE" was separated
from the rest of the completions done for this command, so group
everything together.

Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm1u83jtD2wysdw9XwokEacSXEyUpELajEvOMgJTc3pQ7g@mail.gmail.com

2 years agoFix outdated --help message for postgres -f
Michael Paquier [Mon, 15 Aug 2022 04:36:36 +0000 (13:36 +0900)]
Fix outdated --help message for postgres -f

This option switch supports a total of 8 values, as told by
set_plan_disabling_options() and the documentation, but this was not
reflected in the output generated by --help.

Author: Junwang Zhao
Discussion: https://postgr.es/m/CAEG8a3+pT3cWzyjzKs184L1XMNm8NDnoJLiSjAYSO7XqpRh_vA@mail.gmail.com
Backpatch-through: 10

2 years agoPreserve memory context of VarStringSortSupport buffers.
Tom Lane [Sun, 14 Aug 2022 16:05:27 +0000 (12:05 -0400)]
Preserve memory context of VarStringSortSupport buffers.

When enlarging the work buffers of a VarStringSortSupport object,
varstrfastcmp_locale was careful to keep them in the ssup_cxt
memory context; but varstr_abbrev_convert just used palloc().
The latter creates a hazard that the buffers could be freed out
from under the VarStringSortSupport object, resulting in stomping
on whatever gets allocated in that memory later.

In practice, because we only use this code for ICU collations
(cf. 3df9c374e), the problem is confined to use of ICU collations.
I believe it may have been unreachable before the introduction
of incremental sort, too, as traditional sorting usually just
uses one context for the duration of the sort.

We could fix this by making the broken stanzas in varstr_abbrev_convert
match the non-broken ones in varstrfastcmp_locale.  However, it seems
like a better idea to dodge the issue altogether by replacing the
pfree-and-allocate-anew coding with repalloc, which automatically
preserves the chunk's memory context.  This fix does add a few cycles
because repalloc will copy the chunk's content, which the existing
coding assumes is useless.  However, we don't expect that these buffer
enlargement operations are performance-critical.  Besides that, it's
far from obvious that copying the buffer contents isn't required, since
these stanzas make no effort to mark the buffers invalid by resetting
last_returned, cache_blob, etc.  That seems to be safe upon examination,
but it's fragile and could easily get broken in future, which wouldn't
get revealed in testing with short-to-moderate-size strings.

Per bug #17584 from James Inform.  Whether or not the issue is
reachable in the older branches, this code has been broken on its
own terms from its introduction, so patch all the way back.

Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org

2 years agoAdd new win32 header to headerscheck and cpluspluscheck
Thomas Munro [Sat, 13 Aug 2022 23:15:23 +0000 (11:15 +1200)]
Add new win32 header to headerscheck and cpluspluscheck

Commit 5579388d added src/include/port/win32/netdb.h but forgot to
filter it out in the header checking scripts.  Per build farm animal
crake.

2 years agoRemove configure probe for gethostbyname_r.
Thomas Munro [Sat, 13 Aug 2022 21:57:48 +0000 (09:57 +1200)]
Remove configure probe for gethostbyname_r.

It was only used by src/port/getaddrinfo.c, removed by the previous
commit.

Discussion: https://postgr.es/m/CA%2BhUKGJFLPCtAC58EAimF6a6GPw30TU_59FUY%3DGWB_kC%3DJEmVQ%40mail.gmail.com

2 years agoRemove replacement code for getaddrinfo.
Thomas Munro [Sat, 13 Aug 2022 21:53:28 +0000 (09:53 +1200)]
Remove replacement code for getaddrinfo.

SUSv3, all targeted Unixes and modern Windows have getaddrinfo() and
related interfaces.  Drop the replacement implementation, and adjust
some headers slightly to make sure that the APIs are visible everywhere
using standard POSIX headers and names.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com

2 years agoRemove configure probe for struct sockaddr_storage.
Thomas Munro [Sat, 13 Aug 2022 20:52:08 +0000 (08:52 +1200)]
Remove configure probe for struct sockaddr_storage.

<sys/socket.h> provides sockaddr_storage in SUSv3 and all targeted Unix
systems have it.  Windows has it too.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com

2 years agoAvoid misbehavior when hash_table_bytes < bucket_size.
Tom Lane [Sat, 13 Aug 2022 20:59:58 +0000 (16:59 -0400)]
Avoid misbehavior when hash_table_bytes < bucket_size.

It's possible to reach this case when work_mem is very small and tupsize
is (relatively) very large.  In that case ExecChooseHashTableSize would
get an assertion failure, or with asserts off it'd compute nbuckets = 0,
which'd likely cause misbehavior later (I've not checked).  To fix,
clamp the number of buckets to be at least 1.

This is due to faulty conversion of old my_log2() coding in 28d936031.
Back-patch to v13, as that was.

Zhang Mingli

Discussion: https://postgr.es/m/beb64ca0-91e2-44ac-bf4a-7ea36275ec02@Spark

2 years agoRemove HAVE_UNIX_SOCKETS.
Thomas Munro [Sat, 13 Aug 2022 20:46:53 +0000 (08:46 +1200)]
Remove HAVE_UNIX_SOCKETS.

Since HAVE_UNIX_SOCKETS is now defined unconditionally, remove the macro
and drop a small amount of dead code.

The last known systems not to have them (as far as I know at least) were
QNX, which we de-supported years ago, and Windows, which now has them.

If a new OS ever shows up with the POSIX sockets API but without working
AF_UNIX, it'll presumably still be able to compile the code, and fail at
runtime with an unsupported address family error.  We might want to
consider adding a HINT that you should turn off the option to use it if
your network stack doesn't support it at that point, but it doesn't seem
worth making the relevant code conditional at compile time.

Also adjust a couple of places in the docs and comments that referred to
builds without Unix-domain sockets, since there aren't any.  Windows
still gets a special mention in those places, though, because we don't
try to use them by default there yet.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/CA%2BhUKG%2BL_3brvh%3D8e0BW_VfX9h7MtwgN%3DnFHP5o7X2oZucY9dg%40mail.gmail.com