users/rhaas/postgres.git
2 years agoPrevent long-term memory leakage in autovacuum launcher.
Tom Lane [Wed, 31 Aug 2022 20:23:20 +0000 (16:23 -0400)]
Prevent long-term memory leakage in autovacuum launcher.

get_database_list() failed to restore the caller's memory context,
instead leaving current context set to TopMemoryContext which is
how CommitTransactionCommand() leaves it.  The callers both think
they are using short-lived contexts, for the express purpose of
not having to worry about cleaning up individual allocations.
The net effect therefore is that supposedly short-lived allocations
could accumulate indefinitely in the launcher's TopMemoryContext.

Although this has been broken for a long time, it seems we didn't
have any obvious memory leak here until v15's rearrangement of the
stats logic.  I (tgl) am not entirely convinced that there's no
other leak at all, though, and we're surely at risk of adding one
in future back-patched fixes.  So back-patch to all supported
branches, even though this may be only a latent bug in pre-v15.

Reid Thompson

Discussion: https://postgr.es/m/972a4e12b68b0f96db514777a150ceef7dcd2e0f[email protected]

2 years agoDerive freeze cutoff from nextXID, not OldestXmin.
Peter Geoghegan [Wed, 31 Aug 2022 18:37:35 +0000 (11:37 -0700)]
Derive freeze cutoff from nextXID, not OldestXmin.

Before now, the cutoffs that VACUUM used to determine which XIDs/MXIDs
to freeze were determined at the start of each VACUUM by taking related
cutoffs that represent which XIDs/MXIDs VACUUM should treat as still
running, and subtracting an XID/MXID age based value controlled by GUCs
like vacuum_freeze_min_age.  The FreezeLimit cutoff (XID freeze cutoff)
was derived by subtracting an XID age value from OldestXmin, while the
MultiXactCutoff cutoff (MXID freeze cutoff) was derived by subtracting
an MXID age value from OldestMxact.  This approach didn't match the
approach used nearby to determine whether this VACUUM operation should
be an aggressive VACUUM or not.

VACUUM now uses the standard approach instead: it subtracts the same
age-based values from next XID/next MXID (rather than subtracting from
OldestXmin/OldestMxact).  This approach is simpler and more uniform.
Most of the time it will have only a negligible impact on how and when
VACUUM freezes.  It will occasionally make VACUUM more robust in the
event of problems caused by long running transaction.  These are cases
where OldestXmin and OldestMxact are held back by so much that they
attain an age that is a significant fraction of the value of age-based
settings like vacuum_freeze_min_age.

There is no principled reason why freezing should be affected in any way
by the presence of a long-running transaction -- at least not before the
point that the OldestXmin and OldestMxact limits used by each VACUUM
operation attain an age that makes it unsafe to freeze some of the
XIDs/MXIDs whose age exceeds the value of the relevant age-based
settings.  The new approach should at least make freezing degrade more
gracefully than before, even in the most extreme cases.

Author: Peter Geoghegan <[email protected]>
Reviewed-By: Nathan Bossart <[email protected]>
Reviewed-By: Matthias van de Meent <[email protected]>
Discussion: https://postgr.es/m/CAH2-WzkOv5CEeyOO=c91XnT5WBR_0gii0Wn5UbZhJ=4TTykDYg@mail.gmail.com

2 years agoFix MSVC warning in compat_informix/rnull.pgc
Andres Freund [Wed, 31 Aug 2022 16:31:22 +0000 (09:31 -0700)]
Fix MSVC warning in compat_informix/rnull.pgc

Building the ecpg tests with MSVC, with warnings enabled, results in the
following warning:
src/interfaces/ecpg/test/compat_informix/rnull.pgc(19,1): warning C4305: 'initializing': truncation from 'double' to 'float'

The more obvious fix would be an 'f' suffix, but ecpg can't parse that.

Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/2180a97c-c026-1b6c-cec8-d6e499f97017@enterprisedb.com

2 years agoIn the Snowball dictionary, don't try to stem excessively-long words.
Tom Lane [Wed, 31 Aug 2022 14:42:05 +0000 (10:42 -0400)]
In the Snowball dictionary, don't try to stem excessively-long words.

If the input word exceeds 1000 bytes, don't pass it to the stemmer;
just return it as-is after case folding.  Such an input is surely
not a word in any human language, so whatever the stemmer might
do to it would be pretty dubious in the first place.  Adding this
restriction protects us against a known recursion-to-stack-overflow
problem in the Turkish stemmer, and it seems like good insurance
against any other safety or performance issues that may exist in
the Snowball stemmers.  (I note, for example, that they contain no
CHECK_FOR_INTERRUPTS calls, so we really don't want them running
for a long time.)  The threshold of 1000 bytes is arbitrary.

An alternative definition could have been to treat such words as
stopwords, but that seems like a bigger break from the old behavior.

Per report from Egor Chindyaskin and Alexander Lakhin.
Thanks to Olly Betts for the recommendation to fix it this way.

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

2 years agoFix a bug in roles_is_member_of.
Robert Haas [Tue, 30 Aug 2022 12:32:35 +0000 (08:32 -0400)]
Fix a bug in roles_is_member_of.

Commit e3ce2de09d814f8770b2e3b3c152b7671bcdb83f rearranged this
function to be able to identify which inherited role had admin option
on the target role, but it got the order of operations wrong, causing
the function to return wrong answers in the presence of non-inherited
grants.

Fix that, and add a test case that verifies the correct behavior.

Patch by me, reviewed by Nathan Bossart

Discussion: http://postgr.es/m/CA+TgmoYamnu-xt-u7CqjYWnRiJ6BQaSpYOHXP=r4QGTfd1N_EA@mail.gmail.com

2 years agodoc: Fix typo in user inheritance documentation
Daniel Gustafsson [Wed, 31 Aug 2022 11:32:52 +0000 (13:32 +0200)]
doc: Fix typo in user inheritance documentation

Commit 620ac285483 accidentally introduced a typo in the privilege
inheritance documentation

2 years agoRefactor check_ functions to use filehandle for status
Daniel Gustafsson [Wed, 31 Aug 2022 11:06:50 +0000 (13:06 +0200)]
Refactor check_ functions to use filehandle for status

When reporting failure in check_ functions there is (typically) a text-
file mentioned in the error report which contains further details. Some
check_ functions kept a separate flag variable to indicate failure, and
some just checked the state of the filehandle as it's guaranteed to be
open when the check failed. This refactors the functions to consistently
do the same check on error reporting. As the error report contains the
filepath, it makes more sense to check the filehandle state and skip the
flag variable.

Reviewed-by: Nathan Bossart <[email protected]>
Reviewed-by: Bruce Momjian <[email protected]>
Discussion: https://postgr.es/m/595759F6-625B-4ED7-8125-91AF00437F83@yesql.se

2 years agoplpython: Don't create pgxsdir subdirectory in installdir target
Peter Eisentraut [Wed, 31 Aug 2022 05:42:01 +0000 (07:42 +0200)]
plpython: Don't create pgxsdir subdirectory in installdir target

As of db23464715f4792298c639153dda7bfd9ad9d602, we don't install
anything there anymore from plpython, so we don't need to create the
installation directory anymore.

2 years agoOn NetBSD, force dynamic symbol resolution at postmaster start.
Tom Lane [Tue, 30 Aug 2022 21:28:32 +0000 (17:28 -0400)]
On NetBSD, force dynamic symbol resolution at postmaster start.

The default of lazy symbol resolution means that when the postmaster
first reaches the select() call in ServerLoop, it'll need to resolve
the link to that libc entry point.  NetBSD's dynamic loader takes
an internal lock while doing that, and if a signal interrupts the
operation then there is a risk of self-deadlock should the signal
handler do anything that requires that lock, as several of the
postmaster signal handlers do.  The window for this is pretty narrow,
and timing considerations make it unlikely that a signal would arrive
right then anyway.  But it's semi-repeatable on slow single-CPU
machines, and in principle the race could happen with any hardware.

The least messy solution to this is to force binding of dynamic
symbols at postmaster start, using the "-z now" linker option.
While we're at it, also use "-z relro" so as to provide a small
security gain.

It's not entirely clear whether any other platforms share this
issue, but for now we'll assume it's NetBSD-specific.  (We might
later try to use "-z now" on more platforms for performance
reasons, but that would not likely be something to back-patch.)

Report and patch by me; the idea to fix it this way is from
Andres Freund.

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

2 years agoVarious cleanups of the new memory context header code
David Rowley [Tue, 30 Aug 2022 19:33:54 +0000 (07:33 +1200)]
Various cleanups of the new memory context header code

Robert Haas reported that his older clang compiler didn't like the two
Asserts which were verifying that the given MemoryContextMethodID was <=
MEMORY_CONTEXT_METHODID_MASK when building with
-Wtautological-constant-out-of-range-compare.  In my (David's) opinion,
the compiler is wrong to warn about that.  Newer versions of clang don't
warn about the out of range enum value, so perhaps this was a bug that has
now been fixed.  To keep older clang versions happy, let's just cast the
enum value to int to stop the compiler complaining.

The main reason for the Asserts mentioned above to exist are to inform
future developers which are adding new MemoryContexts if they run out of
bit space in MemoryChunk to store the MemoryContextMethodID.  As pointed
out by Tom Lane, it seems wise to also add a comment to the header for
that enum to document the restriction on these enum values.

Additionally, also fix an incorrect usage of UINT64CONST() which was
introduced in c6e0fe1f2.

Author: Robert Haas, David Rowley
Discussion: https://postgr.es/m/CA+TgmoYGG2C7Vbw1cjkQRRBL3zOk8SmhrQnsJgzscX=N9AwPrw@mail.gmail.com

2 years agoRevert "Add missing padding from MemoryChunk struct"
David Rowley [Tue, 30 Aug 2022 15:06:31 +0000 (03:06 +1200)]
Revert "Add missing padding from MemoryChunk struct"

This reverts commit df0f4feef.  It turns out the problem which was causing
the 32-bit ARM and PPC animals to fail was due to a MAXALIGN problem in
slab.c.  This was fixed by d5ee4db0e.  The padding that was added in
df0f4feef would only do anything on machines where uint64 was not aligned
to 8 bytes.  The 32-bit machines which were failing are not in that
category, so revert this commit.

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

2 years agoUpdate the comment in rmgrlist.h to match it to the code.
Amit Kapila [Tue, 30 Aug 2022 03:46:41 +0000 (09:16 +0530)]
Update the comment in rmgrlist.h to match it to the code.

Author: Hayato Kuroda
Reviwed-by: Amit Kapila
Discussion: https://postgr.es/m/TYAPR01MB58665F20F412EDF27B0759CFF5769@TYAPR01MB5866.jpnprd01.prod.outlook.com

2 years agoDrop replication origin slots before tablesync worker exits.
Amit Kapila [Tue, 30 Aug 2022 03:21:41 +0000 (08:51 +0530)]
Drop replication origin slots before tablesync worker exits.

Currently, the replication origin tracking of the tablesync worker is
dropped by the apply worker. So, there will be a small lag between the
tablesync worker exit and its origin tracking got removed. In the
meantime, new tablesync workers can be launched and will try to set up
a new origin tracking. This can lead the system to reach max configured
limit (max_replication_slots) even if the user has configured the max
limit considering the number of tablesync workers required in the system.

We decided not to back-patch as this can occur in very narrow
circumstances and users have to option to increase the configured limit by
increasing max_replication_slots.

Reported-by: Hubert Depesz Lubaczewski
Author: Ajin Cherian
Reviwed-by: Masahiko Sawada, Peter Smith, Hou Zhijie, Amit Kapila
Discussion: https://postgr.es/m/20220714115155[email protected]

2 years agoFurther code review of port/simd.h
John Naylor [Tue, 30 Aug 2022 02:44:44 +0000 (09:44 +0700)]
Further code review of port/simd.h

Add missing declaration per existing style, and fix a couple typos.

Nathan Bossart and Julien Rouhaud

Discussion: https://www.postgresql.org/message-id/20220829171712.GA509233%40nathanxps13
Discussion: https://www.postgresql.org/message-id/20220830022636.qrcbcecmhztbxrwa%40jrouhaud

2 years agoAdjust comments that called MultiXactIds "XMIDs".
Peter Geoghegan [Tue, 30 Aug 2022 02:42:30 +0000 (19:42 -0700)]
Adjust comments that called MultiXactIds "XMIDs".

Oversights in commits 0b018fab and f3c15cbe.

2 years agoUse MAXALIGN() in calculations using sizeof(SlabBlock)
David Rowley [Tue, 30 Aug 2022 02:36:04 +0000 (14:36 +1200)]
Use MAXALIGN() in calculations using sizeof(SlabBlock)

c6e0fe1f2 added a new pointer field to SlabBlock to make it 4 bytes larger
on 32-bit machines.  Prior to that commit, the size of that struct was a
multiple of 8, which meant that MAXALIGN(sizeof(SlabBlock)) was the same
as sizeof(SlabBlock), however, after c6e0fe1f2, due to the addition of the
new pointer field to store a pointer to the owning context, that was no
longer true on builds with sizeof(void *) == 4.

This problem was highlighted by an Assert failure which was checking that
the pointer given to pfree() was MAXALIGNED.  Various 32-bit ARM buildfarm
animals were failing.  These have MAXIMUM_ALIGNOF of 8.  The only 32-bit
testing I'd managed to do on c6e0fe1f2 had been on x86, which has a
MAXIMUM_ALIGNOF of 4, therefore did not exhibit this issue.

Here we define Slab_BLOCKHDRSZ and copy what is being done in aset.c and
generation.c for doing calculations based on the size of the context's
block type.  This means that SlabAlloc() will now always return a
MAXALIGNed pointer.

This also fixes an incorrect sentinel_ok() check in SlabCheck() which was
incorrectly checking the wrong sentinel byte.  This must have previously
not caused any issues due to the fullChunkSize never being large enough to
store the sentinel byte.

Diagnosed-by: Tomas Vondra, Tom Lane
Author: Tomas Vondra, David Rowley
Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com

2 years agoCleanup more code and comments related to Windows NT4 (XP days)
Michael Paquier [Tue, 30 Aug 2022 00:52:58 +0000 (09:52 +0900)]
Cleanup more code and comments related to Windows NT4 (XP days)

All the code and comments cleaned up here is irrelevant since 495ed0e.
Note that this removes an assumption that CreateRestrictedToken() may
not exist, something that could have happened when running under Windows
NT as the code stated.  Rather than assuming that it may not exist, this
causes pg_ctl to fail hard if the function cannot be loaded.

Reported-by: Justin Pryzby
Discussion: https://postgr.es/m/20220826112637[email protected]

2 years agoClean up inconsistent use of fflush().
Tom Lane [Mon, 29 Aug 2022 17:55:38 +0000 (13:55 -0400)]
Clean up inconsistent use of fflush().

More than twenty years ago (79fcde48b), we hacked the postmaster
to avoid a core-dump on systems that didn't support fflush(NULL).
We've mostly, though not completely, hewed to that rule ever since.
But such systems are surely gone in the wild, so in the spirit of
cleaning out no-longer-needed portability hacks let's get rid of
multiple per-file fflush() calls in favor of using fflush(NULL).

Also, we were fairly inconsistent about whether to fflush() before
popen() and system() calls.  While we've received no bug reports
about that, it seems likely that at least some of these call sites
are at risk of odd behavior, such as error messages appearing in
an unexpected order.  Rather than expend a lot of brain cells
figuring out which places are at hazard, let's just establish a
uniform coding rule that we should fflush(NULL) before these calls.
A no-op fflush() is surely of trivial cost compared to launching
a sub-process via a shell; while if it's not a no-op then we likely
need it.

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

2 years agoRemove stray "the".
Robert Haas [Mon, 29 Aug 2022 16:35:46 +0000 (12:35 -0400)]
Remove stray "the".

Per off-list report.

2 years agoPrevent WAL corruption after a standby promotion.
Robert Haas [Mon, 29 Aug 2022 14:47:12 +0000 (10:47 -0400)]
Prevent WAL corruption after a standby promotion.

When a PostgreSQL instance performing archive recovery but not using
standby mode is promoted, and the last WAL segment that it attempted
to read ended in a partial record, the previous code would create
invalid WAL on the new timeline. The WAL from the previously timeline
would be copied to the new timeline up until the end of the last valid
record, but instead of beginning to write WAL at immediately
afterwards, the promoted server would write an overwrite contrecord at
the beginning of the next segment. The end of the previous segment
would be left as all-zeroes, resulting in failures if anything tried
to read WAL from that file.

The root of the issue is that ReadRecord() decides whether to set
abortedRecPtr and missingContrecPtr based on the value of StandbyMode,
but ReadRecord() switches to a new timeline based on the value of
ArchiveRecoveryRequested. We shouldn't try to write an overwrite
contrecord if we're switching to a new timeline, so change the test in
ReadRecod() to check ArchiveRecoveryRequested instead.

Code fix by Dilip Kumar. Comments by me incorporating suggested
language from Álvaro Herrera. Further review from Kyotaro Horiguchi
and Sami Imseih.

Discussion: http://postgr.es/m/CAFiTN-t7umki=PK8dT1tcPV=mOUe2vNhHML6b3T7W7qqvvajjg@mail.gmail.com
Discussion: http://postgr.es/m/FB0DEA0B-E14E-43A0-811F-C1AE93D00FF3%40amazon.com

2 years agodocs: Fix up some out-of-date references to INHERIT/NOINHERIT.
Robert Haas [Mon, 29 Aug 2022 14:10:09 +0000 (10:10 -0400)]
docs: Fix up some out-of-date references to INHERIT/NOINHERIT.

Commit e3ce2de09d814f8770b2e3b3c152b7671bcdb83f should have updated
these sections of the documentation, but failed to do so.

Patch by me, reviewed by Nathan Bossart.

Discussion: http://postgr.es/m/CA+TgmoaKMnde2W_=u7CqeCKi=FKnfbNQPwOR=c_3c8qD7b2nhQ@mail.gmail.com

2 years agoAdd missing padding from MemoryChunk struct
David Rowley [Mon, 29 Aug 2022 11:20:25 +0000 (23:20 +1200)]
Add missing padding from MemoryChunk struct

Buildfarm animals skate, grison and mamba are Assert failing on the
pointer being given to repalloc not being MAXALIGNED.  c6e0fe1f2a made
changes in that area.

All of these animals are 32-bit with a MAXIMUM_ALIGNOF of 8 and a
SIZEOF_VOID_P of 4.  I suspect that the pointer is not properly aligned due
to the lack of padding in the MemoryChunk struct.

Here we add the same type of padding that was previously used in
AllocChunkData and GenerationChunk that c6e0fe1f2a neglected to add.

Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com

2 years agoFix broken cast on MSVC
John Naylor [Mon, 29 Aug 2022 10:25:59 +0000 (17:25 +0700)]
Fix broken cast on MSVC

Per buildfarm animal drongo, casting a vector type to the same type
causes a compile error. We still need the cast on ARM64, so invent a
wrapper function that does the casting only where necessary.

Discussion: https://www.postgresql.org/message-id/CAFBsxsEouaTwbmpqV%2BEW2%3DwFbhw2vHRe26NQTRcd0%3DNaOFDy7A%40mail.gmail.com

2 years agoUse ARM Advanced SIMD (NEON) intrinsics where available
John Naylor [Mon, 29 Aug 2022 07:32:54 +0000 (14:32 +0700)]
Use ARM Advanced SIMD (NEON) intrinsics where available

NEON support is required on the Aarch64 architecture for standard
implementations. Hardware designers for specialized markets can choose
not to support it, but that's true of floating point as well, which
we assume is supported. As with x86, some SIMD support is available
on 32-bit platforms, but those are not interesting from a performance
standpoint and would require an inconvenient runtime check.

Nathan Bossart

Reviewed by John Naylor, Andres Freund, Thomas Munro, and Tom Lane
Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsEyR9JkfbPcDXBRYEfdfC__OkwVGdwEAgY4Rv0cvw35EA%40mail.gmail.com#aba7a64b11503494ffd8dd27067626a9

2 years agoAbstract some more architecture-specific details away from SIMD functionality
John Naylor [Mon, 29 Aug 2022 06:40:53 +0000 (13:40 +0700)]
Abstract some more architecture-specific details away from SIMD functionality

Add a typedef to represent vectors containing four 32-bit integers,
and add functions operating on them. Also separate out saturating
subtraction into its own function. The motivation for this is to
prepare for a future commit to add ARM NEON support.

Nathan Bossart

Reviewed by John Naylor and Tom Lane
Discussion: https://www.postgresql.org/message-id/flat/CAFBsxsEyR9JkfbPcDXBRYEfdfC__OkwVGdwEAgY4Rv0cvw35EA%40mail.gmail.com#aba7a64b11503494ffd8dd27067626a9

2 years agoImprove performance of and reduce overheads of memory management
David Rowley [Mon, 29 Aug 2022 05:15:00 +0000 (17:15 +1200)]
Improve performance of and reduce overheads of memory management

Whenever we palloc a chunk of memory, traditionally, we prefix the
returned pointer with a pointer to the memory context to which the chunk
belongs.  This is required so that we're able to easily determine the
owning context when performing operations such as pfree() and repalloc().

For the AllocSet context, prior to this commit we additionally prefixed
the pointer to the owning context with the size of the chunk.  This made
the header 16 bytes in size.  This 16-byte overhead was required for all
AllocSet allocations regardless of the allocation size.

For the generation context, the problem was worse; in addition to the
pointer to the owning context and chunk size, we also stored a pointer to
the owning block so that we could track the number of freed chunks on a
block.

The slab allocator had a 16-byte chunk header.

The changes being made here reduce the chunk header size down to just 8
bytes for all 3 of our memory context types.  For small to medium sized
allocations, this significantly increases the number of chunks that we can
fit on a given block which results in much more efficient use of memory.

Additionally, this commit completely changes the rule that pointers to
palloc'd memory must be directly prefixed by a pointer to the owning
memory context and instead, we now insist that they're directly prefixed
by an 8-byte value where the least significant 3-bits are set to a value
to indicate which type of memory context the pointer belongs to.  Using
those 3 bits as an index (known as MemoryContextMethodID) to a new array
which stores the methods for each memory context type, we're now able to
pass the pointer given to functions such as pfree() and repalloc() to the
function specific to that context implementation to allow them to devise
their own methods of finding the memory context which owns the given
allocated chunk of memory.

The reason we're able to reduce the chunk header down to just 8 bytes is
because of the way we make use of the remaining 61 bits of the required
8-byte chunk header.  Here we also implement a general-purpose MemoryChunk
struct which makes use of those 61 remaining bits to allow the storage of
a 30-bit value which the MemoryContext is free to use as it pleases, and
also the number of bytes which must be subtracted from the chunk to get a
reference to the block that the chunk is stored on (also 30 bits).  The 1
additional remaining bit is to denote if the chunk is an "external" chunk
or not.  External here means that the chunk header does not store the
30-bit value or the block offset.  The MemoryContext can use these
external chunks at any time, but must use them if any of the two 30-bit
fields are not large enough for the value(s) that need to be stored in
them.  When the chunk is marked as external, it is up to the MemoryContext
to devise its own means to determine the block offset.

Using 3-bits for the MemoryContextMethodID does mean we're limiting
ourselves to only having a maximum of 8 different memory context types.
We could reduce the bit space for the 30-bit value a little to make way
for more than 3 bits, but it seems like it might be better to do that only
if we ever need more than 8 context types.  This would only be a problem
if some future memory context type which does not use MemoryChunk really
couldn't give up any of the 61 remaining bits in the chunk header.

With this MemoryChunk, each of our 3 memory context types can quickly
obtain a reference to the block any given chunk is located on.  AllocSet
is able to find the context to which the chunk is owned, by first
obtaining a reference to the block by subtracting the block offset as is
stored in the 'hdrmask' field and then referencing the block's 'aset'
field.  The Generation context uses the same method, but GenerationBlock
did not have a field pointing back to the owning context, so one is added
by this commit.

In aset.c and generation.c, all allocations larger than allocChunkLimit
are stored on dedicated blocks.  When there's just a single chunk on a
block like this, it's easy to find the block from the chunk, we just
subtract the size of the block header from the chunk pointer.  The size of
these chunks is also known as we store the endptr on the block, so we can
just subtract the pointer to the allocated memory from that.  Because we
can easily find the owning block and the size of the chunk for these
dedicated blocks, we just always use external chunks for allocation sizes
larger than allocChunkLimit.  For generation.c, this sidesteps the problem
of non-external MemoryChunks being unable to represent chunk sizes >= 1GB.
This is less of a problem for aset.c as we store the free list index in
the MemoryChunk's spare 30-bit field (the value of which will never be
close to using all 30-bits).  We can easily reverse engineer the chunk size
from this when needed.  Storing this saves AllocSetFree() from having to
make a call to AllocSetFreeIndex() to determine which free list to put the
newly freed chunk on.

For the slab allocator, this commit adds a new restriction that slab
chunks cannot be >= 1GB in size.  If there happened to be any users of
slab.c which used chunk sizes this large, they really should be using
AllocSet instead.

Here we also add a restriction that normal non-dedicated blocks cannot be
1GB or larger.  It's now not possible to pass a 'maxBlockSize' >= 1GB
during the creation of an AllocSet or Generation context.  Allocations can
still be larger than 1GB, it's just these will always be on dedicated
blocks (which do not have the 1GB restriction).

Author: Andres Freund, David Rowley
Discussion: https://postgr.es/m/CAApHDvpjauCRXcgcaL6+e3eqecEHoeRm9D-kcbuvBitgPnW=vw@mail.gmail.com

2 years agoFix the incorrect assertion introduced in commit 7f13ac8123.
Amit Kapila [Mon, 29 Aug 2022 02:40:10 +0000 (08:10 +0530)]
Fix the incorrect assertion introduced in commit 7f13ac8123.

It has been incorrectly assumed in commit 7f13ac8123 that we can either
purge all or none in the catalog modifying xids list retrieved from a
serialized snapshot. It is quite possible that some of the xids in that
array are old enough to be pruned but not others.

As per buildfarm

Author: Amit Kapila and Masahiko Sawada
Reviwed-by: Masahiko Sawada
Discussion: https://postgr.es/m/CAA4eK1LBtv6ayE+TvCcPmC-xse=DVg=SmbyQD1nv_AaqcpUJEg@mail.gmail.com

2 years agoDoc: fix example of recursive query.
Tom Lane [Sun, 28 Aug 2022 14:44:52 +0000 (10:44 -0400)]
Doc: fix example of recursive query.

Compute total number of sub-parts correctly, per [email protected]

Simon Riggs

Discussion: https://postgr.es/m/166161184718.1235920.6304070286124217754@wrigleys.postgresql.org

2 years agoAdd more detail why repalloc and pfree do not accept NULL pointers
Peter Eisentraut [Sun, 28 Aug 2022 07:55:04 +0000 (09:55 +0200)]
Add more detail why repalloc and pfree do not accept NULL pointers

Per discussion, we choose not to change this.  This just gives a
little bit more information.

Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com

2 years agoEnable RandomizedBaseAddress (ASLR) on Windows with MSVC builds
Michael Paquier [Sun, 28 Aug 2022 07:04:58 +0000 (16:04 +0900)]
Enable RandomizedBaseAddress (ASLR) on Windows with MSVC builds

This has as effect to add /DYNAMICBASE to the .dll and .exe files
generated by the builds, undoing 7f3e17b.  Note that ASLR was already
enabled in MinGW as we have never added --disable-dynamicbase there.

This change will ease a bit the integration of arm64 with MSVC, as ASLR
support is mandatory in this case.  So, thanks to this commit, we have
no need to make ASLR conditional depending on the architecture used for
the build.

Andres Freund has done a lot of testing with this option while working
on meson, without seeing /DYNAMICBASE as being a problem in the Windows
builds of the CI.  Personally, not supporting anything older than
Windows 10 on HEAD makes me feel safer about this change, as we have
seen ASLR with being a problem in process invocation particularly with
Windows 8 and server 2012 back in 2014, even if Windows 10 was not
really a thing back then.  45e004f is also something that can help in
making the process invocation more stable.  We are very early in the
development of Postgres 16, giving a lot of room to detect stability
issues if any.

Discussion: https://postgr.es/m/20220826012907[email protected]

2 years agoAvoid casting away const in sepgsql's quote_object_name.
Tom Lane [Sat, 27 Aug 2022 16:52:39 +0000 (12:52 -0400)]
Avoid casting away const in sepgsql's quote_object_name.

quote_identifier's API is designed on the assumption that it's
not worth worrying about a short-term memory leak when we have
to produce a quoted version of the given identifier.  Whoever wrote
quote_object_name took it on themselves to override that judgment,
but the only way to do so is to cast away const someplace.  We can
avoid that and substantially shorten the function by going along
with quote_identifier's opinion.  AFAICS quote_object_name is not
used in any way where this would be unsustainable.

Per discussion of commit 45987aae2, which exposed that we had
a casting-away-const situation here.

Discussion: https://postgr.es/m/20220827112304[email protected]

2 years agoDoc: add comment about bug fixed in back branches as of 3f7323cbb.
Tom Lane [Sat, 27 Aug 2022 16:16:21 +0000 (12:16 -0400)]
Doc: add comment about bug fixed in back branches as of 3f7323cbb.

While the bug I just fixed in the back branches doesn't exist in
HEAD, the requirement that MULTIEXPR SubPlans not share output
parameters still does.  Add a comment to memorialize that, because
perhaps it could be an issue again someday.

Discussion: https://postgr.es/m/17596-c5357f61427a81dc@postgresql.org

2 years agoFix typo in comment for writetuple() function
Alexander Korotkov [Sat, 27 Aug 2022 11:46:15 +0000 (14:46 +0300)]
Fix typo in comment for writetuple() function

Reported-by: David Rowley
Discussion: https://postgr.es/m/CAApHDvrZ9Ky2LcWwcKsbdYChA850JE5qS%3DkGJiTNWS8mbBXZHw%40mail.gmail.com

2 years agoBe more careful to avoid including system headers after perl.h
John Naylor [Sat, 27 Aug 2022 04:17:36 +0000 (11:17 +0700)]
Be more careful to avoid including system headers after perl.h

Commit 121d2d3d70 included simd.h into pg_wchar.h. This caused a problem
on Windows, since Perl has "#define free" (referring to globals), which
breaks the Windows' header. To fix, move the static inline function
definitions from plperl_helpers.h, into plperl.h, where we already
document the necessary inclusion order. Since those functions were the
only reason for the existence of plperl_helpers.h, remove it.

First reported by Justin Pryzby

Diagnosis and review by Andres Freund, patch by myself per suggestion
from Tom Lane

Discussion: https://www.postgresql.org/message-id/20220826115546.GE2342%40telsasoft.com

2 years agoUse correct connection for cancellation in frontend's parallel slots
Michael Paquier [Sat, 27 Aug 2022 06:21:31 +0000 (15:21 +0900)]
Use correct connection for cancellation in frontend's parallel slots

While waiting for slots to become available in wait_on_slots() in
parallel_slot.c, the cancellation always relied on the first connection
in the set to do the job.  This could cause problems when this slot's
socket is gone as PQgetCancel() would return NULL in this case.  Rather
than always using the first connection, this changes the logic to use
the first valid connection for the cancellation.

Author: Ranier Vilela
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/CAEudQAokk1h_pUwGXsYS4oVOuf35s1O2o3TXGHpV8=AWikvgHA@mail.gmail.com
Backpatch-through: 14

2 years agoRemove unneeded null pointer checks before PQfreemem()
Peter Eisentraut [Fri, 26 Aug 2022 17:16:28 +0000 (19:16 +0200)]
Remove unneeded null pointer checks before PQfreemem()

PQfreemem() just calls free(), and the latter already checks for null
pointers.

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com

2 years agoRemove unnecessary casts in free() and pfree()
Peter Eisentraut [Fri, 26 Aug 2022 13:55:57 +0000 (15:55 +0200)]
Remove unnecessary casts in free() and pfree()

Reviewed-by: Tom Lane <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/cf26e970-8e92-59f1-247a-aa265235075b%40enterprisedb.com

2 years agoUse SSE2 in is_valid_ascii() where available.
John Naylor [Fri, 26 Aug 2022 08:01:24 +0000 (15:01 +0700)]
Use SSE2 in is_valid_ascii() where available.

Per flame graph from Jelte Fennema, COPY FROM ... USING BINARY shows
input validation taking at least 5% of the profile, so it's worth trying
to be more efficient here. With this change, validation of pure ASCII is
nearly 40% faster on contemporary Intel hardware. To make this change
legible and easier to adopt to additional architectures, use helper
functions to abstract the platform details away.

Reviewed by Nathan Bossart

Discussion: https://www.postgresql.org/message-id/CAFBsxsG%3Dk8t%3DC457FXnoBXb%3D8iA4OaZkbFogFMachWif7mNnww%40mail.gmail.com

2 years agoRemove obsolete comment
Peter Eisentraut [Fri, 26 Aug 2022 08:33:55 +0000 (10:33 +0200)]
Remove obsolete comment

The comment in basebackup.c updated by 33bd4698c11 was actually
obsolete to begin with, since the symbols it was referring to haven't
existed in that header file for quite some time.  The header file is
still needed for other reasons, though, so keep the #include, just
drop the comment.

2 years agoFix typo in comment.
Etsuro Fujita [Fri, 26 Aug 2022 07:55:00 +0000 (16:55 +0900)]
Fix typo in comment.

2 years agoAdd optimized functions for linear search within byte arrays
John Naylor [Sun, 21 Aug 2022 04:14:01 +0000 (21:14 -0700)]
Add optimized functions for linear search within byte arrays

In similar vein to b6ef167564, add pg_lfind8() and pg_lfind8_le()
to search for bytes equal or less-than-or-equal to a given byte,
respectively. To abstract away platform details, add helper functions
and typedefs to simd.h.

John Naylor and Nathan Bossart, per suggestion from Andres Freund

Discussion: https://www.postgresql.org/message-id/CAFBsxsGzaaGLF%3DNuq61iRXTyspbO9rOjhSqFN%3DV6ozzmta5mXg%40mail.gmail.com

2 years agoRemove configure probe for sockaddr_in6 and require AF_INET6.
Thomas Munro [Thu, 25 Aug 2022 22:13:22 +0000 (10:13 +1200)]
Remove configure probe for sockaddr_in6 and require AF_INET6.

SUSv3 <netinet/in.h> defines struct sockaddr_in6, and all targeted Unix
systems have it.  Windows has it in <ws2ipdef.h>.  Remove the configure
probe, the macro and a small amount of dead code.

Also remove a mention of IPv6-less builds from the documentation, since
there aren't any.

This is similar to commits f5580882 and 077bf2f2 for Unix sockets.  Even
though AF_INET6 is an "optional" component of SUSv3, there are no known
modern operating system without it, and it seems even less likely to be
omitted from future systems than AF_UNIX.

Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CA+hUKGKErNfhmvb_H0UprEmp4LPzGN06yR2_0tYikjzB-2ECMw@mail.gmail.com

2 years agolibpq code should use libpq_gettext(), not _()
Peter Eisentraut [Thu, 25 Aug 2022 18:46:58 +0000 (20:46 +0200)]
libpq code should use libpq_gettext(), not _()

Fix some wrong use and install a safeguard against future mistakes.

2 years agoFix doc oversight for custom WAL resource managers.
Jeff Davis [Thu, 25 Aug 2022 17:26:31 +0000 (10:26 -0700)]
Fix doc oversight for custom WAL resource managers.

Reported-by: Bharath Rupireddy
Backpatch-through: 15
Discussion: https://postgr.es/m/CALj2ACU+at7RqnWEzS59QsFg3ZOF4C4GSp7pt+PWiLEp0zrEKg@mail.gmail.com

2 years agoSmall refactor to get rid of -Wshadow=compatible-local warning
David Rowley [Thu, 25 Aug 2022 14:46:56 +0000 (02:46 +1200)]
Small refactor to get rid of -Wshadow=compatible-local warning

Further reduce -Wshadow=compatible-local warnings by 1 by refactoring the
code in gistRelocateBuildBuffersOnSplit() to make use of
foreach_current_index() instead of manually incrementing a variable on
each loop.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvpGZX-X=Bn4moyXgfFa0CdSUwoa04d3isit3=1qo8F8Bw@mail.gmail.com

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[email protected]

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 <[email protected]>
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 <[email protected]>
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[email protected]

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[email protected]

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[email protected]

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 <[email protected]>
Discussion: https://postgr.es/m/20220823083436[email protected]
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 <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
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 <[email protected]>
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[email protected]

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 <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
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 <[email protected]>
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[email protected]

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[email protected]

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[email protected]

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 <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
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 <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
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 <[email protected]>
Reviewed-by: "Drouvot, Bertrand" <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Author: Kyotaro Horiguchi <[email protected]>
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 <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
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 <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
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 <[email protected]>
Reviewed-by: Julien Rouhaud <[email protected]>
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 <[email protected]>
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 <[email protected]>
Author: Andres Freund <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/20220718202327[email protected]

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 <[email protected]>
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 <[email protected]>
Reported-By: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20220818133406[email protected]
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