Commit Graph

24306 Commits

Author SHA1 Message Date
Nathan Bossart 562bee0fc1 Don't truncate database and user names in startup packets.
Unlike commands such as CREATE DATABASE, ProcessStartupPacket()
does not perform multibyte-aware truncation of overlength names.
This means that connection attempts might fail even if the user
provides the same overlength names that were used in CREATE
DATABASE, CREATE ROLE, etc.  Ideally, we'd do the same multibyte-
aware truncation in both code paths, but it doesn't seem worth the
added complexity of trying to discover the encoding of the names.
Instead, let's simply skip truncating the names in the startup
packet and let the user/database lookup fail later on.  With this
change, users must provide the exact names stored in the catalogs,
even if the names were truncated.

This reverts commit d18c1d1f51.

Author: Bertrand Drouvot
Reviewed-by: Kyotaro Horiguchi, Tom Lane
Discussion: https://postgr.es/m/07436793-1426-29b2-f924-db7422a05fb7%40gmail.com
2023-07-03 13:18:05 -07:00
Tomas Vondra 29cf61ade3 Consider fillfactor when estimating relation size
When table_block_relation_estimate_size() estimated the number of tuples
in a relation without statistics (e.g. right after load), it did not
consider fillfactor when calculating density. With non-default
fillfactor values, this may result in significant overestimate of the
number of tuples - up to 10x with the minimum 10% fillfactor. This may
have unexpected consequences, e.g. when creating hash indexes.

This considers the current fillfactor value in the "no statistics" code
path.  If the fillfactor changes after loading data into the table, the
estimate may be off. But that seems much less likely than changing the
fillfactor before the data load.

Reviewed-by: Corey Huinker, Peter Eisentraut
Discussion: https://postgr.es/m/cf154ef9-6bac-d268-b735-67a3443debba@enterprisedb.com
2023-07-03 18:55:31 +02:00
Tomas Vondra a4cfeeca5a Fix code indentation violations
Commits ce5aaea8cd, 2b8b2852bb and 28d03feac3 violated the expected code
indentation rules, upsetting the new buildfarm member "koel."

Discussion: https://postgr.es/m/ZKIU4mhWpgJOM0W0%40paquier.xyz
2023-07-03 12:47:49 +02:00
Peter Eisentraut 6d56c501a7 A minor simplification for List manipulation
Fix one place that was using lfirst(list_head(list)) by using linitial(list)
instead.  They are equivalent but the latter is simpler.  We did the same in
9d299a49.

Author: Richard Guo <guofenglinux@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAMbWs49dJnpezDQDDxCPKq7+=_3NyqLqGqnhqCjd+dYe4MS15w@mail.gmail.com
2023-07-03 11:39:03 +02:00
Peter Eisentraut c69bdf837f Take pg_attribute out of VacAttrStats
The VacAttrStats structure contained the whole Form_pg_attribute for a
column, but it actually only needs attstattarget from there.  So
remove the Form_pg_attribute field and make a separate field for
attstattarget.  This simplifies some code for extended statistics that
doesn't deal with a column but an expression, which had to fake up
pg_attribute rows to satisfy internal APIs.  Also, we can remove some
comments that essentially said "don't look at pg_attribute directly".

Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org
2023-07-03 07:18:57 +02:00
Peter Eisentraut 7a7f60aef8 Add macro for maximum statistics target
The number of places where 10000 was hardcoded had grown a bit beyond
the comfort level.  Introduce a macro MAX_STATISTICS_TARGET instead.

Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org
2023-07-03 07:18:57 +02:00
Peter Eisentraut 3ee2f25d21 Change type of pg_statistic_ext.stxstattarget
Change from int32 to int16, to match attstattarget (changed in
90189eefc1).

Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/d6069765-5971-04d3-c10d-e4f7b2e9c459%40eisentraut.org
2023-07-03 07:18:57 +02:00
Michael Paquier 8e278b6576 Remove support for OpenSSL 1.0.1
Here are some notes about this change:
- As X509_get_signature_nid() should always exist (OpenSSL and
LibreSSL), hence HAVE_X509_GET_SIGNATURE_NID is now gone.
- OPENSSL_API_COMPAT is bumped to 0x10002000L.
- One comment related to 1.0.1e introduced by 74242c2 is removed.

Upstream OpenSSL still provides long-term support for 1.0.2 in a closed
fashion, so removing it is out of scope for a few years, at least.

Reviewed-by: Jacob Champion, Daniel Gustafsson
Discussion: https://postgr.es/m/ZG3JNursG69dz1lr@paquier.xyz
2023-07-03 13:20:27 +09:00
Michael Paquier 2aeaf80e57 Refactor some code related to wait events "BufferPin" and "Extension"
The following changes are done:
- Addition of WaitEventBufferPin and WaitEventExtension, that hold a
list of wait events related to each category.
- Addition of two functions that encapsulate the list of wait events for
each category.
- Rename BUFFER_PIN to BUFFERPIN (only this wait event class used an
underscore, requiring a specific rule in the automation script).

These changes make a bit easier the automatic generation of all the code
and documentation related to wait events, as all the wait event
categories are now controlled by consistent structures and functions.

Author: Bertrand Drouvot
Discussion: https://postgr.es/m/c6f35117-4b20-4c78-1df5-d3056010dcf5@gmail.com
Discussion: https://postgr.es/m/77a86b3a-c4a8-5f5d-69b9-d70bbf2e9b98@gmail.com
2023-07-03 11:01:02 +09:00
David Rowley c65102006b Remove redundant PARTITION BY columns from WindowClauses
Here we adjust the query planner to have it remove items from a window
clause's PARTITION BY clause in cases where the pathkey for a column in
the PARTITION BY clause is redundant.

Doing this allows the optimization added in 9d9c02ccd to stop window
aggregation early rather than going into "pass-through" mode to find
tuples belonging to the next partition.  Also, when we manage to remove
all PARTITION BY columns, we now no longer needlessly check that the
current tuple belongs to the same partition as the last tuple in
nodeWindowAgg.c.  If the pathkey was redundant then all tuples must
contain the same value for the given redundant column, so there's no point
in checking that during execution.

Author: David Rowley
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/CAApHDvo2ji+hdxrxfXtRtsfSVw3to2o1nCO20qimw0dUGK8hcQ@mail.gmail.com
2023-07-03 12:49:43 +12:00
Thomas Munro 4637a6ac0b Silence "missing contrecord" error.
Commit dd38ff28ad added a new error message "missing contrecord" when
we fail to reassemble a record.  Unfortunately that caused noisy
messages to be logged by pg_waldump at end of segment, and by walsender
when asked to shut down on a segment boundary.

Remove the new error message, so that this condition signals end-of-
WAL without a message.  It's arguably a reportable condition that should
not be silenced while performing crash recovery, but fixing that without
introducing noise in the other cases will require more research.

Back-patch to 15.

Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Discussion: https://postgr.es/m/6a1df56e-4656-b3ce-4b7a-a9cb41df8189%40enterprisedb.com
2023-07-03 11:16:27 +12:00
Tomas Vondra ce5aaea8cd Fix oversight in handling of modifiedCols since f24523672d
Commit f24523672d fixed a memory leak by moving the modifiedCols bitmap
into the per-row memory context. In the case of AFTER UPDATE triggers,
the bitmap is however referenced from an event kept until the end of the
query, resulting in a use-after-free bug.

Fixed by copying the bitmap into the AfterTriggerEvents memory context,
which is the one where we keep the trigger events. There's only one
place that needs to do the copy, but the memory context may not exist
yet. Doing that in a separate function seems more readable.

Report by Alexander Pyhalov, fix by me. Backpatch to 13, where the
bitmap was added to the event by commit 71d60e2aa0.

Reported-by: Alexander Pyhalov
Backpatch-through: 13
Discussion: https://postgr.es/m/acddb17c89b0d6cb940eaeda18c08bbe@postgrespro.ru
2023-07-02 22:21:02 +02:00
Tomas Vondra 98640f960e Fix memory leak in Incremental Sort rescans
The Incremental Sort had a couple issues, resulting in leaking memory
during rescans, possibly triggering OOM. The code had a couple of
related flaws:

1. During rescans, the sort states were reset but then also set to NULL
   (despite the comment saying otherwise). ExecIncrementalSort then
   sees NULL and initializes a new sort state, leaking the memory used
   by the old one.

2. Initializing the sort state also automatically rebuilt the info about
   presorted keys, leaking the already initialized info. presorted_keys
   was also unnecessarily reset to NULL.

Patch by James Coleman, based on patches by Laurenz Albe and Tom Lane.
Backpatch to 13, where Incremental Sort was introduced.

Author: James Coleman, Laurenz Albe, Tom Lane
Reported-by: Laurenz Albe, Zu-Ming Jiang
Backpatch-through: 13
Discussion: https://postgr.es/m/b2bd02dff61af15e3526293e2771f874cf2a3be7.camel%40cybertec.at
Discussion: https://postgr.es/m/db03c582-086d-e7cd-d4a1-3bc722f81765%40inf.ethz.ch
2023-07-02 20:03:30 +02:00
Tomas Vondra 0457109344 Improve BRIN minmax-multi opclass test coverage
Per the code coverage report, the existing regression tests did not
exercice some a couple important BRIN minmax-multi code paths.

- The tests focused on testing planning with a range of scan key
  strategies, but not the execution. Fixed by adding queries that
  actually test query execution for both equality and inequality.

- All tests created indexes after inserting data, but this only
  exercises the CREATE INDEX strategy that sees all values at once, not
  incremental summary updates. The new tests flip the order and create
  the index before adding data.

- The assert check(s) validating correctness of expanded ranges were
  present only in the "union" code path, which is not covered by
  regression tests at all (as it requires concurrency etc.). Fixed by
  adding the asserts to a couple more places.

Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/57020b2e-d9c9-9bc7-4892-b36d9bb07563%40enterprisedb.com
2023-07-02 10:33:38 +02:00
Tomas Vondra 2b8b2852bb Introduce bloom_filter_size for BRIN bloom opclass
Move the calculation of Bloom filter parameters (for BRIN indexes) into
a separate function to make reuse easier. At the moment we only call it
from one place, but that may change and it's easier to read anyway.

Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/0e1f3350-c9cf-ab62-43a5-5dae314de89c%40enterprisedb.com
2023-07-02 10:24:29 +02:00
Tomas Vondra 28d03feac3 Minor cleanups in the BRIN code
BRIN bloom and minmax-multi opclasses were somewhat inconsistent when
dealing with bool variables, assigning to them Datum values etc. While
not a bug, it makes the code harder to understand, so fix that.

While at it, update an incorrect comment copied to bloom opclass from
minmax, talking about strategies not supported by bloom.

Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/0e1f3350-c9cf-ab62-43a5-5dae314de89c%40enterprisedb.com
2023-07-02 10:21:17 +02:00
Thomas Munro 4f49b3f849 Trust signalfd on illumos, again.
Commit 3ab4fc5d avoided choosing signalfd by default on illumos, because
it triggered kernel panics.  That was fixed, so we can remove a kludge
from our code.  Users/packagers can still override the default choice at
compile time if desired, and we'll leave the back-branches unchanged so
they keep choosing self-pipe by default, but we'll default to signalfd
(like we do for Linux) in 17.  Fixed kernels should be everywhere by the
time 17 ships.

The illumos issues were:

 * https://www.illumos.org/issues/13700
 * https://www.illumos.org/issues/14892

Discussion: https://postgr.es/m/CA+hUKG+NK-K_G_i1H3OpDTwYPEsiwQi_jw58PGcW2H+-N2eVCA@mail.gmail.com
2023-07-02 15:28:48 +12:00
Heikki Linnakangas e251e780bf Remove redundant check for fast_forward.
We already checked for it earlier in the function.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/1ba2899e-77f8-7866-79e5-f3b7d1251a3e@iki.fi
2023-06-30 18:31:10 +03:00
Heikki Linnakangas a0dd4c95b9 Improve comment on why we need ctid->(cmin,cmax) mapping.
Combocids are only part of the problem. Explain the problem in more detail.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/1ba2899e-77f8-7866-79e5-f3b7d1251a3e@iki.fi
2023-06-30 18:30:32 +03:00
Michael Paquier cfc43aeb38 Fix marking of indisvalid for partitioned indexes at creation
The logic that introduced partitioned indexes missed a few things when
invalidating a partitioned index when these are created, still the code
is written to handle recursions:
1) If created from scratch because a mapping index could not be found,
the new index created could be itself invalid, if for example it was a
partitioned index with one of its leaves invalid.
2) A CCI was missing when indisvalid is set for a parent index, leading
to inconsistent trees when recursing across more than one level for a
partitioned index creation if an invalidation of the parent was
required.

This could lead to the creation of a partition index tree where some of
the partitioned indexes are marked as invalid, but some of the parents
are marked valid, which is not something that should happen (as
validatePartitionedIndex() defines, indisvalid is switched to true for a
partitioned index iff all its partitions are themselves valid).

This patch makes sure that indisvalid is set to false on a partitioned
index if at least one of its partition is invalid.  The flag is set to
true if *all* its partitions are valid.

The regression test added in this commit abuses of a failed concurrent
index creation, marked as invalid, that maps with an index created on
its partitioned table afterwards.

Reported-by: Alexander Lakhin
Reviewed-by: Alexander Lakhin
Discussion: https://postgr.es/m/14987634-43c0-0cb3-e075-94d423607e08@gmail.com
Backpatch-through: 11
2023-06-30 13:54:48 +09:00
Michael Paquier 23d8624fe5 Use named captures in Catalog::ParseHeader()
Using at least perl 5.14 is required since 4c15327, meaning that it is
possible to use named captures and the %+ hash instead of having to
count parenthesis groups manually.

While on it, CATALOG is made more flexible in its handling of
whitespaces for parameter lists (see the addition of \s* in this
case).  The generated postgres.bki remains exactly the same before and
after this commit.

Author: Dagfinn Ilmari Mannsåker
Reviewed-by: John Naylor
Discussion: https://postgr.es/m/87y1l3s7o9.fsf@wibble.ilmari.org
2023-06-30 09:16:27 +09:00
Michael Paquier 97d8910104 Fix pg_depend entry to AMs after ALTER TABLE .. SET ACCESS METHOD
ALTER TABLE .. SET ACCESS METHOD was not registering a dependency to the
new access method with the relation altered in its rewrite phase, making
possible the drop of an access method even if there are relations that
depend on it.  During the rewrite, a temporary relation is created to
build the new relation files before swapping the new and old files, and,
while the temporary relation was registering a correct dependency to the
new AM, the old relation did not do that.  A dependency on the access
method is added when the relation files are swapped, which is the point
where pg_class is updated.

Materialized views and tables use the same code path, hence both were
impacted.

Backpatch down to 15, where this command has been introduced.

Reported-by: Alexander Lakhin
Reviewed-by: Nathan Bossart, Andres Freund
Discussion: https://postgr.es/m/18000-9145c25b1af475ca@postgresql.org
Backpatch-through: 15
2023-06-30 07:49:01 +09:00
Tom Lane a798660ebe Defend against bogus parameterization of join input paths.
An outer join cannot be formed using an input path that is parameterized
by a value that is supposed to be nulled by the outer join.  This is
obviously nonsensical, and it could lead to a bad plan being selected;
although currently it seems that we'll hit various sanity-check
assertions first.

I think that such cases were formerly prevented by the delay_upper_joins
mechanism, but now that that's gone we need an explicit check.

(Perhaps we should avoid generating baserel paths that could
lead to this situation in the first place; but it seems like
having a defense at the join level would be a good idea anyway.)

Richard Guo and Tom Lane, per report from Jaime Casanova

Discussion: https://postgr.es/m/CAJKUy5g2uZRrUDZJ8p-=giwcSHVUn0c9nmdxPSY0jF0Ov8VoEA@mail.gmail.com
2023-06-29 12:12:52 -04:00
Tom Lane 43af714def Fix order of operations in ExecEvalFieldStoreDeForm().
If the given composite datum is toasted out-of-line,
DatumGetHeapTupleHeader will perform database accesses to detoast it.
That can invalidate the result of get_cached_rowtype, as documented
(perhaps not plainly enough) in that function's API spec; which leads
to strange errors or crashes when we try to use the TupleDesc to read
the tuple.  In short then, trying to update a field of a composite
column could fail intermittently if the overall column value is wide
enough to require toasting.

We can fix the bug at no cost by just changing the order of
operations, since we don't need the TupleDesc until after detoasting.
(Other callers of get_cached_rowtype appear to get this right already,
so there's only one bug.)

Note that the added regression test case reveals this bug reliably
only with debug_discard_caches/CLOBBER_CACHE_ALWAYS.

Per bug #17994 from Alexander Lakhin.  Sadly, this patch does not fix
the missing-values issue revealed in the bug discussion; we'll need
some more work to cover that.

Discussion: https://postgr.es/m/17994-5c7100b51b4790e9@postgresql.org
2023-06-29 10:19:10 -04:00
Peter Eisentraut efcf55f8fe Remove inappropriate raw_expression_tree_walker() code
It was walking into the ColumnDef->compression field, which is not a
node but a string.  This code is currently not reachable (because the
compression field is only set in situations that don't go through
raw_expression_tree_walker()), but if it had been, this could have
behaved erratically.
2023-06-29 10:34:53 +02:00
Peter Eisentraut 39a584dc90 Error message wording improvements 2023-06-29 09:14:55 +02:00
Peter Eisentraut 046c8c5c8f Reword error messages for consistency 2023-06-28 19:30:26 +02:00
Michael Paquier fc55c7ff8d Ignore invalid indexes when enforcing index rules in ALTER TABLE ATTACH PARTITION
A portion of ALTER TABLE .. ATTACH PARTITION is to ensure that the
partition being attached to the partitioned table has a correct set of
indexes, so as there is a consistent index mapping between the
partitioned table and its new-to-be partition.  However, as introduced
in 8b08f7d, the current logic could choose an invalid index as a match,
which is something that can exist when dealing with more than two levels
of partitioning, like attaching a partitioned table (that has
partitions, with an index created by CREATE INDEX ON ONLY) to another
partitioned table.

A partitioned index with indisvalid set to false is equivalent to an
incomplete partition tree, meaning that an invalid partitioned index
does not have indexes defined in all its partitions.  Hence, choosing an
invalid partitioned index can create inconsistent partition index trees,
where the parent attaching to is valid, but its partition may be
invalid.

In the report from Alexander Lakhin, this showed up as an assertion
failure when validating an index.  Without assertions enabled, the
partition index tree would be actually broken, as indisvalid should
be switched to true for a partitioned index once all its partitions are
themselves valid.  With two levels of partitioning, the top partitioned
table used a valid index and was able to link to an invalid index stored
on its partition, itself a partitioned table.

I have studied a few options here (like the possibility to switch
indisvalid to false for the parent), but came down to the conclusion
that we'd better rely on a simple rule: invalid indexes had better never
be chosen, so as the partition attached uses and creates indexes that
the parent expects.  Some regression tests are added to provide some
coverage.  Note that the existing coverage is not impacted.

This is a problem since partitioned indexes exist, so backpatch all the
way down to v11.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/14987634-43c0-0cb3-e075-94d423607e08@gmail.com
Backpatch-through: 11
2023-06-28 15:57:31 +09:00
Michael Paquier 2ecbb0a493 Remove dependency to query text in JumbleQuery()
Since 3db72eb, the query ID of utilities is generated using the Query
structure, making the use of the query string in JumbleQuery()
unnecessary.  This commit removes the argument "querytext" from
JumbleQuery().

Reported-by: Joe Conway
Reviewed-by: Nathan Bossart
Discussion: https://postgr.es/m/ZJlQAWE4COFqHuAV@paquier.xyz
2023-06-28 08:59:36 +09:00
Peter Eisentraut 3f0199da5a Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: ab77975e9d2cde44da796c18af3ec1a66f0df7ae
2023-06-26 12:02:02 +02:00
Heikki Linnakangas b491a15f8c Change "..." to cstring in old input/output function comments.
It was not clear what the "..." meant.

Author: Steve Chavez
Discussion: https://www.postgresql.org/message-id/CAGRrpzZzeh7zC3yaVG9di%3DydJ%2BiHwdXnFPB3evGFKvC1zf6ajA@mail.gmail.com
2023-06-26 11:52:31 +03:00
Tom Lane 691594acd6 Check for interrupts and stack overflow in TParserGet().
TParserGet() recurses for some token types, meaning it's possible
to drive it to stack overflow.  Since this is a minority behavior,
I chose to add the check_stack_depth() call to the two places that
recurse rather than doing it during every single call.

While at it, add CHECK_FOR_INTERRUPTS(), because this can run
unpleasantly long for long inputs.

Per bug #17995 from Zuming Jiang.  This is old, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/17995-9f20ff3e6389db4c@postgresql.org
2023-06-24 17:18:08 -04:00
Peter Eisentraut 3ad5f07c0f Error message refactoring
Take some untranslatable things out of the message and replace by
format placeholders, to reduce translatable strings and reduce
translation mistakes.
2023-06-23 16:36:17 +02:00
Nathan Bossart 4dbdb82513 Fix cache lookup hazards introduced by ff9618e82a.
ff9618e82a introduced has_partition_ancestor_privs(), which is used
to check whether a user has MAINTAIN on any partition ancestors.
This involves syscache lookups, and presently this function does
not take any relation locks, so it is likely subject to the same
kind of cache lookup failures that were fixed by 19de0ab23c.

To fix this problem, this commit partially reverts ff9618e82a.
Specifically, it removes the partition-related changes, including
the has_partition_ancestor_privs() function mentioned above.  This
means that MAINTAIN on a partitioned table is no longer sufficient
to perform maintenance commands on its partitions.  This is more
like how privileges for maintenance commands work on supported
versions.  Privileges are checked for each partition, so a command
that flows down to all partitions might refuse to process them
(e.g., if the current user doesn't have MAINTAIN on the partition).

In passing, adjust a few related comments and error messages, and
add a test for the privilege checks for CLUSTER on a partitioned
table.

Reviewed-by: Michael Paquier, Jeff Davis
Discussion: https://postgr.es/m/20230613211246.GA219055%40nathanxps13
2023-06-22 15:48:20 -07:00
Peter Geoghegan 5f0762f147 nbtree VACUUM: cope with topparent inconsistencies.
Avoid "right sibling %u of block %u is not next child" errors when
vacuuming a corrupt nbtree index.  Just LOG the issue and press on.
That way VACUUM will have a decent chance of finishing off all required
processing for the index (and for the table as a whole).

This is similar to recent work from commit 5abff197, as well as work
from commit 5b861baa (later backpatched as commit 43e409ce), which
taught nbtree VACUUM to keep going when its "re-find" check fails.  The
hardening added by this commit takes place directly after the "re-find"
check, right before the critical section for the first stage of page
deletion.

Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wz=dayg0vjs4+er84TS9ami=csdzjpuiCGbEw=idhwqhzQ@mail.gmail.com
Backpatch: 11- (all supported versions).
2023-06-21 17:41:58 -07:00
Jeff Davis f3a01af29b ICU: do not convert locale 'C' to 'en-US-u-va-posix'.
Older versions of ICU canonicalize "C" to "en-US-u-va-posix"; but
starting in ICU version 64, the "C" locale is considered
obsolete. Postgres commit ea1db8ae70 introduced code to always
canonicalize "C" to "en-US-u-va-posix" for consistency and
convenience, but it was deemed too confusing.

This commit removes that code, so that "C" is treated like other ICU
locale names: canonicalization is attempted, and if it fails, the
behavior is controlled by icu_validation_level.

A similar change was previously committed as f7faa9976c, then reverted
due to an ICU-version-dependent test failure. This commit un-reverts
it, omitting the test because we now expect the behavior to depend on
the version of ICU being used.

Discussion: https://postgr.es/m/3a200aca-4672-4b37-fc91-5d198a323503%40eisentraut.org
Discussion: https://postgr.es/m/f83f089ee1e9acd5dbbbf3353294d24e1f196e95.camel@j-davis.com
Discussion: https://postgr.es/m/37520ec1ae9591f83132f82dbd625f3fc2d69c16.camel@j-davis.com
2023-06-21 13:18:25 -07:00
Tom Lane 555b929bbe Avoid Assert failure when processing empty statement in aborted xact.
exec_parse_message() wants to create a cached plan in all cases,
including for empty input.  The empty-input path does not have
a test for being in an aborted transaction, making it possible
that plancache.c will fail due to trying to do database lookups
even though there's no real work to do.

One solution would be to throw an aborted-transaction error in
this path too, but it's not entirely clear whether the lack of
such an error was intentional or whether some clients might be
relying on non-error behavior.  Instead, let's hack plancache.c
so that it treats empty statements with the same logic it
already had for transaction control commands, ensuring that it
can soldier through even in an already-aborted transaction.

Per bug #17983 from Alexander Lakhin.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/17983-da4569fcb878672e@postgresql.org
2023-06-21 11:07:24 -04:00
Amit Kapila a734caa25f Fix the errhint message and docs for drop subscription failure.
The existing errhint message and docs were missing the fact that we can't
disassociate from the slot unless the subscription is disabled.

Author: Robert Sjöblom, Peter Smith
Reviewed-by: Peter Eisentraut, Amit Kapila
Backpatch-through: 11
Discussion: https://postgr.es/m/807bdf85-61ea-88e2-5712-6d9fcd4eabff@fortnox.se
2023-06-21 10:36:09 +05:30
Nathan Bossart 5b1a879943 Move bool parameter for vacuum_rel() to option bits.
ff9618e82a introduced the skip_privs parameter, which is used to
skip privilege checks when recursing to a relation's TOAST table.
This parameter should have been added as a flag bit in
VacuumParams->options instead.

Suggested-by: Michael Paquier
Reviewed-by: Michael Paquier, Jeff Davis
Discussion: https://postgr.es/m/ZIj4v1CwqlDVJZfB%40paquier.xyz
2023-06-20 15:14:58 -07:00
Tom Lane 45392626c9 Fix hash join when inner hashkey expressions contain Params.
If the inner-side expressions contain PARAM_EXEC Params, we must
re-hash whenever the values of those Params change.  The executor
mechanism for that exists already, but we failed to invoke it because
finalize_plan() neglected to search the Hash.hashkeys field for
Params.  This allowed a previous scan's hash table to be re-used
when it should not be, leading to rows missing from the join's output.
(I believe incorrectly-included join rows are impossible however,
since checking the real hashclauses would reject false matches.)

This bug is very ancient, dating probably to d24d75ff1 of 7.4.
Sadly, this simple fix depends on the plan representational changes
made by 2abd7ae9b, so it will only work back to v12.  I thought
about trying to make some kind of hack for v11, but I'm leery
of putting code significantly different from what is used in the
newer branches into a nearly-EOL branch.  Seeing that the bug
escaped detection for a full twenty years, problematic cases
must be rare; so I don't feel too awful about leaving v11 as-is.

Per bug #17985 from Zuming Jiang.  Back-patch to v12.

Discussion: https://postgr.es/m/17985-748b66607acd432e@postgresql.org
2023-06-20 17:47:53 -04:00
Tom Lane 3af87736bf Fix another cause of "wrong varnullingrels" planner failures.
I removed the delay_upper_joins mechanism in commit b448f1c8d,
reasoning that it was only needed when we have a single-table
(SELECT ... WHERE) as the immediate RHS child of a left join,
and we could get rid of that by hoisting the WHERE condition into
the parent join's quals.  However that new code missed a case:
we could have "foo LEFT JOIN ((SELECT ... WHERE) LEFT JOIN bar)",
and if the two left joins can be commuted then we now have the
problematic query shape.  We can fix this too easily enough,
by allowing the syntactically-lower left join to pass through
its parent qual location pointer recursively.  That lets
prepjointree.c discard the SELECT by temporarily hoisting the
WHERE condition into the ancestor join's qual.

Per bug #17978 from Zuming Jiang.

Discussion: https://postgr.es/m/17978-12f3d93a55297266@postgresql.org
2023-06-20 11:09:56 -04:00
Tom Lane efeb12ef0b Don't include outer join relids in lateral_relids bitmapsets.
This avoids an assertion failure when outer joins are rearranged
per identity 3.  Listing only the baserels from a PlaceHolderVar's
ph_lateral set should be enough to ensure that the required values
are available when we need to compute the PHV --- it's what we
did before inventing nullingrel sets, after all.  It's a bit
unsatisfying; but with beta2 hard upon us, there's not time to
look for an aesthetically cleaner fix.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/CAMbWs48Jcw-NvnxT23WiHP324wG44DvzcH1j4hc0Zn+3sR9cfg@mail.gmail.com
2023-06-20 10:29:57 -04:00
Tom Lane 0655c03ef9 Centralize fixups for mismatched nullingrels in nestloop params.
It turns out that the fixes we applied in commits bfd332b3f
and 63e4f13d2 were not nearly enough to solve the problem.
We'd focused narrowly on subquery RTEs with lateral references,
but lateral references can occur in several other RTE kinds
such as function RTEs.  Putting the same hack into half a dozen
code paths seems quite unattractive.  Hence, revert the code changes
(but not the test cases) from those commits and instead solve it
centrally in identify_current_nestloop_params(), as Richard proposed
originally.  This is a bit annoying because it could mask erroneous
nullingrels in nestloop params that are generated from non-LATERAL
parameterized paths; but on balance I don't see a better way.
Maybe at some future time we'll be motivated to find a more rigorous
approach to nestloop params, but that's not happening for beta2.

Richard Guo and Tom Lane

Discussion: https://postgr.es/m/CAMbWs48Jcw-NvnxT23WiHP324wG44DvzcH1j4hc0Zn+3sR9cfg@mail.gmail.com
2023-06-20 10:22:52 -04:00
Tom Lane b334612b8a Pre-beta2 mechanical code beautification.
Run pgindent and pgperltidy.  It seems we're still some ways
away from all committers doing this automatically.  Now that
we have a buildfarm animal that will whine about poorly-indented
code, we'll try to keep the tree more tidy.

Discussion: https://postgr.es/m/3156045.1687208823@sss.pgh.pa.us
2023-06-20 09:50:43 -04:00
Michael Paquier 68cb5af46c Enable archiving in recovery TAP test 009_twophase.pl
This is a follow-up of f663b00, that has been committed to v13 and v14,
tweaking the TAP test for two-phase transactions so as it provides
coverage for the bug that has been fixed.  This change is done in its
own commit for clarity, as v15 and HEAD did not show the problematic
behavior, still missed coverage for it.

While on it, this adds a comment about the dependency of the last
partial segment rename and RecoverPreparedTransactions() at the end of
recovery, as that can be easy to miss.

Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/743b9b45a2d4013bd90b6a5cba8d6faeb717ee34.camel@cybertec.at
Backpatch-through: 13
2023-06-20 10:25:27 +09:00
Andres Freund 0d369ac650 fd.c: Retry after EINTR in more places
Starting with 4d330a61bb we can use posix_fallocate() to extend
files. Unfortunately in some situation, e.g. on tmpfs filesystems, EINTR may
be returned. See also 4518c798b2.

To fix, add a retry path to FileFallocate(). In contrast to 4518c798b2 the
amount we extend by is limited and the extending may happen at a high
frequency, so disabling signals does not appear to be the correct path here.

Also add retry paths to other file operations currently lacking them (around
fdatasync(), fsync(), ftruncate(), posix_fadvise(), sync_file_range(),
truncate()) - they are all documented or have been observed to return EINTR.

Even though most of these functions used in the back branches, it does not
seem worth the risk to backpatch - outside of the new-to-16 case of
posix_fallocate() I am not aware of problem reports due to the lack of
retries.

Reported-by: Christoph Berg <myon@debian.org>
Discussion: https://postgr.es/m/ZEZDj1H61ryrmY9o@msg.df7cb.de
Backpatch: -
2023-06-19 14:11:32 -07:00
David Rowley 7fcd7ef2a9 Don't use partial unique indexes for unique proofs in the planner
Here we adjust relation_has_unique_index_for() so that it no longer makes
use of partial unique indexes as uniqueness proofs.  It is incorrect to
use these as the predicates used by check_index_predicates() to set
predOK makes use of not only baserestrictinfo quals as proofs, but also
qual from join conditions.  For relation_has_unique_index_for()'s case, we
need to know the relation is unique for a given set of columns before any
joins are evaluated, so if predOK was only set to true due to some join
qual, then it's unsafe to use such indexes in
relation_has_unique_index_for().  The final plan may not even make use
of that index, which could result in reading tuples that are not as
unique as the planner previously expected them to be.

Bug: #17975
Reported-by: Tor Erik Linnerud
Backpatch-through: 11, all supported versions
Discussion: https://postgr.es/m/17975-98a90c156f25c952%40postgresql.org
2023-06-19 13:00:42 +12:00
Jeff Davis a14e75eb0b CREATE DATABASE: make LOCALE apply to all collation providers.
For CREATE DATABASE, make LOCALE parameter apply regardless of the
provider used. Also affects initdb and createdb --locale arguments.

Previously, LOCALE (and --locale) only affected the database default
collation when using the libc provider.

Discussion: https://postgr.es/m/1a63084d-221e-4075-619e-6b3e590f673e@enterprisedb.com
Reviewed-by: Peter Eisentraut
2023-06-16 10:27:32 -07:00
Amit Langote 160c23b5fa Fix typo in comment.
Back-patch down to 11.

Author: Sho Kato (<kato-sho@fujitsu.com>)
Discussion: https://postgr.es/m/TYCPR01MB68499042A33BC32241193AAF9F5BA%40TYCPR01MB6849.jpnprd01.prod.outlook.com
2023-06-16 10:17:15 +09:00
Tom Lane f4c00d138f When removing a left join, clean out references in EquivalenceClasses.
Since commit b448f1c8d, we've been able to remove left joins
(that are otherwise removable) even when they are underneath
other left joins, a case that was previously prevented by a
delay_upper_joins check.  This is a clear improvement, but
it has a surprising side-effect: it's now possible that there
are EquivalenceClasses whose relid sets mention the removed
baserel and/or outer join.  If we fail to clean those up,
we may drop essential join quals due to not having any join
level that appears to satisfy their relid sets.

(It's not quite 100% clear that this was impossible before.
But the lack of complaints since we added join removal a dozen
years ago strongly suggests that it was impossible.)

Richard Guo and Tom Lane, per bug #17976 from Zuming Jiang

Discussion: https://postgr.es/m/17976-4b638b525e9a983b@postgresql.org
2023-06-15 15:24:50 -04:00