users/c2main/postgres.git
13 months agoAllow planner to use Merge Append to efficiently implement UNION
David Rowley [Mon, 25 Mar 2024 01:31:14 +0000 (14:31 +1300)]
Allow planner to use Merge Append to efficiently implement UNION

Until now, UNION queries have often been suboptimal as the planner has
only ever considered using an Append node and making the results unique
by either using a Hash Aggregate, or by Sorting the entire Append result
and running it through the Unique operator.  Both of these methods
always require reading all rows from the union subqueries.

Here we adjust the union planner so that it can request that each subquery
produce results in target list order so that these can be Merge Appended
together and made unique with a Unique node.  This can improve performance
significantly as the union child can make use of the likes of btree
indexes and/or Merge Joins to provide the top-level UNION with presorted
input.  This is especially good if the top-level UNION contains a LIMIT
node that limits the output rows to a small subset of the unioned rows as
cheap startup plans can be used.

Author: David Rowley
Reviewed-by: Richard Guo, Andy Fan
Discussion: https://postgr.es/m/CAApHDvpb_63XQodmxKUF8vb9M7CxyUyT4sWvEgqeQU-GB7QFoQ@mail.gmail.com

13 months agoreindexdb: Add the index-level REINDEX with multiple jobs
Alexander Korotkov [Mon, 25 Mar 2024 00:07:14 +0000 (02:07 +0200)]
reindexdb: Add the index-level REINDEX with multiple jobs

Straight-forward index-level REINDEX is not supported with multiple jobs as
we cannot control the concurrent processing of multiple indexes depending on
the same relation.  Instead, we dedicate the whole table to certain reindex
job.  Thus, if indexes in the lists belong to different tables, that gives us
a fair level of parallelism.

This commit teaches get_parallel_object_list() to fetch table names for
indexes in the case of index-level REINDEX.  The same tables are grouped
together in the output order, and the list of indexes is also rebuilt to
match that order.  Later during processingof that list, we push indexes
belonging to the same table into the same job.

Discussion: https://postgr.es/m/CACG%3DezZU_VwDi-1PN8RUSE6mcYG%2BYx1NH_rJO4%2BKe-mKqLp%3DNw%40mail.gmail.com
Author: Maxim Orlov, Svetlana Derevyanko, Alexander Korotkov
Reviewed-by: Michael Paquier
13 months agoFix convert_case(), introduced in 5c40364dd6.
Jeff Davis [Sun, 24 Mar 2024 23:28:34 +0000 (16:28 -0700)]
Fix convert_case(), introduced in 5c40364dd6.

Check source length before checking for NUL terminator to avoid
reading one byte past the string end. Also fix unreachable bug when
caller does not expect NUL-terminated result.

Add unit test coverage of convert_case() in case_test.c, which makes
it easier to reproduce the valgrind failure.

Discussion: https://postgr.es/m/7a9fd36d-7a38-4dc2-e676-fc939491a95a@gmail.com
Reported-by: Alexander Lakhin
13 months agodoc: Clarify requirements for SET ROLE.
Nathan Bossart [Sun, 24 Mar 2024 20:23:55 +0000 (15:23 -0500)]
doc: Clarify requirements for SET ROLE.

Since commit 3d14e171e9, SET ROLE has required the current session
user to have membership with the SET option in the target role, but
the SET ROLE documentation only mentions the membership
requirement.  This commit adds this important detail to the SET
ROLE page.

Reviewed-by: Robert Haas
Discussion: https://postgr.es/m/CA%2BRLCQysHtME0znk2KUMJN343ksboSRQSU-hCnOjesX6VK300Q%40mail.gmail.com
Backpatch-through: 16

13 months agoAllow more cases to pass the unsafe-use-of-new-enum-value restriction.
Tom Lane [Sun, 24 Mar 2024 18:30:29 +0000 (14:30 -0400)]
Allow more cases to pass the unsafe-use-of-new-enum-value restriction.

Up to now we've rejected cases like

BEGIN;
CREATE TYPE rainbow AS ENUM ();
ALTER TYPE rainbow ADD VALUE 'red';
-- use the value 'red', perhaps in a constraint or index
COMMIT;

The concern is that the uncommitted enum value 'red' might get into
an index and then break the index if we roll back the ALTER ADD.
If the ALTER is in the same transaction as the CREATE then it's really
perfectly safe, but we weren't taking the trouble to identify that.

pg_dump in binary-upgrade mode will emit enum definitions that look
like the above, which up to now didn't fall foul of the unsafe-usage
check because we processed each restore command as a separate
transaction.  However an upcoming patch proposes to bundle the restore
commands into large transactions to reduce XID consumption during
pg_upgrade, and that makes this behavior a problem.

To fix, remember the OIDs of enum types created in the current
transaction, and allow use of enum values that are added to one later
in the same transaction.  To do this fully correctly in the presence
of subtransactions, we'd have to track subtransaction nesting level of
the CREATE and do maintenance work at every subsequent subtransaction
exit.  That seems expensive, and we don't need it to satisfy pg_dump's
usage.  Hence, apply the additional optimization only when the CREATE
and ALTER are at outermost transaction level.

Patch by me, reviewed by Andrew Dunstan

Discussion: https://postgr.es/m/1548468.1711220438@sss.pgh.pa.us

13 months agoRelease PQconninfoOptions array in GetDbnameFromConnectionOptions().
Tom Lane [Sun, 24 Mar 2024 16:31:05 +0000 (12:31 -0400)]
Release PQconninfoOptions array in GetDbnameFromConnectionOptions().

It wasn't getting freed in one code path, which Coverity identified as
a resource leak.  It's probably of little consequence, but re-ordering
the code into the correct sequence is no more work than dismissing the
complaint.  Minor oversight in commit a145f424d.

While here, improve the unreasonably clunky coding of
FindDbnameInConnParams: use of an output parameter is unnecessary
and prone to uninitialized-variable problems.

13 months agoRelease temporary array in check_for_data_types_usage().
Tom Lane [Sun, 24 Mar 2024 16:13:35 +0000 (12:13 -0400)]
Release temporary array in check_for_data_types_usage().

Coverity identified this as a resource leak.  It's surely of no
consequence given that the function is called only once per run, but
freeing the storage is no more work than dismissing the complaint.
Minor oversight in commit 347758b12.

13 months agoci: freebsd repartition script didn't copy .git directory
Peter Eisentraut [Sun, 24 Mar 2024 07:41:14 +0000 (08:41 +0100)]
ci: freebsd repartition script didn't copy .git directory

We need a slightly different "cp" incantation to make sure top-level
"dot" files, such as ".git", are also copied.

This is relevant for example if a script wants to execute a git
command.  This currently does not happen, but it has come up while
testing other patches.

Reviewed-by: Tristan Partin <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/40e80f77-a294-4f29-a16f-e21bc7bc75fc%40eisentraut.org

13 months agoAdd temporal FOREIGN KEY contraints
Peter Eisentraut [Sun, 24 Mar 2024 06:37:13 +0000 (07:37 +0100)]
Add temporal FOREIGN KEY contraints

Add PERIOD clause to foreign key constraint definitions.  This is
supported for range and multirange types.  Temporal foreign keys check
for range containment instead of equality.

This feature matches the behavior of the SQL standard temporal foreign
keys, but it works on PostgreSQL's native ranges instead of SQL's
"periods", which don't exist in PostgreSQL (yet).

Reference actions ON {UPDATE,DELETE} {CASCADE,SET NULL,SET DEFAULT}
are not supported yet.

Author: Paul A. Jungwirth <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Reviewed-by: jian he <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

13 months agoamcheck: Normalize index tuples containing uncompressed varlena
Alexander Korotkov [Sat, 23 Mar 2024 21:00:06 +0000 (23:00 +0200)]
amcheck: Normalize index tuples containing uncompressed varlena

It might happen that the varlena value wasn't compressed by index_form_tuple()
due to current storage parameters.  If compression is currently enabled, we
need to compress such values to match index tuple coming from the heap.

Backpatch to all supported versions.

Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru
Author: Andrey Borodin
Reviewed-by: Alexander Lakhin, Michael Zhilin, Jian He, Alexander Korotkov
Backpatch-through: 12

13 months agoamcheck: Support for different header sizes of short varlena datum
Alexander Korotkov [Sat, 23 Mar 2024 20:59:56 +0000 (22:59 +0200)]
amcheck: Support for different header sizes of short varlena datum

In the heap, tuples may contain short varlena datum with both 1B header and 4B
headers.  But the corresponding index tuple should always have such varlena's
with 1B headers.  So, for fingerprinting, we need to convert.

Backpatch to all supported versions.

Discussion: https://postgr.es/m/flat/7bdbe559-d61a-4ae4-a6e1-48abdf3024cc%40postgrespro.ru
Author: Michael Zhilin
Reviewed-by: Alexander Lakhin, Andrey Borodin, Jian He, Alexander Korotkov
Backpatch-through: 12

13 months agoRevert "Add notBefore and notAfter to SSL cert info display"
Daniel Gustafsson [Fri, 22 Mar 2024 21:58:41 +0000 (22:58 +0100)]
Revert "Add notBefore and notAfter to SSL cert info display"

This reverts commit 6acb0a628eccab8764e0306582c2b7e2a1441b9b since
LibreSSL didn't support ASN1_TIME_diff until OpenBSD 7.1, leaving
the older OpenBSD animals in the buildfarm complaining.

Per plover in the buildfarm.

Discussion: https://postgr.es/m/F0DF7102-192D-4C21-96AE-9A01AE153AD1@yesql.se

13 months agoUse a hash table for catcache.c's CatCList objects.
Tom Lane [Fri, 22 Mar 2024 21:13:53 +0000 (17:13 -0400)]
Use a hash table for catcache.c's CatCList objects.

Up to now, all of the "catcache list" objects within a catalog cache
were just chained together on a single dlist, requiring O(N) time to
search.  Remarkably, we've not had serious performance problems with
that so far; but we got a complaint of a bad performance regression
from v15 in a case with a large number of roles in the system, which
traced down to O(N^2) total time when we probed N catcache lists.

Replace that data structure with a hashtable having an enlargeable
number of dlists, in an exactly parallel way to the data structure
we've used for years for the plain CatCTup cache members.  The extra
cost of maintaining a hash table seems negligible, since we were
already computing a hash value for list searches.

Normally this'd be HEAD-only material, but in view of the performance
regression it seems advisable to back-patch into v16.  In the v16
version of the patch, leave the dead cc_lists field where it is and
add the new fields at the end of struct catcache, to avoid possible
ABI breakage in case any external code is looking at these structs.
(We assume no external code is actually allocating new catcache
structs.)

Per report from alex work.

Discussion: https://postgr.es/m/CAGvXd3OSMbJQwOSc-Tq-Ro1CAz=vggErdSG7pv2s6vmmTOLJSg@mail.gmail.com

13 months agoAdd notBefore and notAfter to SSL cert info display
Daniel Gustafsson [Fri, 22 Mar 2024 20:25:25 +0000 (21:25 +0100)]
Add notBefore and notAfter to SSL cert info display

This adds the X509 attributes notBefore and notAfter to sslinfo
as well as pg_stat_ssl to allow verifying and identifying the
validity period of the current client certificate. OpenSSL has
APIs for extracting notAfter and notBefore, but they are only
supported in recent versions so we have to calculate the dates
by hand in order to make this work for the older versions of
OpenSSL that we still support.

Original patch by Cary Huang with additional hacking by Jacob
and myself.

Author: Cary Huang <[email protected]>
Co-author: Jacob Champion <[email protected]>
Co-author: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/182b8565486.10af1a86f158715.2387262617218380588@highgo.ca

13 months agoFix an oversight in refactoring in 06b10f80ba4.
Alexander Korotkov [Fri, 22 Mar 2024 13:25:53 +0000 (15:25 +0200)]
Fix an oversight in refactoring in 06b10f80ba4.

It was against intended skipping prechecking keys optimization in the
first page of range queries to not influence point queries performance.

Reported-by: Anton Melnikov
Discussion: https://postgr.es/m/30cd7524-b9f1-4cf8-9c4a-223eb2e34441%40postgrespro.ru
Author: Pavel Borisov

13 months agoDo not output actual value of location fields in node serialization by default
Peter Eisentraut [Fri, 22 Mar 2024 08:13:35 +0000 (09:13 +0100)]
Do not output actual value of location fields in node serialization by default

This changes nodeToString() to not output the actual value of location
fields in nodes, but instead it writes -1.  This mirrors the fact that
stringToNode() also does not read location field values but always
stores -1.

For most uses of nodeToString(), which is to store nodes in catalog
fields, this is more useful.  We don't store original query texts in
catalogs, so any lingering query location values are not meaningful.

For debugging purposes, there is a new nodeToStringWithLocations(),
which mirrors the existing stringToNodeWithLocations().  This is used
for WRITE_READ_PARSE_PLAN_TREES and nodes/print.c functions, which
covers all the debugging uses.

Reviewed-by: Matthias van de Meent <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAEze2WgrCiR3JZmWyB0YTc8HV7ewRdx13j0CqD6mVkYAW+SFGQ@mail.gmail.com

13 months agoTrack invalidation_reason in pg_replication_slots.
Amit Kapila [Fri, 22 Mar 2024 08:22:05 +0000 (13:52 +0530)]
Track invalidation_reason in pg_replication_slots.

Till now, the reason for replication slot invalidation is not tracked
directly in pg_replication_slots. A recent commit 007693f2a3 added
'conflict_reason' to show the reasons for slot conflict/invalidation, but
only for logical slots.

This commit adds a new column 'invalidation_reason' to show invalidation
reasons for both physical and logical slots. And, this commit also turns
'conflict_reason' text column to 'conflicting' boolean column (effectively
reverting commit 007693f2a3). The 'conflicting' column is true for
invalidation reasons 'rows_removed' and 'wal_level_insufficient' because
those make the slot conflict with recovery. When 'conflicting' is true,
one can now look at the new 'invalidation_reason' column for the reason
for the logical slot's conflict with recovery.

The new 'invalidation_reason' column will also be useful to track other
invalidation reasons in the future commit.

Author: Bharath Rupireddy
Reviewed-by: Bertrand Drouvot, Amit Kapila, Shveta Malik
Discussion: https://www.postgresql.org/message-id/ZfR7HuzFEswakt/a%40ip-10-97-1-34.eu-west-3.compute.internal
Discussion: https://www.postgresql.org/message-id/CALj2ACW4aUe-_uFQOjdWCEN-xXoLGhmvRFnL8SNw_TZ5nJe+aw@mail.gmail.com

13 months agoMake RangeTblEntry dump order consistent
Peter Eisentraut [Fri, 22 Mar 2024 06:12:28 +0000 (07:12 +0100)]
Make RangeTblEntry dump order consistent

Put the fields alias and eref earlier in the struct, so that it
matches the order in _outRangeTblEntry()/_readRangeTblEntry().  This
helps if we ever want to fully automate out/read of RangeTblEntry.
Also, it makes dumps in the debugger easier to read in the same way.
Internally, this makes no difference.

Reviewed-by: Andrew Dunstan <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca645@eisentraut.org

13 months agoRemove custom _jumbleRangeTblEntry()
Peter Eisentraut [Fri, 22 Mar 2024 06:12:28 +0000 (07:12 +0100)]
Remove custom _jumbleRangeTblEntry()

This is part of an effort to reduce the number of special cases in the
automatically generated node support functions.

This patch removes _jumbleRangeTblEntry() and instead adds per-field
query_jumble_ignore annotations to match the behavior of the previous
custom code.  The pg_stat_statements test suite has some coverage of
this.  It gets rid of the switch on rtekind; this should be
technically correct, since we do the equal and copy functions like
this also.

The list of fields to jumble has been checked and is considered
correct as of 8b29a119fd.

Reviewed-by: Andrew Dunstan <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca645@eisentraut.org

13 months agoReformat some node comments
Peter Eisentraut [Fri, 22 Mar 2024 06:12:28 +0000 (07:12 +0100)]
Reformat some node comments

Reformat some comments in node field definitions to avoid long lines.
This makes room for per-field annotations.  Similar to 835d476fd2.

Reviewed-by: Andrew Dunstan <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca645@eisentraut.org

13 months agoImprove comment
Peter Eisentraut [Fri, 22 Mar 2024 06:12:28 +0000 (07:12 +0100)]
Improve comment

Clarify that RangeTblEntry.lateral reflects whether LATERAL was
specified in the statement (as opposed to whether lateralness is
implicit).  Also, the list of applicable entry types was incomplete.

Reviewed-by: Andrew Dunstan <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca645@eisentraut.org

13 months agoRemove obsolete comment
Peter Eisentraut [Fri, 22 Mar 2024 06:12:28 +0000 (07:12 +0100)]
Remove obsolete comment

The idea to use a union in the definition of RangeTblEntry is clearly
not being pursued.

Reviewed-by: Andrew Dunstan <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4b27fc50-8cd6-46f5-ab20-88dbaadca645@eisentraut.org

13 months agoAvoid splitting errmsg string to span multiple lines
Amit Langote [Fri, 22 Mar 2024 03:00:14 +0000 (12:00 +0900)]
Avoid splitting errmsg string to span multiple lines

The error message being fixed was added in 6185c9737c.

While at it, add an "a" to the sentence.

Reported-by: Kyotaro Horiguchi <[email protected]>
Discussion: https://postgr.es/m/20240322.095149.895185546948714852.horikyota.ntt%40gmail.com

13 months agoFix dumping role comments when using --no-role-passwords
Daniel Gustafsson [Thu, 21 Mar 2024 22:31:57 +0000 (23:31 +0100)]
Fix dumping role comments when using --no-role-passwords

Commit 9a83d56b38c added support for allowing pg_dumpall to dump
roles without including passwords, which accidentally made dumps
omit COMMENTs on roles.  This fixes it by using pg_authid to get
the comment.

Backpatch to all supported versions. Patch simultaneously written
independently by Álvaro and myself.

Author: Álvaro Herrera <[email protected]>
Author: Daniel Gustafsson <[email protected]>
Reported-by: Bartosz Chroł <[email protected]>
Discussion: https://postgr.es/m/AS8P194MB1271CDA0ADCA7B75FCD8E767F7332@AS8P194MB1271.EURP194.PROD.OUTLOOK.COM
Discussion: https://postgr.es/m/CAEP4nAz9V4H41_4ESJd1Gf0v%3DdevkqO1%3Dpo91jUw-GJSx8Hxqg%40mail.gmail.com
Backpatch-through: v12

13 months agoAdd hash support functions and hash opclass for contrib/ltree.
Tom Lane [Thu, 21 Mar 2024 22:27:49 +0000 (18:27 -0400)]
Add hash support functions and hash opclass for contrib/ltree.

This also enables hash join and hash aggregation on ltree columns.

Tommy Pavlicek, reviewed by jian he

Discussion: https://postgr.es/m/CAEhP-W9ZEoHeaP_nKnPCVd_o1c3BAUvq1gWHrq8EbkNRiS9CvQ@mail.gmail.com

13 months agoAdd TupleTableSlotOps.is_current_xact_tuple() method
Alexander Korotkov [Thu, 21 Mar 2024 21:00:43 +0000 (23:00 +0200)]
Add TupleTableSlotOps.is_current_xact_tuple() method

This allows us to abstract how/whether table AM uses transaction identifiers.
A custom table AM can use a custom slot, which may not store xmin directly,
but determine the tuple belonging to the current transaction in the other way.

Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Reviewed-by: Matthias van de Meent, Mark Dilger, Pavel Borisov
Reviewed-by: Nikita Malakhov, Japin Li
13 months agoAllow table AM tuple_insert() method to return the different slot
Alexander Korotkov [Thu, 21 Mar 2024 21:00:40 +0000 (23:00 +0200)]
Allow table AM tuple_insert() method to return the different slot

This allows table AM to return a native tuple slot even if
VirtualTupleTableSlot is given as an input.  Native tuple slots have knowledge
about system attributes, which could be accessed in the future.
table_multi_insert() method already can modify the input 'slots' array.

Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Reviewed-by: Matthias van de Meent, Mark Dilger, Pavel Borisov
Reviewed-by: Nikita Malakhov, Japin Li
13 months agoAllow table AM to store complex data structures in rd_amcache
Alexander Korotkov [Thu, 21 Mar 2024 21:00:34 +0000 (23:00 +0200)]
Allow table AM to store complex data structures in rd_amcache

The new table AM method free_rd_amcache is responsible for freeing all the
memory related to rd_amcache and setting free_rd_amcache to NULL.  If the new
method is not specified, we still assume rd_amcache to be a single chunk of
memory, which could be just pfree'd.

Discussion: https://postgr.es/m/CAPpHfdurb9ycV8udYqM%3Do0sPS66PJ4RCBM1g-bBpvzUfogY0EA%40mail.gmail.com
Reviewed-by: Matthias van de Meent, Mark Dilger, Pavel Borisov
Reviewed-by: Nikita Malakhov, Japin Li
13 months agodocs: Make claims about the benefits of HOT updates more precise.
Robert Haas [Thu, 21 Mar 2024 16:54:58 +0000 (12:54 -0400)]
docs: Make claims about the benefits of HOT updates more precise.

The old text claims that HOT completely removes old row versions.
It was unclear whether it just meant the tuples themselves, or the
tuples together with their line pointers. If it meant the former,
it was wrong because we can remove dead row versions even when no
HOT updates have occurred, so it's not describing a benefit of HOT.
If it meant the latter, it was wrong because HOT doesn't allow
reclaiming the root tuple's line pointer.

This section does seems like it's intended to be more of an
informal introduction to HOT than a precise technical description
of every detail of how it works, but we still don't want it to
say things that are just not true, so update the text enough
to avoid that.

Patch by me, reviewed by James Coleman (although he would have
preferred more extensive changes) and Shubham Khanna.

Discussion: http://postgr.es/m/CA+TgmobH6DPmR-u--Xgeg8cYUwhDhypNsv38nDrAJyf_xno=TQ@mail.gmail.com

13 months agoRevise the style of a paragraph in README.md.
Nathan Bossart [Thu, 21 Mar 2024 15:16:41 +0000 (10:16 -0500)]
Revise the style of a paragraph in README.md.

Presently, one of the lines in README.md has trailing whitespace,
which was added by commit 363eb05996 to maintain a line break that
was in the non-Markdown version.  Instead of changing
.gitattributes, let's match this paragraph's style with the style
of the following paragraph, thereby removing the trailing
whitespace.

Reported-by: Peter Eisentraut
Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/c0a4e906-c6fe-4519-bd15-28dfcd03fdb2%40eisentraut.org

13 months agoExplicitly require password for SCRAM exchange
Daniel Gustafsson [Thu, 21 Mar 2024 13:45:54 +0000 (14:45 +0100)]
Explicitly require password for SCRAM exchange

This refactors the SASL init flow to set password_needed on the two
SCRAM exchanges currently supported. The code already required this
but was set up in such a way that all SASL exchanges required using
a password, a restriction which may not hold for all exchanges (the
example at hand being the proposed OAuthbearer exchange).

This was extracted from a larger patchset to introduce OAuthBearer
authentication and authorization.

Author: Jacob Champion <[email protected]>
Discussion: https://postgr.es/m/d1b467a78e0e36ed85a09adf979d04cf124a9d4b[email protected]

13 months agoRefactor SASL exchange to return tri-state status
Daniel Gustafsson [Thu, 21 Mar 2024 13:45:46 +0000 (14:45 +0100)]
Refactor SASL exchange to return tri-state status

The SASL exchange callback returned state in to output variables:
done and success.  This refactors that logic by introducing a new
return variable of type SASLStatus which makes the code easier to
read and understand, and prepares for future SASL exchanges which
operate asynchronously.

This was extracted from a larger patchset to introduce OAuthBearer
authentication and authorization.

Author: Jacob Champion <[email protected]>
Discussion: https://postgr.es/m/d1b467a78e0e36ed85a09adf979d04cf124a9d4b[email protected]

13 months agoTemporarily install debugging in partition_prune test
David Rowley [Thu, 21 Mar 2024 08:21:05 +0000 (21:21 +1300)]
Temporarily install debugging in partition_prune test

The buildfarm animal parula has been sporadically failing in the
partition_prune test for the past week or so.  It appears like an
auto-vacuum or auto-analyze has run on one of the partitions of the "ab"
table, causing the plan to change.  This is unexpected as the table is
empty.

Here we install some telemetry to find out if this is the case.  This
also joins in pg_index to see if something has gone wrong with the index
which could result in the planner being unable to use that index.

We can revert this once we've figured out the cause of the plan
instability.

Reported-by: Tom Lane
Investigation-by: Tom Lane
Discussion: https://postgr.es/m/4009739.1710878318%40sss.pgh.pa.us

13 months agoAdd SQL/JSON query functions
Amit Langote [Thu, 21 Mar 2024 08:06:27 +0000 (17:06 +0900)]
Add SQL/JSON query functions

This introduces the following SQL/JSON functions for querying JSON
data using jsonpath expressions:

JSON_EXISTS(), which can be used to apply a jsonpath expression to a
JSON value to check if it yields any values.

JSON_QUERY(), which can be used to to apply a jsonpath expression to
a JSON value to get a JSON object, an array, or a string.  There are
various options to control whether multi-value result uses array
wrappers and whether the singleton scalar strings are quoted or not.

JSON_VALUE(), which can be used to apply a jsonpath expression to a
JSON value to return a single scalar value, producing an error if it
multiple values are matched.

Both JSON_VALUE() and JSON_QUERY() functions have options for
handling EMPTY and ERROR conditions, which can be used to specify
the behavior when no values are matched and when an error occurs
during jsonpath evaluation, respectively.

Author: Nikita Glukhov <[email protected]>
Author: Teodor Sigaev <[email protected]>
Author: Oleg Bartunov <[email protected]>
Author: Alexander Korotkov <[email protected]>
Author: Andrew Dunstan <[email protected]>
Author: Amit Langote <[email protected]>
Author: Peter Eisentraut <[email protected]>
Author: Jian He <[email protected]>

Reviewers have included (in no particular order):

Andres Freund, Alexander Korotkov, Pavel Stehule, Andrew Alsup,
Erik Rijkers, Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson,
Justin Pryzby, Álvaro Herrera, Jian He, Anton A. Melnikov,
Nikita Malakhov, Peter Eisentraut, Tomas Vondra

Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru
Discussion: https://postgr.es/m/20220616233130[email protected]
Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org
Discussion: https://postgr.es/m/CA+HiwqHROpf9e644D8BRqYvaAPmgBZVup-xKMDPk-nd4EpgzHw@mail.gmail.com
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com

13 months agoAllow dbname to be written as part of connstring via pg_basebackup's -R option.
Amit Kapila [Thu, 21 Mar 2024 05:18:59 +0000 (10:48 +0530)]
Allow dbname to be written as part of connstring via pg_basebackup's -R option.

Commit cca97ce6a665 allowed dbname in pg_basebackup connstring and in this
commit we allow it to be written in postgresql.auto.conf when -R option is
used. The database name in the connection string will be used by the
logical replication slot synchronization on standby.

The dbname will be recorded only if specified explicitly in the connection
string or environment variable.

Masahiko Sawada hasn't reviewed the code in detail but endorsed the idea.

Author: Vignesh C, Kuroda Hayato
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/CAB8KJ=hdKdg+UeXhReeHpHA6N6v3e0qFF+ZsPFHk9_ThWKf=2A@mail.gmail.com

13 months agoAdd TIDStore, to store sets of TIDs (ItemPointerData) efficiently.
Masahiko Sawada [Thu, 21 Mar 2024 01:08:42 +0000 (10:08 +0900)]
Add TIDStore, to store sets of TIDs (ItemPointerData) efficiently.

TIDStore is a data structure designed to efficiently store large sets
of TIDs. For TID storage, it employs a radix tree, where the key is
a block number, and the value is a bitmap representing offset
numbers. The TIDStore can be created on a DSA area and used by
multiple backend processes simultaneously.

There are potential future users such as tidbitmap.c, though it's very
likely the interface will need to evolve as we come to understand the
needs of different kinds of users. For example, we can support
updating the offset bitmap of existing values.

Currently, the TIDStore is not used for anything yet, aside from the
test code. But an upcoming patch will use it.

This includes a unit test module, in src/test/modules/test_tidstore.

Co-authored-by: John Naylor
Discussion: https://postgr.es/m/CAD21AoAfOZvmfR0j8VmZorZjL7RhTiQdVttNuC4W-Shdc2a-AA%40mail.gmail.com

13 months agoUn-break genbki.pl's error reporting capabilities.
Tom Lane [Wed, 20 Mar 2024 22:02:50 +0000 (18:02 -0400)]
Un-break genbki.pl's error reporting capabilities.

This essentially reverts commit 69eb643b2, which added a fast path
in Catalog::ParseData, but neglected to preserve the behavior of
adding a line_number field in each hash.  That makes it impossible
for genbki.pl to provide any localization of error reports, which is
bad enough; but actually the affected error reports failed entirely,
producing useless bleats like "use of undefined value in sprintf".

69eb643b2 claimed to get a 15% speedup, but I'm not sure I believe
that: the time to rebuild the bki files changes by less than 1% for
me.  In any case, making debugging of mistakes in .dat files more
difficult would not be justified by even an order of magnitude
speedup here; it's just not that big a chunk of the total build time.

Per report from David Wheeler.  Although it's also broken in v16,
I don't think this is worth a back-patch, since we're very unlikely
to touch the v16 catalog data again.

Discussion: https://postgr.es/m/19238.1710953049@sss.pgh.pa.us

13 months agoAdd to_regtypemod function to extract typemod from a string type name.
Tom Lane [Wed, 20 Mar 2024 21:11:23 +0000 (17:11 -0400)]
Add to_regtypemod function to extract typemod from a string type name.

In combination with to_regtype, this allows converting a string to
the "canonicalized" form emitted by format_type.  That usage requires
parsing the string twice, which is slightly annoying but not really
too expensive.  We considered alternatives such as returning a record
type, but that way was notationally uglier than this, and possibly
less flexible.

Like to_regtype(), we'd rather that this return NULL for any bad
input, but the underlying type-parsing logic isn't yet capable of
not throwing syntax errors.  Adjust the documentation for both
functions to point that out.

In passing, fix up a couple of nearby entries in the System Catalog
Information Functions table that had not gotten the word about our
since-v13 convention for displaying function usage examples.

David Wheeler and Erik Wienhold, reviewed by Pavel Stehule, Jim Jones,
and others.

Discussion: https://postgr.es/m/DF2324CA-2673-4ABE-B382-26B5770B6AA3@justatheory.com

13 months agoAvoid overflow in MaybeRemoveOldWalSummaries().
Nathan Bossart [Wed, 20 Mar 2024 18:31:58 +0000 (13:31 -0500)]
Avoid overflow in MaybeRemoveOldWalSummaries().

This commit limits the maximum value of wal_summary_keep_time to
INT_MAX / SECS_PER_MINUTE to avoid overflow when it is converted to
seconds.  In passing, use the HOURS_PER_DAY, MINS_PER_HOUR, and
SECS_PER_MINUTE macros in the code for this GUC instead of hard-
coding those values.

Discussion: https://postgr.es/m/20240314210010.GA3056455%40nathanxps13

13 months agoInline basic UTF-8 functions.
Jeff Davis [Wed, 20 Mar 2024 16:40:57 +0000 (09:40 -0700)]
Inline basic UTF-8 functions.

Shows a measurable speedup when processing UTF-8 data, such as with
the new builtin collation provider.

Discussion: https://postgr.es/m/163f4e2190cdf67f67016044e503c5004547e5a9[email protected]
Reviewed-by: Peter Eisentraut
13 months agoRevert "Temporary patch to help debug pg_walsummary test failures."
Nathan Bossart [Wed, 20 Mar 2024 16:34:00 +0000 (11:34 -0500)]
Revert "Temporary patch to help debug pg_walsummary test failures."

Thanks to commits ea18eb7d62b6ee30ec08, and 19a829a327, the
002_blocks.pl test now consistently passes, so we can remove this
temporary debugging code.

This reverts commit 5ddf9973477729cf161b4ad0a1efd52f4fea9c88.

Discussion: https://postgr.es/m/20240314210010.GA3056455%40nathanxps13

13 months agoReview wording on tablespaces w.r.t. partitioned tables
Alvaro Herrera [Wed, 20 Mar 2024 14:28:14 +0000 (15:28 +0100)]
Review wording on tablespaces w.r.t. partitioned tables

Remove a redundant comment, and document pg_class.reltablespace properly
in catalogs.sgml.

After commits a36c84c3e4a987259588d0ab and others.

Backpatch to 12.

Discussion: https://postgr.es/m/202403191013[email protected]

13 months agoRework lwlocknames.txt to become lwlocklist.h
Alvaro Herrera [Wed, 20 Mar 2024 10:46:11 +0000 (11:46 +0100)]
Rework lwlocknames.txt to become lwlocklist.h

This way, we can fold the list of lock names to occur in
BuiltinTrancheNames instead of having its own separate array.  This
saves two lines of code in GetLWTrancheName and some space in
BuiltinTrancheNames, as foreseen in commit 74a730631065, as well as
removing the need for a separate lwlocknames.c file.

We still have to build lwlocknames.h using Perl code, which initially I
wanted to avoid, but it gives us the chance to cross-check
wait_event_names.txt.

Discussion: https://postgr.es/m/202401231025[email protected]

13 months agoCatalog domain not-null constraints
Peter Eisentraut [Wed, 20 Mar 2024 08:29:08 +0000 (09:29 +0100)]
Catalog domain not-null constraints

This applies the explicit catalog representation of not-null
constraints introduced by b0e96f3119 for table constraints also to
domain not-null constraints.

Reviewed-by: Aleksander Alekseev <[email protected]>
Reviewed-by: jian he <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org

13 months agoRemove unused PruneState member rel
Heikki Linnakangas [Wed, 20 Mar 2024 08:13:42 +0000 (10:13 +0200)]
Remove unused PruneState member rel

PruneState->rel is no longer being used, so just remove it.

Author: Melanie Plageman <[email protected]>
Discussion: https://www.postgresql.org/message-id/20240320013602.6sypr4cx6sefpemg@liskov

13 months agoReorganize heap_page_prune() function comment
Heikki Linnakangas [Wed, 20 Mar 2024 08:13:39 +0000 (10:13 +0200)]
Reorganize heap_page_prune() function comment

heap_page_prune()'s function header comment didn't explain the
parameters in the same order they appear in the function. Fix that.

Author: Melanie Plageman <[email protected]>
Discussion: https://www.postgresql.org/message-id/20240320013602.6sypr4cx6sefpemg@liskov

13 months agoAdd "--exclude-extension" to pg_dump's options.
Dean Rasheed [Wed, 20 Mar 2024 08:05:44 +0000 (08:05 +0000)]
Add "--exclude-extension" to pg_dump's options.

This option (or equivalently specifying "exclude extension pattern" in
a filter file) allows extensions matching the specified pattern to be
excluded from the dump.

Ayush Vatsa, reviewed by Junwang Zhao, Dean Rasheed, and Daniel
Gustafsson.

Discussion: https://postgr.es/m/CACX+KaP=VgVy9h-EUh598DTu+-fNr1jyEmpghC8rRp9s=w33Kg@mail.gmail.com

13 months agoRemove assertions that some compiler say are tautological
Heikki Linnakangas [Wed, 20 Mar 2024 07:14:51 +0000 (09:14 +0200)]
Remove assertions that some compiler say are tautological

To avoid the compiler warnings:

    launch_backend.c:211:39: warning: comparison of constant 16 with expression of type 'BackendType' (aka 'enum BackendType') is always true [-Wtautological-constant-out-of-range-compare]
    launch_backend.c:233:39: warning: comparison of constant 16 with expression of type 'BackendType' (aka 'enum BackendType') is always true [-Wtautological-constant-out-of-range-compare]

The point of the assertions was to fail more explicitly if someone
adds a new BackendType to the end of the enum, but forgets to add it
to the child_process_kinds array. It was a pretty weak assertion to
begin with, because it wouldn't catch if you added a new BackendType
in the middle of the enum. So let's just remove it.

Per buildfarm member ayu and a few others, spotted by Tom Lane.

Discussion: https://www.postgresql.org/message-id/4119680.1710913067@sss.pgh.pa.us

13 months agoAdd tests for domain-related information schema views
Peter Eisentraut [Wed, 20 Mar 2024 06:08:01 +0000 (07:08 +0100)]
Add tests for domain-related information schema views

Reviewed-by: Aleksander Alekseev <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/9ec24d7b-633d-463a-84c6-7acff769c9e8%40eisentraut.org

13 months agoSupport C.UTF-8 locale in the new builtin collation provider.
Jeff Davis [Tue, 19 Mar 2024 22:24:41 +0000 (15:24 -0700)]
Support C.UTF-8 locale in the new builtin collation provider.

The builtin C.UTF-8 locale has similar semantics to the libc locale of
the same name. That is, code point sort order (fast, memcmp-based)
combined with Unicode semantics for character operations such as
pattern matching, regular expressions, and
LOWER()/INITCAP()/UPPER(). The character semantics are based on
Unicode simple case mappings.

The builtin provider's C.UTF-8 offers several important advantages
over libc:

 * faster sorting -- benefits from additional optimizations such as
   abbreviated keys and varstrfastcmp_c
 * faster case conversion, e.g. LOWER(), at least compared with some
   libc implementations
 * available on all platforms with identical semantics, and the
   semantics are stable, testable, and documentable within a given
   Postgres major version

Being based on memcmp, the builtin C.UTF-8 locale does not offer
natural language sort order. But it is an improvement for most use
cases that might otherwise use libc's "C.UTF-8" locale, as well as
many use cases that use libc's "C" locale.

Discussion: https://postgr.es/m/ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com
Reviewed-by: Daniel Vérité, Peter Eisentraut, Jeremy Schneider
13 months agoImprove EXPLAIN's display of SubPlan nodes and output parameters.
Tom Lane [Tue, 19 Mar 2024 22:19:24 +0000 (18:19 -0400)]
Improve EXPLAIN's display of SubPlan nodes and output parameters.

Historically we've printed SubPlan expression nodes as "(SubPlan N)",
which is pretty uninformative.  Trying to reproduce the original SQL
for the subquery is still as impractical as before, and would be
mighty verbose as well.  However, we can still do better than that.
Displaying the "testexpr" when present, and adding a keyword to
indicate the SubLinkType, goes a long way toward showing what's
really going on.

In addition, this patch gets rid of EXPLAIN's use of "$n" to represent
subplan and initplan output Params.  Instead we now print "(SubPlan
N).colX" or "(InitPlan N).colX" to represent the X'th output column
of that subplan.  This eliminates confusion with the use of "$n" to
represent PARAM_EXTERN Params, and it's useful for the first part of
this change because it eliminates needing some other indication of
which subplan is referenced by a SubPlan that has a testexpr.

In passing, this adds simple regression test coverage of the
ROWCOMPARE_SUBLINK code paths, which were entirely unburdened
by testing before.

Tom Lane and Dean Rasheed, reviewed by Aleksander Alekseev.
Thanks to Chantal Keller for raising the question of whether
this area couldn't be improved.

Discussion: https://postgr.es/m/2838538.1705692747@sss.pgh.pa.us

13 months agoInline pg_popcount{32,64} into pg_popcount().
Nathan Bossart [Tue, 19 Mar 2024 19:46:16 +0000 (14:46 -0500)]
Inline pg_popcount{32,64} into pg_popcount().

On some systems, calls to pg_popcount{32,64} are indirected through
a function pointer.  This commit converts pg_popcount() to a
function pointer on those systems so that we can inline the
appropriate pg_popcount{32,64} implementations into each of the
pg_popcount() implementations.  Since pg_popcount() may call
pg_popcount{32,64} several times, this can significantly improve
its performance.

Suggested-by: David Rowley
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAApHDvrb7MJRB6JuKLDEY2x_LKdFHwVbogpjZBCX547i5%2BrXOQ%40mail.gmail.com

13 months agoPostpone reparameterization of paths until create_plan().
Tom Lane [Tue, 19 Mar 2024 18:51:58 +0000 (14:51 -0400)]
Postpone reparameterization of paths until create_plan().

When considering nestloop paths for individual partitions within
a partitionwise join, if the inner path is parameterized, it is
parameterized by the topmost parent of the outer rel, not the
corresponding outer rel itself.  Therefore, we need to translate the
parameterization so that the inner path is parameterized by the
corresponding outer rel.

Up to now, we did this while generating join paths.  However, that's
problematic because we must also translate some expressions that are
shared across all paths for a relation, such as restriction clauses
(kept in the RelOptInfo and/or IndexOptInfo) and TableSampleClauses
(kept in the RangeTblEntry).  The existing code fails to translate
these at all, leading to wrong answers, odd failures such as
"variable not found in subplan target list", or executor crashes.
But we can't modify them during path generation, because that would
break things if we end up choosing some non-partitioned-join path.

So this patch postpones reparameterization of the inner path until
createplan.c, where it is safe to modify the referenced RangeTblEntry,
RelOptInfo or IndexOptInfo, because we have made a final choice of which
Path to use.  We do still have to check during path generation that
the reparameterization will be possible.  So we introduce a new
function path_is_reparameterizable_by_child() to detect that.

The duplication between path_is_reparameterizable_by_child() and
reparameterize_path_by_child() is a bit annoying, but there seems
no other good answer.  A small benefit is that we can avoid building
useless reparameterized trees in cases where a non-partitioned join
is ultimately chosen.  Also, reparameterize_path_by_child() can now
be allowed to scribble on the input paths, saving a few cycles.

This fix repairs the same problems previously addressed in the
back branches by commits 62f120203 et al.

Richard Guo, reviewed at various times by Ashutosh Bapat, Andrei
Lepikhov, Alena Rybakina, Robert Haas, and myself

Discussion: https://postgr.es/m/CAMbWs496+N=UAjOc=rcD3P7B6oJe4rZw08e_TZRUsWbPxZW3Tw@mail.gmail.com

13 months agogen_node_support.pl: Mark location fields as type alias ParseLoc
Peter Eisentraut [Tue, 19 Mar 2024 15:55:00 +0000 (16:55 +0100)]
gen_node_support.pl: Mark location fields as type alias ParseLoc

Instead of the rather ugly type=int + name ~= location$, we now have a
marker type for offset pointers or sizes that are only relevant when a
query text is included, which decreases the complexity required in
gen_node_support.pl for handling these values.

Author: Matthias van de Meent <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CAEze2WgrCiR3JZmWyB0YTc8HV7ewRdx13j0CqD6mVkYAW+SFGQ@mail.gmail.com

13 months agopg_upgrade: run all data type checks per connection
Daniel Gustafsson [Tue, 19 Mar 2024 12:32:50 +0000 (13:32 +0100)]
pg_upgrade: run all data type checks per connection

The checks for data type usage were each connecting to all databases
in the cluster and running their query. On clusters which have a lot
of databases this can become unnecessarily expensive. This moves the
checks to run in a single connection instead to minimize setup and
teardown overhead.

Reviewed-by: Nathan Bossart <[email protected]>
Reviewed-by: Justin Pryzby <[email protected]>
Reviewed-by: Peter Eisentraut <[email protected]>
Discussion: https://postgr.es/m/BB4C76F-D416-4F9F-949E-DBE950D37787@yesql.se

13 months agoUse half-open interval notation in without_overlaps tests
Peter Eisentraut [Tue, 19 Mar 2024 10:44:14 +0000 (11:44 +0100)]
Use half-open interval notation in without_overlaps tests

This way, the input literals match the output in any error messages.

Author: Paul A. Jungwirth <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

13 months agoFix misleading comments
Peter Eisentraut [Tue, 19 Mar 2024 09:55:51 +0000 (10:55 +0100)]
Fix misleading comments

To match code changes in 229fb58d4f.

13 months agoUse daterange and YMD in without_overlaps tests instead of tsrange.
Peter Eisentraut [Tue, 19 Mar 2024 09:17:03 +0000 (10:17 +0100)]
Use daterange and YMD in without_overlaps tests instead of tsrange.

This makes things a lot easier to read, especially when we get to the
FOREIGN KEY tests later.

Author: Paul A. Jungwirth <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com

13 months agoAdd some UUID support functions
Peter Eisentraut [Tue, 19 Mar 2024 08:30:24 +0000 (09:30 +0100)]
Add some UUID support functions

Add uuid_extract_timestamp() and uuid_extract_version().

Author: Andrey Borodin
Reviewed-by: Sergey Prokhorenko, Kirk Wolak, Przemysław Sztoch
Reviewed-by: Nikolay Samokhvalov, Jelte Fennema-Nio, Aleksander Alekseev
Reviewed-by: Peter Eisentraut, Chris Travers, Lukas Fittl
Discussion: https://postgr.es/m/CAAhFRxitJv%3DyoGnXUgeLB_O%2BM7J2BJAmb5jqAT9gZ3bij3uLDA%40mail.gmail.com

13 months agoActivate perlcritic InputOutput::RequireCheckedSyscalls and fix resulting warnings
Peter Eisentraut [Tue, 19 Mar 2024 06:01:22 +0000 (07:01 +0100)]
Activate perlcritic InputOutput::RequireCheckedSyscalls and fix resulting warnings

This checks that certain I/O-related Perl functions properly check
their return value.  Some parts of the PostgreSQL code had been a bit
sloppy about that.  The new perlcritic warnings are fixed here.  I
didn't design any beautiful error messages, mostly just used "or die
$!", which mostly matches existing code, and also this is
developer-level code, so having the system error plus source code
reference should be ok.

Initially, we only activate this check for a subset of what the
perlcritic check would warn about.  The effective list is

    chmod flock open read rename seek symlink system

The initial set of functions is picked because most existing code
already checked the return value of those, so any omissions are
probably unintended, or because it seems important for test
correctness.

The actual perlcritic configuration is written as an exclude list.
That seems better so that we are clear on what we are currently not
checking.  Maybe future patches want to investigate checking some of
the other functions.  (In principle, we might eventually want to check
all of them, but since this is test and build support code, not
production code, there are probably some reasonable compromises to be
made.)

Reviewed-by: Daniel Gustafsson <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/88b7d4f2-46d9-4cc7-b1f7-613c90f9a76a%40eisentraut.org

13 months agoFix documentation oversights from 2d819a08a1.
Jeff Davis [Tue, 19 Mar 2024 00:13:18 +0000 (17:13 -0700)]
Fix documentation oversights from 2d819a08a1.

13 months agoUpdate src/common/unicode/README.
Jeff Davis [Mon, 18 Mar 2024 23:36:40 +0000 (16:36 -0700)]
Update src/common/unicode/README.

Change the test description to include the case mapping
test. Oversight in 5c40364dd6.

13 months agoFix another warning, introduced by 846311051e.
Jeff Davis [Mon, 18 Mar 2024 22:34:43 +0000 (15:34 -0700)]
Fix another warning, introduced by 846311051e.

Discussion: https://postgr.es/m/3703896.1710799495@sss.pgh.pa.us
Reported-by: Tom Lane
13 months agoAddress more review comments on commit 2d819a08a1.
Jeff Davis [Mon, 18 Mar 2024 18:56:45 +0000 (11:56 -0700)]
Address more review comments on commit 2d819a08a1.

Based on comments from Peter Eisentraut.

 * Document CREATE DATABASE ... BUILTIN_LOCALE.
 * Determine required encoding based on locale name for CREATE
   COLLATION. Use -1 for "C" (requires catversion bump).
 * initdb output fixups.
 * Make ctype_is_c a constant true for now.
 * Fixups to ICU 010_create_database.pl test.

Discussion: https://postgr.es/m/4135cf11-206d-40ed-96c0-9363c1232379@eisentraut.org

13 months agodblink/isolationtester/fe_utils: Use new cancel API
Alvaro Herrera [Mon, 18 Mar 2024 18:28:58 +0000 (19:28 +0100)]
dblink/isolationtester/fe_utils: Use new cancel API

Commit 61461a300c1c introduced new functions to libpq for cancelling
queries.  This replaces the usage of the old ones in parts of the
codebase with these newer ones.  This specifically leaves out changes to
psql and pgbench, as those would need a much larger refactor to be able
to call them due to the new functions not being signal-safe; and also
postgres_fdw, because the original code there is not clear to me
(Álvaro) and not fully tested.

Author: Jelte Fennema-Nio <[email protected]>
Discussion: https://postgr.es/m/CAGECzQT_VgOWWENUqvUV9xQmbaCyXjtRRAYO8W07oqashk_N+g@mail.gmail.com

13 months agoFix unreachable code warning from commit 2d819a08a1.
Jeff Davis [Mon, 18 Mar 2024 16:15:47 +0000 (09:15 -0700)]
Fix unreachable code warning from commit 2d819a08a1.

Found by Coverity.

Discussion: https://postgr.es/m/3422201.1710711993@sss.pgh.pa.us
Reported-by: Tom Lane
13 months agoAdd missing source files to nls.mk
Peter Eisentraut [Mon, 18 Mar 2024 15:45:51 +0000 (16:45 +0100)]
Add missing source files to nls.mk

13 months agoPut libpq_pipeline cancel test back
Alvaro Herrera [Mon, 18 Mar 2024 12:14:55 +0000 (13:14 +0100)]
Put libpq_pipeline cancel test back

I disabled it in cc6e64afda53 because it was unstable; hopefully the
changes here make it more stable.  (Jelte proposed to submit the queries
using simple query protocol, but I chose to instead make the monitor
connection wait until the PgSleep wait event is seen.  This is probably
not a terribly meaningful increase in coverage ...)

Discussion: https://postgr.es/m/CAGECzQRQh_5tSy+5cndgv08eNJ2O0Zpwn2YddJtSsmC=Wpy1BQ@mail.gmail.com

13 months agoFix EXPLAIN Bitmap heap scan to count pages with no visible tuples
Heikki Linnakangas [Mon, 18 Mar 2024 12:03:58 +0000 (14:03 +0200)]
Fix EXPLAIN Bitmap heap scan to count pages with no visible tuples

Previously, bitmap heap scans only counted lossy and exact pages for
explain when there was at least one visible tuple on the page.

heapam_scan_bitmap_next_block() returned true only if there was a
"valid" page with tuples to be processed. However, the lossy and exact
page counters in EXPLAIN should count the number of pages represented
in a lossy or non-lossy way in the constructed bitmap, regardless of
whether or not the pages ultimately contained visible tuples.

Backpatch to all supported versions.

Author: Melanie Plageman
Discussion: https://www.postgresql.org/message-id/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA@mail.gmail.com
Discussion: https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%[email protected]

13 months agoAdd some const decorations
Peter Eisentraut [Mon, 18 Mar 2024 11:07:09 +0000 (12:07 +0100)]
Add some const decorations

Discussion: https://www.postgresql.org/message-id/flat/5ac50071-f2ed-4ace-a8fd-b892cffd33eb@www.fastmail.com

13 months agoMove code for backend startup to separate file
Heikki Linnakangas [Mon, 18 Mar 2024 09:38:10 +0000 (11:38 +0200)]
Move code for backend startup to separate file

This is code that runs in the backend process after forking, rather
than postmaster. Move it out of postmaster.c for clarity.

Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi

13 months agoRefactor postmaster child process launching
Heikki Linnakangas [Mon, 18 Mar 2024 09:35:08 +0000 (11:35 +0200)]
Refactor postmaster child process launching

Introduce new postmaster_child_launch() function that deals with the
differences in EXEC_BACKEND mode.

Refactor the mechanism of passing information from the parent to child
process. Instead of using different command-line arguments when
launching the child process in EXEC_BACKEND mode, pass a
variable-length blob of startup data along with all the global
variables. The contents of that blob depend on the kind of child
process being launched. In !EXEC_BACKEND mode, we use the same blob,
but it's simply inherited from the parent to child process.

Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi

13 months agoMove some functions from postmaster.c to a new source file
Heikki Linnakangas [Mon, 18 Mar 2024 09:35:05 +0000 (11:35 +0200)]
Move some functions from postc to a new source file

This just moves the functions, with no other changes, to make the next
commits smaller and easier to review. The moved functions are related
to launching postmaster child processes in EXEC_BACKEND mode.

Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi

13 months agoSplit registration of Win32 deadchild callback to separate function
Heikki Linnakangas [Mon, 18 Mar 2024 09:35:01 +0000 (11:35 +0200)]
Split registration of Win32 deadchild callback to separate function

The next commit will move the internal_forkexec() function to a
different source file, but it makes sense to keep all the code related
to the win32 waitpid() emulation in postmaster.c. Split it off to a
separate function now, to make the next commit more mechanical.

Reviewed-by: Tristan Partin, Andres Freund
Discussion: https://www.postgresql.org/message-id/7a59b073-5b5b-151e-7ed3-8b01ff7ce9ef@iki.fi

13 months agoRemove references to backup_fs_hot() in Cluster.pm
Michael Paquier [Mon, 18 Mar 2024 05:21:32 +0000 (14:21 +0900)]
Remove references to backup_fs_hot() in Cluster.pm

This routine has been removed in 39969e2a1e4d with the exclusive backup
mode but there were still references to it.

Issue noticed while working on 071e3ad59d6f.

13 months agoInitialize variables to placate compiler.
Nathan Bossart [Mon, 18 Mar 2024 01:16:15 +0000 (20:16 -0500)]
Initialize variables to placate compiler.

Since commit 012460ee93, some compilers have been warning that a
couple of variables may be used uninitialized.  There doesn't
appear to be any actual risk, so let's just initialize these
variables to 0 to silence the compiler warnings.

Discussion: https://postgr.es/m/20240317192927.GA3978212%40nathanxps13

13 months agoSupport json_errdetail in FRONTEND code
Daniel Gustafsson [Sun, 17 Mar 2024 22:56:15 +0000 (23:56 +0100)]
Support json_errdetail in FRONTEND code

Allocate memory for the error message inside memory owned by the
JsonLexContext and move responsibility away from the caller for
freeing it.  This means that we can partially revert b44669b2ca
as this is now safe to use in FRONTEND code.  The motivation for
this comes from the OAuth and incremental JSON patchsets but it
also adds value on its own.

Author: Jacob Champion <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/CAOYmi+mWdTd6ujtyF7MsvXvk7ToLRVG_tYAcaGbQLvf=N4KrQw@mail.gmail.com

13 months agoMark hash_corrupted() as pg_attribute_noreturn.
Tom Lane [Sun, 17 Mar 2024 21:54:45 +0000 (17:54 -0400)]
Mark hash_corrupted() as pg_attribute_noreturn.

Coverity started complaining about this after cc5ef90ed.
The code's not really different from before, but might
as well clarify its intent.

13 months agoFix PDF doc generation.
Dean Rasheed [Sun, 17 Mar 2024 14:48:39 +0000 (14:48 +0000)]
Fix PDF doc generation.

Commit c649fa24a4 broke PDF generation, due to a misplaced id
attribute.

Per buildfarm member crake.

13 months agoAdd RETURNING support to MERGE.
Dean Rasheed [Sun, 17 Mar 2024 13:58:59 +0000 (13:58 +0000)]
Add RETURNING support to MERGE.

This allows a RETURNING clause to be appended to a MERGE query, to
return values based on each row inserted, updated, or deleted. As with
plain INSERT, UPDATE, and DELETE commands, the returned values are
based on the new contents of the target table for INSERT and UPDATE
actions, and on its old contents for DELETE actions. Values from the
source relation may also be returned.

As with INSERT/UPDATE/DELETE, the output of MERGE ... RETURNING may be
used as the source relation for other operations such as WITH queries
and COPY commands.

Additionally, a special function merge_action() is provided, which
returns 'INSERT', 'UPDATE', or 'DELETE', depending on the action
executed for each row. The merge_action() function can be used
anywhere in the RETURNING list, including in arbitrary expressions and
subqueries, but it is an error to use it anywhere outside of a MERGE
query's RETURNING list.

Dean Rasheed, reviewed by Isaac Morland, Vik Fearing, Alvaro Herrera,
Gurjeet Singh, Jian He, Jeff Davis, Merlin Moncure, Peter Eisentraut,
and Wolfgang Walther.

Discussion: http://postgr.es/m/CAEZATCWePEGQR5LBn-vD6SfeLZafzEm2Qy_L_Oky2=qw2w3Pzg@mail.gmail.com

13 months agoAdd attstattarget to FormExtraData_pg_attribute
Peter Eisentraut [Sun, 17 Mar 2024 11:38:27 +0000 (12:38 +0100)]
Add attstattarget to FormExtraData_pg_attribute

This allows setting attstattarget when a relation is created.

We make use of this by having index_concurrently_create_copy() copy
over the attstattarget values when the new index is created, instead
of having index_concurrently_swap() fix it up later.

Reviewed-by: Tomas Vondra <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org

13 months agoGeneralize handling of nullable pg_attribute columns in DDL
Peter Eisentraut [Sun, 17 Mar 2024 11:19:30 +0000 (12:19 +0100)]
Generalize handling of nullable pg_attribute columns in DDL

DDL code uses tuple descriptors to pass around pg_attribute values
during table and index creation.  But tuple descriptors don't include
the variable-length/nullable columns of pg_attribute, so they have to
be handled separately.  Right now, the attoptions field is handled in
a one-off way with a separate argument passed to
InsertPgAttributeTuples().  The other affected fields of pg_attribute
are right now not needed at relation creation time.

The goal of this patch is to generalize this to allow handling
additional variable-length/nullable columns of pg_attribute in a
similar manner.  For that, create a new struct
FormExtraData_pg_attribute, which is to be passed around in parallel
to the tuple descriptor and optionally supplies the additional
columns.  Right now, this struct only contains one field for
attoptions, so no functionality is actually changed by this.

Reviewed-by: Tomas Vondra <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org

13 months agoMake stxstattarget nullable
Peter Eisentraut [Sun, 17 Mar 2024 11:22:05 +0000 (12:22 +0100)]
Make stxstattarget nullable

To match attstattarget change (commit 4f622503d6d).  The logic inside
CreateStatistics() is clarified a bit compared to that previous patch,
and so here we also update ATExecSetStatistics() to match.

Reviewed-by: Tomas Vondra <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/4da8d211-d54d-44b9-9847-f2a9f1184c76@eisentraut.org

13 months agoFix EXPLAIN output for subplans in MERGE.
Dean Rasheed [Sun, 17 Mar 2024 10:17:11 +0000 (10:17 +0000)]
Fix EXPLAIN output for subplans in MERGE.

Given a subplan in a MERGE query, EXPLAIN would sometimes fail to
properly display expressions involving Params referencing variables in
other parts of the plan tree.

This would affect subplans outside the topmost join plan node, for
which expansion of Params would go via the top-level ModifyTable plan
node.  The problem was that "inner_tlist" for the ModifyTable node's
deparse_namespace was set to the join node's targetlist, but
"inner_plan" was set to the ModifyTable node itself, rather than the
join node, leading to incorrect results when descending to the
referenced variable.

Fix and backpatch to v15, where MERGE was introduced.

Discussion: https://postgr.es/m/CAEZATCWAv-sZuH%2BwG5xJ-%2BGt7qGNGX8wUQd3XYydMFDKgRB9nw%40mail.gmail.com

14 months agoSeparate equalRowTypes() from equalTupleDescs()
Peter Eisentraut [Sun, 17 Mar 2024 04:58:04 +0000 (05:58 +0100)]
Separate equalRowTypes() from equalTupleDescs()

This introduces a new function equalRowTypes() that is effectively a
subset of equalTupleDescs() but only compares the number of attributes
and attribute name, type, typmod, and collation.  This is enough for
most existing uses of equalTupleDescs(), which are changed to use the
new function.  The only remaining callers of equalTupleDescs() are
those that really want to check the full tuple descriptor as such,
without concern about record or row or record type semantics.

The existing function hashTupleDesc() is renamed to hashRowType(),
because it now corresponds more to equalRowTypes().

The purpose of this change is to be clearer about the semantics of the
equality asked for by each caller.  (At least one caller had a comment
that questioned whether equalTupleDescs() was too restrictive.)  For
example, 4f622503d6d removed attstattarget from the tuple descriptor
structure.  It was not fully clear at the time how this should affect
equalTupleDescs().  Now the answer is clear: By their own definitions,
equalRowTypes() does not care, and equalTupleDescs() just compares
whatever is in the tuple descriptor but does not care why it is in
there.

Reviewed-by: Tomas Vondra <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/f656d6d9-6660-4518-a006-2f65cafbebd1%40eisentraut.org

14 months agoAdd destroyStringInfo function for cleaning up StringInfos
Daniel Gustafsson [Sat, 16 Mar 2024 22:18:28 +0000 (23:18 +0100)]
Add destroyStringInfo function for cleaning up StringInfos

destroyStringInfo() is a counterpart to makeStringInfo(), freeing a
palloc'd StringInfo and its data. This is a convenience function to
align the StringInfo API with the PQExpBuffer API. Originally added
in the OAuth patchset, it was extracted and committed separately in
order to aid upcoming JSON work.

Author: Daniel Gustafsson <[email protected]>
Author: Jacob Champion <[email protected]>
Reviewed-by: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/CAOYmi+mWdTd6ujtyF7MsvXvk7ToLRVG_tYAcaGbQLvf=N4KrQw@mail.gmail.com

14 months agopsql: fix variable existence tab completion
Alexander Korotkov [Sat, 16 Mar 2024 21:49:10 +0000 (23:49 +0200)]
psql: fix variable existence tab completion

psql has the :{?name} syntax for testing for a psql variable existence.  This
commit implements a tab completion for this syntax.  Notably, in order to
implement this we have to remove '{' from WORD_BREAKS.  It appears that
'{' here from the very beginning and it comes from the default value of
rl_basic_word_break_characters.  And :{?name} is the only psql syntax using
the '{' sign.  So, removing it from WORD_BREAKS shouldn't break anything.

Discussion: https://postgr.es/m/CAGRrpzZU48F2oV3d8eDLr%3D4TU9xFH5Jt9ED%2BqU1%2BX91gMH68Sw%40mail.gmail.com
Author: Steve Chavez
Reviewed-by: Erik Wienhold
14 months agoUse locale-aware value for \watch in 005_timeouts.pl
Alexander Korotkov [Fri, 15 Mar 2024 19:35:18 +0000 (21:35 +0200)]
Use locale-aware value for \watch in 005_timeouts.pl

Reported-by: Alexander Lakhin
14 months agoFix handling of expecteddir in pg_regress
Daniel Gustafsson [Fri, 15 Mar 2024 16:02:07 +0000 (17:02 +0100)]
Fix handling of expecteddir in pg_regress

Commit c855872074b introduced a new parameter to pg_regress to set
the directory where to look for expected files, but accidentally
only implemented it for when compiling pg_regress for ECPG tests.
Fix by adding support for the parameter to the main regression test
compilation of pg_regress as well.

Backpatch to v16 where --expecteddir was introduced.

Author: Anthonin Bonnefoy <[email protected]>
Discussion: https://postgr.es/m/CAO6_Xqq5yKJHcJsq__LPcKwSY0XHRqVERNWGxx5ttNXXj7+W=A@mail.gmail.com
Backpatch-through: 16

14 months agoFix backstop in gin test if injection point is not reached
Heikki Linnakangas [Fri, 15 Mar 2024 15:55:12 +0000 (17:55 +0200)]
Fix backstop in gin test if injection point is not reached

Per Tom Lane's observation that the test got stuck in infinite loop if
the injection_points module was not loaded. It was supposed to give up
after 10000 iterations, but the backstop was broken.

Discussion: https://www.postgresql.org/message-id/2498595.1710511222%40sss.pgh.pa.us

14 months agoTry to unbreak injection-fault tests in the buildfarm
Heikki Linnakangas [Fri, 15 Mar 2024 13:18:44 +0000 (15:18 +0200)]
Try to unbreak injection-fault tests in the buildfarm

The buildfarm script attempts to run all tests marked as
NO_INSTALLCHECK under src/test/modules without paying attention to
whether they are enabled or disabled in the parent Makefile. That
hasn't been a problem so far, because all the tests marked with
NO_INSTALLCHECK ran unconditionally in "make check". But commit
e2e3b8ae9e changed that: the injection fault tests are marked as
NO_INSTALLCHECK, and also depend on --enable-injection-points.

Try to work around that by ensuring that "make check" does nothing in
the those subdirectories. We can hopefully get rid of this hack soon,
after fixing the buildfarm client, or by switching to meson.

Discussion: https://www.postgresql.org/message-id/8e4cf596-dd70-432e-9068-16466ed596ed%40iki.fi

14 months agoFix wordings in timeouts TAP test
Alexander Korotkov [Fri, 15 Mar 2024 12:32:25 +0000 (14:32 +0200)]
Fix wordings in timeouts TAP test

Reported-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20240315.104235.1835366724413653745.horikyota.ntt%40gmail.com
Author: Andrey Borodin

14 months agoFix race condition in transaction timeout TAP tests
Alexander Korotkov [Fri, 15 Mar 2024 12:31:25 +0000 (14:31 +0200)]
Fix race condition in transaction timeout TAP tests

The interruption handler within the injection point can get stuck in an
infinite loop while handling transaction timeout. To avoid this situation
we reset the timeout flag before invoking the injection point.

Author: Alexander Korotkov
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZfPchPC6oNN71X2J%40paquier.xyz

14 months agoImprove log messages referring to background worker processes
Heikki Linnakangas [Fri, 15 Mar 2024 11:14:38 +0000 (13:14 +0200)]
Improve log messages referring to background worker processes

"Worker" could also mean autovacuum worker or slot sync worker, so
let's be more explicit.

Per Tristan Partin's suggestion.

Discussion: https://www.postgresql.org/message-id/[email protected]

14 months agoDisable tests using injection points in installcheck
Heikki Linnakangas [Fri, 15 Mar 2024 11:06:57 +0000 (13:06 +0200)]
Disable tests using injection points in installcheck

The 'gin' test injections faults to GIN index build. If another test
running concurrently in the same cluster also tries to create a GIN
index, it will hit the fault, too.

To fix, disable tests using injection points when running against an
existing cluster. A better long-term solution would be to make the
injection points scoped to the database or process, but this will do
for now.

Discussion: https://www.postgresql.org/message-id/CA%2BhUKGJYhcG_o2nwSK6r01eOZJwNWUJUbX%3D%3DAVnW84f-%[email protected]
Discussion: https://www.postgresql.org/message-id/10fd6cdd-c5d9-46fe-9fa1-7e661191309e@iki.fi

14 months agoAdd basic TAP tests for the low-level backup method, take two
Michael Paquier [Thu, 14 Mar 2024 23:29:54 +0000 (08:29 +0900)]
Add basic TAP tests for the low-level backup method, take two

There are currently no tests for the low-level backup method where
pg_backup_start() and pg_backup_stop() are involved while taking a
file-system backup.  The tests introduced in this commit rely on a
background psql process to make sure that the backup is taken while the
session doing the SQL start and stop calls remains alive.

Two cases are checked here with the backup taken:
- Recovery without a backup_label, leading to a corrupted state.
- Recovery with a backup_label, with a consistent state reached.
Both cases cross-check some patterns in the logs generated when running
recovery.

Compared to the first attempt in 99b4a63bef94, this includes a couple of
fixes making the CI stable (5 runs succeeded here):
- Add the file to the list of tests in meson.build.
- Race condition with the first WAL segment that we expect in the
primary's archives, by adding a poll on pg_stat_archiver.  The second
segment with the checkpoint record is archived thanks to pg_backup_stop
waiting for it.
- Fix failure of test where the backup_label does not exist.  The
cluster inherits the configuration of the first node; it was attempting
to store segments in the first node's archives, triggering failures with
copy on Windows.
- Fix failure of test on Windows because of incorrect parsing of the
backup_file in the success case.  The data of the backup_label file is
retrieved from the output pg_backup_stop() from a BackgroundPsql written
directly to the backup's data folder.  This would include CRLFs (\r\n),
causing the startup process to fail at the beginning of recovery when
parsing the backup_label because only LFs (\n) are allowed.

Author: David Steele
Discussion: https://postgr.es/m/f20fcc82-dadb-478d-beb4-1e2ffb0ace76@pgmasters.net

14 months agoRefactor initial hash lookup in dynahash.c
Michael Paquier [Thu, 14 Mar 2024 22:57:17 +0000 (07:57 +0900)]
Refactor initial hash lookup in dynahash.c

The same pattern is used three times in dynahash.c to retrieve a bucket
number and a hash bucket from a hash value.  This has popped up while
discussing improvements for the type cache, where this piece of
refactoring would become useful.

Note that hash_search_with_hash_value() does not need the bucket number,
just the hash bucket.

Author: Teodor Sigaev
Reviewed-by: Aleksander Alekseev, Michael Paquier
Discussion: https://postgr.es/m/5812a6e5-68ae-4d84-9d85-b443176966a1@sigaev.ru

14 months agoTrim ORDER BY/DISTINCT aggregate pathkeys in gather_grouping_paths
David Rowley [Thu, 14 Mar 2024 22:54:36 +0000 (11:54 +1300)]
Trim ORDER BY/DISTINCT aggregate pathkeys in gather_grouping_paths

Similar to d8a295389, trim off any PathKeys which are for ORDER BY /
DISTINCT aggregate functions from the PathKey List for the Gather Merge
paths created by gather_grouping_paths().  These additional PathKeys are
not valid to use after grouping has taken place as these PathKeys belong
to columns which are inputs to an aggregate function and, therefore are
unavailable after aggregation.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/cf63174c-8c89-3953-cb49-48f41f74941a@gmail.com
Backpatch-through: 16, where 1349d2790 was added

14 months agoLogin event trigger documentation wordsmithing
Daniel Gustafsson [Thu, 14 Mar 2024 22:35:35 +0000 (23:35 +0100)]
Login event trigger documentation wordsmithing

Minor wordsmithing on the login trigger documentation and code
comments to improve readability, as well as fixing a few small
incorrect statements in the comments.

Author: Robert Treat <[email protected]>
Discussion: https://postgr.es/m/CAJSLCQ0aMWUh1m6E9YdjeqV61baQ=EhteJX8XOxXg8H_2Lcr0Q@mail.gmail.com

14 months agoMake INSERT-from-multiple-VALUES-rows handle domain target columns.
Tom Lane [Thu, 14 Mar 2024 18:57:16 +0000 (14:57 -0400)]
Make INSERT-from-multiple-VALUES-rows handle domain target columns.

Commit a3c7a993d fixed some cases involving target columns that are
arrays or composites by applying transformAssignedExpr to the VALUES
entries, and then stripping off any assignment ArrayRefs or
FieldStores that the transformation added.  But I forgot about domains
over arrays or composites :-(.  Such cases would either fail with
surprising complaints about mismatched datatypes, or insert unexpected
coercions that could lead to odd results.  To fix, extend the
stripping logic to get rid of CoerceToDomain if it's atop an ArrayRef
or FieldStore.

While poking at this, I realized that there's a poorly documented and
not-at-all-tested behavior nearby: we coerce each VALUES column to
the domain type separately, and rely on the rewriter to merge those
operations so that the domain constraints are checked only once.
If that merging did not happen, it's entirely possible that we'd get
unexpected domain constraint failures due to checking a
partially-updated container value.  There's no bug there, but while
we're here let's improve the commentary about it and add some test
cases that explicitly exercise that behavior.

Per bug #18393 from Pablo Kharo.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18393-65fedb1a0de9260d@postgresql.org