Commit Graph

54435 Commits

Author SHA1 Message Date
Etsuro Fujita
11e5f99d39 Doc: Update struct Trigger definition.
Commit 487e9861d added a new field to struct Trigger, but failed to
update the documentation to match; backpatch to v13 where that came in.

Reviewed by Richard Guo.

Discussion: https://postgr.es/m/CAPmGK17NY92CyxJ%2BBG7A3JZurmng4jfRfzPiBTtNupGMF0xW1g%40mail.gmail.com
2022-09-02 16:45:00 +09:00
John Naylor
0a8de93a48 Speed up lexing of long JSON strings
Use optimized linear search when looking ahead for end quotes,
backslashes, and non-printable characters. This results in nearly 40%
faster JSON parsing on x86-64 when most values are long strings, and
all platforms should see some improvement.

Reviewed by Andres Freund and Nathan Bossart
Discussion: https://www.postgresql.org/message-id/CAFBsxsGhaR2KQ5eisaK%3D6Vm60t%3DaxhD8Ckj1qFoCH1pktZi%2B2w%40mail.gmail.com
Discussion: https://www.postgresql.org/message-id/CAFBsxsESLUyJ5spfOSyPrOvKUEYYNqsBosue9SV1j8ecgNXSKA%40mail.gmail.com
2022-09-02 09:36:22 +07:00
Andres Freund
05519126a0 Move darwin sysroot determination into separate file
The sysroot determination is fairly complex and will soon also be needed when
building with meson. Instead of duplicating the logic, move it to a dedicated
shell script invoked both by configure and meson.

Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/2180a97c-c026-1b6c-cec8-d6e499f97017@enterprisedb.com
2022-09-01 16:54:19 -07:00
Andrew Dunstan
2f2b18bd3f Revert SQL/JSON features
The reverts the following and makes some associated cleanups:

    commit f79b803dc: Common SQL/JSON clauses
    commit f4fb45d15: SQL/JSON constructors
    commit 5f0adec25: Make STRING an unreserved_keyword.
    commit 33a377608: IS JSON predicate
    commit 1a36bc9db: SQL/JSON query functions
    commit 606948b05: SQL JSON functions
    commit 49082c2cc: RETURNING clause for JSON() and JSON_SCALAR()
    commit 4e34747c8: JSON_TABLE
    commit fadb48b00: PLAN clauses for JSON_TABLE
    commit 2ef6f11b0: Reduce running time of jsonb_sqljson test
    commit 14d3f24fa: Further improve jsonb_sqljson parallel test
    commit a6baa4bad: Documentation for SQL/JSON features
    commit b46bcf7a4: Improve readability of SQL/JSON documentation.
    commit 112fdb352: Fix finalization for json_objectagg and friends
    commit fcdb35c32: Fix transformJsonBehavior
    commit 4cd8717af: Improve a couple of sql/json error messages
    commit f7a605f63: Small cleanups in SQL/JSON code
    commit 9c3d25e17: Fix JSON_OBJECTAGG uniquefying bug
    commit a79153b7a: Claim SQL standard compliance for SQL/JSON features
    commit a1e7616d6: Rework SQL/JSON documentation
    commit 8d9f9634e: Fix errors in copyfuncs/equalfuncs support for JSON node types.
    commit 3c633f32b: Only allow returning string types or bytea from json_serialize
    commit 67b26703b: expression eval: Fix EEOP_JSON_CONSTRUCTOR and EEOP_JSONEXPR size.

The release notes are also adjusted.

Backpatch to release 15.

Discussion: https://postgr.es/m/40d2c882-bcac-19a9-754d-4299e1d87ac7@postgresql.org
2022-09-01 17:07:14 -04:00
Tom Lane
90247e742f Add a regression test for contrib/pgrowlocks.
Dong Wook Lee, revised a bit by me

Discussion: https://postgr.es/m/20220629055326.tdswmcjcr5jzbrsk@home-desktop
2022-09-01 15:02:41 -04:00
Andres Freund
e5484554ba aix: when building with gcc, tell gcc we're building a shared library
Not passing -shared to gcc when building a shared library triggers linking to
the wrong libgcc (libgcc.a instead of libgcc_s.a) and prevents emitting
correct unwind information. It's somewhat surprising that this hasn't caused
known problems so far.

Doing so requires adding path to libgcc to libpath, or linking statically to
libgcc - as the latter increases .so size substantially (for not entirely
obvious reasons), shared linking seems preferrable.  It likely is worth
building executables with -shared-libgcc too, but I've not done that here.

Discussion: https://postgr.es/m/20220820174213.d574qde4ptwdzoqz@awork3.anarazel.de
2022-09-01 11:49:36 -07:00
Tom Lane
a1b9b14cc8 Use --load-extension to set up for contrib/tcn's isolation tests.
Oversight in commit 418ec3207: it's better to do it like this,
else you have to drop and recreate the extension for each
permutation.  tcn.spec only has one permutation at present,
so this doesn't speed it up any, but it's still a bad example.
2022-09-01 14:30:41 -04:00
Bruce Momjian
97fe6d2210 doc: in create statistics docs, mention analyze for parent info
Discussion: https://postgr.es/m/Yv1Bw8J+1pYfHiRl@momjian.us

Backpatch-through: 10
2022-08-31 23:11:46 -04:00
Bruce Momjian
dd03dfeb3b doc: mention "bloom" as a possible index access method
Also remove USING erroneously added recently.

Reported-by: Jeff Janes

Discussion: https://postgr.es/m/CAMkU=1zhCpC7hottyMWM5Pimr9vRLprSwzLg+7PgajWhKZqRzw@mail.gmail.com

Backpatch-through: 10
2022-08-31 22:35:09 -04:00
Tom Lane
4ea07e7cf3 Adjust XML test case to avoid unstable behavior.
Buildfarm member bowerbird is (inconsistently) showing different
results for this test case since we enabled ASLR for MSVC builds.
It's not very clear whether that's a bug in its version of libxml2
or the test case is relying on nominally-undefined behavior, ie the
ordering of results from XPath's node().  It seems quite unlikely
that it's *our* bug though, and what's more, using node() adds
nothing to the test coverage so far as our code is concerned.
So, tweak the test to not use node().

For the moment, only change HEAD because we've only seen the
problem there.  Perhaps a case will emerge for back-patching.

Discussion: https://postgr.es/m/2655387.1661695793@sss.pgh.pa.us
2022-08-31 22:21:39 -04:00
Bruce Momjian
fede154172 doc: use FILTER in aggregate example
Reported-by: michal.palenik@freemap.sk

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

Backpatch-through: 10
2022-08-31 22:19:06 -04:00
Bruce Momjian
fbd597e0b8 doc: clarify that pgcrypto's gen_random_uuid calls core func.
Previously it was just marked as a duplicate of the core function.

Reported-by: Andreas Dijkman

Discussion: https://postgr.es/m/17349-24d61e214429e8c1@postgresql.org

Backpatch-through: 13
2022-08-31 22:04:36 -04:00
Bruce Momjian
3e337b585a doc: split out the NATURAL/CROSS JOIN in SELECT syntax
This allows the syntax to be more accurate about what clauses are
supported.  Also switch an example query to use the ANSI join syntax.

Reported-by: Joel Jacobson

Discussion: https://postgr.es/m/67b71d3e-0c22-44df-a223-351f14418319@www.fastmail.com

Backpatch-through: 11
2022-08-31 21:46:14 -04:00
Bruce Momjian
1703726488 doc: warn of SECURITY DEFINER schemas for non-sql_body functions
Non-sql_body functions are evaluated at runtime.

Reported-by: Erki Eessaar

Discussion: https://postgr.es/m/AM9PR01MB8268BF5E74E119828251FD34FE409@AM9PR01MB8268.eurprd01.prod.exchangelabs.com

Backpatch-through: 10
2022-08-31 21:10:37 -04:00
Bruce Momjian
a009cb3141 doc: mention that SET TIME ZONE often needs to be quoted
Also mention that time zone abbreviations are not supported.

Reported-by: philippe.godfrin@nov.com

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

Backpatch-through: 10
2022-08-31 20:27:27 -04:00
Bruce Momjian
62c49fca13 doc: document the maximum char/varchar length value
Reported-by: Japin Li

Discussion: https://postgr.es/m/MEYP282MB1669B13E98AE531617CB1386B6979@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

Backpatch-through: 10
2022-08-31 19:43:06 -04:00
Bruce Momjian
9bdad1b515 doc: show direction is optional in FETCH/MOVE's FROM/IN syntax
It used to show direction was required for FROM/IN.

Reported-by: Rob <rirans@comcast.net>

Discussion: https://postgr.es/m/20211015165248.isqjceyilelhnu3k@localhost

Author: Rob <rirans@comcast.net>

Backpatch-through: 10
2022-08-31 19:29:06 -04:00
David Rowley
1083f94dac Be smarter about freeing tuples during tuplesorts
During dumptuples() the call to writetuple() would pfree any non-null
tuple.  This was quite wasteful as this happens just before we perform a
reset of the context which stores all of those tuples.

It seems to make sense to do a bit of a code refactor to make this work,
so here we just get rid of the writetuple function and adjust the WRITETUP
macro to call the state's writetup function.  The WRITETUP usage in
mergeonerun() always has state->slabAllocatorUsed == true, so writetuple()
would never free the tuple or do any memory accounting.  The only call
path that needs memory accounting done is in dumptuples(), so let's just
do it manually there.

In passing, let's get rid of the state->memtupcount-- code that counts the
memtupcount down to 0 one tuple at a time inside the loop.  That seems to
be a rather inefficient way to set memtupcount to 0, so let's just zero it
after the loop instead.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqZXoDCyrfCzZJR0-xH+7_q+GgitcQiYXUjRani7h4j8Q@mail.gmail.com
2022-09-01 11:08:10 +12:00
Bruce Momjian
349baa87ae doc: simplify WITH clause syntax in CREATE DATABASE
Reported-by: Rob <rirans@comcast.net>

Discussion: https://postgr.es/m/20211016171149.yaouvlw5kvux6dvk@localhost

Author: Rob <rirans@comcast.net>

Backpatch-through: 10
2022-08-31 17:08:44 -04:00
Tom Lane
1c1294be71 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.camel@crunchydata.com
2022-08-31 16:23:35 -04:00
Peter Geoghegan
c3ffa731a5 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 <pg@bowt.ie>
Reviewed-By: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-By: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzkOv5CEeyOO=c91XnT5WBR_0gii0Wn5UbZhJ=4TTykDYg@mail.gmail.com
2022-08-31 11:37:35 -07:00
Andres Freund
483ac64761 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 <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/2180a97c-c026-1b6c-cec8-d6e499f97017@enterprisedb.com
2022-08-31 09:31:22 -07:00
Tom Lane
1058555a5e 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
2022-08-31 10:42:05 -04:00
Robert Haas
0101f770a0 Fix a bug in roles_is_member_of.
Commit e3ce2de09d 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
2022-08-31 08:22:24 -04:00
Daniel Gustafsson
c7892c253f doc: Fix typo in user inheritance documentation
Commit 620ac28548 accidentally introduced a typo in the privilege
inheritance documentation
2022-08-31 13:32:52 +02:00
Daniel Gustafsson
cad4323cd3 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 <nathandbossart@gmail.com>
Reviewed-by: Bruce Momjian <bruce@momjian.us>
Discussion: https://postgr.es/m/595759F6-625B-4ED7-8125-91AF00437F83@yesql.se
2022-08-31 13:06:50 +02:00
Peter Eisentraut
7d5852ca83 plpython: Don't create pgxsdir subdirectory in installdir target
As of db23464715, we don't install
anything there anymore from plpython, so we don't need to create the
installation directory anymore.
2022-08-31 07:42:01 +02:00
Tom Lane
8acd8f8690 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
2022-08-30 17:28:37 -04:00
David Rowley
05f9084236 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
2022-08-31 07:33:54 +12:00
David Rowley
5495796ad1 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
2022-08-31 03:06:31 +12:00
Amit Kapila
c98b6acdb2 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
2022-08-30 09:16:41 +05:30
Amit Kapila
f6c5edb8ab 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.GA5439@depesz.com
2022-08-30 08:51:41 +05:30
John Naylor
865424627d 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
2022-08-30 09:50:00 +07:00
Peter Geoghegan
9887dd38f9 Adjust comments that called MultiXactIds "XMIDs".
Oversights in commits 0b018fab and f3c15cbe.
2022-08-29 19:42:30 -07:00
David Rowley
d5ee4db0ea 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
2022-08-30 14:36:04 +12:00
Michael Paquier
b1ec7f47e3 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.GD2342@telsasoft.com
2022-08-30 09:52:58 +09:00
Tom Lane
7fed801135 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
2022-08-29 13:55:41 -04:00
Robert Haas
20796536c1 Remove stray "the".
Per off-list report.
2022-08-29 12:35:46 -04:00
Robert Haas
6672d79139 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
2022-08-29 11:07:37 -04:00
Robert Haas
620ac28548 docs: Fix up some out-of-date references to INHERIT/NOINHERIT.
Commit e3ce2de09d 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
2022-08-29 10:13:47 -04:00
David Rowley
df0f4feef8 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
2022-08-29 23:20:25 +12:00
John Naylor
c6a43c25a8 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
2022-08-29 17:44:35 +07:00
John Naylor
82739d4a80 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
2022-08-29 14:43:03 +07:00
John Naylor
f8f19f7086 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
2022-08-29 14:20:09 +07:00
David Rowley
c6e0fe1f2a 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
2022-08-29 17:15:00 +12:00
Amit Kapila
d2169c9985 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
2022-08-29 08:10:10 +05:30
Tom Lane
8c7fc86ca9 Doc: fix example of recursive query.
Compute total number of sub-parts correctly, per jason@banfelder.net

Simon Riggs

Discussion: https://postgr.es/m/166161184718.1235920.6304070286124217754@wrigleys.postgresql.org
2022-08-28 10:44:52 -04:00
Peter Eisentraut
805a397db4 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
2022-08-28 09:55:04 +02:00
Michael Paquier
36389a060c 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.gjw3jdqdgsts5y65@awork3.anarazel.de
2022-08-28 16:04:58 +09:00
Tom Lane
94077df1c2 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.GL2342@telsasoft.com
2022-08-27 12:52:39 -04:00