Michael Paquier [Fri, 9 Sep 2022 01:00:40 +0000 (10:00 +0900)]
Add more error context to RestoreBlockImage() and consume it
On failure in restoring a block image, no details were provided, while
it is possible to see failure with an inconsistent record state, a
failure in processing decompression or a failure in decompression
because a build does not support this option.
RestoreBlockImage() is used in two code paths in the backend code,
during recovery and when checking a page consistency after applying
masking, and both places are changed to consume the error message
produced by the internal routine when it returns a false status. All
the error messages are reported under ERRCODE_INTERNAL_ERROR, that gets
used also when attempting to access a page compressed by a method
not supported by the build attempting the decompression. This is
something that can happen in core when doing physical replication with
primary and standby using inconsistent build options, for example.
This routine is available since
2c03216d and it has never provided any
context about the error happening when it failed. This change is
justified even more after
57aa5b2, that introduced compression of FPWs
in WAL.
Reported-by: Justin Prysby
Author: Michael Paquier
Discussion: https://postgr.es/m/
20220905002320.GD31833@telsasoft.com
Backpatch-through: 15
Peter Geoghegan [Thu, 8 Sep 2022 17:29:39 +0000 (10:29 -0700)]
Instrument freezing in autovacuum log reports.
Add a new line to log reports from autovacuum (as well as VACUUM VERBOSE
output) that shows information about freezing. Emphasis is placed on
the total number of heap pages that had one or more tuples frozen by
VACUUM. The total number of tuples frozen is also shown.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Jeff Janes <jeff.janes@gmail.com>
Discussion: https://postgr.es/m/CAH2-WznTY6D0zyE8VLrC6Gd4kh_HGAXxnTPtcOQOOsxzLx9zog@mail.gmail.com
David Rowley [Thu, 8 Sep 2022 12:28:38 +0000 (00:28 +1200)]
Temporarily make MemoryContextContains return false
5265e91fd changed MemoryContextContains to update it so that it works
correctly with the new MemoryChunk code added in
c6e0fe1f2. However,
5265e91fd was done with the assumption that MemoryContextContains would
only ever be given pointers to memory that had been returned by one of our
MemoryContext allocators. It seems that's not true and many of our 32-bit
buildfarm animals are clearly showing that.
There are some code paths that call MemoryContextContains with a pointer
pointing part way into an allocated chunk. The example of this found by
the 32-bit buildfarm animals is the int2int4_sum() function. This
function returns transdata->sum, which is not a pointer to memory that was
allocated directly. This return value is then subsequently passed to
MemoryContextContains which causes it to crash due to it thinking the
memory directly prior to that pointer is a MemoryChunk. What's actually
in that memory is the field in the struct that comes prior to the "sum"
field. This problem didn't occur in 64-bit world because BIGINT is a
byval type and the code which was calling MemoryContextContains with the
bad pointer only does so with non-byval types.
Here, instead of reverting
5265e91fd and making MemoryContextContains
completely broken again, let's just make it always return false for now.
Effectively prior to
5265e91fd it was doing that anyway, this at least
makes that more explicit. The only repercussions of this with the current
MemoryContextContains calls are that we perform a datumCopy() when we
might not need to. This should make the 32-bit buildfarm animals happy
again and give us more time to consider a long-term fix.
Discussion: https://postgr.es/m/
20220907130552.sfjri7jublfxyyi4%40jrouhaud
Alvaro Herrera [Thu, 8 Sep 2022 11:17:02 +0000 (13:17 +0200)]
Choose FK name correctly during partition attachment
During ALTER TABLE ATTACH PARTITION, if the name of a parent's foreign
key constraint is already used on the partition, the code tries to
choose another one before the FK attributes list has been populated,
so the resulting constraint name was "<relname>__fkey" instead of
"<relname>_<attrs>_fkey". Repair, and add a test case.
Backpatch to 12. In 11, the code to attach a partition was not smart
enough to cope with conflicting constraint names, so the problem doesn't
exist there.
Author: Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
Discussion: https://postgr.es/m/
20220901184156.
738ebee5@karst
Thomas Munro [Thu, 8 Sep 2022 08:25:20 +0000 (20:25 +1200)]
Fix recovery_prefetch with low maintenance_io_concurrency.
We should process completed IOs *before* trying to start more, so that
it is always possible to decode one more record when the decoded record
queue is empty, even if maintenance_io_concurrency is set so low that a
single earlier WAL record might have saturated the IO queue.
That bug was hidden because the effect of maintenance_io_concurrency was
arbitrarily clamped to be at least 2. Fix the ordering, and also remove
that clamp. We need a special case for 0, which is now treated the same
as recovery_prefetch=off, but otherwise the number is used directly.
This allows for testing with 1, which would have made the problem
obvious in simple test scenarios.
Also add an explicit error message for missing contrecords. It was a
bit strange that we didn't report an error already, and became a latent
bug with prefetching, since the internal state that tracks aborted
contrecords would not survive retrying, as revealed by
026_overwrite_contrecord.pl with this adjustment. Reporting an error
prevents that.
Back-patch to 15.
Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/
20220831140128.GS31833%40telsasoft.com
Alvaro Herrera [Thu, 8 Sep 2022 09:16:09 +0000 (11:16 +0200)]
Fix perltidy breaking perlcritic
perltidying a "##no critic" line moves the marker to where it becomes
useless. Put the line back to how it was, and protect it from further
malfeasance.
Per buildfarm member crake.
Daniel Gustafsson [Thu, 8 Sep 2022 07:56:50 +0000 (09:56 +0200)]
doc: Fix PL/pgSQL casing to be consistent
Ensure that all mentions of PL/pgSQL is cased equally, a few instances
of PL/PgSQL had snuck in.
Reviewed-by: Pavel Stehule <pavel.stehule@gmail.com>
Discussion: https://postgr.es/m/
DDCF61C3-9E25-48A8-97BE-
6113A93D54A5@yesql.se
John Naylor [Thu, 8 Sep 2022 07:05:40 +0000 (14:05 +0700)]
Add
b2e6e7682 to .git-blame-ignore-revs
John Naylor [Thu, 8 Sep 2022 06:54:14 +0000 (13:54 +0700)]
Run perltidy over Catalog.pm
Commit
69eb643b2 deliberately left indentation unchanged to make the changes
more legible. Rather than waiting until next year's perltidy run, do it now
to avoid confusion
Per suggestion from Álvaro Herrera
Discussion: https://www.postgresql.org/message-id/
20220907083558.vfvb5hcauaictgum%40alvherre.pgsql
John Naylor [Thu, 8 Sep 2022 06:23:13 +0000 (13:23 +0700)]
Parse catalog .dat files as a whole when compiling the backend
Previously Catalog.pm eval'd each individual hash reference
so that comments and whitespace can be preserved when running
reformat-dat-files. This is unnecessary when building, and we can save
~15% off the run time of genbki.pl by simply slurping and eval'-ing
the whole file at once. This saves a bit of time, especially in highly
parallel builds, since most build targets depend on this script's outputs.
Report and review by Andres Freund
Discussion: https://www.postgresql.org/message-id/CAFBsxsGW%3DWRbnxXrc8UqqR479XuxtukSFWV-hnmtgsbuNAUO6w%40mail.gmail.com
Amit Kapila [Thu, 8 Sep 2022 04:17:43 +0000 (09:47 +0530)]
Fix the test case introduced by commit
8756930190.
Before dropping a relation, ensure that it has reached a 'ready' state
after initial synchronization.
Author: Vignesh C
Reviewed-By: Amit Kapila
Discussion: https://www.postgresql.org/message-id/CALDaNm0gwjY_4HFxvvty01BOT01q_fJLKQ3pWP9=9orqubhjcQ@mail.gmail.com
Amit Kapila [Thu, 8 Sep 2022 01:24:13 +0000 (06:54 +0530)]
Raise a warning if there is a possibility of data from multiple origins.
This commit raises a warning message for a combination of options
('copy_data = true' and 'origin = none') during CREATE/ALTER subscription
operations if the publication tables were also replicated from other
publishers.
During replication, we can skip the data from other origins as we have that
information in WAL but that is not possible during initial sync so we raise
a warning if there is such a possibility.
Author: Vignesh C
Reviewed-By: Peter Smith, Amit Kapila, Jonathan Katz, Shi yu, Wang wei
Discussion: https://www.postgresql.org/message-id/CALDaNm0gwjY_4HFxvvty01BOT01q_fJLKQ3pWP9=9orqubhjcQ@mail.gmail.com
Alvaro Herrera [Wed, 7 Sep 2022 15:33:49 +0000 (17:33 +0200)]
Message style fixes
David Rowley [Wed, 7 Sep 2022 12:20:20 +0000 (00:20 +1200)]
Make MemoryContextContains work correctly again
c6e0fe1f2 recently changed the way we store headers for allocated chunks
of memory. Prior to that commit, we stored a pointer to the owning
MemoryContext directly prior to the pointer to the allocated memory.
That's no longer true and
c6e0fe1f2 neglected to update
MemoryContextContains() so that it correctly obtains the owning context
with the new method.
A side effect of this change and
c6e0fe1f2, in general, is that it's even
less safe than it was previously to pass MemoryContextContains() an
arbitrary pointer which was not allocated by one of our MemoryContexts.
Previously some comments in MemoryContextContains() seemed to indicate
that the worst that could happen by passing an arbitrary pointer would be
a false positive return value. It seems to me that this was a rather
wishful outlook as we subsequently proceeded to subtract sizeof(void *)
from the given pointer and then dereferenced that memory. So it seems
quite likely that we could have segfaulted instead of returning a false
positive. However, it's not impossible that the memory sizeof(void *)
bytes before the pointer could have been owned by the process, but it's
far less likely to work now as obtaining a pointer to the owning
MemoryContext is less direct than before
c6e0fe1f2 and will access memory
that's possibly much further away to obtain the owning MemoryContext.
Because of this, I took the liberty of updating the comment to warn
against any future usages of the function and checked the existing core
usages to ensure that we only ever pass in a pointer to memory allocated
by a MemoryContext.
Extension authors updating their code for PG16 who are using
MemoryContextContains should check to ensure that only NULL pointers and
pointers to chunks allocated with a MemoryContext will ever be passed to
MemoryContextContains.
Reported-by: Andres Freund
Discussion: https://postgr.es/m/
20220905230949.kb3x2fkpfwtngz43@awork3.anarazel.de
Peter Eisentraut [Wed, 7 Sep 2022 09:03:53 +0000 (11:03 +0200)]
Renumber confusing value for GUC_UNIT_BYTE
It had a power-of-two value, which looks right, and causes the other
values which aren't powers-of-two to look wrong. But this is tested
for equality and not a bitwise test.
See also:
6e7baa322773ff8c79d4d8883c99fdeff5bfa679
https://www.postgresql.org/message-id/CAOG9ApEu8bXVwBxkOO9J7ZpM76TASK_vFMEEiCEjwhMmSLiaqQ%40mail.gmail.com
Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://www.postgresql.org/message-id/flat/
20220720145220.GJ12702@telsasoft.com
David Rowley [Wed, 7 Sep 2022 03:46:57 +0000 (15:46 +1200)]
Make more effort to put a sentinel at the end of allocated memory
Traditionally, in MEMORY_CONTEXT_CHECKING builds, we only ever marked a
sentinel byte just beyond the requested size if there happened to be
enough space on the chunk to do so. For Slab and Generation context
types, we only rounded the size of the chunk up to the next maxalign
boundary, so it was often not that likely that those would ever have space
for the sentinel given that the majority of allocation requests are going
to be for sizes which are maxaligned. For AllocSet, it was a little
different as smaller allocations are rounded up to the next power-of-2
value rather than the next maxalign boundary, so we're a bit more likely
to have space for the sentinel byte, especially when we get away from tiny
sized allocations such as 8 or 16 bytes.
Here we make more of an effort to allow space so that there is enough room
for the sentinel byte in more cases. This makes it more likely that we'll
detect when buggy code accidentally writes beyond the end of any of its
memory allocations.
Each of the 3 MemoryContext types has been changed as follows:
The Slab allocator will now always set a sentinel byte. Both the current
usages of this MemoryContext type happen to use chunk sizes which were on
the maxalign boundary, so these never used sentinel bytes previously.
For the Generation allocator, we now always ensure there's enough space in
the allocation for a sentinel byte.
For AllocSet, this commit makes an adjustment for allocation sizes which
are greater than allocChunkLimit. We now ensure there is always space for
a sentinel byte. We don't alter the sentinel behavior for request sizes
<= allocChunkLimit. Making way for the sentinel byte for power-of-2
request sizes would require doubling up to the next power of 2. Some
analysis done on the request sizes made during installcheck shows that a
fairly large portion of allocation requests are for power-of-2 sizes. The
amount of additional memory for the sentinel there seems prohibitive, so
we do nothing for those here.
Author: David Rowley
Discussion: https://postgr.es/m/
3478405.
1661824539@sss.pgh.pa.us
Amit Kapila [Wed, 7 Sep 2022 03:28:31 +0000 (08:58 +0530)]
Doc: Explain about Column List feature.
Add a new logical replication section for "Column Lists" (analogous to the
Row Filters page). This explains how the feature can be used and the
caveats in it.
Author: Peter Smith
Reviewed-by: Shi yu, Vignesh C, Erik Rijkers, Amit Kapila
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAHut+PvOuc9=_4TbASc5=VUqh16UWtFO3GzcKQK_5m1hrW3vqg@mail.gmail.com
Tom Lane [Tue, 6 Sep 2022 22:00:32 +0000 (18:00 -0400)]
Fix new pg_publication_tables query.
The addition of published column names forgot to filter on attisdropped,
leading to cases where you could see "........pg.dropped.1........"
or the like as a reportedly-published column.
While we're here, rewrite the new subquery to get a more efficient plan
for it.
Hou Zhijie, per report from Jaime Casanova. Back-patch to v15 where
the bug was introduced. (Sadly, this means we need a post-beta4
catversion bump before beta4 has even hit the streets. I see no
good alternative though.)
Discussion: https://postgr.es/m/Yxa1SU4nH2HfN3/i@ahch-to
John Naylor [Tue, 6 Sep 2022 05:46:41 +0000 (12:46 +0700)]
Fix cplusplusscheck in vpath builds
Same solution as
829906fb6.
Michael Paquier [Tue, 6 Sep 2022 06:36:42 +0000 (15:36 +0900)]
Add psql tab compression for SET COMPRESSION with ALTER TABLE
Author: Aleksander Alekseev
Reviewed-by: Shinya Kato
Discussion: https://postgr.es/m/CAJ7c6TMuT+=P7uDepjVpdqSEp2xOmXET3Y2K-xWAO=sCz-28gg@mail.gmail.com
John Naylor [Tue, 6 Sep 2022 05:46:41 +0000 (12:46 +0700)]
Fix headerscheck in vpath builds
Oversight in
dac048f71e per buildfarm animal crake.
Fix per suggestion from Andrew Dunstan.
Discussion: https://www.postgresql.org/message-id/
e3f4a3d0-dfcc-41cc-1ed2-
acc15700ddef%40dunslane.net
John Naylor [Tue, 6 Sep 2022 05:30:56 +0000 (12:30 +0700)]
Fix failure to maintainer-clean jsonpath_gram.h
Oversight in
dac048f71e
David Rowley [Tue, 6 Sep 2022 03:59:15 +0000 (15:59 +1200)]
Fix typo in
16d69ec29
As noted by Justin Pryzby, just I forgot to commit locally before creating
a patch file.
Discussion: https://postgr.es/m/
20220901053146.GI31833@telsasoft.com
David Rowley [Tue, 6 Sep 2022 03:51:44 +0000 (15:51 +1200)]
Remove buggy and dead code from CreateTriggerFiringOn
Here we remove some dead code from CreateTriggerFiringOn() which was
attempting to find the relevant child partition index corresponding to the
given indexOid. As it turned out, thanks to -Wshadow=compatible-local,
this code was buggy as the code which was finding the child indexes
assigned those to a shadowed variable that directly went out of scope.
The code which thought it was looking at the List of child indexes was
always referencing an empty List.
On further investigation, this code is dead. We never call
CreateTriggerFiringOn() passing a valid indexOid in a way that the
function would actually ever execute the code in question. So, for lack
of a way to test if a fix actually works, let's just remove the dead code
instead.
As a reminder, if there is ever a need to resurrect this code, an Assert()
has been added to remind future feature developers that they might need to
write some code to find the corresponding child index.
Reported-by: Justin Pryzby
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/
20220819211824.GX26426@telsasoft.com
John Naylor [Tue, 6 Sep 2022 03:27:05 +0000 (10:27 +0700)]
Add missing exceptions to cpluspluscheck
dac048f71 added exceptions to headerscheck but failed to do the
same for cpluspluscheck
Per report from Andres Freund regarding CI
Discussion:https://www.postgresql.org/message-id/
20220904205743.y3ntq6ij3aibmxvy%40awork3.anarazel.de
David Rowley [Tue, 6 Sep 2022 01:19:44 +0000 (13:19 +1200)]
Fix an assortment of improper usages of string functions
In a similar effort to
f736e188c and
110d81728, fixup various usages of
string functions where a more appropriate function is available and more
fit for purpose.
These changes include:
1. Use cstring_to_text_with_len() instead of cstring_to_text() when
working with a StringInfoData and the length can easily be obtained.
2. Use appendStringInfoString() instead of appendStringInfo() when no
formatting is required.
3. Use pstrdup(...) instead of psprintf("%s", ...)
4. Use pstrdup(...) instead of psprintf(...) (with no formatting)
5. Use appendPQExpBufferChar() instead of appendPQExpBufferStr() when the
length of the string being appended is 1.
6. appendStringInfoChar() instead of appendStringInfo() when no formatting
is required and string is 1 char long.
7. Use appendPQExpBufferStr(b, .) instead of appendPQExpBuffer(b, "%s", .)
8. Don't use pstrdup when it's fine to just point to the string constant.
I (David) did find other cases of #8 but opted to use #4 instead as I
wasn't certain enough that applying #8 was ok (e.g in hba.c)
Author: Ranier Vilela, David Rowley
Discussion: https://postgr.es/m/CAApHDvo2j2+RJBGhNtUz6BxabWWh2Jx16wMUMWKUjv70Ver1vg@mail.gmail.com
Peter Eisentraut [Sun, 28 Aug 2022 08:47:10 +0000 (10:47 +0200)]
Fix incorrect uses of Datum conversion macros
Since these macros just cast whatever you give them to the designated
output type, and many normal uses also cast the output type further, a
number of incorrect uses go undiscovered. The fixes in this patch
have been discovered by changing these macros to inline functions,
which is the subject of a future patch.
Reviewed-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/flat/
8528fb7e-0aa2-6b54-85fb-
0c0886dbd6ed%40enterprisedb.com
Daniel Gustafsson [Mon, 5 Sep 2022 09:11:18 +0000 (11:11 +0200)]
Update out of date comments in pg_trgm
Commit
be8a7a68662 changed the check_only parameter to a flag array
but missed updating all comments. Update, and fix a related typo.
Discussion: https://postgr.es/m/
9732D8A2-EABD-4F5B-8BA0-
A97DA4AB51A7@yesql.se
Daniel Gustafsson [Mon, 5 Sep 2022 09:10:57 +0000 (11:10 +0200)]
Check for interrupts in pg_trgm word similarity
Calculating similarity between large strings can be timesconsuming
and overrun configured statement timeouts. Check for interrupts in
the main loop to ensure query cancellation can be performed.
Author: Robins Tharakan <tharakan@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAEP4nAxvmfc_XWTz73bqXRhgjONi=1HaX4_NhsopA3L6UvnN1g@mail.gmail.com
David Rowley [Mon, 5 Sep 2022 06:43:03 +0000 (18:43 +1200)]
Doc: clarify partitioned table limitations
Improve documentation regarding the limitations of unique and primary key
constraints on partitioned tables. The existing documentation didn't make
it clear that the constraint columns had to be present in the partition
key as bare columns. The reader could be led to believe that it was ok to
include the constraint columns as part of a function call's parameters or
as part of an expression. Additionally, the documentation didn't mention
anything about the fact that we disallow unique and primary key
constraints if the partition keys contain *any* function calls or
expressions, regardless of if the constraint columns appear as columns
elsewhere in the partition key.
The confusion here was highlighted by a report on the general mailing list
by James Vanns.
Discussion: https://postgr.es/m/CAH7vdhNF0EdYZz3GLpgE3RSJLwWLhEk7A_fiKS9dPBT3Dz_3eA@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvoU-u9iTqKjteYRFfi+UNEk7dbSAcyxEQD==vZt9B1KnA@mail.gmail.com
Reviewed-by: Erik Rijkers
Backpatch-through: 11
Tomas Vondra [Sun, 4 Sep 2022 22:09:17 +0000 (00:09 +0200)]
Force parallelism in partition_aggregate
Commit
db0d67db2 tweaked sort costing, which however resulted in a
couple plan changes in our regression tests. Most of the new plans were
fine, but partition_aggregate were meant to test parallel plans and the
new plans were serial.
Fix that by lowering parallel_setup_cost to 0, which is enough to switch
to the parallel plan again.
Commit
1349d2790 already made the plans parallel again, but do this
anyway to keep the tests in sync with 15, to make backpatching simpler.
Report and patch by David Rowley.
Author: David Rowley
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAApHDvpVFgWzXdtUQkjyOPhNrNvumRi_=ftgS79KeAZ92tnHKQ@mail.gmail.com
John Naylor [Sun, 4 Sep 2022 09:49:00 +0000 (16:49 +0700)]
Fix MSVC linker error for specparse.obj
Per buildfarm animals drongo
John Naylor [Sun, 4 Sep 2022 04:33:31 +0000 (11:33 +0700)]
Build all Flex files standalone
The proposed Meson build system will need a way to ignore certain
generated files in order to coexist with the autoconf build system,
and C files generated by Flex which are #include'd into .y files make
this more difficult. In similar vein to
72b1e3a21, arrange for all Flex
C files to compile to their own .o targets.
Reviewed by Andres Freund
Discussion: https://www.postgresql.org/message-id/
20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de
Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
John Naylor [Tue, 16 Aug 2022 05:01:41 +0000 (12:01 +0700)]
Move private declarations shared between guc.c and guc-file.l to new header
Further preparatory refactoring for compiling guc-file.c standalone.
Reviewed by Andres Freund
Discussion: https://www.postgresql.org/message-id/
20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de
Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
John Naylor [Tue, 16 Aug 2022 03:42:19 +0000 (10:42 +0700)]
Preparatory refactoring for compiling guc-file.c standalone
Mostly this involves moving ProcessConfigFileInternal() to guc.c
and fixing the shared API to match.
Reviewed by Andres Freund
Discussion: https://www.postgresql.org/message-id/
20220810171935.7k5zgnjwqzalzmtm%40awork3.anarazel.de
Discussion: https://www.postgresql.org/message-id/CAFBsxsF8Gc2StS3haXofshHCzqNMRXiSxvQEYGwnFsTmsdwNeg@mail.gmail.com
John Naylor [Sun, 4 Sep 2022 02:23:57 +0000 (09:23 +0700)]
Fix sign-compare warnings arising from port/simd.h
Noted while building an extension using -Wsign-compare.
Per gripe from Pavel Stehule
Discussion: https://www.postgresql.org/message-id/CAFj8pRAagKQHfw71aQbL8PbL0S_360M61V0_vPqJXbpUFvqnRA%40mail.gmail.com
Michael Paquier [Sat, 3 Sep 2022 11:57:16 +0000 (20:57 +0900)]
doc: Fix two queries related to jsonb functions
These have been updated by the revert done in
2f2b18b, but the
pre-revert state was correct. Note that the result was incorrectly
formatted in the first case.
Author: Erik Rijkers
Discussion: https://postgr.es/m/
13777e96-24b6-396b-cb16-
8ad01b6ac130@xs4all.nl
Backpatch-through: 13
Bruce Momjian [Sat, 3 Sep 2022 03:32:19 +0000 (23:32 -0400)]
doc: simplify docs about analyze and inheritance/partitions
Discussion: https://postgr.es/m/YxAqYijOsLzgLQgy@momjian.us
Backpatch-through: 10
Bruce Momjian [Sat, 3 Sep 2022 01:57:41 +0000 (21:57 -0400)]
doc: clarify recursion internal behavior
Reported-by: Drew DeVault
Discussion: https://postgr.es/m/
20211018091720.31299-1-sir@cmpwn.com
Backpatch-through: 10
Thomas Munro [Sat, 3 Sep 2022 00:58:16 +0000 (12:58 +1200)]
Fix cache invalidation bug in recovery_prefetch.
XLogPageRead() can retry internally after a pread() system call has
succeeded, in the case of short reads, and page validation failures
while in standby mode (see commit
0668719801). Due to an oversight in
commit
3f1ce973, these cases could leave stale data in the internal
cache of xlogreader.c without marking it invalid. The main defense
against stale cached data on failure to read a page was in the error
handling path of the calling function ReadPageInternal(), but that
wasn't quite enough for errors handled internally by XLogPageRead()'s
retry loop if we then exited with XLREAD_WOULDBLOCK.
1. ReadPageInternal() now marks the cache invalid before calling the
page_read callback, by setting state->readLen to 0. It'll be set to
a non-zero value only after a successful read. It'll stay valid as
long as the caller requests data in the cached range.
2. XLogPageRead() no long performs internal retries while reading
ahead. While such retries should work, the general philosophy is
that we should give up prefetching if anything unusual happens so we
can handle it when recovery catches up, to reduce the complexity of
the system. Let's do that here too.
3. While here, a new function XLogReaderResetError() improves the
separation between xlogrecovery.c and xlogreader.c, where the former
previously clobbered the latter's internal error buffer directly.
The new function makes this more explicit, and also clears a related
flag, without which a standby would needlessly retry in the outer
function.
Thanks to Noah Misch for tracking down the conditions required for a
rare build farm failure in src/bin/pg_ctl/t/003_promote.pl, and
providing a reproducer.
Back-patch to 15.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/
20220807003627.GA4168930%40rfd.leadboat.com
Tom Lane [Fri, 2 Sep 2022 21:01:51 +0000 (17:01 -0400)]
Fix planner to consider matches to boolean columns in extension indexes.
The planner has to special-case indexes on boolean columns, because
what we need for an indexscan on such a column is a qual of the shape
of "boolvar = pseudoconstant". For plain bool constants, previous
simplification will have reduced this to "boolvar" or "NOT boolvar",
and we have to reverse that if we want to make an indexqual. There is
existing code to do so, but it only fires when the index's opfamily
is BOOL_BTREE_FAM_OID or BOOL_HASH_FAM_OID. Thus extension AMs, or
extension opclasses such as contrib/btree_gin, are out in the cold.
The reason for hard-wiring the set of relevant opfamilies was mostly
to avoid a catalog lookup in a hot code path. We can improve matters
while not taking much of a performance hit by relying on the
hard-wired set when the opfamily OID is visibly built-in, and only
checking the catalogs when dealing with an extension opfamily.
While here, rename IsBooleanOpfamily to IsBuiltinBooleanOpfamily
to remind future users of that macro of its limitations. At some
point we might want to make indxpath.c's improved version of the
test globally accessible, but it's not presently needed elsewhere.
Zongliang Quan and Tom Lane
Discussion: https://postgr.es/m/
f293b91d-1d46-d386-b6bb-
4b06ff5c667b@yeah.net
Daniel Gustafsson [Fri, 2 Sep 2022 18:38:34 +0000 (20:38 +0200)]
Remove unused code from sepgsql
Commit
4232c4b40 removed all callers of sepgsql_check_perms but left
the function in place. This removes the function as well.
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/
3BD5C3BF-FECA-4496-AE53-
5E447997AA0B@yesql.se
Peter Eisentraut [Fri, 2 Sep 2022 15:56:14 +0000 (17:56 +0200)]
Fix PL/Perl build on Cygwin
This was broken by
b4e936859dc441102eb0b6fb7a104f3948c90490. The
reason why this fixes it are not entirely clear, but it seemed the
best way to get it working again.
Discussion: https://www.postgresql.org/message-id/flat/
8c4fcb72-2574-ff7c-4c25-
1f032d4a2a57%40enterprisedb.com
Amit Kapila [Fri, 2 Sep 2022 11:14:52 +0000 (16:44 +0530)]
Doc: fix column list vs. replica identity rules.
It was not strictly correct to say that a column list must always include
replica identity columns because that is true for only updates and
deletes.
Author: Peter Smith
Reviwed-by: Vignesh C, Amit Kapila
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAHut+PvOuc9=_4TbASc5=VUqh16UWtFO3GzcKQK_5m1hrW3vqg@mail.gmail.com
Michael Paquier [Fri, 2 Sep 2022 07:58:06 +0000 (16:58 +0900)]
Expand the use of get_dirent_type(), shaving a few calls to stat()/lstat()
Several backend-side loops scanning one or more directories with
ReadDir() (WAL segment recycle/removal in xlog.c, backend-side directory
copy, temporary file removal, configuration file parsing, some logical
decoding logic and some pgtz stuff) already know the type of the entry
being scanned thanks to the dirent structure associated to the entry, on
platforms where we know about DT_REG, DT_DIR and DT_LNK to make the
difference between a regular file, a directory and a symbolic link.
Relying on the direct structure of an entry saves a few system calls to
stat() and lstat() in the loops updated here, shaving some code while on
it. The logic of the code remains the same, calling stat() or lstat()
depending on if it is necessary to look through symlinks.
Authors: Nathan Bossart, Bharath Rupireddy
Reviewed-by: Andres Freund, Thomas Munro, Michael Paquier
Discussion: https://postgr.es/m/CALj2ACV8n-J-f=yiLUOx2=HrQGPSOZM3nWzyQQvLPcccPXxEdg@mail.gmail.com
Etsuro Fujita [Fri, 2 Sep 2022 07:45:00 +0000 (16:45 +0900)]
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
John Naylor [Wed, 31 Aug 2022 03:39:17 +0000 (10:39 +0700)]
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
Andres Freund [Thu, 1 Sep 2022 23:54:19 +0000 (16:54 -0700)]
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
Andrew Dunstan [Thu, 1 Sep 2022 21:07:14 +0000 (17:07 -0400)]
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
Tom Lane [Thu, 1 Sep 2022 19:02:34 +0000 (15:02 -0400)]
Add a regression test for contrib/pgrowlocks.
Dong Wook Lee, revised a bit by me
Discussion: https://postgr.es/m/
20220629055326.tdswmcjcr5jzbrsk@home-desktop
Andres Freund [Thu, 1 Sep 2022 18:49:36 +0000 (11:49 -0700)]
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
Tom Lane [Thu, 1 Sep 2022 18:30:41 +0000 (14:30 -0400)]
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.
Bruce Momjian [Thu, 1 Sep 2022 03:11:46 +0000 (23:11 -0400)]
doc: in create statistics docs, mention analyze for parent info
Discussion: https://postgr.es/m/Yv1Bw8J+1pYfHiRl@momjian.us
Backpatch-through: 10
Bruce Momjian [Thu, 1 Sep 2022 02:35:09 +0000 (22:35 -0400)]
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
Tom Lane [Thu, 1 Sep 2022 02:21:32 +0000 (22:21 -0400)]
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
Bruce Momjian [Thu, 1 Sep 2022 02:19:06 +0000 (22:19 -0400)]
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
Bruce Momjian [Thu, 1 Sep 2022 02:04:36 +0000 (22:04 -0400)]
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
Bruce Momjian [Thu, 1 Sep 2022 01:46:14 +0000 (21:46 -0400)]
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
Bruce Momjian [Thu, 1 Sep 2022 01:10:37 +0000 (21:10 -0400)]
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
Bruce Momjian [Thu, 1 Sep 2022 00:27:27 +0000 (20:27 -0400)]
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
Bruce Momjian [Wed, 31 Aug 2022 23:43:06 +0000 (19:43 -0400)]
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
Bruce Momjian [Wed, 31 Aug 2022 23:28:42 +0000 (19:28 -0400)]
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
David Rowley [Wed, 31 Aug 2022 23:08:10 +0000 (11:08 +1200)]
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
Bruce Momjian [Wed, 31 Aug 2022 21:08:44 +0000 (17:08 -0400)]
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
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.camel@crunchydata.com
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 <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
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 <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/
2180a97c-c026-1b6c-cec8-
d6e499f97017@enterprisedb.com
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
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
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
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 <nathandbossart@gmail.com>
Reviewed-by: Bruce Momjian <bruce@momjian.us>
Discussion: https://postgr.es/m/
595759F6-625B-4ED7-8125-
91AF00437F83@yesql.se
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.
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
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
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
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
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.GA5439@depesz.com
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
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.
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
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.GD2342@telsasoft.com
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
Robert Haas [Mon, 29 Aug 2022 16:35:46 +0000 (12:35 -0400)]
Remove stray "the".
Per off-list report.
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
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
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
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
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
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
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
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
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 jason@banfelder.net
Simon Riggs
Discussion: https://postgr.es/m/
166161184718.
1235920.
6304070286124217754@wrigleys.postgresql.org
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
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.gjw3jdqdgsts5y65@awork3.anarazel.de
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.GL2342@telsasoft.com
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
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
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
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
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 <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/flat/
cf26e970-8e92-59f1-247a-
aa265235075b%40enterprisedb.com