1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-03 20:02:46 +03:00
Commit Graph

23074 Commits

Author SHA1 Message Date
23bcc8d877 Message style improvements 2022-09-17 08:10:59 +02:00
c946425176 Message wording improvements 2022-09-16 16:37:53 +02:00
148f66d59e Don't allow creation of database with ICU locale with unsupported encoding
Check in CREATE DATABASE and initdb that the selected encoding is
supported by ICU.  Before, they would pass but users would later get
an error from the server when they tried to use the database.

Also document that initdb sets the encoding to UTF8 by default if the
ICU locale provider is chosen.

Author: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://www.postgresql.org/message-id/6dd6db0984d86a51b7255ba79f111971@postgrespro.ru
2022-09-16 09:41:25 +02:00
e36cbef04b Reset InstallXLogFileSegmentActive after walreceiver self-initiated exit.
After commit cc2c7d65fc added this flag,
failure to reset it caused assertion failures.  In non-assert builds, it
made the system fail to achieve the objectives listed in that commit;
chiefly, we might emit a spurious log message.  Back-patch to v15, where
that commit first appeared.

Bharath Rupireddy and Kyotaro Horiguchi.  Reviewed by Dilip Kumar,
Nathan Bossart and Michael Paquier.  Reported by Dilip Kumar.

Discussion: https://postgr.es/m/CAFiTN-sE3ry=ycMPVtC+Djw4Fd7gbUGVv_qqw6qfzp=JLvqT3g@mail.gmail.com
2022-09-15 06:45:28 -07:00
eed548822c Fix grammar in error message
While at it, make ellipses formatting consistent when describing SQL statements.

Ekaterina Kiryanova and Alexander Lakhin

Reviewed by myself and Álvaro Herrera
Discussion: https://www.postgresql.org/message-id/eed5cec0-a542-53da-6a5e-7789c6ed9817%40postgrespro.ru
Backpatch only the grammar fix to v15
2022-09-15 11:43:00 +07:00
d068b4108c Fix outdated convert_saop_to_hashed_saop comment
In 29f45e299, we added support for optimizing the execution of NOT
IN(values) by using a hash table instead of a linear search over the
array.  That commit neglected to update the header comment for
convert_saop_to_hashed_saop() to mention this fact.  Here we fix that.

Author: James Coleman
Discussion: https://postgr.es/m/CAAaqYe99NUpAPcxgchGstgM23fmiGjqQPot8627YgkBgNt=BfA@mail.gmail.com
Backpatch-through: 15, where 29f45e299 was added.
2022-09-15 09:41:32 +12:00
12a5214df4 Small wording improvements 2022-09-14 22:32:12 +02:00
ef81b7f869 Remove duplicate initialization
This appears to be a merge mistake in 96ef3237bf.  We could put it
back the way it was before JSON_TABLE and it'd be two lines shorter, but
it's likely that JSON_TABLE will be back and will prefer things this
way.  It makes no other difference in practice.

Backpatch to 15.

Reported by Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAr4nOcNQskC4oBEZN4S+4heJ=1ch_ZKOxU+_Ef-FQSf-g@mail.gmail.com
2022-09-14 15:36:21 +02:00
53332eacaf Simplify handling of compression level with compression specifications
PG_COMPRESSION_OPTION_LEVEL is removed from the compression
specification logic, and instead the compression level is always
assigned with each library's default if nothing is directly given.  This
centralizes the checks on the compression methods supported by a given
build, and always assigns a default compression level when parsing a
compression specification.  This results in complaining at an earlier
stage than previously if a build supports a compression method or not,
aka when parsing a specification in the backend or the frontend, and not
when processing it.  zstd, lz4 and zlib are able to handle in their
respective routines setting up the compression level the case of a
default value, hence the backend or frontend code (pg_receivewal or
pg_basebackup) has now no need to know what the default compression
level should be if nothing is specified: the logic is now done so as the
specification parsing assigns it.  It can also be enforced by passing
down a "level" set to the default value, that the backend will accept
(the replication protocol is for example able to handle a command like
BASE_BACKUP (COMPRESSION_DETAIL 'gzip:level=-1')).

This code simplification fixes an issue with pg_basebackup --gzip
introduced by ffd5365, where the tarball of the streamed WAL segments
would be created as of pg_wal.tar.gz with uncompressed contents, while
the intention is to compress the segments with gzip at a default level.
The origin of the confusion comes from the handling of the default
compression level of gzip (-1 or Z_DEFAULT_COMPRESSION) and the value of
0 was getting assigned, which is what walmethods.c would consider
as equivalent to no compression when streaming WAL segments with its tar
methods.  Assigning always the compression level removes the confusion
of some code paths considering a value of 0 set in a specification as
either no compression or a default compression level.

Note that 010_pg_basebackup.pl has to be adjusted to skip a few tests
where the shape of the compression detail string for client and
server-side compression was checked using gzip.  This is a result of the
code simplification, as gzip specifications cannot be used if a build
does not support it.

Reported-by: Tom Lane
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/1400032.1662217889@sss.pgh.pa.us
Backpatch-through: 15
2022-09-14 12:17:03 +09:00
892cac9124 Improve wal_decode_buffer_size description some more
Per Thomas Munro

Discussion: https://postgr.es/m/CA+hUKGJ9wP9kpvgoxHvqA=4g1d9-y_w3LhhdhFVU=mFiqjwHww@mail.gmail.com
2022-09-13 12:02:56 +02:00
4d5d35858c Don't reference out-of-bounds array elements in brin_minmax_multi.c
The primary fix here is to fix has_matching_range() so it does not
reference ranges->values[-1] when nranges == 0.  Similar problems existed
in AssertCheckRanges() too.  It does not look like any of these problems
could lead to a crash as the array in question is at the end of the Ranges
struct, and values[-1] is memory that belongs to other fields in the
struct.  However, let's get rid of these rather unsafe coding practices.

In passing, I (David) adjusted some comments to try to make it more clear
what some of the fields are for in the Ranges struct.  I had to study the
code to find out what nsorted was for as I couldn't tell from the
comments.

Author: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAqJQzPitufX-jR=YUbJafpCDAKUnwgdbX_MzSc93wuvdw@mail.gmail.com
Backpatch-through: 14, where multi-range brin was added.
2022-09-13 11:04:37 +12:00
56d238846f Fix NaN comparison in circle_same test
Commit c4c340088 changed geometric operators to use float4 and float8
functions, and handle NaN's in a better way. The circle sameness test
had a typo in the code which resulted in all comparisons with the left
circle having a NaN radius considered same.

  postgres=# select '<(0,0),NaN>'::circle ~= '<(0,0),1>'::circle;
  ?column?
  ----------
  t
  (1 row)

This fixes the sameness test to consider the radius of both the left
and right circle.

Backpatch to v12 where this was introduced.

Author: Ranier Vilela <ranier.vf@gmail.com>
Discussion: https://postgr.es/m/CAEudQAo8dK=yctg2ZzjJuzV4zgOPBxRU5+Kb+yatFiddtQk6Rw@mail.gmail.com
Backpatch-through: v12
2022-09-12 12:59:06 +02:00
68b0da6779 Fix GetForeignKey*Triggers for self-referential FKs
Because of inadequate filtering, the check triggers were confusing the
search for action triggers in GetForeignKeyActionTriggers and vice-versa
in GetForeignKeyCheckTriggers; this confusion results in seemingly
random assertion failures, and can have real impact in non-asserting
builds depending on catalog order.  Change these functions so that they
correctly ignore triggers that are not relevant to each side.

To reduce the odds of further problems, do not break out of the
searching loop in assertion builds.  This break is likely to hide bugs;
without it, we would have detected this bug immediately.

This problem was introduced by f4566345cf, so backpatch to 15 where
that commit first appeared.

Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/20220908172029.sejft2ppckbo6oh5@awork3.anarazel.de
Discussion: https://postgr.es/m/4104619.1662663056@sss.pgh.pa.us
2022-09-09 12:22:20 +02:00
0a7c9ee500 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
2022-09-09 10:01:14 +09:00
ade2409b18 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
2022-09-08 13:17:02 +02:00
dd38ff28ad 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
2022-09-08 20:36:44 +12:00
144cefac92 Message style fixes 2022-09-07 17:33:49 +02:00
e25631265a 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
2022-09-06 18:00:32 -04:00
77ce482e9e Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: f2c273bb8723eb69911a4b0b9d02ca15bbc7c60f
2022-09-05 14:37:22 +02:00
d0d9344900 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
2022-09-03 13:29:36 +12:00
96ef3237bf 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:10:42 -04:00
c4e861b7bb Fix some possibly latent bugs in slab.c
Primarily, this fixes an incorrect calculation in SlabCheck which was
looking in the wrong byte for the sentinel check.  The reason that we've
never noticed this before in the form of a failing sentinel check is
because the pre-check to this always fails because all current core users
of slab contexts have a chunk size which is already MAXALIGNed, therefore
there's never any space for the sentinel byte.  It is possible that an
extension needs to use a slab context and if they do with a chunk size
that's not MAXALIGNed, then they'll likely get errors about overwritten
sentinel bytes.

Additionally, this patch changes various calculations which are being done
based on the sizeof(SlabBlock).  Currently, sizeof(SlabBlock) is a
multiple of 8, therefore sizeof(SlabBlock) is the same as
MAXALIGN(sizeof(SlabBlock)), however, if we were to ever have to add any
fields to that struct as part of a bug fix, then SlabAlloc could end up
returning a non-MAXALIGNed pointer.  To be safe, let's ensure we always
MAXALIGN sizeof(SlabBlock) before using it in any calculations.

This patch has already been applied to master in d5ee4db0e.

Diagnosed-by: Tomas Vondra, Tom Lane
Author: Tomas Vondra, David Rowley
Discussion: https://postgr.es/m/CAA4eK1%2B1JyW5TiL%3DyV-3Uq1CrfnTyn0Xrk5uArt31Z%3D8rgPhXQ%40mail.gmail.com
Backpatch-through: 10
2022-09-01 19:21:58 +12:00
0c599e748f 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:20 -04:00
61f6e5c236 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
eb29fa3889 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:18:55 -04:00
73cbdb7f19 Switch format specifier for replication origins to %d
Using %u with uint16 causes warnings with -Wformat-signedness. There are many
other warnings, but for now change only these since c920fe4818 already changed
the message string for most of them.

Per report from Peter Eisentraut
Discussion: https://www.postgresql.org/message-id/31e63649-0355-7088-831e-b07d5f908a8c%40enterprisedb.com
2022-08-28 16:04:35 +02:00
d4e9bca403 Remove obsolete comment
The comment in basebackup.c updated by 33bd4698c1 was actually
obsolete to begin with, since the symbols it was referring to haven't
existed in that header file for quite some time.  The header file is
still needed for other reasons, though, so keep the #include, just
drop the comment.
2022-08-26 10:45:01 +02:00
2829cfaf78 Fix typo in comment. 2022-08-26 16:55:01 +09:00
36a8881c4b Fix code comments still referring to pg_start/stop_backup()
pg_start_backup() and pg_stop_backup() have been respectively renamed to
pg_backup_start() and pg_backup_stop() as of 39969e2, but a few comments
did not get the call.

Reviewed-by: Kyotaro Horiguchi, David Steele
Discussion: https://postgr.es/m/YrqGlj1+4DF3dbZ/@paquier.xyz
2022-08-24 22:10:29 +02:00
3d3c05c70f Defend against stack overrun in a few more places.
SplitToVariants() in the ispell code, lseg_inside_poly() in geo_ops.c,
and regex_selectivity_sub() in selectivity estimation could recurse
until stack overflow; fix by adding check_stack_depth() calls.
So could next() in the regex compiler, but that case is better fixed by
converting its tail recursion to a loop.  (We probably get better code
that way too, since next() can now be inlined into its sole caller.)

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

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

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/1661334672.728714027@f473.i.mail.ru
2022-08-24 13:02:19 -04:00
f02147bd23 Fix ICU locale option handling in CREATE DATABASE
The code took the LOCALE option as the default/fallback for
ICU_LOCALE, but this was neither documented nor intended, so remove
it.  (It was probably left in from an earlier patch version.)

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e
2022-08-24 13:34:53 +02:00
d53ff6a44b Doc: prefer sysctl to /proc/sys in docs and comments.
sysctl is more portable than Linux's /proc/sys file tree, and
often easier to use too.  That's why most of our docs refer to
sysctl when talking about how to adjust kernel parameters.
Bring the few stragglers into line.

Discussion: https://postgr.es/m/361175.1661187463@sss.pgh.pa.us
2022-08-23 09:41:53 -04:00
42681dffaf Add CHECK_FOR_INTERRUPTS while decoding changes.
While decoding changes in a loop, if we skip all the changes there is no
CFI making the loop uninterruptible.

Reported-by: Whale Song and Andrey Borodin
Bug: 17580
Author: Masahiko Sawada
Reviwed-by: Amit Kapila
Backpatch-through: 10
Discussion: https://postgr.es/m/17580-849c1d5b6d7eb422@postgresql.org
Discussion: https://postgr.es/m/B319ECD6-9A28-4CDF-A8F4-3591E0BF2369@yandex-team.ru
2022-08-23 10:08:43 +05:30
045ec34785 pgstat: Acquire lock when reading variable-numbered stats
Somewhere during the development of the patch acquiring a lock during read
access to variable-numbered stats got lost. The missing lock acquisition won't
cause corruption, but can lead to reading torn values when accessing
stats. Add the missing lock acquisitions.

Reported-by: Greg Stark <stark@mit.edu>
Reviewed-by: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Author: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CAM-w4HMYkM_DkYhWtUGV+qE_rrBxKOzOF0+5faozxO3vXrc9wA@mail.gmail.com
Backpatch: 15-
2022-08-22 20:17:45 -07:00
a2caf182a2 Fix assertion failure in CREATE DATABASE
An assertion would fail when creating a database with libc locale
provider from a template database with icu locale provider.

Reported-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/f385ba25e7f8be427b8c582e5cca7d79%40postgrespro.ru#515a31c5429d6d37ad1d5c9d66962a1e
2022-08-22 15:31:50 +02:00
254d79f0b2 Use logical operator && instead of & in vacuumparallel.c.
As such the current usage of & won't produce incorrect results but it
would be better to use && to short-circuit the evaluation of second
condition when the same is not required.

Author: Ranier Vilela
Reviewed-by: Tom Lane, Bharath Rupireddy
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAEudQApL8QcoYwQuutkWKY_h7gBY8F0Xs34YKfc7-G0i83K_pw@mail.gmail.com
2022-08-22 08:51:25 +05:30
24f457aa2b Remove shadowed local variables that are new in v15
Compiling with -Wshadow=compatible-local yields quite a few warnings about
local variables being shadowed by compatible local variables in an inner
scope.  Of course, this is perfectly valid in C, but we have had bugs in
the past as a result of developers failing to notice this.  af7d270dd is a
recent example.

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

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

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220817145434.GC26426%40telsasoft.com
Backpatch-through: 15
2022-08-20 11:40:59 +12:00
4496020e6d Avoid reltuples distortion in very small tables.
Consistently avoid trusting a sample of only one page at the point that
VACUUM determines a new reltuples for the target table (though only when
the table is larger than a single page).  This is follow-up work to
commit 74388a1a, which added a heuristic to prevent reltuples from
becoming distorted by successive VACUUM operations that each scan only a
single heap page (which was itself more or less a bugfix for an issue in
commit 44fa8488, which simplified VACUUM's handling of scanned pages).

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

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzk7d4m3oEbEWkWQKd+gz-eD_peBvdXVk1a_KBygXadFeg@mail.gmail.com
Backpatch: 15-, where the rules for scanned pages changed.
2022-08-19 09:26:06 -07:00
08c23f4204 Initialize index stats during parallel VACUUM.
Initialize shared memory allocated for index stats to avoid a hard
crash.  This was possible when parallel VACUUM became confused about the
current phase of index processing.

Oversight in commit 8e1fae1938, which refactored parallel VACUUM.

Author: Masahiko Sawada <sawada.mshk@gmail.com>
Reported-By: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220818133406.GL26426@telsasoft.com
Backpatch: 15-, the first version with the refactoring commit.
2022-08-18 17:34:12 -07:00
2a42c1c51d Fix subtly-incorrect matching of parent and child partitioned indexes.
When creating a partitioned index, DefineIndex tries to identify
any existing indexes on the partitions that match the partitioned
index, so that it can absorb those as child indexes instead of
building new ones.  Part of the matching is to compare IndexInfo
structs --- but that wasn't done quite right.  We're comparing
the IndexInfo built within DefineIndex itself to one made from
existing catalog contents by BuildIndexInfo.  Notably, while
BuildIndexInfo will run index expressions and predicates through
expression preprocessing, that has not happened to DefineIndex's
struct.  The result is failure to match and subsequent creation
of duplicate indexes.

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

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

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

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/8864BFAA-81FD-4BF9-8E06-7DEB8D4164ED@thebuild.com
2022-08-18 12:12:03 -04:00
576bb0fc93 When using the WAL-logged CREATE DATABASE strategy, bulk extend.
This should improve performance, and was suggested by Andres Freund.
Back-patch to v15 to keep the code consistent across branches.

Dilip Kumar

Discussion: http://postgr.es/m/C3458199-FEDD-4356-865A-08DFAA5D4065@anarazel.de
Discussion: http://postgr.es/m/CAFiTN-sJ0vVpJrZ=R5M+g7Tr8=NN4wKOtrqOcDEsfFfnZgivVA@mail.gmail.com
2022-08-18 11:22:48 -04:00
9c4c22fdfe Simplify and clarify an error message 2022-08-18 11:33:53 +02:00
7cadaac513 Refer to replication origin roident as "ID" in user facing messages and docs
The table column that stores this is of type oid, but is actually limited
to uint16 and has a different path for creating new values. Some of
the documentation already referred to it as an ID, so let's standardize
on that.

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

Per suggestions from Tom Lane and Justin Pryzby
Discussion: https://www.postgresql.org/message-id/3437166.1659620465%40sss.pgh.pa.us
Backpatch to v15
2022-08-18 09:10:39 +07:00
be6aadf07d Fix assert in logicalmsg_desc
The assert, introduced by 9f1cf97bb5, is intended to check if the prefix
is terminated by a \0 byte, but it has two flaws. Firstly, prefix_size
includes the \0 byte, so prefix[prefix_size] points to the byte after
the null byte. Secondly, the check ensures the byte is not equal \0,
while it should be checking the opposite.

Backpatch-through: 14
Discussion: https://postgr.es/m/b99b6101-2f14-3796-3dfa-4a6cd7d4326d@enterprisedb.com
2022-08-17 00:13:42 +02:00
76d2579259 Fix replica identity check for a partitioned table.
The current publisher code checks if UPDATE or DELETE can be executed with
the replica identity of the table even if it's a partitioned table. We can
skip checking the replica identity for partitioned tables because the
operations are actually performed on the leaf partitions (not the
partitioned table).

Reported-by: Brad Nicholson
Author: Hou Zhijie
Reviewed-by: Peter Smith, Amit Kapila
Backpatch-through: 13
Discussion: https://postgr.es/m/CAMMnM%3D8i5DohH%3DYKzV0_wYuYSYvuOJoL9F5nzXTc%2ByzsG1f6rg%40mail.gmail.com
2022-08-16 15:14:27 +05:30
a5d2337005 Fix outdated --help message for postgres -f
This option switch supports a total of 8 values, as told by
set_plan_disabling_options() and the documentation, but this was not
reflected in the output generated by --help.

Author: Junwang Zhao
Discussion: https://postgr.es/m/CAEG8a3+pT3cWzyjzKs184L1XMNm8NDnoJLiSjAYSO7XqpRh_vA@mail.gmail.com
Backpatch-through: 10
2022-08-15 13:37:32 +09:00
9ae484121f Preserve memory context of VarStringSortSupport buffers.
When enlarging the work buffers of a VarStringSortSupport object,
varstrfastcmp_locale was careful to keep them in the ssup_cxt
memory context; but varstr_abbrev_convert just used palloc().
The latter creates a hazard that the buffers could be freed out
from under the VarStringSortSupport object, resulting in stomping
on whatever gets allocated in that memory later.

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

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

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

Discussion: https://postgr.es/m/17584-95c79b4a7d771f44@postgresql.org
2022-08-14 12:05:27 -04:00
aee9543736 Avoid misbehavior when hash_table_bytes < bucket_size.
It's possible to reach this case when work_mem is very small and tupsize
is (relatively) very large.  In that case ExecChooseHashTableSize would
get an assertion failure, or with asserts off it'd compute nbuckets = 0,
which'd likely cause misbehavior later (I've not checked).  To fix,
clamp the number of buckets to be at least 1.

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

Zhang Mingli

Discussion: https://postgr.es/m/beb64ca0-91e2-44ac-bf4a-7ea36275ec02@Spark
2022-08-13 16:59:58 -04:00
00ac61695e Catch stack overflow when recursing in transformFromClauseItem().
Most parts of the parser can expect that the stack overflow check
in transformExprRecurse() will trigger before things get desperate.
However, transformFromClauseItem() can recurse directly to self
without having analyzed any expressions, so it's possible to drive
it to a stack-overrun crash.  Add a check to prevent that.

Per bug #17583 from Egor Chindyaskin.  Back-patch to all supported
branches.

Richard Guo

Discussion: https://postgr.es/m/17583-33be55b9f981f75c@postgresql.org
2022-08-13 15:21:28 -04:00
94478f44a4 Add missing fields to _outConstraint()
As of 897795240c, check constraints can
be declared invalid.  But that patch didn't update _outConstraint() to
also show the relevant struct fields (which were only applicable to
foreign keys before that).  This currently only affects debugging
output, so no impact in practice.
2022-08-13 10:37:38 +02:00