Commit Graph

7729 Commits

Author SHA1 Message Date
e6a9637488 Revert "Allow parallel workers to cope with a newly-created session user ID."
This reverts commit f5f30c22ed69fb37b896c4d4546b2ab823c3fd61.

Some buildfarm animals are failing with "cannot change
"client_encoding" during a parallel operation".  It looks like
assign_client_encoding is unhappy at being asked to roll back a
client_encoding setting after a parallel worker encounters a
failure.  There must be more to it though: why didn't I see this
during local testing?  In any case, it's clear that moving the
RestoreGUCState() call is not as side-effect-free as I thought.
Given that the bug f5f30c22e intended to fix has gone unreported
for years, it's not something that's urgent to fix; I'm not
willing to risk messing with it further with only days to our
next release wrap.
2024-07-31 20:57:00 -04:00
f5f30c22ed Allow parallel workers to cope with a newly-created session user ID.
Parallel workers failed after a sequence like
	BEGIN;
	CREATE USER foo;
	SET SESSION AUTHORIZATION foo;
because check_session_authorization could not see the uncommitted
pg_authid row for "foo".  This is because we ran RestoreGUCState()
in a separate transaction using an ordinary just-created snapshot.
The same disease afflicts any other GUC that requires catalog lookups
and isn't forgiving about the lookups failing.

To fix, postpone RestoreGUCState() into the worker's main transaction
after we've set up a snapshot duplicating the leader's.  This affects
check_transaction_isolation and check_transaction_deferrable, which
think they should only run during transaction start.  Make them
act like check_transaction_read_only, which already knows it should
silently accept the value when InitializingParallelWorker.

Per bug #18545 from Andrey Rachitskiy.  Back-patch to all
supported branches, because this has been wrong for awhile.

Discussion: https://postgr.es/m/18545-feba138862f19aaa@postgresql.org
2024-07-31 18:54:10 -04:00
c8b06bb969 Introduce pg_sequence_read_tuple().
This new function returns the data for the given sequence, i.e.,
the values within the sequence tuple.  Since this function is a
substitute for SELECT from the sequence, the SELECT privilege is
required on the sequence in question.  It returns all NULLs for
sequences for which we lack privileges, other sessions' temporary
sequences, and unlogged sequences on standbys.

This function is primarily intended for use by pg_dump in a
follow-up commit that will use it to optimize dumpSequenceData().
Like pg_sequence_last_value(), which is a support function for the
pg_sequences system view, pg_sequence_read_tuple() is left
undocumented.

Bumps catversion.

Reviewed-by: Michael Paquier, Tom Lane
Discussion: https://postgr.es/m/20240503025140.GA1227404%40nathanxps13
2024-07-31 10:12:42 -05:00
0dcea330ba Fix random failure in 021_twophase.
After disabling the subscription, the failed test was changing the
two_phase option for the subscription. We can't change the two_phase
option for a subscription till the corresponding apply worker is active.
The check to ensure that the replication apply worker has exited was
incorrect.

Author: Vignesh C
Discussion: https://postgr.es/m/CALDaNm3YY+bzj+JWJbY+DsUgJ2mPk8OR1ttjVX2cywKr4BUgxw@mail.gmail.com
2024-07-31 08:53:55 +05:30
524d490a9f Preserve tz when converting to jsonb timestamptz
This removes an inconsistency in the treatment of different datatypes by
the jsonpath timestamp_tz() function. Conversions from data types that
are not timestamp-aware, such as date and timestamp, are now treated
consistently with conversion from those that are such as timestamptz.

Author: David Wheeler
Reviewed-by: Junwang Zhao and Jeevan Chalke

Discussion: https://postgr.es/m/7DE080CE-6D8C-4794-9BD1-7D9699172FAB%40justatheory.com

Backpatch to release 17.
2024-07-30 07:57:38 -04:00
e25626677f Remove --disable-spinlocks.
A later change will require atomic support, so it wouldn't make sense
for a hypothetical new system not to be able to implement spinlocks.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (concept, not the patch)
Reviewed-by: Andres Freund <andres@anarazel.de> (concept, not the patch)
Discussion: https://postgr.es/m/3351991.1697728588%40sss.pgh.pa.us
2024-07-30 22:58:37 +12:00
800cd3e923 Stabilize xid_wraparound tests
The tests had a race condition if autovacuum was set to off. Instead we
create all the tables we are interested in with autovacuum disabled, so
they are only ever touched when in danger of wraparound.

Discussion: https://postgr.es/m/3e2cbd24-f45e-4b2b-ba83-8149214f0a4d@dunslane.net

Masahiko Sawada (slightly tweaked by me)

Backpatch to release 17 where these tests were introduced.
2024-07-30 06:24:59 -04:00
72fe6d24a3 Make collation not depend on setlocale().
Now that the result of pg_newlocale_from_collation() is always
non-NULL, then we can move the collate_is_c and ctype_is_c flags into
pg_locale_t. That simplifies the logic in lc_collate_is_c() and
lc_ctype_is_c(), removing the dependence on setlocale().

This commit also eliminates the multi-stage initialization of the
collation cache.

As long as we have catalog access, then it's now safe to call
pg_newlocale_from_collation() without checking lc_collate_is_c()
first.

Discussion: https://postgr.es/m/cfd9eb85-c52a-4ec9-a90e-a5e4de56e57d@eisentraut.org
Reviewed-by: Peter Eisentraut, Andreas Karlsson
2024-07-30 00:58:06 -07:00
9b282a9359 Fix partitionwise join with partially-redundant join clauses
To determine if the two relations being joined can use partitionwise
join, we need to verify the existence of equi-join conditions
involving pairs of matching partition keys for all partition keys.
Currently we do that by looking through the join's restriction
clauses.  However, it has been discovered that this approach is
insufficient, because there might be partition keys known equal by a
specific EC, but they do not form a join clause because it happens
that other members of the EC than the partition keys are constrained
to become a join clause.

To address this issue, in addition to examining the join's restriction
clauses, we also check if any partition keys are known equal by ECs,
by leveraging function exprs_known_equal().  To accomplish this, we
enhance exprs_known_equal() to check equality per the semantics of the
opfamily, if provided.

It could be argued that exprs_known_equal() could be called O(N^2)
times, where N is the number of partition key expressions, resulting
in noticeable performance costs if there are a lot of partition key
expressions.  But I think this is not a problem.  The number of a
joinrel's partition key expressions would only be equal to the join
degree, since each base relation within the join contributes only one
partition key expression.  That is to say, it does not scale with the
number of partitions.  A benchmark with a query involving 5-way joins
of partitioned tables, each with 3 partition keys and 1000 partitions,
shows that the planning time is not significantly affected by this
patch (within the margin of error), particularly when compared to the
impact caused by partitionwise join.

Thanks to Tom Lane for the idea of leveraging exprs_known_equal() to
check if partition keys are known equal by ECs.

Author: Richard Guo, Tom Lane
Reviewed-by: Tom Lane, Ashutosh Bapat, Robert Haas
Discussion: https://postgr.es/m/CAN_9JTzo_2F5dKLqXVtDX5V6dwqB0Xk+ihstpKEt3a1LT6X78A@mail.gmail.com
2024-07-30 15:51:54 +09:00
7f56eaff2f SQL/JSON: Fix casting for integer EXISTS columns in JSON_TABLE
The current method of coercing the boolean result value of
JsonPathExists() to the target type specified for an EXISTS column,
which is to call the type's input function via json_populate_type(),
leads to an error when the target type is integer, because the
integer input function doesn't recognize boolean literal values as
valid.

Instead use the boolean-to-integer cast function for coercion in that
case so that using integer or domains thereof as type for EXISTS
columns works. Note that coercion for ON ERROR values TRUE and FALSE
already works like that because the parser creates a cast expression
including the cast function, but the coercion of the actual result
value is not handled by the parser.

Tests by Jian He.

Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
2024-07-30 10:34:17 +09:00
74c96699be SQL/JSON: Some fixes to JsonBehavior expression casting
1. Remove the special case handling when casting the JsonBehavior
   expressions to types with typmod, like 86d33987 did for the casting
   of SQL/JSON constructor functions.

2. Fix casting for fixed-length character and bit string types by
   using assignment-level casts.  This is again similar to what
   86d33987 did, but for ON ERROR / EMPTY expressions.

3. Use runtime coercion for the boolean ON ERROR constants so that
   using fixed-length character string types, for example, for an
   EXISTS column doesn't cause a "value too long for type
   character(n)" when the parser tries to coerce the default ON ERROR
   value "false" to that type, that is, even when clause is not
   specified.

4. Simplify the conditions of when to use runtime coercion vs
   creating the cast expression in the parser itself.  jsonb-valued
   expressions are now always coerced at runtime and boolean
   expressions too if the target type is a string type for the
   reasons mentioned above.

Tests are taken from a patch that Jian He posted.

Reported-by: Jian He <jian.universality@gmail.com>
Author: Jian He <jian.universality@gmail.com>
Author: Amit Langote <amitlangote09@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
2024-07-30 10:34:17 +09:00
b181062aa5 Fix incorrect return value for pg_size_pretty(bigint)
pg_size_pretty(bigint) would return the value in bytes rather than PB
for the smallest-most bigint value.  This happened due to an incorrect
assumption that the absolute value of -9223372036854775808 could be
stored inside a signed 64-bit type.

Here we fix that by instead storing that value in an unsigned 64-bit type.

This bug does exist in versions prior to 15 but the code there is
sufficiently different and the bug seems sufficiently non-critical that
it does not seem worth risking backpatching further.

Author: Joseph Koshakow <koshy44@gmail.com>
Discussion: https://postgr.es/m/CAAvxfHdTsMZPWEHUrZ=h3cky9Ccc3Mtx2whUHygY+ABP-mCmUw@mail.gmail.com
Backpatch-through: 15
2024-07-28 22:22:52 +12:00
5d1d8b3c82 Clarify error message and documentation related to typed tables.
We restrict typed tables (those declared as "OF composite_type")
to be based on stand-alone composite types, not composite types
that are the implicitly-created rowtypes of other tables.
But if you tried to do that, you got the very confusing error
message "type foo is not a composite type".  Provide a more specific
message for that case.  Also clarify related documentation in the
CREATE TABLE man page.

Erik Wienhold and David G. Johnston, per complaint from Hannu Krosing.

Discussion: https://postgr.es/m/CAMT0RQRysCb_Amy5CTENSc5GfsvXL1a4qX3mv_hx31_v74P==g@mail.gmail.com
2024-07-26 12:39:45 -04:00
4fc6a55560 SQL/JSON: Respect OMIT QUOTES when RETURNING domains over jsonb
populate_domain() didn't take into account the omit_quotes flag passed
down to json_populate_type() by ExecEvalJsonCoercion() and that led
to incorrect behavior when the RETURNING type is a domain over
jsonb.  Fix that by passing the flag by adding a new function
parameter to populate_domain().

Reported-by: Jian He <jian.universality@gmail.com>
Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
2024-07-26 16:08:13 +09:00
231b7d670b SQL/JSON: Improve error-handling of JsonBehavior expressions
Instead of returning a NULL when the JsonBehavior expression value
could not be coerced to the RETURNING type, throw the error message
informing the user that it is the JsonBehavior expression that caused
the error with the actual coercion error message shown in its DETAIL
line.

Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
2024-07-26 16:00:56 +09:00
63e6c5f4a2 SQL/JSON: Fix error-handling of some JsonBehavior expressions
To ensure that the errors of executing a JsonBehavior expression that
is coerced in the parser are caught instead of being thrown directly,
pass ErrorSaveContext to ExecInitExprRec() when initializing it.
Also, add a EEOP_JSONEXPR_COERCION_FINISH step to handle the errors
that are caught that way.

Discussion: https://postgr.es/m/CACJufxEo4sUjKCYtda0_qt9tazqqKPmF1cqhW9KBOUeJFqQd2g@mail.gmail.com
Backpatch-through: 17
2024-07-26 16:00:06 +09:00
ab61c40bfa Add extern declarations for Bison global variables
This adds extern declarations for some global variables produced by
Bison that are not already declared in its generated header file.
This is a workaround to be able to add -Wmissing-variable-declarations
to the global set of warning options in the near future.

Another longer-term solution would be to convert these grammars to
"pure" parsers in Bison, to avoid global variables altogether.  Note
that the core grammar is already pure, so this patch did not need to
touch it.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c95ce@eisentraut.org
2024-07-25 09:26:08 +02:00
32d3ed8165 Add path column to pg_backend_memory_contexts view
"path" provides a reliable method of determining the parent/child
relationships between memory contexts.  Previously this could be done in
a non-reliable way by writing a recursive query and joining the "parent"
and "name" columns.  This wasn't reliable as the names were not unique,
which could result in joining to the wrong parent.

To make this reliable, "path" stores an array of numerical identifiers
starting with the identifier for TopLevelMemoryContext.  It contains an
element for each intermediate parent between that and the current context.

Incompatibility: Here we also adjust the "level" column to make it
1-based rather than 0-based.  A 1-based level provides a convenient way
to access elements in the "path" array. e.g. path[level] gives the
identifier for the current context.

Identifiers are not stable across multiple evaluations of the view.  In
an attempt to make these more stable for ad-hoc queries, the identifiers
are assigned breadth-first.  Contexts closer to TopLevelMemoryContext
are less likely to change between queries and during queries.

Author: Melih Mutlu <m.melihmutlu@gmail.com>
Discussion: https://postgr.es/m/CAGPVpCThLyOsj3e_gYEvLoHkr5w=tadDiN_=z2OwsK3VJppeBA@mail.gmail.com
Reviewed-by: Andres Freund, Stephen Frost, Atsushi Torikoshi,
Reviewed-by: Michael Paquier, Robert Haas, David Rowley
2024-07-25 15:03:28 +12:00
3dd637f3d5 Reset relhassubclass upon attaching table as a partition
We don't allow inheritance parents as partitions, and have checks to
prevent this; but if a table _was_ in the past an inheritance parents
and all their children are removed, the pg_class.relhassubclass flag
may remain set, which confuses the partition pruning code (most
obviously, it results in an assertion failure; in production builds it
may be worse.)

Fix by resetting relhassubclass on attach.

Backpatch to all supported versions.

Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/18550-d5e047e9a897a889@postgresql.org
2024-07-24 12:38:18 +02:00
f6bef362ca Refactor tidstore.c iterator buffering.
Previously, TidStoreIterateNext() would expand the set of offsets for
each block into an internal buffer that it overwrote each time.  In
order to be able to collect the offsets for multiple blocks before
working with them, change the contract.  Now, the offsets are obtained
by a separate call to TidStoreGetBlockOffsets(), which can be called at
a later time.  TidStoreIteratorResult objects are safe to copy and store
in a queue.

Reviewed-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/CAAKRu_bbkmwAzSBgnezancgJeXrQZXy4G4kBTd+5=cr86H5yew@mail.gmail.com
2024-07-24 17:32:35 +12:00
1462aad2e4 Allow altering of two_phase option of a SUBSCRIPTION.
The two_phase option is controlled by both the publisher (as a slot
option) and the subscriber (as a subscription option), so the slot option
must also be modified.

Changing the 'two_phase' option for a subscription from 'true' to 'false'
is permitted only when there are no pending prepared transactions
corresponding to that subscription. Otherwise, the changes of already
prepared transactions can be replicated again along with their corresponding
commit leading to duplicate data or errors.

To avoid data loss, the 'two_phase' option for a subscription can only be
changed from 'false' to 'true' once the initial data synchronization is
completed. Therefore this is performed later by the logical replication worker.

Author: Hayato Kuroda, Ajin Cherian, Amit Kapila
Reviewed-by: Peter Smith, Hou Zhijie, Amit Kapila, Vitaly Davydov, Vignesh C
Discussion: https://postgr.es/m/8fab8-65d74c80-1-2f28e880@39088166
2024-07-24 10:13:36 +05:30
991f8cf8ab Detect integer overflow in array_set_slice().
When provided an empty initial array, array_set_slice() fails to
check for overflow when computing the new array's dimensions.
While such overflows are ordinarily caught by ArrayGetNItems(),
commands with the following form are accepted:

	INSERT INTO t (i[-2147483648:2147483647]) VALUES ('{}');

To fix, perform the hazardous computations using overflow-detecting
arithmetic routines.  As with commit 18b585155a, the added test
cases generate errors that include a platform-dependent value, so
we again use psql's VERBOSITY parameter to suppress printing the
message text.

Reported-by: Alexander Lakhin
Author: Joseph Koshakow
Reviewed-by: Jian He
Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com
Backpatch-through: 12
2024-07-23 21:59:02 -05:00
f68d85bf69 ldapurl is supported with simple bind
The docs currently imply that ldapurl is for search+bind only, but
that's not true.  Rearrange the docs to cover this better.

Add a test ldapurl with simple bind.  This was previously allowed but
unexercised, and now that it's documented it'd be good to pin the
behavior.

Improve error when mixing LDAP bind modes.  The option names had gone
stale; replace them with a more general statement.

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://www.postgresql.org/message-id/flat/CAOYmi+nyg9gE0LeP=xQ3AgyQGR=5ZZMkVVbWd0uR8XQmg_dd5Q@mail.gmail.com
2024-07-23 10:17:55 +02:00
581df21487 Fix rowcount estimate for gather (merge) paths
In the case of a parallel plan, when computing the number of tuples
processed per worker, we divide the total number of tuples by the
parallel_divisor obtained from get_parallel_divisor(), which accounts
for the leader's contribution in addition to the number of workers.

Accordingly, when estimating the number of tuples for gather (merge)
nodes, we should multiply the number of tuples per worker by the same
parallel_divisor to reverse the division.  However, currently we use
parallel_workers rather than parallel_divisor for the multiplication.
This could result in an underestimation of the number of tuples for
gather (merge) nodes, especially when there are fewer than four
workers.

This patch fixes this issue by using the same parallel_divisor for the
multiplication.  There is one ensuing plan change in the regression
tests, but it looks reasonable and does not compromise its original
purpose of testing parallel-aware hash join.

In passing, this patch removes an unnecessary assignment for path.rows
in create_gather_merge_path, and fixes an uninitialized-variable issue
in generate_useful_gather_paths.

No backpatch as this could result in plan changes.

Author: Anthonin Bonnefoy
Reviewed-by: Rafia Sabih, Richard Guo
Discussion: https://postgr.es/m/CAO6_Xqr9+51NxgO=XospEkUeAg-p=EjAWmtpdcZwjRgGKJ53iA@mail.gmail.com
2024-07-23 10:33:26 +09:00
efcbb76efe Revert "Test that vacuum removes tuples older than OldestXmin"
This reverts commit aa607980aee08416211f003ab41aa750f5559712.

This test proved to be unstable on the buildfarm, timing out before the
standby could catch up on 32-bit machines where more rows were required
and failing to reliably trigger multiple index vacuum rounds on 64-bit
machines where fewer rows should be required.

Because the instability is only known to be present on versions of
Postgres with TIDStore used for dead TID storage by vacuum, this is only
being reverted on master and REL_17_STABLE.

As having this coverage may be valuable, there is a discussion on the
thread of possible ways to stabilize the test. If that happens, a fixed
test can be committed again.

Backpatch-through: 17
Reported-by: Tom Lane

Discussion: https://postgr.es/m/614152.1721580711%40sss.pgh.pa.us
2024-07-22 16:58:15 -04:00
5d2e1cc117 Replace some strtok() with strsep()
strtok() considers adjacent delimiters to be one delimiter, which is
arguably the wrong behavior in some cases.  Replace with strsep(),
which has the right behavior: Adjacent delimiters create an empty
token.

Affected by this are parsing of:

- Stored SCRAM secrets
  ("SCRAM-SHA-256$<iterations>:<salt>$<storedkey>:<serverkey>")

- ICU collation attributes
  ("und@colStrength=primary;colCaseLevel=yes") for ICU older than
  version 54

- PG_COLORS environment variable
  ("error=01;31:warning=01;35:note=01;36:locus=01")

- pg_regress command-line options with comma-separated list arguments
  (--dbname, --create-role) (currently only used pg_regress_ecpg)

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: David Steele <david@pgmasters.net>
Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c3944222a@eisentraut.org
2024-07-22 15:45:46 +02:00
7e187a7386 Fix unstable test in select_parallel.sql
One test case added in 22d946b0f verifies the plan of a non-parallel
nestloop join.  The planner's choice of join order is arbitrary, and
slight variations in underlying statistics could result in a different
displayed plan.  To stabilize the test result, here we enforce the
join order using a lateral join.

While here, modify the test case to verify that parallel nestloop join
is not generated if the inner path is not parallel-safe, which is what
we wanted to test in 22d946b0f.

Reported-by: Alexander Lakhin as per buildfarm
Author: Richard Guo
Discussion: https://postgr.es/m/7c09a439-e48d-5460-cfa0-a371b1a57066@gmail.com
2024-07-22 11:29:21 +09:00
220003b9b9 Correctly check updatability of columns targeted by INSERT...DEFAULT.
If a view has some updatable and some non-updatable columns, we failed
to verify updatability of any columns for which an INSERT or UPDATE
on the view explicitly specifies a DEFAULT item (unless the view has
a declared default for that column, which is rare anyway, and one
would almost certainly not write one for a non-updatable column).
This would lead to an unexpected "attribute number N not found in
view targetlist" error rather than the intended error.

Per bug #18546 from Alexander Lakhin.  This bug is old, so back-patch
to all supported branches.

Discussion: https://postgr.es/m/18546-84a292e759a9361d@postgresql.org
2024-07-20 13:40:15 -04:00
22b0ccd65d Add overflow checks to money type.
None of the arithmetic functions for the the money type handle
overflow.  This commit introduces several helper functions with
overflow checking and makes use of them in the money type's
arithmetic functions.

Fixes bug #18240.

Reported-by: Alexander Lakhin
Author: Joseph Koshakow
Discussion: https://postgr.es/m/18240-c5da758d7dc1ecf0%40postgresql.org
Discussion: https://postgr.es/m/CAAvxfHdBPOyEGS7s%2Bxf4iaW0-cgiq25jpYdWBqQqvLtLe_t6tw%40mail.gmail.com
Backpatch-through: 12
2024-07-19 11:52:32 -05:00
aa607980ae Test that vacuum removes tuples older than OldestXmin
If vacuum fails to prune a tuple killed before OldestXmin, it will
decide to freeze its xmax and later error out in pre-freeze checks.

Add a test reproducing this scenario to the recovery suite which creates
a table on a primary, updates the table to generate dead tuples for
vacuum, and then, during the vacuum, uses a replica to force
GlobalVisState->maybe_needed on the primary to move backwards and
precede the value of OldestXmin set at the beginning of vacuuming the
table.

This commit is separate from the fix in case there are test stability
issues.

Author: Melanie Plageman
Reviewed-by: Peter Geoghegan
Discussion: https://postgr.es/m/CAAKRu_apNU2MPBK96V%2BbXjTq0RiZ-%3DA4ZTaysakpx9jxbq1dbQ%40mail.gmail.com
2024-07-19 12:04:11 -04:00
3a137ab7e5 Add more test coverage for jsonpath "$.*" with arrays
There was no coverage for the code path to unwrap an array before
applying ".*" to it, so add tests to provide more coverage for both
objects and arrays.

This shows, for example, that no results are returned for an array of
scalars, and what results are returned when the array contains an
object.  A few more scenarios are covered with the strict/lax modes and
the operator "@?".

Author: David Wheeler
Reported-by: David G. Johnston, Stepan Neretin
Discussion: https://postgr.es/m/A95346F9-6147-46E0-809E-532A485D71D6@justatheory.com
2024-07-19 14:17:56 +09:00
a0a5869a85 Add INJECTION_POINT_CACHED() to run injection points directly from cache
This new macro is able to perform a direct lookup from the local cache
of injection points (refreshed each time a point is loaded or run),
without touching the shared memory state of injection points at all.

This works in combination with INJECTION_POINT_LOAD(), and it is better
than INJECTION_POINT() in a critical section due to the fact that it
would avoid all memory allocations should a concurrent detach happen
since a LOAD(), as it retrieves a callback from the backend-private
memory.

The documentation is updated to describe in more details how to use this
new macro with a load.  Some tests are added to the module
injection_points based on a new SQL function that acts as a wrapper of
INJECTION_POINT_CACHED().

Based on a suggestion from Heikki Linnakangas.

Author: Heikki Linnakangas, Michael Paquier
Discussion: https://postgr.es/m/58d588d0-e63f-432f-9181-bed29313dece@iki.fi
2024-07-18 09:50:41 +09:00
f2a0d5808c Avoid error in recovery test if history file is not yet present
Error was detected when testing use of libpq sessions instead of psql
for polling queries.

Discussion: https://postgr.es/m/e86b6d2d-20d8-4ac9-9a98-165fff7db886@dunslane.net

Backpatch to all live branches
2024-07-17 10:44:20 -04:00
4b74ebf726 When creating materialized views, use REFRESH to load data.
Previously, CREATE MATERIALIZED VIEW ... WITH DATA populated the MV
the same way as CREATE TABLE ... AS.

Instead, reuse the REFRESH logic, which locks down security-restricted
operations and restricts the search_path. This reduces the chance that
a subsequent refresh will fail.

Reported-by: Noah Misch
Backpatch-through: 17
Discussion: https://postgr.es/m/20240630222344.db.nmisch@google.com
2024-07-16 15:41:29 -07:00
49546ae9c7 Adjust recently added test for pg_signal_autovacuum role
This test was added by commit d2b74882ca, but fails if
log_error_verbosity is set to verbose. Adjust the regex that checks the
error message to allow for it containing an SQL status code.
2024-07-16 10:05:48 -04:00
d2b74882ca Add tap test for pg_signal_autovacuum role
This commit provides testig coverage for ccd38024bc3c, checking that a
role granted pg_signal_autovacuum_worker is able to stop a vacuum
worker.

An injection point with a wait is placed at the beginning of autovacuum
worker startup to make sure that a worker is still alive when sending
and processing the signal sent.

Author: Anthony Leung, Michael Paquier, Kirill Reshke
Reviewed-by: Andrey Borodin, Nathan Bossart
Discussion: https://postgr.es/m/CALdSSPiQPuuQpOkF7x0g2QkA5eE-3xXt7hiJFvShV1bHKDvf8w@mail.gmail.com
2024-07-16 10:05:46 +09:00
4e5d6c4091 Fix unstable tests in partition_merge.sql and partition_split.sql.
The tests added by commit c086896625 were unstable due to
missing schema names when checking pg_tables and pg_indexes.

Backpatch to v17.

Reported by buildfarm.
2024-07-15 14:11:13 +09:00
c086896625 Fix tablespace handling in MERGE/SPLIT partition commands.
As commit ca4103025d stated, new partitions without a specified tablespace
should inherit the parent relation's tablespace. However, previously,
ALTER TABLE MERGE PARTITIONS and ALTER TABLE SPLIT PARTITION commands
always created new partitions in the default tablespace, ignoring
the parent's tablespace. This commit ensures new partitions inherit
the parent's tablespace.

Backpatch to v17 where these commands were introduced.

Author: Fujii Masao
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/abaf390b-3320-40a5-8815-ef476db5cfe7@oss.nttdata.com
2024-07-15 13:11:51 +09:00
069d0ff022 Check lateral references within PHVs for memoize cache keys
If we intend to generate a Memoize node on top of a path, we need
cache keys of some sort.  Currently we search for the cache keys in
the parameterized clauses of the path as well as the lateral_vars of
its parent.  However, it turns out that this is not sufficient because
there might be lateral references derived from PlaceHolderVars, which
we fail to take into consideration.

This oversight can cause us to miss opportunities to utilize the
Memoize node.  Moreover, in some plans, failing to recognize all the
cache keys could result in performance regressions.  This is because
without identifying all the cache keys, we would need to purge the
entire cache every time we get a new outer tuple during execution.

This patch fixes this issue by extracting lateral Vars from within
PlaceHolderVars and subsequently including them in the cache keys.

In passing, this patch also includes a comment clarifying that Memoize
nodes are currently not added on top of join relation paths.  This
explains why this patch only considers PlaceHolderVars that are due to
be evaluated at baserels.

Author: Richard Guo
Reviewed-by: Tom Lane, David Rowley, Andrei Lepikhov
Discussion: https://postgr.es/m/CAMbWs48jLxn0pAPZpJ50EThZ569Xrw+=4Ac3QvkpQvNszbeoNg@mail.gmail.com
2024-07-15 10:26:33 +09:00
f96c2c7278 Avoid unhelpful internal error for incorrect recursive-WITH queries.
checkWellFormedRecursion would issue "missing recursive reference"
if a WITH RECURSIVE query contained a single self-reference but
that self-reference was inside a top-level WITH, ORDER BY, LIMIT,
etc, rather than inside the second arm of the UNION as expected.
We already intended to throw more-on-point errors for such cases,
but those error checks must be done before examining the UNION arm
in order to have the desired results.  So this patch need only
move some code (and improve the comments).

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

Discussion: https://postgr.es/m/18536-0a342ec07901203e@postgresql.org
2024-07-14 13:49:46 -04:00
d5e6891502 Fix new assertion for MERGE view_name ... DO NOTHING.
Such queries don't expand automatically updatable views, and ModifyTable
uses the wholerow attribute unconditionally.  The user-visible behavior
is fine, so change to more-specific assertions.  Commit
d5f788b41dc2cbdde6e7694c70dda54d829a5ed5 added the wrong assertion.
Back-patch to v17, where commit 5f2e179bd31e5f5803005101eb12a8d7bf8db8f3
introduced MERGE view_name.

Reported by Alexander Lakhin.

Discussion: https://postgr.es/m/e4b40a88-c134-6926-3196-bc4501cb87a2@gmail.com
2024-07-13 08:09:33 -07:00
7102070329 Don't lose partitioned table reltuples=0 after relhassubclass=f.
ANALYZE sets relhassubclass=f when a partitioned table no longer has
partitions.  An ANALYZE doing that proceeded to apply the inplace update
of pg_class.reltuples to the old pg_class tuple instead of the new
tuple, losing that reltuples=0 change if the ANALYZE committed.
Non-partitioning inheritance trees were unaffected.  Back-patch to v14,
where commit 375aed36ad83f0e021e9bdd3a0034c0c992c66dc introduced
maintenance of partitioned table pg_class.reltuples.

Reported by Alexander Lakhin.

Discussion: https://postgr.es/m/a295b499-dcab-6a99-c06e-01cf60593344@gmail.com
2024-07-13 08:09:33 -07:00
291c420747 Use diff --strip-trailing-cr in pg_regress.c
This was reverted in commit c194de0713. However with a correct
collate.windows.win1252.out we can now re-enable it.

Discussion: https://postgr.es/m/CAN55FZ1objLz3Vn5Afu4ojNESMQpxjxKcp2q18yrKF4eKMLENg@mail.gmail.com
2024-07-12 18:25:11 -04:00
74e12db19c Add ORDER BY to new test query
Per buildfarm.
2024-07-12 13:44:19 +02:00
8391779138 Fix ALTER TABLE DETACH for inconsistent indexes
When a partitioned table has an index that doesn't support a constraint,
but a partition has an equivalent index that does, then a DETACH
operation would misbehave: a crash in assertion-enabled systems (because
we fail to find the constraint in the parent that we expect to), or a
broken coninhcount value (-1) in production systems (because we blindly
believe that we've successfully detached the parent).

While we should reject an ATTACH of a partition with such an index, we
have failed to do so in existing releases, so adding an error in stable
releases might break the (unlikely) existing applications that rely on
this behavior.  At this point I don't even want to reject them in
master, because it'd break pg_upgrade if such databases exist, and there
would be no easy way to fix existing databases without expensive index
rebuilds.

(Later on we could add ALTER TABLE ... ADD CONSTRAINT USING INDEX to
partitioned tables, which would allow the user to fix such patterns.  At
that point we could add more restrictions to prevent the problem from
its root.)

Also, add a test case that leaves one table in this condition, so that
we can verify that pg_upgrade continues to work if we later decide to
change the policy on the master branch.

Backpatch to all supported branches.

Co-authored-by: Tender Wang <tndrwang@gmail.com>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/18500-62948b6fe5522f56@postgresql.org
2024-07-12 12:54:01 +02:00
22d946b0f8 Consider materializing the cheapest inner path in parallel nestloop
When generating non-parallel nestloop paths for each available outer
path, we always consider materializing the cheapest inner path if
feasible.  Similarly, in this patch, we also consider materializing
the cheapest inner path when building partial nestloop paths.  This
approach potentially reduces the need to rescan the inner side of a
partial nestloop path for each outer tuple.

Author: Tender Wang
Reviewed-by: Richard Guo, Robert Haas, David Rowley, Alena Rybakina
Reviewed-by: Tomasz Rybak, Paul Jungwirth, Yuki Fujii
Discussion: https://postgr.es/m/CAHewXNkPmtEXNfVQMou_7NqQmFABca9f4etjBtdbbm0ZKDmWvw@mail.gmail.com
2024-07-12 11:16:43 +09:00
0d8bd0a72e Improve logical replication connection-failure messages.
These messages mostly said "could not connect to the publisher: %s"
which is lacking context.  Add some verbiage to indicate which
subscription or worker process is failing.

Nisha Moond

Discussion: https://postgr.es/m/CABdArM7q1=zqL++cYd0hVMg3u_tc0S=0Of=Um-KvDhLony0cSg@mail.gmail.com
2024-07-11 13:21:13 -04:00
a0f1fce80c Add min and max aggregates for composite types (records).
Like min/max for arrays, these are just thin wrappers around
the existing btree comparison function for records.

Aleksander Alekseev

Discussion: https://postgr.es/m/CAO=iB8L4WYSNxCJ8GURRjQsrXEQ2-zn3FiCsh2LMqvWq2WcONg@mail.gmail.com
2024-07-11 11:50:50 -04:00
c194de0713 Change pg_regress.c back to using diff -w on Windows
This partially reverts commit 628c1d1f2c.

It appears that there are non line-end differences in some regression
tests on Windows. To keep the buildfarm and CI clients happy, change
this back for now, pending further investigation.

Per reports from Tatsuo Ishii and Nazir Bilal Yavuz.
2024-07-11 09:34:27 -04:00
628c1d1f2c Use diff's --strip-trailing-cr flag where appropriate on Windows
Test result files might be checked out using Unix or Windows style line
endings, depening on git flags, so on Windows we use the
--strip-trailing-cr flag to tell diff to ignore line endings
differences.

The flag is added to the diff invocation for the test_json_parser module
tests and the pg_bsd_indent tests. in pg_regress.c we replace the
current use of the "-w" flag, which ignore all white space differences,
with this one which only ignores line end differences.

Discussion: https://postgr.es/m/20240707052030.r77hbdkid3mwksop@awork3.anarazel.de
2024-07-10 09:53:47 -04:00