postgresql.git
15 months agoUse correct format placeholder for timeline IDs
Peter Eisentraut [Tue, 13 Feb 2024 05:54:58 +0000 (06:54 +0100)]
Use correct format placeholder for timeline IDs

Should be %u rather than %d.

15 months agoDoc: Improve upgrade for streaming replication section.
Amit Kapila [Tue, 13 Feb 2024 04:15:01 +0000 (09:45 +0530)]
Doc: Improve upgrade for streaming replication section.

Currently the documentation of upgrade for streaming replication section
says that logical replication slots will be copied, but the logical
replication slots are copied only if the old primary is version 17.0 or
later.

Author: Shubham Khanna
Discussion: https://postgr.es/m/CAHv8RjJHCw0jpUo9PZxjcguzGt3j2W1_NH=QuREoN0nYiVdVeA@mail.gmail.com

15 months agoRead WAL directly from WAL buffers.
Jeff Davis [Mon, 12 Feb 2024 18:36:18 +0000 (10:36 -0800)]
Read WAL directly from WAL buffers.

If available, read directly from WAL buffers, avoiding the need to go
through the filesystem. Only for physical replication for now, but can
be expanded to other callers.

In preparation for replicating unflushed WAL data.

Author: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACXKKK%3DwbiG5_t6dGao5GoecMwRkhr7GjVBM_jg54%2BNa%3DQ%40mail.gmail.com
Reviewed-by: Andres Freund, Alvaro Herrera, Nathan Bossart, Dilip Kumar, Nitin Jadhav, Melih Mutlu, Kyotaro Horiguchi
15 months agoRemove "#ifdef WIN32" guards from src/port/win32*.c
Heikki Linnakangas [Mon, 12 Feb 2024 09:57:45 +0000 (11:57 +0200)]
Remove "#ifdef WIN32" guards from src/port/win32*.c

These files are only compiled on Windows, and most of them didn't have
"#ifdef WIN32" guards. Remove them from the few that did, for
consistency.

Author: Tristan Partin
Discussion: https://www.postgresql.org/message-id/CXGM9RYSXA2J.1DBO4MRXGZA9P@neon.tech

15 months agoRemove unnecessary smgropen() calls
Heikki Linnakangas [Mon, 12 Feb 2024 08:59:45 +0000 (10:59 +0200)]
Remove unnecessary smgropen() calls

Now that RelationCreateStorage() returns the SmgrRelation (since
commit 5c1560606dc), use that.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/ME3P282MB316600FA62F6605477F26F6AB6742@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM

15 months agoFix some typos in event trigger docs
Alexander Korotkov [Sun, 11 Feb 2024 22:33:51 +0000 (00:33 +0200)]
Fix some typos in event trigger docs

Discussion: https://postgr.es/m/CALj2ACWFUW4jX9EW7CLxbzSS%2Bb7b0Z%3DxKYrqzj2Rstc9MCEx7g%40mail.gmail.com
Author: Bharath Rupireddy

15 months agoUse heap_inplace_update() to unset pg_database.dathasloginevt
Alexander Korotkov [Sun, 11 Feb 2024 22:33:44 +0000 (00:33 +0200)]
Use heap_inplace_update() to unset pg_database.dathasloginevt

Doing this instead of regular updates serves two purposes. First, that avoids
possible waiting on the row-level lock.  Second, that avoids dealing with
TOAST.

It's known that changes made by heap_inplace_update() may be lost due to
concurrent normal updates.  However, we are OK with that.  The subsequent
connections will still have a chance to set "dathasloginevt" to false.

Reported-by: Alexander Lakhin
Discussion: https://postgr.es/m/e2a0248e-5f32-af0c-9832-a90d303c2c61%40gmail.com

15 months agoRemove obsolete script related to MSVC build system
Peter Eisentraut [Sun, 11 Feb 2024 22:42:40 +0000 (23:42 +0100)]
Remove obsolete script related to MSVC build system

15 months agoFix gai_strerror() thread-safety on Windows.
Thomas Munro [Sun, 11 Feb 2024 21:47:57 +0000 (10:47 +1300)]
Fix gai_strerror() thread-safety on Windows.

Commit 5579388d removed code that supplied a fallback implementation of
getaddrinfo(), which was dead code on modern systems.  One tiny piece of
the removed code was still doing something useful on Windows, though:
that OS's own gai_strerror()/gai_strerrorA() function returns a pointer
to a static buffer that it overwrites each time, so it's not
thread-safe.  In rare circumstances, a multi-threaded client program
could get an incorrect or corrupted error message.

Restore the replacement gai_strerror() function, though now that it's
only for Windows we can put it into a win32-specific file and cut it
down to the errors that Windows documents.  The error messages here are
taken from FreeBSD, because Windows' own messages seemed too verbose.

Back-patch to 16.

Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGKz%2BF9d2PTiXwfYV7qJw%2BWg2jzACgSDgPizUw7UG%3Di58A%40mail.gmail.com

15 months agoUse extensible buffers to assemble command lines
Peter Eisentraut [Sun, 11 Feb 2024 08:08:35 +0000 (09:08 +0100)]
Use extensible buffers to assemble command lines

This makes use of StringInfo to assemble command lines, instead of
using fixed-size buffers and the (remote) possibility of "command too
long" errors.  Also makes the code a bit simpler.

This covers the test driver programs pg_regress and
pg_isolation_regress.

Similar to the changes done for pg_rewind in a33e17f210.

Discussion: https://www.postgresql.org/message-id/2be4fee5-738f-4749-b9f8-b452032c7ade%40eisentraut.org

15 months agoDisallow jsonpath methods involving TZ in immutable functions
Andrew Dunstan [Sat, 10 Feb 2024 17:12:39 +0000 (12:12 -0500)]
Disallow jsonpath methods involving TZ in immutable functions

Timezones are not immutable and so neither is any function that relies on
them. In commit 66ea94e8, we introduced a few methods which do casting
from one time to another and thus may involve the current timezone.  To
preserve the immutability of jsonpath functions currently marked
immutable, disallow these methods from being called from non-TZ aware
functions.

Jeevan Chalke, per a report from Jian He.

15 months agoRemove race condition in pg_get_expr().
Tom Lane [Fri, 9 Feb 2024 17:29:41 +0000 (12:29 -0500)]
Remove race condition in pg_get_expr().

Since its introduction, pg_get_expr() has intended to silently
return NULL if called with an invalid relation OID, as can happen
when scanning the catalogs concurrently with relation drops.
However, there is a race condition: we check validity of the OID
at the start, but it could get dropped just afterward, leading to
failures.  This is the cause of some intermittent instability we're
seeing in a proposed new test case, and presumably it's a hazard in
the field as well.

We can fix this by AccessShareLock-ing the target relation for the
duration of pg_get_expr().  Since we don't require any permissions
on the target relation, this is semantically a bit undesirable.  But
it turns out that the set_relation_column_names() subroutine already
takes a transient AccessShareLock on that relation, and has done since
commit 2ffa740be in 2012.  Given the lack of complaints about that, it
seems like there should be no harm in holding the lock a bit longer.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/31ddcc01-a71b-4e8c-9948-01d1c47293ca@eisentraut.org

15 months agoAvoid concurrent calls to bindtextdomain().
Tom Lane [Fri, 9 Feb 2024 16:21:08 +0000 (11:21 -0500)]
Avoid concurrent calls to bindtextdomain().

We previously supposed that it was okay for different threads to
call bindtextdomain() concurrently (cf. commit 1f655fdc3).
It now emerges that there's at least one gettext implementation
in which that triggers an abort() crash, so let's stop doing that.
Add mutexes guarding libpq's and ecpglib's calls, which are the
only ones that need worry about multithreaded callers.

Note: in libpq, we could perhaps have piggybacked on
default_threadlock() to avoid defining a new mutex variable.
I judge that not terribly safe though, since libpq_gettext could
be called from code that is holding the default mutex.  If that
were the first such call in the process, it'd fail.  An extra
mutex is cheap insurance against unforeseen interactions.

Per bug #18312 from Christian Maurer.  Back-patch to all
supported versions.

Discussion: https://postgr.es/m/18312-bbbabc8113592b78@postgresql.org
Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us

15 months agoClean up Windows-specific mutex code in libpq and ecpglib.
Tom Lane [Fri, 9 Feb 2024 16:11:39 +0000 (11:11 -0500)]
Clean up Windows-specific mutex code in libpq and ecpglib.

Fix pthread-win32.h and pthread-win32.c to provide a more complete
emulation of POSIX pthread mutexes: define PTHREAD_MUTEX_INITIALIZER
and make sure that pthread_mutex_lock() can operate on a mutex
object that's been initialized that way.  Then we don't need the
duplicative platform-specific logic in default_threadlock() and
pgtls_init(), which we'd otherwise need yet a third copy of for
an upcoming bug fix.

Also, since default_threadlock() supposes that pthread_mutex_lock()
cannot fail, try to ensure that that's actually true, by getting
rid of the malloc call that was formerly involved in initializing
an emulated mutex.  We can define an extra state for the spinlock
field instead.

Also, replace the similar code in ecpglib/misc.c with this version.
While ecpglib's version at least had a POSIX-compliant API, it
also had the potential of failing during mutex init (but here,
because of CreateMutex failure rather than malloc failure).  Since
all of misc.c's callers ignore failures, it seems like a wise idea
to avoid failures here too.

A further improvement in this area could be to unify libpq's and
ecpglib's implementations into a src/port/pthread-win32.c file.
But that doesn't seem like a bug fix, so I'll desist for now.

In preparation for the aforementioned bug fix, back-patch to all
supported branches.

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

15 months agoRefactor pipe_read_line to return the full line
Daniel Gustafsson [Fri, 9 Feb 2024 14:03:16 +0000 (15:03 +0100)]
Refactor pipe_read_line to return the full line

Commit 5b2f4afffe6 refactored find_other_exec() and in the process
created pipe_read_line() into a static routine for reading a single
line of output, aimed at reading version numbers.  Commit a7e8ece41
later exposed it externally in order to read a postgresql.conf GUC
using "postgres -C ..".  Further, f06b1c598 also made use of it for
reading a version string much like find_other_exec().  The internal
variable remained "pgver", even when used for other purposes.

Since the function requires passing a buffer and its size, and at
most size - 1 bytes will be read via fgets(), there is a truncation
risk when using this for reading GUCs (like how pg_rewind does,
though the risk in this case is marginal).

To keep this as generic functionality for reading a line from a pipe,
this refactors pipe_read_line() into returning an allocated buffer
containing all of the line to remove the risk of silent truncation.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: John Naylor <johncnaylorls@gmail.com>
Discussion: https://postgr.es/m/DEDF73CE-D528-49A3-9089-B3592FD671A9@yesql.se

15 months agoFix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()
Alexander Korotkov [Fri, 9 Feb 2024 10:56:26 +0000 (12:56 +0200)]
Fix usage of aggregate pathkeys in group_keys_reorder_by_pathkeys()

group_keys_reorder_by_pathkeys() function searched for matching pathkeys
within root->group_pathkeys.  That could lead to picking an aggregate pathkey
and using its pathkey->pk_eclass->ec_sortref as an argument of
get_sortgroupref_clause_noerr().  Given that ec_sortref of an aggregate pathkey
references aggregate targetlist not query targetlist, this leads to incorrect
query optimization.

Fix this by looking for matching pathkeys only within the first
num_groupby_pathkeys pathkeys.

Reported-by: David G. Johnston
Discussion: https://postgr.es/m/CAKFQuwY3Ek%3DcLThgd8FdaSc5JRDVt0FaV00gMcWra%2BTAR4gGUw%40mail.gmail.com
Author: Andrei Lepikhov, Alexander Korotkov

15 months agoFix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN
Peter Eisentraut [Fri, 9 Feb 2024 06:57:31 +0000 (07:57 +0100)]
Fix propagation of persistence to sequences in ALTER TABLE / ADD COLUMN

Fix for 344d62fb9a9: That commit introduced unlogged sequences and
made it so that identity/serial sequences automatically get the
persistence level of their owning table.  But this works only for
CREATE TABLE and not for ALTER TABLE / ADD COLUMN.  The latter would
always create the sequence as logged (default), independent of the
persistence setting of the table.  This is fixed here.

Note: It is allowed to change the persistence of identity sequences
directly using ALTER SEQUENCE.  So mistakes in existing databases can
be fixed manually.

Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/c4b6e2ed-bcdf-4ea7-965f-e49761094827%40eisentraut.org

15 months agoAdd previous commit to .git-blame-ignore-revs
Michael Paquier [Fri, 9 Feb 2024 02:06:32 +0000 (11:06 +0900)]
Add previous commit to .git-blame-ignore-revs

15 months agoFix indentation of copyto.c
Michael Paquier [Fri, 9 Feb 2024 02:05:01 +0000 (11:05 +0900)]
Fix indentation of copyto.c

Issue introduced by b619852086ed.

Per buildfarm member koel.

15 months agoImprove COPY TO performance when server and client encodings match
Michael Paquier [Fri, 9 Feb 2024 00:30:53 +0000 (09:30 +0900)]
Improve COPY TO performance when server and client encodings match

This commit fixes an oversight introduced in c61a2f58418e, where COPY TO
would attempt to do encoding conversions even if the encodings of the
client and the server matched for multi-byte encodings.  All conversions
go through pg_any_to_server() that makes the conversion a no-op when the
encodings of the client and the server match, even for multi-byte
encodings.

The logic was fine, but setting CopyToStateData->need_transcoding would
cause strlen() to be called for nothing for each attribute of all the
rows copied, and that was showing high in some profiles (more attributes
make that easier to reach).  This change improves the runtime of some
worst-case COPY TO queries by 15%~ (number present at least here).

This is a performance improvement, so no backpatch is done out of
caution as this is not a regression.

Reported-by: Andres Freund
Analyzed-by: Andres Freund
Author: Michael Paquier
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/20240206020504.edijzczkgd25ek6z@awork3.anarazel.de

15 months agoApply pg_dump test cleanups to test_pg_dump as well
Peter Eisentraut [Thu, 8 Feb 2024 20:19:03 +0000 (21:19 +0100)]
Apply pg_dump test cleanups to test_pg_dump as well

Apply the changes from 41a284411e0 to the test_pg_dump module as well.
Here, we just apply the new test consistency checks, but we don't need
to fix any existing tests.

Discussion: https://www.postgresql.org/message-id/flat/1f8cb371-e84e-434e-0367-6b716fb16fa1@eisentraut.org

15 months agoFix gcc >= 10 warning
Alexander Korotkov [Thu, 8 Feb 2024 19:59:28 +0000 (21:59 +0200)]
Fix gcc >= 10 warning

Reported-by: Bharath Rupireddy
Discussion: https://postgr.es/m/CALj2ACVQoFXxFm2kCmhHcdM7DjA84_bOjoM8HVAKHbE%2BKrZ1uA%40mail.gmail.com

15 months agodoc: Remove superfluous bracket in synopsis
Daniel Gustafsson [Thu, 8 Feb 2024 11:19:34 +0000 (12:19 +0100)]
doc: Remove superfluous bracket in synopsis

Commit 9c08aea6a30 accidentally added one too many end brackets
in the synopsis for CREATE DATABASE .. strategy = strat. Fix by
removing. Backpatch to v15 where it was introduced.

Reported-by: tim.needham2@gmail.com
Discussion: https://postgr.es/m/170734160862.3279712.810853722572951776@wrigleys.postgresql.org
Backpatch-through: v15

15 months agoFix wrong logic in TransactionIdInRecentPast()
Alexander Korotkov [Thu, 8 Feb 2024 10:45:26 +0000 (12:45 +0200)]
Fix wrong logic in TransactionIdInRecentPast()

The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid.  However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.

This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.

Backpatch to all supported versions.

Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12

15 months agoFix documentation build with older docbook-xsl
Peter Eisentraut [Thu, 8 Feb 2024 10:37:11 +0000 (11:37 +0100)]
Fix documentation build with older docbook-xsl

Commit b0f0a9432d0 backpatched some code from upstream DocBook XSL to
our customization layer.  It turned out that this failed to work with
anything but the latest DocBook XSL upstream version (1.79.*), because
the backpatched code references an XSLT parameter (autolink.index.see)
that is not defined in earlier versions (because the feature it is
used for did not exist yet).

There is no way in XSLT to test whether a parameter is declared before
the stylesheet processor tries and fails to access it.  So the
possibilities to fix this would be to either remove the code that uses
the parameter (and thus give up on the feature it is used for) or
declare the parameter in our customization layer.  The latter seems
easier, and with a few more lines of code we can backport the entire
autolink.index.see feature, so let's do that.  (If we didn't, then
with older stylesheets the parameter will appear as on, but it won't
actually do anything, because of the way the stylesheets are laid out,
so it's less confusing to just make it work.)

With this, the documentation build should work again with docbook-xsl
versions 1.77.*, 1.78.*, and 1.79.* (which already worked before).
Version 1.76.1 already didn't work before all this, so was not
considered here.

Reported-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/9077b779-a9f8-09c8-6e85-da1ebfba15af@eisentraut.org

15 months agoUpdate comment
Peter Eisentraut [Thu, 8 Feb 2024 09:18:50 +0000 (10:18 +0100)]
Update comment

The documentation output format htmlhelp is no longer supported, but a
comment still mentioned it.

15 months agoFix meson installation of xid_wraparound test.
Masahiko Sawada [Thu, 8 Feb 2024 08:03:59 +0000 (17:03 +0900)]
Fix meson installation of xid_wraparound test.

Fix for e255b646a, to prevent installation of xid_wraparound test
module during main install.

Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/84cd416a-0e37-4019-8380-1c8a3cdd8c5c%40eisentraut.org

15 months agoFix warnings in cpluspluscheck
John Naylor [Thu, 8 Feb 2024 03:04:57 +0000 (10:04 +0700)]
Fix warnings in cpluspluscheck

Various int variables were compared to macros that are of type size_t,
which caused -Wsign-compare warnings in cpluspluscheck.  Change those
to size_t, which also better describes their purpose.

Per report from Peter Eisentraut

Discussion: https://postgr.es/m/486847dc-6de5-464a-938e-bac98ec2438b%40eisentraut.org

15 months agoRename static function to avoid conflicting names
Daniel Gustafsson [Wed, 7 Feb 2024 21:16:21 +0000 (22:16 +0100)]
Rename static function to avoid conflicting names

Commit a4fd3aa719e moved setup_cancel_handler out of psql and
exporeted it as a global function.  While pg_dump isn't using
the header it's exported in,  having a conflicting name still
risks causing confusion when grepping the code for callsites,
so rename the static function in pg_dump to avoid this.

Author: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/20240126094245.cf6718cc659273765f3ab69a@sraoss.co.jp

15 months agoRemove Start* macros in postmaster.c.
Nathan Bossart [Wed, 7 Feb 2024 18:50:48 +0000 (12:50 -0600)]
Remove Start* macros in postmaster.c.

These macros are just shorthands for calling StartChildProcess()
with the appropriate process type, and they arguably make the code
harder to understand.

Suggested-by: Andres Freund
Author: Reid Thompson
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/e88934c02a5c66f5e8caab2025f85da6b9026d0b.camel%40crunchydata.com

15 months agoUpdate PQparameterStatus and ParameterStatus docs
Alvaro Herrera [Wed, 7 Feb 2024 18:25:07 +0000 (19:25 +0100)]
Update PQparameterStatus and ParameterStatus docs

Cover scram_iterations, which was added in commit b577743000cd.  While
at it, turn the list into a <simplelist> with 2 columns, which is much
nicer to read.

In master, remove mentions of antediluvian versions before which some
parameters were not reported.

Noticed while investigating a question by Maiquel Grassi.

Backpatch to 16.

Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/202401301236.mc5ebrohhtsd@alvherre.pgsql

15 months agoAdjust reltarget assignment for UPPERREL_PARTIAL_DISTINCT rel
David Rowley [Wed, 7 Feb 2024 08:22:34 +0000 (21:22 +1300)]
Adjust reltarget assignment for UPPERREL_PARTIAL_DISTINCT rel

A comment in grouping_planner() claimed that the PlannerInfo
upper_targets array was not used in core code.  However, the code that
generated the paths for the UPPERREL_PARTIAL_DISTINCT rel made that
comment untrue.

Here we adjust the create_distinct_paths() function signature to pass
down the PathTarget the same as is done for create_grouping_paths(),
thus making the aforementioned comment true again.

In passing adjust the order of the upper_targets[] assignments.  These
seem to be following the reverse enum order apart from
UPPERREL_PARTIAL_DISTINCT.

Also, update the header comment for generate_gather_paths() to mention
the function is also used to create gather paths for partial distinct
paths.

Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com

15 months agoSet LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.
Amit Kapila [Wed, 7 Feb 2024 05:40:12 +0000 (11:10 +0530)]
Set LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.

Commit 861f86beea used REGBUF_NO_CHANGE at one of the places in the hash
index to register the clean buffers but forgot to avoid setting LSN in
that case.

Reported-by: Michael Paquier
Author: Kuroda Hayato
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz

15 months agoClean-ups for 776621a5e4 and 7329240437.
Amit Kapila [Wed, 7 Feb 2024 04:34:04 +0000 (10:04 +0530)]
Clean-ups for 776621a5e4 and 7329240437.

Following are a few clean-ups related to failover option support in slots:
1. Improve the documentation in create_subscription.sgml.
2. Remove the spurious blank line in subscriptioncmds.c.
3. Remove the NOTICE for alter_replication_slot in subscriptioncmds.c as
we would sometimes print it even when nothing has changed. One can find
the change by enabling log_replication_commands on the publisher.
4. Optimize ReplicationSlotAlter() function to prevent disk flushing when
the slot's data remains unchanged.

Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB57164904651FB588A518E98894472@OS0PR01MB5716.jpnprd01.prod.outlook.com

15 months agoSimplify signature of CopyAttributeOutCSV() in copyto.c
Michael Paquier [Wed, 7 Feb 2024 03:28:55 +0000 (12:28 +0900)]
Simplify signature of CopyAttributeOutCSV() in copyto.c

This has come up in 2889fd23be56, reverted later on, and is still useful
on its own to reduce a bit the differences between the code paths
dedicated to CSV and text.

Discussion: https://postgr.es/m/ZcCKwAeFrlOqPBuN@paquier.xyz

15 months agoRevert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"
Michael Paquier [Tue, 6 Feb 2024 23:04:26 +0000 (08:04 +0900)]
Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

This reverts commit 2889fd23be56, following a discussion with Andres
Freund as this callback, being called once per attribute when sending a
relation's row, can involve a lot of indirect function calls (more
attributes to deal with means more impact).  The effects of a dispatch
at this level would become more visible when improving the per-row code
execution of COPY TO, impacting future potential performance
improvements.

Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de

15 months agoChange initial use of pg_atomic_write_u64 to init
Alvaro Herrera [Tue, 6 Feb 2024 11:08:39 +0000 (12:08 +0100)]
Change initial use of pg_atomic_write_u64 to init

This only matters when using atomics emulation with semaphores.

Per buildfarm member rorqual.

15 months agoUse atomic access for SlruShared->latest_page_number
Alvaro Herrera [Tue, 6 Feb 2024 09:54:10 +0000 (10:54 +0100)]
Use atomic access for SlruShared->latest_page_number

The new concurrency model proposed for slru.c to improve performance
does not include any single lock that would coordinate processes
doing concurrent reads/writes on SlruShared->latest_page_number.
We can instead use atomic reads and writes for that variable.

Author: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Andrey M. Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAFiTN-vzDvNz=ExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A@mail.gmail.com

15 months agoFurther cosmetic review of hashfn_unstable.h
John Naylor [Tue, 6 Feb 2024 06:09:03 +0000 (13:09 +0700)]
Further cosmetic review of hashfn_unstable.h

In follow-up to e97b672c8,
* Flesh out comments explaining the incremental interface
* Clarify detection of zero bytes when hashing aligned C strings

The latter was suggested and reviewed by Jeff Davis

Discussion: https://postgr.es/m/48e8f8bbe0be9c789f98776c7438244ab7a7cc63.camel%40j-davis.com

15 months agoSimplify initialization of incremental hash state
John Naylor [Sun, 21 Jan 2024 12:19:14 +0000 (19:19 +0700)]
Simplify initialization of incremental hash state

The standalone functions fasthash{32,64} use length for two purposes:
how many bytes to hash, and how to perturb the internal seed.

Developers using the incremental interface may not know the length
ahead of time (e.g. for C strings). In this case, it's advised to
pass length to the finalizer, but initialization still needed some
length up front, in the form of a placeholder macro.

Separate the concerns by having the standalone functions perturb the
internal seed themselves from their own length parameter, allowing
to remove "len" from fasthash_init(), as well as the placeholder macro.

Discussion: https://postgr.es/m/CANWCAZbTUk2LOyhsFo33gjLyLAHZ7ucXCi5K9u%3D%2BPtnTShDKtw%40mail.gmail.com

15 months agodoc: Spell I/O consistently
Michael Paquier [Tue, 6 Feb 2024 04:29:22 +0000 (13:29 +0900)]
doc: Spell I/O consistently

The pg_stat_io and pg_stat_copy_progress view docs spelled "I/O" as "IO"
or even "io" in some places when not referring to literal names or
string values.

Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87fry6lx5s.fsf@wibble.ilmari.org

15 months agoFix meson installation of new generated files
Peter Eisentraut [Mon, 5 Feb 2024 14:45:29 +0000 (15:45 +0100)]
Fix meson installation of new generated files

Fix for 9b1a6f50b9: We want to install catalog/syscache_ids.h but not
catalog/syscache_info.h.  The meson code has this backwards.  The
makefiles are ok.

Reported-by: Aleksander Alekseev <aleksander@timescale.com>
Discussion: https://www.postgresql.org/message-id/CAJ7c6TMDGmAiozDjJQ3%3DP3cd-1BidC_GpitcAuU0aqq-r1eSoQ%40mail.gmail.com

15 months agoFix assertion if index is dropped during REFRESH CONCURRENTLY
Heikki Linnakangas [Mon, 5 Feb 2024 09:01:30 +0000 (11:01 +0200)]
Fix assertion if index is dropped during REFRESH CONCURRENTLY

When assertions are disabled, the built SQL statement is invalid and
you get a "syntax error". So this isn't a serious problem, but let's
avoid the assertion failure.

Backpatch to all supported versions.

Reviewed-by: Noah Misch
15 months agoRun REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context
Heikki Linnakangas [Mon, 5 Feb 2024 09:01:23 +0000 (11:01 +0200)]
Run REFRESH MATERIALIZED VIEW CONCURRENTLY in right security context

The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are
correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for
creating the temporary "diff" table, because you cannot create
temporary tables in SRO mode. But creating the temporary "diff" table
is a pretty complex CTAS command that selects from another temporary
table created earlier in the command. If you can cajole that CTAS
command to execute code defined by the table owner, the table owner
can run code with the privileges of the user running the REFRESH
command.

The proof-of-concept reported to the security team relied on CREATE
RULE to convert the internally-built temp table to a view. That's not
possible since commit b23cd185fd, and I was not able to find a
different way to turn the SELECT on the temp table into code
execution, so as far as I know this is only exploitable in v15 and
below. That's a fiddly assumption though, so apply this patch to
master and all stable versions.

Thanks to Pedro Gallegos for the report.

Security: CVE-2023-5869
Reviewed-by: Noah Misch
15 months agoEnhance libpqrcv APIs to support slot synchronization.
Amit Kapila [Mon, 5 Feb 2024 05:15:34 +0000 (10:45 +0530)]
Enhance libpqrcv APIs to support slot synchronization.

This patch provides support for regular (non-replication) connections in
libpqrcv_connect(). This can be used to execute SQL statements on the
primary server without starting a walsender.

A new API libpqrcv_get_dbname_from_conninfo() is also added to extract the
database name from the given connection-info.

Note that this patch doesn't change any existing functionality but later
patches implementing the slot synchronization will use this functionality
to connect to the primary server to fetch required slot information.

Author: Shveta Malik, Hou Zhijie, Ajin Cherian
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com

15 months agoImprove the comments in 004_subscription.pl.
Amit Kapila [Mon, 5 Feb 2024 03:21:33 +0000 (08:51 +0530)]
Improve the comments in 004_subscription.pl.

It was not clear whether the subscriptions in the upgraded instance were
the ones that retained the previous subscription's properties.

Author: Peter Smith
Reviewed-by: Vignesh C, Alvaro Herrera
Discussion: https://postgr.es/m/CAHut+Pu1usLPHRySPTacY1K_Q-ddSRXNFhmj_2u1NfqBC1ytng@mail.gmail.com

15 months agoRefactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO
Michael Paquier [Mon, 5 Feb 2024 02:12:37 +0000 (11:12 +0900)]
Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

These routines are used by the text and CSV formats to send an output
representation of a string, applying quotes if required.  This is
similar to 95fb5b49024a, reducing the number of "if" branches that need
to be checked on a per-row basis when sending representation of fields
in text or CSV mode.

While on it, this simplifies the signature of CopyAttributeOutCSV() as
it is possible to know that an attribute is alone on a line thanks to
CopyToState.  Headers should not use quotes, even if forced at query
level.

Extracted from a larger patch by the same author.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com

15 months agoRefactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM
Michael Paquier [Mon, 5 Feb 2024 00:46:02 +0000 (09:46 +0900)]
Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

CopyReadAttributes{CSV,Text}() are used to parse lines for text and CSV
format.  This reduces the number of "if" branches that need to be
checked when parsing fields in CSV and text mode when dealing with a
COPY FROM, something that can become more noticeable with more
attributes and more lines to process.

Extracted from a larger patch by the same author.

Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com

15 months agolibpq: Change some static functions to extern
Alvaro Herrera [Sun, 4 Feb 2024 15:35:16 +0000 (16:35 +0100)]
libpq: Change some static functions to extern

This is in preparation of a follow up commit that starts using these
functions from fe-cancel.c.

Author: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com

15 months agolibpq: Add pqReleaseConnHosts function
Alvaro Herrera [Sun, 4 Feb 2024 15:19:20 +0000 (16:19 +0100)]
libpq: Add pqReleaseConnHosts function

In a follow up commit we'll need to free this connhost field in a
function defined in fe-cancel.c, so here we extract the logic to a
dedicated extern function.

Author: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com

15 months agopg_basebackup: Fix check for MINIMUM_VERSION_FOR_WAL_SUMMARIES
Michael Paquier [Sun, 4 Feb 2024 01:51:53 +0000 (10:51 +0900)]
pg_basebackup: Fix check for MINIMUM_VERSION_FOR_WAL_SUMMARIES

MINIMUM_VERSION_FOR_WAL_SUMMARIES is used to check if the directory
pg_wal/summaries/ should be created in a base backup, but its condition
was reversed: pg_wal/summaries/ would be created when taking base
backups from backends of v16 and older versions, but it should be
created in base backups taken from backends of v17 and newer versions.

Author: Artur Zakirov
Reviewed-by: Yugo Nagata, Nazir Bilal Yavuz
Discussion: https://postgr.es/m/CAKNkYnzkkQ0gb_ZmLTY0r2-qV1q6imXgcCWxdA6UoA6yJkujGg@mail.gmail.com

15 months agoImprove documentation for COPY ... ON_ERROR ...
Alexander Korotkov [Fri, 2 Feb 2024 23:49:51 +0000 (01:49 +0200)]
Improve documentation for COPY ... ON_ERROR ...

Discussion: https://postgr.es/m/20240126112829.d420b28859fbe84379fdb7ad%40sraoss.co.jp
Author: Yugo Nagata
Reviewed-by: Masahiko Sawada, David G. Johnston, Atsushi Torikoshi
15 months agoFix typo in comments
Heikki Linnakangas [Fri, 2 Feb 2024 22:18:21 +0000 (00:18 +0200)]
Fix typo in comments

Backpatch-through: v16

15 months agoTranslate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().
Tom Lane [Fri, 2 Feb 2024 20:34:29 +0000 (15:34 -0500)]
Translate ENOMEM to ERRCODE_OUT_OF_MEMORY in errcode_for_file_access().

Previously you got ERRCODE_INTERNAL_ERROR, which seems inappropriate,
especially given that we're trying to avoid emitting that in reachable
cases.

Alexander Kuzmenkov

Discussion: https://postgr.es/m/CALzhyqzgQph0BY8-hFRRGdHhF8CoqmmDHW9S=hMZ-HMzLxRqDQ@mail.gmail.com

15 months agoFix bug in bulk extending temp relation after failure
Heikki Linnakangas [Fri, 2 Feb 2024 19:12:30 +0000 (21:12 +0200)]
Fix bug in bulk extending temp relation after failure

A ResourceOwnerEnlarge() call was missing. That led to an error:

ERROR:  ResourceOwnerRemember called but array was full

and an assertion failure, if you tried to extend a temp relation again
after a failure. Alexander's test case used running out of disk space
to trigger the original failure.

This bug was introduced in the large ResourceOwner rewrite commit
b8bff07daa. Before that, the UnpinLocalBuffer() call guaranteed that
the subsequent PinLocalBuffer() will succeed, but after the rewrite,
releasing an old resource doesn't guarantee that there is space for a
new one.

Add a comment explaining why the UnpinBuffer + PinBuffer calls in
BufferAlloc(), with no ResourceOwnerEnlarge() in between, are safe.

Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/dc574fea-c83e-a600-08cd-10881762e4fa@gmail.com

15 months agoAllow Gather Merge in more cases for parallel DISTINCT
David Rowley [Fri, 2 Feb 2024 11:20:18 +0000 (00:20 +1300)]
Allow Gather Merge in more cases for parallel DISTINCT

Here we adjust the partial path generation for parallel DISTINCT queries
to add Sort nodes on top of any unsorted partial distinct paths.

This increases the likelihood of the planner pushing a Sort below a Gather
Merge which enables the final phase of the parallel distinct to be
implemented using a Unique node in more cases.

Sorting the partial distinct paths is particularly useful when the
DISTINCT query has an ORDER BY and LIMIT clause as this can allow cheaper
plans by having the workers Hash Aggregate then Sort before feeding the
results into the Gather Merge.  The non-parallel portion of the plan then
becomes very cheap as it leaves only Unique and Limit to do in the leader
process.

Author: Richard Guo
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com

15 months agoSync PG_VERSION file in CREATE DATABASE.
Noah Misch [Thu, 1 Feb 2024 21:44:19 +0000 (13:44 -0800)]
Sync PG_VERSION file in CREATE DATABASE.

An OS crash could leave PG_VERSION empty or missing.  The same symptom
appeared in a backup by block device snapshot, taken after the next
checkpoint and before the OS flushes the PG_VERSION blocks.  Device
snapshots are not a documented backup method, however.  Back-patch to
v15, where commit 9c08aea6a3090a396be334cc58c511edab05776a introduced
STRATEGY=WAL_LOG and made it the default.

Discussion: https://postgr.es/m/20240130195003.0a.nmisch@google.com

15 months agoHandle interleavings between CREATE DATABASE steps and base backup.
Noah Misch [Thu, 1 Feb 2024 21:44:19 +0000 (13:44 -0800)]
Handle interleavings between CREATE DATABASE steps and base backup.

Restoring a base backup taken in the middle of CreateDirAndVersionFile()
or write_relmap_file() would lose the function's effects.  The symptom
was absence of the database directory, PG_VERSION file, or
pg_filenode.map.  If missing the directory, recovery would fail.  Either
missing file would not fail recovery but would render the new database
unusable.  Fix CreateDirAndVersionFile() with the transam/README "action
first and then write a WAL entry" strategy.  That has a side benefit of
moving filesystem mutations out of a critical section, reducing the ways
to PANIC.  Fix the write_relmap_file() call with a lock acquisition, so
it interacts with checkpoints like non-CREATE DATABASE calls do.
Back-patch to v15, where commit 9c08aea6a3090a396be334cc58c511edab05776a
introduced STRATEGY=WAL_LOG and made it the default.

Discussion: https://postgr.es/m/20240130195003.0a.nmisch@google.com

15 months agoUpdate time zone data files to tzdata release 2024a.
Tom Lane [Thu, 1 Feb 2024 20:57:53 +0000 (15:57 -0500)]
Update time zone data files to tzdata release 2024a.

DST law changes in Ittoqqortoormiit, Greenland (America/Scoresbysund),
Kazakhstan (Asia/Almaty and Asia/Qostanay) and Palestine; as well as
updates for the Antarctic stations Casey and Vostok.

Historical corrections for Vietnam, Toronto, and Miquelon.

15 months agoAvoid package qualification of $windows_os
Andrew Dunstan [Thu, 1 Feb 2024 20:17:41 +0000 (15:17 -0500)]
Avoid package qualification of $windows_os

Further fallout from commit 6ee26c6a4b. To keep code in sync and avoid
issues on older releases with different package names, simply use the
unqualified name like many other places in our code.

15 months agoContinue my quest to make 002_blocks.pl pass reliably.
Robert Haas [Thu, 1 Feb 2024 16:46:30 +0000 (11:46 -0500)]
Continue my quest to make 002_blocks.pl pass reliably.

The latest buildfarm failures show that after the insert, we don't
actually wait long enough for WAL summarization to catch up, apparently
because the on disk state gets updated before the in-memory state, and
so by checking the on disk state to see whether we're caught up and then
the in-memory state to see where exactly how far we've progressed, we
can, if unlucky, derive an older value of summarized_lsn, messing up
the rest of the test.

Attempt to fix this by using pg_available_wal_summaries() everywhere in
the test and pg_get_wal_summarizer_state() nowhere.

Per buildfarm.

15 months agodoc: improve role option documentation
Bruce Momjian [Thu, 1 Feb 2024 11:11:53 +0000 (06:11 -0500)]
doc:  improve role option documentation

Role option management was changed in Postgres 16.  This patch improves
the docs around these changes, including CREATE ROLE's INHERIT option,
inheritance handling, and grant's ability to change role options.

Discussion: https://postgr.es/m/Zab9GiV63EENDcWG@momjian.us

Co-authored-by: David G. Johnston
Backpatch-through: 16

15 months agodoc: remove incorrect grammar for ALTER EVENT TRIGGER
Daniel Gustafsson [Thu, 1 Feb 2024 09:45:37 +0000 (10:45 +0100)]
doc: remove incorrect grammar for ALTER EVENT TRIGGER

The Parameters subsection had an extra TRIGGER in the grammar
for DISABLE/ENABLE which is incorrect.  Backpatch down to all
supported versions since it's been like this all along.

Discussion: https://postgr.es/m/0AFB171E-7E78-4A90-A140-46AB270212CA@yesql.se
Backpatch-through: v12

15 months agodoc: Fix incorrect openssl option
Daniel Gustafsson [Thu, 1 Feb 2024 08:36:34 +0000 (09:36 +0100)]
doc: Fix incorrect openssl option

The openssl command for displaying the DN of a client certificate was
using --subject and not the single-dash option -subject. While recent
versions of openssl handles double dash options,  earlier does not so
fix by using just -subject  (which is per the openssl documentation).

Backpatch to v14 where this was introduced.

Reported-by: konkove@gmail.com
Discussion: https://postgr.es/m/170672168899.666.10442618407194498217@wrigleys.postgresql.org
Backpatch-through: v14

15 months agoFix stats_fetch_consistency with stats for fixed-numbered objects
Michael Paquier [Thu, 1 Feb 2024 08:12:50 +0000 (17:12 +0900)]
Fix stats_fetch_consistency with stats for fixed-numbered objects

This impacts the statistics retrieved in transactions for the following
views when updating the value of stats_fetch_consistency, leading to
behaviors contrary to what is documented since 605994651b6a as an update
of this parameter should discard all statistics snapshot data:
- pg_stat_archiver
- pg_stat_bgwriter
- pg_stat_checkpointer
- pg_stat_io
- pg_stat_slru
- pg_stat_wal

For example, updating stats_fetch_consistency from "snapshot" to "cache"
in a transaction did not re-fetch any fresh data, using data cached from
the time when "snapshot" was in use.

Author: Shinya Kato
Discussion: https://postgr.es/m/d77fc5190d4dbe1738d77231488e768b@oss.nttdata.com
Backpatch-through: 15

15 months agoFix copy&paste typo in comment
Alvaro Herrera [Wed, 31 Jan 2024 22:11:53 +0000 (23:11 +0100)]
Fix copy&paste typo in comment

15 months agoExclude Threadsanitizer instrumentation in exit check
Daniel Gustafsson [Wed, 31 Jan 2024 21:54:45 +0000 (22:54 +0100)]
Exclude Threadsanitizer instrumentation in exit check

When building libpq there is a check to ensure that we're not
linking against code that calls exit(). This check is using a
heuristic grep with exclusions for known false positives. The
Threadsanitizer library instrumentation for function exits is
named such that it triggers the check, so add an exclusion.

This fix is only applied to the Makefile since the meson build
files don't yet have this check.  Adding the check to meson is
outside the scope of this patch though.

Reported-by: Roman Lozko <lozko.roma@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAEhC_BmNGKgj2wKArH2EAU11BsaHYgLnrRFJGRm5Vs8WJzyiQA@mail.gmail.com

15 months agoFix costing bug in MergeAppend
David Rowley [Wed, 31 Jan 2024 20:48:26 +0000 (09:48 +1300)]
Fix costing bug in MergeAppend

When building a MergeAppendPath which has child paths that are not
sorted correctly for the MergeAppend's sort order, we apply the cost of
sorting those paths to the MergeAppendPath costs.

Here we fix a bug where the number of tuples specified that needed to be
sorted was effectively pg_class.reltuples rather than the number of
expected row in the subpath.  This effectively penalizes MergeAppend
plans any time any filter is present on the MergeAppend subpath as the
sort cost added is to sort all tuples in the table rather than just the
ones expected the path to return.

This did not affect UNION ALL type queries as the RelOptInfo tuples is
set from the subquery's path rows.  It does affect MergeAppends uses for
inheritance and partitioned tables.

This is a long-standing bug introduced when MergeAppend was first added
in 11cad29c9.  No backpatch as this could result in plan changes.

Author: Alexander Kuzmenkov
Reviewed-by: Ashutosh Bapat, Aleksander Alekseev, David Rowley
Discussion: https://postgr.es/m/CALzhyqyhoXQDR-Usd_0HeWk%3DuqNLzoVeT8KhRoo%3DpV_KzgO3QQ%40mail.gmail.com

15 months agoClean pg_walsummary's tmp_check directory.
Tom Lane [Wed, 31 Jan 2024 16:50:35 +0000 (11:50 -0500)]
Clean pg_walsummary's tmp_check directory.

Oversight similar to ba08c10fc.

15 months agoIn 002_blocks.pl, try to prevent a HOT update.
Robert Haas [Wed, 31 Jan 2024 16:35:41 +0000 (11:35 -0500)]
In 002_blocks.pl, try to prevent a HOT update.

Make the new tuple larger than the old one so that it, hopefully, won't
manage to squeeze into leftover freespace on the same page. The test
is trying to verify that the UPDATE touches 2 pages, but if a HOT
update happens, then it doesn't.

Per buildfarm.

15 months agoUpdate pg_walsummary copyright notices to 2024
Alvaro Herrera [Wed, 31 Jan 2024 15:29:22 +0000 (16:29 +0100)]
Update pg_walsummary copyright notices to 2024

15 months agoRevise pg_walsummary's 002_blocks test to avoid spurious failures.
Robert Haas [Wed, 31 Jan 2024 15:12:53 +0000 (10:12 -0500)]
Revise pg_walsummary's 002_blocks test to avoid spurious failures.

Analysis of buildfarm results showed that the code that was intended
to wait for the inserts performed by this test to complete did not
actually do so. Try to make that logic more robust.

Improve error checking elsewhere in the script, too, so that we
don't miss things like poll_query_until failing.

Along the way, fix a bit of pgindent damage introduced by commit
5ddf9973477729cf161b4ad0a1efd52f4fea9c88, which aimed to help us
debug the failures that this commit is trying to fix. It's making
the buildfarm sad.

Discussion: http://postgr.es/m/CA+TgmobWFb8NqyfC31YnKAbZiXf9tLuwmyuvx=iYMXMniPQ4nw@mail.gmail.com

15 months agodoc: Document more that relations share a namespace
Peter Eisentraut [Wed, 31 Jan 2024 10:53:56 +0000 (11:53 +0100)]
doc: Document more that relations share a namespace

This was already documented in the CREATE INDEX reference, but not in
the introductory "Data Definition" chapter.  Also, document that the
index that implements a constraint has the same name as the
constraint.

Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: jian he <jian.universality@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CACJufxFG682tYcP9aH_F-jrqq5End8MHZR77zcp1%3DDUrEsSu1Q%40mail.gmail.com

15 months agoGive SMgrRelation pointers a well-defined lifetime.
Heikki Linnakangas [Wed, 31 Jan 2024 10:31:02 +0000 (12:31 +0200)]
Give SMgrRelation pointers a well-defined lifetime.

After calling smgropen(), it was not clear how long you could continue
to use the result, because various code paths including cache
invalidation could call smgrclose(), which freed the memory.

Guarantee that the object won't be destroyed until the end of the
current transaction, or in recovery, the commit/abort record that
destroys the underlying storage.

smgrclose() is now just an alias for smgrrelease(). It closes files
and forgets all state except the rlocator, but keeps the SMgrRelation
object valid.

A new smgrdestroy() function is used by rare places that know there
should be no other references to the SMgrRelation.

The short version:

 * smgrclose() is now just an alias for smgrrelease(). It releases
   resources, but doesn't destroy until EOX
 * smgrdestroy() now frees memory, and should rarely be used.

Existing code should be unaffected, but it is now possible for code that
has an SMgrRelation object to use it repeatedly during a transaction as
long as the storage hasn't been physically dropped.  Such code would
normally hold a lock on the relation.

This also replaces the "ownership" mechanism of SMgrRelations with a
pin counter.  An SMgrRelation can now be "pinned", which prevents it
from being destroyed at end of transaction.  There can be multiple pins
on the same SMgrRelation.  In practice, the pin mechanism is only used
by the relcache, so there cannot be more than one pin on the same
SMgrRelation.  Except with swap_relation_files XXX

Author: Thomas Munro, Heikki Linnakangas
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Discussion: https://www.postgresql.org/message-id/CA%2BhUKGJ8NTvqLHz6dqbQnt2c8XCki4r2QvXjBQcXpVwxTY_pvA@mail.gmail.com

15 months agoRemove some obsolete smgrcloseall() calls.
Heikki Linnakangas [Wed, 31 Jan 2024 09:40:29 +0000 (11:40 +0200)]
Remove some obsolete smgrcloseall() calls.

Before the advent of PROCSIGNAL_BARRIER_SMGRRELEASE, we didn't have a
comprehensive way to deal with Windows file handles that get in the way
of unlinking directories.  We had smgrcloseall() calls in a few places
to try to mitigate.

It's still a good idea for bgwriter and checkpointer to do that once per
checkpoint so they don't accumulate unbounded SMgrRelation objects, but
there is no longer any reason to close them at other random places such
as the error path, and the explanation as given in the comments is now
obsolete.

Author: Thomas Munro
Reviewed-by: Heikki Linnakangas, Robert Haas
Discussion: https://www.postgresql.org/message-id/CA%2BhUKGJ8NTvqLHz6dqbQnt2c8XCki4r2QvXjBQcXpVwxTY_pvA@mail.gmail.com

15 months agoAdd .gitignore to src/test/modules/gin/
Michael Paquier [Wed, 31 Jan 2024 06:12:22 +0000 (15:12 +0900)]
Add .gitignore to src/test/modules/gin/

This has been forgotten in 6a1ea02c491d.

15 months agoAdd tests for int4_bool() in int.c
Michael Paquier [Wed, 31 Jan 2024 06:02:28 +0000 (15:02 +0900)]
Add tests for int4_bool() in int.c

This cast was previously not covered at all by the regression tests.

Author: Christoph Berg
Discussion: https://postgr.es/m/ZYQZ1hNfLd_4rzkn@msg.df7cb.de

15 months agoConsider the "LIMIT 1" optimization with parallel DISTINCT
David Rowley [Wed, 31 Jan 2024 04:22:02 +0000 (17:22 +1300)]
Consider the "LIMIT 1" optimization with parallel DISTINCT

Similar to what was done in 5543677ec for non-parallel DISTINCT, apply
the same optimization when the distinct_pathkeys are empty for the
partial paths too.

This can be faster than the non-parallel version when the first row
matching the WHERE clause of the query takes a while to find.  Parallel
workers could speed that process up considerably.

Author: Richard Guo
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAMbWs49JC0qvfUbzs-TVzgMpSSBiMJ_6sN=BaA9iohBgYkr=LA@mail.gmail.com

15 months agoFix various issues with ALTER TEXT SEARCH CONFIGURATION
Michael Paquier [Wed, 31 Jan 2024 04:15:21 +0000 (13:15 +0900)]
Fix various issues with ALTER TEXT SEARCH CONFIGURATION

This commit addresses a set of issues when changing token type mappings
in a text search configuration when using duplicated token names:
- ADD MAPPING would fail on insertion because of a constraint failure
after inserting the same mapping.
- ALTER MAPPING with an "overridden" configuration failed with "tuple
already updated by self" when the token mappings are removed.
- DROP MAPPING failed with "tuple already updated by self", like
previously, but in a different code path.

The code is refactored so the token names (with their numbers) are
handled as a List with unique members rather than an array with numbers,
ensuring that no duplicates mess up with the catalog inserts, updates
and deletes.  The list is generated by getTokenTypes(), with the same
error handling as previously while duplicated tokens are discarded from
the list used to work on the catalogs.

Regression tests are expanded to cover much more ground for the cases
fixed by this commit, as there was no coverage for the code touched in
this commit.  A bit more is done regarding the fact that a token name
not supported by a configuration's parser should result in an error even
if IF EXISTS is used in a DROP MAPPING clause.  This is implied in the
code but there was no coverage for that, and it was very easy to miss.

These issues exist since at least their introduction in core with
140d4ebcb46e, so backpatch all the way down.

Reported-by: Alexander Lakhin
Author: Tender Wang, Michael Paquier
Discussion: https://postgr.es/m/18310-1eb233c5908189c8@postgresql.org
Backpatch-through: 12

15 months agoFix 003_extrafiles.pl test for the Windows
Andrew Dunstan [Tue, 30 Jan 2024 22:09:44 +0000 (17:09 -0500)]
Fix 003_extrafiles.pl test for the Windows

File::Find converts backslashes to slashes in the newer Perl versions.
See: https://github.com/Perl/perl5/commit/414f14df98cb1c9a20f92c5c54948b67c09f072d

So, do the same conversion for Windows before comparing paths. To
support all Perl versions, always convert them on Windows regardless of
the Perl's version.

Author: Nazir Bilal Yavuz <byavuz81@gmail.com>

Backpatch to all live branches

15 months agoSimplify partial path generation in GROUP BY/ORDER BY
David Rowley [Tue, 30 Jan 2024 21:10:59 +0000 (10:10 +1300)]
Simplify partial path generation in GROUP BY/ORDER BY

Here we consolidate the generation of partial sort and partial incremental
sort paths in a similar way to what was done in 4a29eabd1.  Since the cost
penalty for incremental sort was removed by that commit, there's no
point in creating a sort path on the cheapest partial path if an
incremental sort could be done instead.

This has the added benefit of reducing the amount of code required to
build these paths.

Author: Richard Guo
Reviewed-by: Etsuro Fujita, Shubham Khanna, David Rowley
Discussion: https://postgr.es/m/CAMbWs49PaKxBZU9cN7k3DKB7id+YfGfOfS9H_Fo5tkqPMt=fDg@mail.gmail.com

15 months agoSplit use of SerialSLRULock, creating SerialControlLock
Alvaro Herrera [Tue, 30 Jan 2024 17:11:17 +0000 (18:11 +0100)]
Split use of SerialSLRULock, creating SerialControlLock

predicate.c has been using SerialSLRULock (the control lock for its SLRU
structure) to coordinate access to SerialControlData, another of its
numerous shared memory structures; this is unnecessary and confuses
further SLRU scalability work.  Create a separate LWLock to cover
SerialControlData.

Extracted from a larger patch from the same author, and some additional
changes by Álvaro.

Author: Dilip Kumar <dilip.kumar@enterprisedb.com>
Discussion: https://postgr.es/m/CAFiTN-vzDvNz=ExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A@mail.gmail.com

15 months agoAdd a failover option to subscriptions.
Amit Kapila [Tue, 30 Jan 2024 11:01:09 +0000 (16:31 +0530)]
Add a failover option to subscriptions.

This commit introduces a new subscription option named 'failover', which
provides users with the ability to set the failover property of the
replication slot on the publisher when creating or altering a
subscription.

This uses the replication commands introduced by commit 7329240437 to
enable the failover option for a logical replication slot.

If the failover option is set to true, the associated replication slots
(i.e. the main slot and the table sync slots) in the upstream database are
enabled to be synchronized to the standbys. Note that the capability to
sync the replication slots will be added in subsequent commits.

Thanks to Masahiko Sawada for the design inputs.

Author: Shveta Malik, Hou Zhijie, Ajin Cherian
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com

15 months agopgcrypto: Fix check for buffer size
Daniel Gustafsson [Tue, 30 Jan 2024 10:15:46 +0000 (11:15 +0100)]
pgcrypto: Fix check for buffer size

The code copying the PGP block into the temp buffer failed to
account for the extra 2 bytes in the buffer which are needed
for the prefix. If the block was oversized, subsequent checks
of the prefix would have exceeded the buffer size.  Since the
block sizes are hardcoded in the list of supported ciphers it
can be verified that there is no live bug here. Backpatch all
the way for consistency though, as this bug is old.

Author: Mikhail Gribkov <youzhick@gmail.com>
Discussion: https://postgr.es/m/CAMEv5_uWvcMCMdRFDsJLz2Q8g16HEa9xWyfrkr+FYMMFJhawOw@mail.gmail.com
Backpatch-through: v12

15 months agoFix incorrect format placeholders for Oid
Peter Eisentraut [Tue, 30 Jan 2024 08:11:41 +0000 (09:11 +0100)]
Fix incorrect format placeholders for Oid

15 months agoDelay build of Memoize hash table until executor run
David Rowley [Mon, 29 Jan 2024 23:37:03 +0000 (12:37 +1300)]
Delay build of Memoize hash table until executor run

Previously this hash table was built during executor startup.  This
could cause long delays in EXPLAIN (without ANALYZE) when the planner
opts to use a large Memoize hash table.

No backpatch for now due to lack of complaints.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvoJktJ5XL=Kjh2a2TFr64R-7eQZV-+jcJrUwoES2GLiWg@mail.gmail.com

15 months agoDoc: mention foreign keys can reference unique indexes
David Rowley [Mon, 29 Jan 2024 21:15:17 +0000 (10:15 +1300)]
Doc: mention foreign keys can reference unique indexes

We seem to have only documented a foreign key can reference the columns of
a primary key or unique constraint.  Here we adjust the documentation
to mention columns in a non-partial unique index can be mentioned too.

The header comment for transformFkeyCheckAttrs() also didn't mention
unique indexes, so fix that too.  In passing make that header comment
reflect reality in the various other aspects where it deviated from it.

Bug: 18295
Reported-by: Gilles PARC
Author: Laurenz Albe, David Rowley
Discussion: https://www.postgresql.org/message-id/18295-0ed0fac5c9f7b17b%40postgresql.org
Backpatch-through: 12

15 months agoMove is_valid_ascii() to ascii.h.
Nathan Bossart [Mon, 29 Jan 2024 18:08:57 +0000 (12:08 -0600)]
Move is_valid_ascii() to ascii.h.

This function requires simd.h, which is a rather large dependency
for a widely-used header file like pg_wchar.h.  Furthermore, there
is a report of a third-party tool that is struggling to use
pg_wchar.h due to its dependence on simd.h (presumably because
simd.h uses several intrinsics).  Moving the function to the much
less popular ascii.h resolves these issues for now.

This commit is back-patched for the benefit of the aforementioned
third-party tool.  The simd.h dependency was only added in v16,
but we've opted to back-patch to v15 so that is_valid_ascii() lives
in the same file for all versions where it exists.  This could
break existing third-party code that uses the function, but we
couldn't find any examples of such code.  It should be possible to
fix any code that this commit breaks by including ascii.h in the
file that uses is_valid_ascii().

Author: Jubilee Young
Reviewed-by: Tom Lane, John Naylor, Andres Freund, Eric Ridge
Discussion: https://postgr.es/m/CAPNHn3oKJJxMsYq%2BqLYzVJOFrUcOr4OF1EC-KtFT-qh8nOOOtQ%40mail.gmail.com
Backpatch-through: 15

15 months agoFix incompatibilities with libxml2 >= 2.12.0.
Tom Lane [Mon, 29 Jan 2024 17:06:07 +0000 (12:06 -0500)]
Fix incompatibilities with libxml2 >= 2.12.0.

libxml2 changed the required signature of error handler callbacks
to make the passed xmlError struct "const".  This is causing build
failures on buildfarm member caiman, and no doubt will start showing
up in the field quite soon.  Add a version check to adjust the
declaration of xml_errorHandler() according to LIBXML_VERSION.

2.12.x also produces deprecation warnings for contrib/xml2/xpath.c's
assignment to xmlLoadExtDtdDefaultValue.  I see no good reason for
that to still be there, seeing that we disabled external DTDs (at a
lower level) years ago for security reasons.  Let's just remove it.

Back-patch to all supported branches, since they might all get built
with newer libxml2 once it gets a bit more popular.  (The back
branches produce another deprecation warning about xpath.c's use of
xmlSubstituteEntitiesDefault().  We ought to consider whether to
back-patch all or part of commit 65c5864d7 to silence that.  It's
less urgent though, since it won't break the buildfarm.)

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

15 months agoAdd EXPLAIN (MEMORY) to report planner memory consumption
Alvaro Herrera [Mon, 29 Jan 2024 16:53:03 +0000 (17:53 +0100)]
Add EXPLAIN (MEMORY) to report planner memory consumption

This adds a new "Memory:" line under the "Planning:" group (which
currently only has "Buffers:") when the MEMORY option is specified.

In order to make the reporting reasonably accurate, we create a separate
memory context for planner activities, to be used only when this option
is given.  The total amount of memory allocated by that context is
reported as "allocated"; we subtract memory in the context's freelists
from that and report that result as "used".  We use
MemoryContextStatsInternal() to obtain the quantities.

The code structure to show buffer usage during planning was not in
amazing shape, so I (Álvaro) modified the patch a bit to clean that up
in passing.

Author: Ashutosh Bapat
Reviewed-by: David Rowley, Andrey Lepikhov, Jian He, Andy Fan
Discussion: https://www.postgresql.org/message-id/CAExHW5sZA=5LJ_ZPpRO-w09ck8z9p7eaYAqq3Ks9GDfhrxeWBw@mail.gmail.com

15 months agoFix locking when fixing an incomplete split of a GIN internal page
Heikki Linnakangas [Mon, 29 Jan 2024 11:46:22 +0000 (13:46 +0200)]
Fix locking when fixing an incomplete split of a GIN internal page

ginFinishSplit() expects the caller to hold an exclusive lock on the
buffer, but when finishing an earlier "leftover" incomplete split of
an internal page, the caller held a shared lock. That caused an
assertion failure in MarkBufferDirty(). Without assertions, it could
lead to corruption if two backends tried to complete the split at the
same time.

On master, add a test case using the new injection point facility.

Report and analysis by Fei Changhong. Backpatch the fix to all
supported versions.

Reviewed-by: Fei Changhong, Michael Paquier
Discussion: https://www.postgresql.org/message-id/tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com

15 months agolibpq: Move cancellation related functions to fe-cancel.c
Alvaro Herrera [Mon, 29 Jan 2024 09:53:34 +0000 (10:53 +0100)]
libpq: Move cancellation related functions to fe-cancel.c

In follow up commits we'll add more functions related to query
cancellations.  This groups those all together instead of mixing them
with the other functions in fe-connect.c.

The formerly static parse_int_param() function had to be exported to
other libpq users, so it's been renamed pqParseIntParam() and moved to a
more reasonable place within fe-connect.c (rather than randomly between
various keepalive-related routines).

Author: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Discussion: https://postgr.es/m/AM5PR83MB0178D3B31CA1B6EC4A8ECC42F7529@AM5PR83MB0178.EURPRD83.prod.outlook.com

15 months agoDoc: Fix incorrect reference to conflicting column in pg_replication_slots.
Amit Kapila [Mon, 29 Jan 2024 06:43:39 +0000 (12:13 +0530)]
Doc: Fix incorrect reference to conflicting column in pg_replication_slots.

Commit 007693f2a3 changes the existing 'conflicting' field to
'conflict_reason' in pg_replication_slots but missed updating one of its
existing references.

Author: Hou Zhijie
Discussion: https://postgr.es/m/OS0PR01MB571690299199ACA80F602D97947E2@OS0PR01MB5716.jpnprd01.prod.outlook.com

15 months agoRemove make function vpathsearch
Peter Eisentraut [Mon, 29 Jan 2024 06:22:43 +0000 (07:22 +0100)]
Remove make function vpathsearch

This function served to support having prebuilt files in the source
tree for vpath builds.  This is no longer possible (since
721856ff24b); all built files are now always in the build tree.  The
invocations of this function are no longer required.

15 months agoFix comments in ReplicationSlotAcquire().
Amit Kapila [Mon, 29 Jan 2024 04:42:58 +0000 (10:12 +0530)]
Fix comments in ReplicationSlotAcquire().

They were incorrectly referring to a slot parameter in
ReplicationSlotAcquire() which is not passed to the API.

Author: Wang Wei
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/OS3PR01MB6275E3CE4DC15FF8B8B80D3A9E7A2@OS3PR01MB6275.jpnprd01.prod.outlook.com

15 months agoAllow setting failover property in the replication command.
Amit Kapila [Mon, 29 Jan 2024 03:40:00 +0000 (09:10 +0530)]
Allow setting failover property in the replication command.

This commit implements a new replication command called
ALTER_REPLICATION_SLOT and a corresponding walreceiver API function named
walrcv_alter_slot. Additionally, the CREATE_REPLICATION_SLOT command has
been extended to support the failover option.

These new additions allow the modification of the failover property of a
replication slot on the publisher. A subsequent commit will make use of
these commands in subscription commands and will add the tests as well to
cover the functionality added/changed by this commit.

Author: Hou Zhijie, Shveta Malik
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda, Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com

15 months agoRemove ReorderBufferTupleBuf structure.
Masahiko Sawada [Mon, 29 Jan 2024 01:37:16 +0000 (10:37 +0900)]
Remove ReorderBufferTupleBuf structure.

Since commit a4ccc1cef, the 'node' and 'alloc_tuple_size' fields of
the ReorderBufferTupleBuf structure are no longer used. This leaves
only the 'tuple' field in the structure. Since keeping a single-field
structure makes little sense, the ReorderBufferTupleBuf is removed
entirely. The code is refactored accordingly.

No back-patching since these are ABI changes in an exposed structure
and functions, and there would be some risk of breaking extensions.

Author: Aleksander Alekseev
Reviewed-by: Amit Kapila, Masahiko Sawada, Reid Thompson
Discussion: https://postgr.es/m/CAD21AoCvnuxiXXfRecp7g9+CeC35POQfhuQeJFr7_9u_Q5jc_Q@mail.gmail.com

15 months agoFix DROP ROLE when specifying duplicated roles
Michael Paquier [Sun, 28 Jan 2024 23:05:59 +0000 (08:05 +0900)]
Fix DROP ROLE when specifying duplicated roles

This commit fixes failures with "tuple already updated by self" when
listing twice the same role and in a DROP ROLE query.

This is an oversight in 6566133c5f52, that has introduced a two-phase
logic in DropRole() where dependencies of all the roles to drop are
removed in a first phase, with the roles themselves removed from
pg_authid in a second phase.

The code is simplified to not rely on a List of ObjectAddress built in
the first phase used to remove the pg_authid entries in the second
phase, switching to a list of OIDs.  Duplicated OIDs can be simply
avoided in the first phase thanks to that.  Using ObjectAddress was not
necessary for the roles as they are not used for anything specific to
dependency.c, building all the ObjectAddress in the List with
AuthIdRelationId as class ID.

In 15 and older versions, where a single phase is used, DROP ROLE with
duplicated role names would fail on "role \"blah\" does not exist" for
the second entry after the CCI() done by the first deletion.  This is
not really incorrect, but it does not seem worth changing based on a
lack of complaints.

Reported-by: Alexander Lakhin
Reviewed-by: Tender Wang
Discussion: https://postgr.es/m/18310-1eb233c5908189c8@postgresql.org
Backpatch-through: 16

15 months agoAttempt to fix newly added Memoize regression test
David Rowley [Fri, 26 Jan 2024 22:17:35 +0000 (11:17 +1300)]
Attempt to fix newly added Memoize regression test

Both drongo and fairywren seem not to like a new regression test added
by 2cca95e17.  These machines show a different number of actual rows in
the EXPLAIN ANALYZE output.  Since the number of actual rows is divided by
the number of loops, I suspect this might be due to some platform
dependant rounding behavior as the total row count is 5 and the number of
loops is 2.  drongo and fairywren seem to be calculating that 5.0 / 2.0 is
3, whereas most other machines think the answer is 2.

Here we tweak the test query's WHERE clause so it's 4.0 / 2.0 instead.
There shouldn't be too much wiggle room for platform dependant-behavior to
be a factor with those numbers.

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

15 months agoCompare varnullingrels too in assign_param_for_var().
Tom Lane [Fri, 26 Jan 2024 20:54:17 +0000 (15:54 -0500)]
Compare varnullingrels too in assign_param_for_var().

Oversight in 2489d76c4.  Preliminary analysis suggests that the
problem may be unreachable --- but if we did have instances of
the same column with different varnullingrels, we'd surely need
to treat them as different Params.

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