postgresql.git
2 years agopsql: Add command to use extended query protocol
Peter Eisentraut [Tue, 15 Nov 2022 12:50:27 +0000 (13:50 +0100)]
psql: Add command to use extended query protocol

This adds a new psql command \bind that sets query parameters and
causes the next query to be sent using the extended query protocol.
Example:

    SELECT $1, $2 \bind 'foo' 'bar' \g

This may be useful for psql scripting, but one of the main purposes is
also to be able to test various aspects of the extended query protocol
from psql and to write tests more easily.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/e8dd1cd5-0e04-3598-0518-a605159fe314@enterprisedb.com

2 years agolibpq error message refactoring, part 2
Peter Eisentraut [Tue, 15 Nov 2022 10:50:04 +0000 (11:50 +0100)]
libpq error message refactoring, part 2

This applies the new APIs to the code.

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com

2 years agolibpq error message refactoring
Peter Eisentraut [Tue, 15 Nov 2022 10:50:04 +0000 (11:50 +0100)]
libpq error message refactoring

libpq now contains a mix of error message strings that end with
newlines and don't end with newlines, due to some newer code paths
with new ways of passing errors around.  This leads to confusion and
mistakes both during development and translation.

This adds new functions libpq_append_error() and
libpq_append_conn_error() that encapsulate common code paths for
producing error message strings.  Notably, these functions append the
newline, so that the string appearing in the code does not end with a
newline.  This makes (almost) all error message strings in libpq
uniform in this regard (and also consistent with how we handle it
outside of libpq code).  (There are a few exceptions that are
difficult to fit into this scheme, but they are only a few.)

Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://www.postgresql.org/message-id/flat/7c0232ef-7b44-68db-599d-b327d0640a77@enterprisedb.com

2 years agoDisallow setting archive_library and archive_command at the same time
Peter Eisentraut [Tue, 15 Nov 2022 09:03:12 +0000 (10:03 +0100)]
Disallow setting archive_library and archive_command at the same time

Setting archive_library and archive_command at the same time is now an
error.  Before, archive_library would take precedence over
archive_command.

Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://www.postgresql.org/message-id/20220914222736.GA3042279%40nathanxps13

2 years agoImprove comments referring snapshot's subxip array.
Amit Kapila [Tue, 15 Nov 2022 04:07:19 +0000 (09:37 +0530)]
Improve comments referring snapshot's subxip array.

It was referred to as subxact array in a few places and subxip array in
others. By changing it to subxip array, we make it consistent with similar
references to xip array.

Author: Japin Li
Reviewd by: Julien Rouhaud, Richard Guo
Discussion: https://postgr.es/m/MEYP282MB1669DCE7AC193A947CED2A95B6009@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoFix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.
Amit Kapila [Mon, 14 Nov 2022 05:13:33 +0000 (10:43 +0530)]
Fix cleanup lock acquisition in SPLIT_ALLOCATE_PAGE replay.

During XLOG_HASH_SPLIT_ALLOCATE_PAGE replay, we were checking for a
cleanup lock on the new bucket page after acquiring an exclusive lock on
it and raising a PANIC error on failure. However, it is quite possible
that checkpointer can acquire the pin on the same page before acquiring a
lock on it, and then the replay will lead to an error. So instead, directly
acquire the cleanup lock on the new bucket page during
XLOG_HASH_SPLIT_ALLOCATE_PAGE replay operation.

Reported-by: Andres Freund
Author: Robert Haas
Reviewed-By: Amit Kapila, Andres Freund, Vignesh C
Backpatch-through: 11
Discussion: https://postgr.es/m/20220810022617.fvjkjiauaykwrbse@awork3.anarazel.de

2 years agoAdd error context callback when tokenizing authentication files
Michael Paquier [Mon, 14 Nov 2022 02:58:10 +0000 (11:58 +0900)]
Add error context callback when tokenizing authentication files

The parsing of the authentication files for HBA and ident entries
happens in two phases:
- Tokenization of the files, creating a list of TokenizedAuthLines.
- Validation of the HBA and ident entries, building a set of HbaLines or
IdentLines.

The second phase doing the validation provides already some error
context about the configuration file and the line where a problem
happens, but there is no such information in the first phase when
tokenizing the files.  This commit adds an ErrorContextCallback in
tokenize_auth_file(), with a context made of the line number and the
configuration file name involved in a problem.  This is useful for files
included in an HBA file for user and database lists, and it will become
much more handy to track problems for files included via a potential
@include[_dir,_if_exists].

The error context is registered so as the full chain of events is
reported when using cascaded inclusions when for example
tokenize_auth_file() recurses over itself on new files, displaying one
context line for each file gone through when tokenizing things.

Author: Michael Paquier
Reviewed-by: Julien Rouhaud
Discussion: https://postgr.es/m/Y2xUBJ+S+Z0zbxRW@paquier.xyz

2 years agoInvent open_auth_file() in hba.c to refactor authentication file opening
Michael Paquier [Mon, 14 Nov 2022 01:21:42 +0000 (10:21 +0900)]
Invent open_auth_file() in hba.c to refactor authentication file opening

This adds a check on the recursion depth when including authentication
configuration files, something that has never been done when processing
'@' files for database and user name lists in pg_hba.conf.  On HEAD,
this was leading to a rather confusing error, as of:
FATAL:  exceeded maxAllocatedDescs (NN) while trying to open file "/path/blah.conf"

This refactors the code so as the error reported is now the following,
which is the same as for GUCs:
FATAL: could not open file "/path/blah.conf": maximum nesting depth exceeded

This reduces a bit the verbosity of the error message used for files
included in user and database lists, reporting only the file name of
what's failing to load, without mentioning the relative or absolute path
specified after '@' in a HBA file.  The absolute path is built upon what
'@' defines anyway, so there is no actual loss of information.  This
makes the future inclusion logic much simpler.  A follow-up patch will
add an error context to be able to track on which line of which file the
inclusion is failing, to close the loop, providing all the information
needed to know the full chain of events.

This logic has been extracted from a larger patch written by Julien,
rewritten by me to have a unique code path calling AllocateFile() on
authentication files, and is useful on its own.  This new interface
will be used later for authentication files included with
@include[_dir,_if_exists], in a follow-up patch.

Author: Michael Paquier, Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/Y2xUBJ+S+Z0zbxRW@paquier.xyz

2 years agolibpq: Add missing newlines to error messages
Peter Eisentraut [Sun, 13 Nov 2022 20:09:09 +0000 (21:09 +0100)]
libpq: Add missing newlines to error messages

2 years agolibpq: Remove unneeded cast and adjust format placeholder
Peter Eisentraut [Sun, 13 Nov 2022 20:09:05 +0000 (21:09 +0100)]
libpq: Remove unneeded cast and adjust format placeholder

2 years agoMake Bitmapsets be valid Nodes.
Tom Lane [Sun, 13 Nov 2022 15:22:45 +0000 (10:22 -0500)]
Make Bitmapsets be valid Nodes.

Add a NodeTag field to struct Bitmapset.  This is free because of
alignment considerations on 64-bit hardware.  While it adds some
space on 32-bit machines, we aren't optimizing for that case anymore.
The advantage is that data structures such as Lists of Bitmapsets
are now first-class objects to the Node infrastructure, and don't
require special-case code to handle.

This patch includes removal of one such special case, in indxpath.c:
bms_equal_any() can now be replaced by list_member().  There may be
more existing code that could be simplified, but I didn't look very
hard.  We also get to drop the read_write_ignore annotations on a
couple of RelOptInfo fields.

The outfuncs/readfuncs support is arranged so that nothing changes
in the string representation of a Bitmapset field; therefore, this
doesn't need a catversion bump.

Amit Langote and Tom Lane

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

2 years agoUse installed postgresql.conf.sample for GUC sanity TAP test
Andrew Dunstan [Sun, 13 Nov 2022 14:07:53 +0000 (09:07 -0500)]
Use installed postgresql.conf.sample for GUC sanity TAP test

The current code looks for the sample file in the source directory, but
it seems better to test against the installed sample file.

Backpatch to release 15 where the test was introduced.

Discussion: https://postgr.es/m/73eea68e-3b6f-5f63-6024-25ed26b52016@dunslane.net

Reviewed by Tom Lane, Alvaro Herrera, Michael Paquier.

2 years agoMake PostgreSQL::Test::Cluster::config_data more flexible
Andrew Dunstan [Sun, 13 Nov 2022 13:45:14 +0000 (08:45 -0500)]
Make PostgreSQL::Test::Cluster::config_data more flexible

Currently this only allows for one argument, which must be present, and
always returns a single string. With this change the following now all
work:

  $all_config = $node->config_data;
  %config_map = ($node->config_data);
  $incdir = $node->config_data('--include-dir');
  ($incdir, $sharedir) = $node->config_data(
      qw(--include-dir --share-dir));

Backpatch to release 15 where this was introduced.

Discussion: https://postgr.es/m/73eea68e-3b6f-5f63-6024-25ed26b52016@dunslane.net

Reviewed by Tom Lane, Alvaro Herrera, Michael Paquier.

2 years agoRefactor aclcheck functions
Peter Eisentraut [Sun, 13 Nov 2022 07:11:17 +0000 (08:11 +0100)]
Refactor aclcheck functions

Instead of dozens of mostly-duplicate pg_foo_aclcheck() functions,
write one common function object_aclcheck() that can handle almost all
of them.  We already have all the information we need, such as which
system catalog corresponds to which catalog table and which column is
the ACL column.

There are a few pg_foo_aclcheck() that don't work via the generic
function and have special APIs, so those stay as is.

I also changed most pg_foo_aclmask() functions to static functions,
since they are not used outside of aclchk.c.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/95c30f96-4060-2f48-98b5-a4392d3b6066@enterprisedb.com

2 years agoRefactor ownercheck functions
Peter Eisentraut [Sun, 13 Nov 2022 07:11:17 +0000 (08:11 +0100)]
Refactor ownercheck functions

Instead of dozens of mostly-duplicate pg_foo_ownercheck() functions,
write one common function object_ownercheck() that can handle almost
all of them.  We already have all the information we need, such as
which system catalog corresponds to which catalog table and which
column is the owner column.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Discussion: https://www.postgresql.org/message-id/flat/95c30f96-4060-2f48-98b5-a4392d3b6066@enterprisedb.com

2 years agoAdd repalloc0 and repalloc0_array
Peter Eisentraut [Sat, 12 Nov 2022 19:31:27 +0000 (20:31 +0100)]
Add repalloc0 and repalloc0_array

These zero out the space added by repalloc.  This is a common pattern
that is quite hairy to code by hand.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://www.postgresql.org/message-id/b66dfc89-9365-cb57-4e1f-b7d31813eeec@enterprisedb.com

2 years agoIf wait_for_catchup fails under has_wal_read_bug, skip balance of test.
Noah Misch [Sat, 12 Nov 2022 19:19:50 +0000 (11:19 -0800)]
If wait_for_catchup fails under has_wal_read_bug, skip balance of test.

Test files should now ignore has_wal_read_bug() so long as
wait_for_catchup() is their only known way of reaching the bug.  That's
at least five files today, a number expected to grow over time.  This
commit removes skip logic from three.  By doing so, systems having the
bug regain the ability to catch other kinds of defects via those three
tests.  The other two, 002_databases.pl and 031_recovery_conflict.pl,
have been unprotected.  Back-patch to v15, where done_testing() first
became our standard.

Discussion: https://postgr.es/m/20221030031639.GA3082137@rfd.leadboat.com

2 years agoFix volatility marking of timestamptz_trunc_zone.
Tom Lane [Sat, 12 Nov 2022 18:29:41 +0000 (13:29 -0500)]
Fix volatility marking of timestamptz_trunc_zone.

It's safe to mark this as immutable, because it does not depend
on the timezone GUC setting.  Oversight in commit 600b04d6b.

(There's an argument that timezone definitions do change from
time to time, but we have not worried about that in marking
other timestamp-related functions; for example AT TIME ZONE
has always been considered immutable.  The situation is no
worse than our problems with time-varying locales, surely.)

Przemysław Sztoch

Discussion: https://postgr.es/m/eaa3fabe-50fc-bbe8-b096-ce62ddadab85@sztoch.pl

2 years agoDocument WAL rules related to PD_ALL_VISIBLE in README.
Jeff Davis [Sat, 12 Nov 2022 16:37:50 +0000 (08:37 -0800)]
Document WAL rules related to PD_ALL_VISIBLE in README.

Also improve comments.

Discussion: https://postgr.es/m/a50005c1c537f89bb359057fd70e66bb83bce969.camel@j-davis.com
Reviewed-by: Peter Geoghegan
2 years agoFix theoretical torn page hazard.
Jeff Davis [Thu, 10 Nov 2022 22:46:30 +0000 (14:46 -0800)]
Fix theoretical torn page hazard.

The original report was concerned with a possible inconsistency
between the heap and the visibility map, which I was unable to
confirm. The concern has been retracted.

However, there did seem to be a torn page hazard when using
checksums. By not setting the heap page LSN during redo, the
protections of minRecoveryPoint were bypassed. Fixed, along with a
misleading comment.

It may have been impossible to hit this problem in practice, because
it would require a page tear between the checksum and the flags, so I
am marking this as a theoretical risk. But, as discussed, it did
violate expectations about the page LSN, so it may have other
consequences.

Backpatch to all supported versions.

Reported-by: Konstantin Knizhnik
Reviewed-by: Konstantin Knizhnik
Discussion: https://postgr.es/m/fed17dac-8cb8-4f5b-d462-1bb4908c029e@garret.ru
Backpatch-through: 11

2 years agoRemove obsolete comments and code from prior to f8f4227976.
Jeff Davis [Fri, 11 Nov 2022 16:40:01 +0000 (08:40 -0800)]
Remove obsolete comments and code from prior to f8f4227976.

XLogReadBufferForRedo() and XLogReadBufferForRedoExtended() only return
BLK_NEEDS_REDO if the record LSN is greater than the page LSN, so
the redo routine doesn't need to do the LSN check again.

Discussion: https://postgr.es/m/0c37b80e62b1f3007d5a6d1292bd8fa0c275627a.camel@j-davis.com

2 years agomeson: Define HAVE_LOCALE_T for msvc
Peter Eisentraut [Fri, 11 Nov 2022 15:00:48 +0000 (16:00 +0100)]
meson: Define HAVE_LOCALE_T for msvc

Meson doesn't see the redefinition of locale_t done in
src/include/port/win32_port.h, so it is not defining HAVE_LOCALE_T,
HAVE_WCSTOMBS_L nor HAVE_MBSTOWCS_L as the current
src/tools/msvc/build.pl script does.  Add manual overrides to fix.

Author: Author: Juan Jose Santamaria Flecha <juanjo.santamaria@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAC%2BAXB1wJEqfKCuVcNpoH%3Dgxd61N%3D7c2fR3Ew6YRPpSfEUA%3DyQ%40mail.gmail.com

2 years agoSupport writing "CREATE/ALTER TABLE ... SET STORAGE DEFAULT".
Tom Lane [Thu, 10 Nov 2022 23:20:49 +0000 (18:20 -0500)]
Support writing "CREATE/ALTER TABLE ... SET STORAGE DEFAULT".

We already allow explicitly writing DEFAULT for SET COMPRESSION,
so it seems a bit inflexible and non-orthogonal to not have it
for STORAGE.

Aleksander Alekseev

Discussion: https://postgr.es/m/CAJ7c6TMX9ui+6y3TQFaXJYVpZyBukvqhQbVDJ8OUokeLRhtnpA@mail.gmail.com

2 years agoFix alter_table.sql test case to test what it claims to.
Tom Lane [Thu, 10 Nov 2022 22:24:26 +0000 (17:24 -0500)]
Fix alter_table.sql test case to test what it claims to.

The stanza "SET STORAGE may need to add a TOAST table" does not
test what it's supposed to, and hasn't done so since we added
the ability to store constant column default values as metadata.
We need to use a non-constant default to get the expected table
rewrite to actually happen.

Fix that, and add the missing checks that would have exposed the
problem to begin with.

Noted while reviewing a patch that made changes in this test case.
Back-patch to v11 where the problem came in.

2 years agoFix comments atop ReorderBufferAddInvalidations.
Amit Kapila [Thu, 10 Nov 2022 11:26:49 +0000 (16:56 +0530)]
Fix comments atop ReorderBufferAddInvalidations.

The comments atop seem to indicate that we always accumulate invalidation
messages in a top-level transaction which is neither required nor matches
with the code.

Author: Amit Kapila
Reviewd by: Masahiko Sawada
Backpatch-through: 14, where it was introduced in commit c55040ccd0
Discussion: https://postgr.es/m/CAA4eK1LxGgnUroPz8STb6OfjVU1yaHoSA+T63URwmGCLdMJ0LA@mail.gmail.com

2 years agoFix comment of SimpleLruInit() in slru.c
Michael Paquier [Thu, 10 Nov 2022 07:32:29 +0000 (16:32 +0900)]
Fix comment of SimpleLruInit() in slru.c

sync_handler was not mentioned in the comment block of the function.

Oversight in dee663f.

Author: Aleksander Alekseev
Discussion: https://postgr.es/m/CAJ7c6TPUd9BwNY47TtMxaijLHSbyHNdhu=kvbGnvO_bi+oC6_Q@mail.gmail.com
Backpatch-through: 14

2 years agoApply a better fix to mdunlinkfork().
Tom Lane [Wed, 9 Nov 2022 19:15:38 +0000 (14:15 -0500)]
Apply a better fix to mdunlinkfork().

Replace the stopgap fix I made in 0e758ae89 with a cleaner one.

The real problem with 4ab5dae94 is that it contorted this function's
logic substantially, by introducing a third code path that required
different behavior in the function's main loop.  That seems quite
unnecessary on closer inspection: the new IsBinaryUpgrade case can
just share the behavior of the other immediate-unlink cases.  Hence,
revert 4ab5dae94 and most of 0e758ae89 (keeping the latter's
save/restore errno fix), and add IsBinaryUpgrade to the set of
conditions tested to choose immediate unlink.

Also fix some additional places with sloppy handling of errno,
to ensure we have an invariant that we always continue processing
after any non-ENOENT failure of do_truncate.  I doubt that that's
fixing any bug of field importance, so I don't feel it necessary to
back-patch; but we might as well get it right while we're here.

Also improve the comments, which had drifted a bit from what the
code actually does, and neglected to mention some important
considerations.

Back-patch to v15, not because this is fixing any bug but because
it doesn't seem like a good idea for v15's mdunlinkfork logic to be
significantly different from both v14 and v16.

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

2 years agoRemove redundant declaration for XidInMVCCSnapshot
Alvaro Herrera [Wed, 9 Nov 2022 17:27:31 +0000 (18:27 +0100)]
Remove redundant declaration for XidInMVCCSnapshot

This was added for no good reason by c91560defc57, after b7eda3e0e334
had just moved the prototype from utils/tqual.h to utils/snapmgr.h.

Author: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/MEYP282MB16693A409F3282A9DB287BADB63E9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoReport a more useful error for reloptions on a partitioned table.
Tom Lane [Wed, 9 Nov 2022 17:28:34 +0000 (12:28 -0500)]
Report a more useful error for reloptions on a partitioned table.

Previously, trying to set storage parameters on a partitioned table
always led to "unrecognized parameter foo", because the code expected
there might be some valid parameters; but there aren't any.  The docs
make clear that it's intended that there never will be any, so let's
replace this useless search with a more to-the-point message.

Simon Riggs and Karina Litskevich

Discussion: https://postgr.es/m/CANbhV-H=eZ9kTR9mUgKGK0Qv9uXP=U+dQg3rinQHfTdFMhBA2A@mail.gmail.com

2 years agoDoc: add comments about PreventInTransactionBlock/IsInTransactionBlock.
Tom Lane [Wed, 9 Nov 2022 16:08:52 +0000 (11:08 -0500)]
Doc: add comments about PreventInTransactionBlock/IsInTransactionBlock.

Add a little to the header comments for these functions to make it
clearer what guarantees about commit behavior are provided to callers.
(See commit f92944137 for context.)

Although this is only a comment change, it's really documentation
aimed at authors of extensions, so it seems appropriate to back-patch.

Yugo Nagata and Tom Lane, per further discussion of bug #17434.

Discussion: https://postgr.es/m/17434-d9f7a064ce2a88a3@postgresql.org

2 years agoProvide sigaction() for Windows.
Thomas Munro [Wed, 9 Nov 2022 00:05:16 +0000 (13:05 +1300)]
Provide sigaction() for Windows.

Commit 9abb2bfc left behind code to block signals inside signal
handlers on Windows, because our signal porting layer didn't have
sigaction().  Provide a minimal implementation that is capable of
blocking signals, to get rid of platform differences.  See also related
commit c94ae9d8.

Discussion: https://postgr.es/m/CA%2BhUKGKKKfcgx6jzok9AYenp2TNti_tfs8FMoJpL8%2B0Gsy%3D%3D_A%40mail.gmail.com

2 years agoUse AbsoluteConfigLocation() when building an included path in hba.c
Michael Paquier [Tue, 8 Nov 2022 23:47:02 +0000 (08:47 +0900)]
Use AbsoluteConfigLocation() when building an included path in hba.c

The code building an absolute path to a file included, as prefixed by
'@' in authentication files, for user and database lists uses the same
logic as for GUCs, except that it has no need to know about DataDir as
there is always a calling file to rely to build the base directory path.
The refactoring done in a1a7bb8 makes this move straight-forward, and
unifies the code used for GUCs and authentication files, and the
intention is to rely also on that for the upcoming patch to be able to
include full files from HBA or ident files.

Note that this gets rid of an inconsistency introduced in 370f909, that
copied the logic coming from GUCs but applied it for files included in
authentication files, where the result buffer given to
join_path_components() must have a size of MAXPGPATH.  Based on a
double-check of the existing code, all the other callers of
join_path_components() already do that, except the code path changed
here.

Discussion: https://postgr.es/m/Y2igk7q8OMpg+Yta@paquier.xyz

2 years agoDoc: improve tutorial section about grouped aggregates.
Tom Lane [Tue, 8 Nov 2022 23:25:03 +0000 (18:25 -0500)]
Doc: improve tutorial section about grouped aggregates.

Commit fede15417 introduced FILTER by jamming it into the existing
example introducing HAVING, which seems pedagogically poor to me;
and it added no information about what the keyword actually does.
Not to mention that the claimed output didn't match the sample
data being used in this running example.

Revert that and instead make an independent example using FILTER.
To help drive home the point that it's a per-aggregate filter,
we need to use two aggregates not just one; for consistency
expand all the examples in this segment to do that.

Also adjust the example using WHERE ... LIKE so that it'd produce
nonempty output with this sample data, and show that output.

Back-patch, as the previous patch was.  (Sadly, v10 is now out
of scope.)

Discussion: https://postgr.es/m/166794307526.652.9073408178177444190@wrigleys.postgresql.org

2 years agoUnify some internal error message wordings
Peter Eisentraut [Tue, 8 Nov 2022 17:45:29 +0000 (18:45 +0100)]
Unify some internal error message wordings

2 years agoProduce more-optimal plans for bitmap scans on boolean columns.
Tom Lane [Tue, 8 Nov 2022 15:36:04 +0000 (10:36 -0500)]
Produce more-optimal plans for bitmap scans on boolean columns.

The planner simplifies boolean comparisons such as "x = true" and
"x = false" down to "x" and "NOT x" respectively, to have a canonical
form to ease comparisons.  However, if we want to use an index on x,
the index AM APIs require us to reconstitute the comparison-operator
form of the indexqual.  While that works, in bitmap indexscans the
canonical form of the qual was emitted as a "filter" condition
although it really only needs to be a "recheck" condition, because
create_bitmap_scan_plan didn't recognize the equivalence of that
form with the generated indexqual.  booleq() is pretty cheap so that
likely doesn't make very much difference, but it's unsightly so
let's clean it up.

To fix, add a case to predicate_implied_by() to recognize the
equivalence of such clauses.  This is a relatively low-cost place to
add a check, and perhaps it will have additional use cases in future.

Richard Guo and Tom Lane, per discussion of bug #17618 from Sindy
Senorita.

Discussion: https://postgr.es/m/17618-7a2240bfaa7e84ae@postgresql.org

2 years agoSuppress useless wakeups in walreceiver.
Thomas Munro [Tue, 8 Nov 2022 07:36:36 +0000 (20:36 +1300)]
Suppress useless wakeups in walreceiver.

Instead of waking up 10 times per second to check for various timeout
conditions, keep track of when we next have periodic work to do.

Author: Thomas Munro <thomas.munro@gmail.com>
Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/CA%2BhUKGJGhX4r2LPUE3Oy9BX71Eum6PBcS8L3sJpScR9oKaTVaA%40mail.gmail.com

2 years agopsql: Add information in \d+ about foreign partitions and child tables
Michael Paquier [Tue, 8 Nov 2022 05:19:09 +0000 (14:19 +0900)]
psql: Add information in \d+ about foreign partitions and child tables

\d+ is already able to show if a partition or a child table is
"PARTITIONED" via its relkind, hence the addition of a keyword for
"FOREIGN" in the relation description is basically free.

Author: Ian Lawrence Barwick
Reviewed-by: Tom Lane, Michael Paquier
Discussion: https://postgr.es/m/CAB8KJ=iwzbEz2HR9EhNxQLVhMk2G_OYtQPJ9V=jWLadseggrOA@mail.gmail.com

2 years agoUse pg_pwrite_zeros() in walmethods.c
Michael Paquier [Tue, 8 Nov 2022 03:37:11 +0000 (12:37 +0900)]
Use pg_pwrite_zeros() in walmethods.c

This change impacts pg_receivewal and pg_basebackup, for the pre-padding
with zeros of all the new non-compressed WAL segments, so as the code is
more robust on partial writes.  This makes the code consistent with the
backend (XLogFileInitInternal) when wal_init_zeros is enabled for the
WAL segment initialization.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Andres Freund, Thomas Munro, Michael
Paquier
Discussion: https://postgr.es/m/CALj2ACUq7nAb7=bJNbK3yYmp-SZhJcXFR_pLk8un6XgDzDF3OA@mail.gmail.com

2 years agoIntroduce pg_pwrite_zeros() in fileutils.c
Michael Paquier [Tue, 8 Nov 2022 03:23:46 +0000 (12:23 +0900)]
Introduce pg_pwrite_zeros() in fileutils.c

This routine is designed to write zeros to a file using vectored I/O,
for a size given by its caller, being useful when it comes to
initializing a file with a final size already known.

XLogFileInitInternal() in xlog.c is changed to use this new routine when
initializing WAL segments with zeros (wal_init_zero enabled).  Note that
the aligned buffers used for the vectored I/O writes have a size of
XLOG_BLCKSZ, and not BLCKSZ anymore, as pg_pwrite_zeros() relies on
PGAlignedBlock while xlog.c originally used PGAlignedXLogBlock.

This routine will be used in a follow-up patch to do the pre-padding of
WAL segments for pg_receivewal and pg_basebackup when these are not
compressed.

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Andres Freund, Thomas Munro, Michael
Paquier
Discussion: https://www.postgresql.org/message-id/CALj2ACUq7nAb7%3DbJNbK3yYmp-SZhJcXFR_pLk8un6XgDzDF3OA%40mail.gmail.com

2 years agoFix initialization of pg_stat_get_lastscan()
Michael Paquier [Tue, 8 Nov 2022 01:50:09 +0000 (10:50 +0900)]
Fix initialization of pg_stat_get_lastscan()

A NULL result should be reported when a stats timestamp is set to 0, but
c037471 missed that, leading to a confusing timestamp value after for
example a DML on a freshly-created relation with no scans done on it
yet.

This impacted the following attributes for two system views:
- pg_stat_all_tables.last_idx_scan
- pg_stat_all_tables.last_seq_scan
- pg_stat_all_indexes.last_idx_scan

Reported-by: Robert Treat
Analyzed-by: Peter Eisentraut
Author: Dave Page
Discussion: https://postgr.es/m/CABV9wwPzMfSaz3EfKXXDxKmMprbxwF5r6WPuxqA=5mzRUqfTGg@mail.gmail.com

2 years agoFix compiler warning on MSVC
David Rowley [Mon, 7 Nov 2022 21:54:04 +0000 (10:54 +1300)]
Fix compiler warning on MSVC

MSVC does not understand that ereport(ERROR) does not return, so just
return the first enum PartitionStrategy value to keep the compiler from
complaining about the missing return.

Discussion: https://postgr.es/m/20221104161934.GB16921@telsasoft.com

2 years agoFix failure to remove non-first segments of temporary tables.
Tom Lane [Mon, 7 Nov 2022 16:36:45 +0000 (11:36 -0500)]
Fix failure to remove non-first segments of temporary tables.

Commit 4ab5dae94 broke mdunlinkfork's logic for removing additional
segments of a multi-gigabyte table, because it neglected to advance
"segno" after unlinking the first segment, in the code path where it
chooses to unlink that one immediately.  Then the main remove loop
gets ENOENT at segment zero and figures it's done, so we never remove
whatever additional segments might exist.

The main problem here is with large temporary tables, but WAL replay
of a drop of a large regular table would also fail to remove extra
segments.  The third case where this path is taken is for non-main
forks; but I doubt it matters for those since they probably never
exceed 1GB.

The simplest fix is just to increment segno after that unlink().
(Probably this logic could do with a more thorough rethink, but not
with mere hours to go before 15.1 wraps.)

While here, also fix an incautious assumption that
register_forget_request cannot change errno.  I don't think that
that has any really bad consequences, as we'd end up trying to unlink
the zero'th segment either way, but it greatly complicates reasoning
about what could happen here.  Also make a couple of other cosmetic
fixes.

Per bug #17679 from Balazs Szilfai.  Back-patch into v15, as the
faulty patch was.

Discussion: https://postgr.es/m/17679-1095d04450cf6a6e@postgresql.org

2 years agoMove code related to configuration files in directories to new file
Michael Paquier [Mon, 7 Nov 2022 03:31:38 +0000 (12:31 +0900)]
Move code related to configuration files in directories to new file

The code in charge of listing and classifying a set of configuration
files in a directory was located in guc-file.l, being used currently for
GUCs under "include_dir".  This code is planned to be used for an
upcoming feature able to include configuration files for ident and HBA
files from a directory, similarly to GUCs.  In both cases, the file
names, suffixed by ".conf", have to be ordered alphabetically.  This
logic is moved to a new file, called conffiles.c, so as it is easier to
share this facility between GUCs and the HBA/ident parsing logic.

Author: Julien Rouhaud, Michael Paquier
Discussion: https://postgr.es/m/Y2IgaH5YzIq2b+iR@paquier.xyz

2 years agoDon't pass down nonnullable_vars while reducing outer joins.
Tom Lane [Sat, 5 Nov 2022 19:58:51 +0000 (15:58 -0400)]
Don't pass down nonnullable_vars while reducing outer joins.

We weren't actually using the passed-down list for anything, other
than computing the new value to be passed down further.  I (tgl)
probably had the idea that we'd need this data eventually; but
no use-case has emerged in a good long while, so let's just stop
expending useless cycles here.

Richard Guo

Discussion: https://postgr.es/m/CAMbWs48KLy9aBb=sZ5MoNmnqAcGHaW_JTGWLCgoE_uMW7S6C-A@mail.gmail.com

2 years agoHandle SubPlan cases in find_nonnullable_rels/vars.
Tom Lane [Sat, 5 Nov 2022 19:24:36 +0000 (15:24 -0400)]
Handle SubPlan cases in find_nonnullable_rels/vars.

We can use some variants of SubPlan to deduce that Vars appearing
in the testexpr must be non-null.

Richard Guo

Discussion: https://postgr.es/m/CAMbWs4-jV=199A2Y_6==99dYnpnmaO_Wz_RGkRTTaCB=Pihw2w@mail.gmail.com

2 years agoRemove redundant breaks in HeapTupleSatisfiesVisibility
Andres Freund [Sat, 5 Nov 2022 04:56:34 +0000 (21:56 -0700)]
Remove redundant breaks in HeapTupleSatisfiesVisibility

Author: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_ZJg_N7zHtWP+JoSY_hrce4+GKioL137Y2c2En-kuXQ7g@mail.gmail.com

2 years agoRemove unneeded includes of <sys/stat.h>
Michael Paquier [Sat, 5 Nov 2022 03:31:28 +0000 (12:31 +0900)]
Remove unneeded includes of <sys/stat.h>

Since bfb9dfd, none of the files updated in this commit have any stat()
calls, so these inclusions are not necessary, for the same reasons as
233cf6e.

Per discussion with John Naylor.

Discussion: https://postgr.es/m/CAFBsxsGGGX7KD6RxbNoSJzuSc8Gz3hOxcfhTOMLB_hJcm68dKQ@mail.gmail.com

2 years agomeson: Split 'main' suite into 'regress' and 'isolation'
Andres Freund [Sat, 5 Nov 2022 01:08:44 +0000 (18:08 -0700)]
meson: Split 'main' suite into 'regress' and 'isolation'

Several people didn't like the 'main' name and found it confusing that the
main regression and isolation tests were in one suite.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20221001221514.2yy257v4zdfhwiy2@awork3.anarazel.de
Discussion: https://postgr.es/m/20221021123435.GU16921@telsasoft.com

2 years agomeson: Mark PROVE as not required
Andres Freund [Sat, 5 Nov 2022 01:08:05 +0000 (18:08 -0700)]
meson: Mark PROVE as not required

In the meson build the prove binary is currently not even used. It will soon
be, for PGXS compatibility, but even then we should build without it around.

Author: Justin Pryzby <pryzbyj@telsasoft.com>
Discussion: https://postgr.es/m/20221021034040.GT16921@telsasoft.com
Discussion: https://postgr.es/m/20221104235412.GE16921@telsasoft.com

2 years agoFix CREATE DATABASE so we can pg_upgrade DBs with OIDs above 2^31.
Tom Lane [Fri, 4 Nov 2022 14:39:52 +0000 (10:39 -0400)]
Fix CREATE DATABASE so we can pg_upgrade DBs with OIDs above 2^31.

Commit aa0105141 repeated one of the oldest mistakes in our book:
thinking that OID is the same as int32.  It isn't of course, and
unsurprisingly the first person who came along with a database
OID above 2 billion broke it.  Repair.

Per bug #17677 from Sergey Pankov.  Back-patch to v15.

Discussion: https://postgr.es/m/17677-a99fa067d7ed71c9@postgresql.org

2 years agoCorrect error message for row-level triggers with transition tables on partitioned...
Etsuro Fujita [Fri, 4 Nov 2022 10:15:00 +0000 (19:15 +0900)]
Correct error message for row-level triggers with transition tables on partitioned tables.

"Triggers on partitioned tables cannot have transition tables." is
incorrect as we allow statement-level triggers on partitioned tables to
have transition tables.

This has been wrong since commit 86f575948; back-patch to v11 where that
commit came in.

Reviewed by Tom Lane.

Discussion: https://postgr.es/m/CAPmGK17gk4vXLzz2iG%2BG4LWRWCoVyam70nZ3OuGm1hMJwDrhcg%40mail.gmail.com

2 years agoDoc: Improve the description of confirmed_flush_lsn in pg_replication_slots.
Amit Kapila [Fri, 4 Nov 2022 03:30:46 +0000 (09:00 +0530)]
Doc: Improve the description of confirmed_flush_lsn in pg_replication_slots.

Make it clear that the data corresponding to the transactions committed
before confirmed_flush_lsn is not available anymore.

Author: Ashutosh Sharma
Reviewd by: Ashutosh Bapat
Discussion: https://postgr.es/m/CAE9k0P=hiqRXUonnmtS-5Pu8SbO=yF6vcrVBcfEf2+93ng_f5Q@mail.gmail.com

2 years agoRemove outdated include
John Naylor [Fri, 4 Nov 2022 00:50:57 +0000 (07:50 +0700)]
Remove outdated include

In the wake of bfb9dfd93, there are no longer any stat() calls in
guc-file.l, but the work leading to dac048f71 did not get the memo.

Noted by Michael Paquier

Discussion: https://www.postgresql.org/message-id/Y2OosGi1Xh9x/lEn%40paquier.xyz

2 years agoCreate FKs properly when attaching table as partition
Alvaro Herrera [Thu, 3 Nov 2022 19:40:21 +0000 (20:40 +0100)]
Create FKs properly when attaching table as partition

Commit f56f8f8da6af added some code in CloneFkReferencing that's way too
lax about a Constraint node it manufactures, not initializing enough
struct members -- initially_valid in particular was forgotten.  This
causes some FKs in partitions added by ALTER TABLE ATTACH PARTITION to
be marked as not validated.  Set initially_valid true, which fixes the
bug.

While at it, make the struct initialization more complete.  Very similar
code was added in two other places by the same commit; make them all
follow the same pattern for consistency, though no bugs are apparent
there.

This bug has never been reported: I only happened to notice while
working on commit 614a406b4ff1.  The test case that was added there with
the improper result is repaired.

Backpatch to 12.

Discussion: https://postgr.es/m/20221005105523.bhuhkdx4olajboof@alvherre.pgsql

2 years agoMake AssertPointerAlignment available to frontend code
Peter Eisentraut [Thu, 3 Nov 2022 15:53:46 +0000 (11:53 -0400)]
Make AssertPointerAlignment available to frontend code

We don't need separate definitions for frontend and backend, since the
contained Assert() will take care of the difference.  So this also
makes it simpler overall.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/f64365b1-d5f9-ef83-41fe-404810f10e5a@enterprisedb.com

2 years agoAvoid crash after function syntax error in a replication worker.
Tom Lane [Thu, 3 Nov 2022 16:01:57 +0000 (12:01 -0400)]
Avoid crash after function syntax error in a replication worker.

If a syntax error occurred in a SQL-language or PL/pgSQL-language
CREATE FUNCTION or DO command executed in a logical replication worker,
we'd suffer a null pointer dereference or assertion failure.  That
seems like a rather contrived case, but nonetheless worth fixing.

The cause is that function_parse_error_transpose assumes it must be
executing within the context of a Portal, but logical/worker.c
doesn't create a Portal since it's not running the standard executor.
We can just back off the hard Assert check and make it fail gracefully
if there's not an ActivePortal.  (I have a feeling that the aggressive
check here was my fault originally, probably because I wasn't sure if
the case would always hold and wanted to find out.  Well, now we know.)

The hazard seems to exist in all branches that have logical replication,
so back-patch to v10.

Maxim Orlov, Anton Melnikov, Masahiko Sawada, Tom Lane

Discussion: https://postgr.es/m/b570c367-ba38-95f3-f62d-5f59b9808226@inbox.ru
Discussion: https://postgr.es/m/adf0452f-8c6b-7def-d35e-ab516c80088e@inbox.ru

2 years agoResolve partition strategy during early parsing
Alvaro Herrera [Thu, 3 Nov 2022 15:25:54 +0000 (16:25 +0100)]
Resolve partition strategy during early parsing

This has little practical value, but there's no reason to let the
partition strategy names travel through DDL as strings.

Reviewed-by: Japin Li <japinli@hotmail.com>
Discussion: https://postgr.es/m/20221021093216.ffupd7epy2mytkux@alvherre.pgsql

2 years agoAdd casts to simplehash.h to silence C++ warnings.
Tom Lane [Thu, 3 Nov 2022 14:47:31 +0000 (10:47 -0400)]
Add casts to simplehash.h to silence C++ warnings.

Casting the result of palloc etc. to the intended type is more per
project style anyway.

(The fact that cpluspluscheck doesn't notice these problems is
because it doesn't expand any macros, which seems like a troubling
shortcoming.  Don't have a good idea about improving that.)

Back-patch to v13, which is as far as the patch applies cleanly;
doesn't seem worth working harder.

David Geier

Discussion: https://postgr.es/m/aa5d88a3-71f4-3455-11cf-82de0372c941@gmail.com

2 years agoStraighten include order in guc-file.l
John Naylor [Thu, 3 Nov 2022 05:38:44 +0000 (12:38 +0700)]
Straighten include order in guc-file.l

Oversight in dac048f71eb

Michael Paquier

Reviewed by Julien Rouhaud
Discussion: https://www.postgresql.org/message-id/Y2IATvRGo347Lvd1%40paquier.xyz

2 years agoAllow use of __sync_lock_test_and_set for spinlocks on any machine.
Tom Lane [Wed, 2 Nov 2022 21:37:26 +0000 (17:37 -0400)]
Allow use of __sync_lock_test_and_set for spinlocks on any machine.

If we have no special-case code in s_lock.h for the current platform,
but the compiler has __sync_lock_test_and_set, use that instead of
failing.  It's unlikely that anybody's __sync_lock_test_and_set
would be so awful as to be worse than our semaphore-based fallback,
but if it is, they can (continue to) use --disable-spinlocks.

This allows removal of the RISC-V special case installed by commit
c32fcac56, which generated exactly the same code but only on that
platform.  Usefully, the RISC-V buildfarm animals should now test
at least the int variant of this patch.

I've manually tested both variants on ARM by dint of removing the
ARM-specific stanza.  We don't want to drop that, because it already
has some special knowledge and is likely to grow more over time.
Likewise, this is not meant to preclude installing special cases
for other arches if that proves worthwhile.

Per discussion of a request to install the same code for loongarch64.
Like the previous patch, we might as well back-patch to supported
branches.

Discussion: https://postgr.es/m/761ac43d44b84d679ba803c2bd947cc0@HSMAILSVR04.hs.handsome.com.cn

2 years agopg_dump: Refactor code that constructs ALTER ... OWNER TO commands
Peter Eisentraut [Wed, 2 Nov 2022 21:17:27 +0000 (17:17 -0400)]
pg_dump: Refactor code that constructs ALTER ... OWNER TO commands

Avoid having to list all the possible object types twice.  Instead,
only _getObjectDescription() needs to know about specific object
types.  It communicates back to _printTocEntry() whether an owner is
to be set.

In passing, remove the logic to use ALTER TABLE to set the owner of
views and sequences.  This is no longer necessary.  Furthermore, if
pg_dump doesn't recognize the object type, this is now a fatal error,
not a warning.

Reviewed-by: Corey Huinker <corey.huinker@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/0a00f923-599a-381b-923f-0d802a727715@enterprisedb.com

2 years agoDefend against unsupported partition relkind in logical replication worker.
Tom Lane [Wed, 2 Nov 2022 16:29:39 +0000 (12:29 -0400)]
Defend against unsupported partition relkind in logical replication worker.

Since partitions can be foreign tables not only plain tables, but
logical replication only supports plain tables, we'd better check the
relkind of a partition after we find it.  (There was some discussion
of checking this when adding a partitioned table to a subscription;
but that would be inadequate since the troublesome partition could be
added later.)  Without this, the situation leads to a segfault or
assertion failure.

In passing, add a separate variable for the target Relation of
a cross-partition UPDATE; reusing partrel seemed mighty confusing
and error-prone.

Shi Yu and Tom Lane, per report from Ilya Gladyshev.  Back-patch
to v13 where logical replication into partitioned tables became
a thing.

Discussion: https://postgr.es/m/6b93e3748ba43298694f376ca8797279d7945e29.camel@gmail.com

2 years agopg_dump: fix failure to dump comments on constraints in some cases.
Tom Lane [Wed, 2 Nov 2022 15:30:04 +0000 (11:30 -0400)]
pg_dump: fix failure to dump comments on constraints in some cases.

Thinko in commit 5209c0ba0: I checked the wrong object's
DUMP_COMPONENT_COMMENT bit in two places.

Per bug #17675 from Franz-Josef Färber.

Discussion: https://postgr.es/m/17675-c69c001e06390867@postgresql.org

2 years agoFix copy-and-pasteo in comment.
Etsuro Fujita [Wed, 2 Nov 2022 09:15:00 +0000 (18:15 +0900)]
Fix copy-and-pasteo in comment.

2 years agoDoc: Update information about manually creating slots.
Amit Kapila [Wed, 2 Nov 2022 06:10:37 +0000 (11:40 +0530)]
Doc: Update information about manually creating slots.

There are some cases (e.g. when the subscription is created using the
connect = false option) where the remote replication slot was not created
automatically and the user must create it manually before the subscription
can be activated. There was not enough information in the docs for users
to do this easily.

Author: Peter Smith
Reviewd by: Shi yu, Amit Kapila
Discussion: https://postgr.es/m/CAHut+PvqdqOanheWSHDyhQiF+Z-7w=-+k4U+bwbT=b6YQ_hrXQ@mail.gmail.com

2 years agoImprove the description of XLOG_RUNNING_XACTS.
Amit Kapila [Wed, 2 Nov 2022 04:36:55 +0000 (10:06 +0530)]
Improve the description of XLOG_RUNNING_XACTS.

Previously, the description of XLOG_RUNNING_XACTS showed only
top-transaction XIDs and whether subtransactions overflowed. This commit
improves it to show individual subtransaction XIDs. This also improves the
description of overflowed subtransactions.

This additional information can be helpful for testing and debugging
purposes.

Author: Masahiko Sawada
Reviewd by: Fujii Masao, Kyotaro Horiguchi, Ashutosh Bapat, Bharath Rupireddy
Discussion: https://postgr.es/m/CAD21AoAqvaE+XEeXHHPdAGQPcCoGXxuoeutq_nWhUSQvTt5+tA@mail.gmail.com

2 years agodoc: Fix some descriptions related to pg_ident_file_mappings
Michael Paquier [Wed, 2 Nov 2022 02:56:06 +0000 (11:56 +0900)]
doc: Fix some descriptions related to pg_ident_file_mappings

pg_ident_file_mappings.line_number was described as a line number in
pg_ident.conf for a "rule" number, but this should refer to a "map".
The same inconsistent term was used in the main paragraph describing the
view.

Extracted from a patch by the same author.  Issue introduced by
a2c8499 where this view has been added.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20221026031948.cbrnzgy5e7glsq2d@jrouhaud
Backpatch-through: 15

2 years agoFix outdated comment in tuplesort.h
David Rowley [Wed, 2 Nov 2022 02:29:31 +0000 (15:29 +1300)]
Fix outdated comment in tuplesort.h

This was outdated by 77bae396d.

Backpatch-through: 15, where 77bae396d was added

2 years agoRemove code handling FORCE_NULL and FORCE_NOT_NULL for COPY TO
Michael Paquier [Wed, 2 Nov 2022 01:15:19 +0000 (10:15 +0900)]
Remove code handling FORCE_NULL and FORCE_NOT_NULL for COPY TO

These two options are only available with COPY FROM, so the extra logic
in charge of checking the validity of the attributes given has no
purpose.

Author: Zhang Mingli
Reviewed-by: Richard Guo, Kyotaro Horiguchi
Discussion: https://postgr.es/m/F28F0B5A-766F-4D33-BF44-43B3A052D833@gmail.com

2 years agoAdd doubly linked count list implementation
David Rowley [Wed, 2 Nov 2022 01:06:05 +0000 (14:06 +1300)]
Add doubly linked count list implementation

We have various requirements when using a dlist_head to keep track of the
number of items in the list.  This, traditionally, has been done by
maintaining a counter variable in the calling code.  Here we tidy this up
by adding "dclist", which is very similar to dlist but also keeps track of
the number of items stored in the list.

Callers may use the new dclist_count() function when they need to know how
many items are stored. Obtaining the count is an O(1) operation.

For simplicity reasons, dclist and dlist both use dlist_node as their node
type and dlist_iter/dlist_mutable_iter as their iterator type. dclists
have all of the same functionality as dlists except there is no function
named dclist_delete().  To remove an item from a list dclist_delete_from()
must be used.  This requires knowing which dclist the given item is stored
in.

Additionally, here we also convert some dlists where additional code
exists to keep track of the number of items stored and to make these use
dclists instead.

Author: David Rowley
Reviewed-by: Bharath Rupireddy, Aleksander Alekseev
Discussion: https://postgr.es/m/CAApHDvrtVxr+FXEX0VbViCFKDGxA3tWDgw9oFewNXCJMmwLjLg@mail.gmail.com

2 years agoAdd more tests for COPY with incorrect option combinations
Michael Paquier [Wed, 2 Nov 2022 00:57:54 +0000 (09:57 +0900)]
Add more tests for COPY with incorrect option combinations

Based on the existing coverage report, some combinations were not
checked at all, so add some tests to do so.  Spotted while looking at
the area.

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

2 years agoUpdate time zone data files to tzdata release 2022f.
Tom Lane [Tue, 1 Nov 2022 21:08:28 +0000 (17:08 -0400)]
Update time zone data files to tzdata release 2022f.

DST law changes in Chile, Fiji, Iran, Jordan, Mexico, Palestine,
and Syria.  Historical corrections for Chile, Crimea, Iran, and
Mexico.

Also, the Europe/Kiev zone has been renamed to Europe/Kyiv
(retaining the old name as a link).

The following zones have been merged into nearby, more-populous zones
whose clocks have agreed since 1970: Antarctica/Vostok, Asia/Brunei,
Asia/Kuala_Lumpur, Atlantic/Reykjavik, Europe/Amsterdam,
Europe/Copenhagen, Europe/Luxembourg, Europe/Monaco, Europe/Oslo,
Europe/Stockholm, Indian/Christmas, Indian/Cocos, Indian/Kerguelen,
Indian/Mahe, Indian/Reunion, Pacific/Chuuk, Pacific/Funafuti,
Pacific/Majuro, Pacific/Pohnpei, Pacific/Wake and Pacific/Wallis.
(This indirectly affects zones that were already links to one of
these: Arctic/Longyearbyen, Atlantic/Jan_Mayen, Iceland,
Pacific/Ponape, Pacific/Truk, and Pacific/Yap.)  America/Nipigon,
America/Rainy_River, America/Thunder_Bay, Europe/Uzhgorod, and
Europe/Zaporozhye were also merged into nearby zones after discovering
that their claimed post-1970 differences from those zones seem to have
been errors.

While the IANA crew have been working on merging zones that have no
post-1970 differences for some time, this batch of changes affects
some zones that are significantly more populous than those merged
in the past, notably parts of Europe.  The loss of pre-1970 timezone
history for those zones may be troublesome for applications
expecting consistency of timestamptz display.  As an example, the
stored value '1944-06-01 12:00 UTC' would previously display as
'1944-06-01 13:00:00+01' if the Europe/Stockholm zone is selected,
but now it will read out as '1944-06-01 14:00:00+02'.

There exists a "packrat" option that will build the timezone data
files with this old data preserved, but the problem is that it also
resurrects a bunch of other, far less well-attested data; so much so
that actually more zones' contents change from 2022a with that option
than without it.  I have chosen not to do that here, for that reason
and because it appears that no major OS distributions are using the
"packrat" option, so that doing so would cause Postgres' behavior
to diverge significantly depending on whether it was built with
--with-system-tzdata.  However, for anyone for whom these changes pose
significant problems, there is a solution: build a set of timezone
files with the "packrat" option and use those with Postgres.

2 years agoFix planner failure with extended statistics on partitioned tables.
Tom Lane [Tue, 1 Nov 2022 18:34:44 +0000 (14:34 -0400)]
Fix planner failure with extended statistics on partitioned tables.

Some cases would result in "cache lookup failed for statistics object",
due to trying to fetch inherited statistics when only non-inherited
ones are available or vice versa.

Richard Guo and Justin Pryzby

Discussion: https://postgr.es/m/20221030170520.GM16921@telsasoft.com

2 years agopg_stat_statements: fetch stmt location/length before it disappears.
Tom Lane [Tue, 1 Nov 2022 16:48:01 +0000 (12:48 -0400)]
pg_stat_statements: fetch stmt location/length before it disappears.

When executing a utility statement, we must fetch everything
we need out of the PlannedStmt data structure before calling
standard_ProcessUtility.  In certain cases (possibly only ROLLBACK
in extended query protocol), that data structure will get freed
during command execution.  The situation is probably often harmless
in production builds, but in debug builds we intentionally overwrite
the freed memory with garbage, leading to picking up garbage values
of statement location and length, typically causing an assertion
failure later in pg_stat_statements.  In non-debug builds, if
something did go wrong it would likely lead to storing garbage
for the query string.

Report and fix by zhaoqigui (with cosmetic adjustments by me).
It's an old problem, so back-patch to all supported versions.

Discussion: https://postgr.es/m/17663-a344fd0675f92128@postgresql.org
Discussion: https://postgr.es/m/1667307420050.56657@hundsun.com

2 years agodoc: Add note about lack of publication privileges
Peter Eisentraut [Tue, 1 Nov 2022 13:18:37 +0000 (14:18 +0100)]
doc: Add note about lack of publication privileges

This gives some additional advice on using row filters and column
lists on publications securely.

Author: Antonin Houska <ah@cybertec.at>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Discussion: https://www.postgresql.org/message-id/flat/20330.1652105397@antos

2 years agopsql: Improve tab completion for ALTER TABLE on identity columns
Peter Eisentraut [Tue, 1 Nov 2022 11:07:40 +0000 (12:07 +0100)]
psql: Improve tab completion for ALTER TABLE on identity columns

- Add tab completion for ALTER SEQUENCE … START …
- Add tab completion for ALTER COLUMN … SET GENERATED …
- Add tab completion for ALTER COLUMN … SET <sequence option>
- Add tab completion for ALTER COLUMN … ADD GENERATED … AS IDENTITY

Author: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Reviewed-by: Matheus Alcantara <mths.dev@pm.me>
Discussion: https://www.postgresql.org/message-id/flat/87mta1jfax.fsf@wibble.ilmari.org

2 years agoAdd basic regression tests for semi/antijoin recognition.
Tom Lane [Mon, 31 Oct 2022 23:52:33 +0000 (19:52 -0400)]
Add basic regression tests for semi/antijoin recognition.

Add some simple tests that the planner recognizes all the
standard idioms for SEMI and ANTI joins.  Failure to optimize
in this way won't necessarily cause any visible change in
query results, so check the plans.  We had no similar coverage
before, at least for some variants of antijoin, as noted by
Richard Guo.

Discussion: https://postgr.es/m/CAMbWs4-mvPPCJ1W6iK6dD5HiNwoJdi6mZp=-7mE8N9Sh+cd0tQ@mail.gmail.com

2 years agoFix ALTER COLLATION "default" REFRESH VERSION.
Jeff Davis [Sat, 29 Oct 2022 21:13:23 +0000 (14:13 -0700)]
Fix ALTER COLLATION "default" REFRESH VERSION.

Issue a helpful error message rather than an internal error.

Discussion: https://postgr.es/m/51fb77507cafd43fc1a2e733c23045873d93ae60.camel%40j-davis.com
Reviewed-by: Thomas Munro
2 years agoEnable pg_collation_actual_version() to work on the default collation.
Jeff Davis [Sat, 29 Oct 2022 20:30:15 +0000 (13:30 -0700)]
Enable pg_collation_actual_version() to work on the default collation.

Previously, it would simply return NULL, which was less useful.

Discussion: https://postgr.es/m/51fb77507cafd43fc1a2e733c23045873d93ae60.camel%40j-davis.com
Reviewed-by: Thomas Munro
2 years agopg_dump test: Make concatenated create_sql commands more readable
Peter Eisentraut [Mon, 31 Oct 2022 12:59:57 +0000 (13:59 +0100)]
pg_dump test: Make concatenated create_sql commands more readable

When the pg_dump 002_pg_dump.pl test generates the command to load the
schema, it does

    # Add terminating semicolon
    $create_sql{$test_db} .= $tests{$test}->{create_sql} . ";";

In some cases, this creates a duplicate semicolon, but more
importantly, this doesn't add any newline.  So if you look at the
result in either the server log or in
tmp_check/log/regress_log_002_pg_dump, it looks like a complete mess.
This patch makes the output look cleaner for manual inspection: add
semicolon only if necessary, and add two newlines.

Discussion: https://www.postgresql.org/message-id/flat/d6aec95a-8729-43cc-2578-f2a5e46640e0%40enterprisedb.com

2 years agoAdd check on initial and boot values when loading GUCs
Michael Paquier [Mon, 31 Oct 2022 04:54:23 +0000 (13:54 +0900)]
Add check on initial and boot values when loading GUCs

This commit adds a function to perform a cross-check between the initial
value of the C declaration associated to a GUC and its actual boot
value in assert-enabled builds.  The purpose of this is to prevent
anybody reading these C declarations from being fooled by mismatched
values before they are loaded at program startup.

The following rules apply depending on the GUC type:
* bool - can be false, or same as boot_val.
* int - can be 0, or same as the boot_val.
* real - can be 0.0, or same as the boot_val.
* string - can be NULL, or strcmp'd equal to the boot_val.
* enum - equal to the boot_val.

This is done for the system as well custom GUCs loaded by external
modules, which may require extension developers to adapt the C
declaration of the variables used by these GUCs (testing this change
with some of my own modules has allowed me to catch some stupid typos,
FWIW).  This may finish by being a bad experiment depending on the
feedbcak received, but let's see how it goes.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Michael Paquier, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

2 years agoClean up some inconsistencies with GUC declarations
Michael Paquier [Mon, 31 Oct 2022 03:44:48 +0000 (12:44 +0900)]
Clean up some inconsistencies with GUC declarations

This is similar to 7d25958, and this commit takes care of all the
remaining inconsistencies between the initial value used in the C
variable associated to a GUC and its default value stored in the GUC
tables (as of pg_settings.boot_val).

Some of the initial values of the GUCs updated rely on a compile-time
default.  These are refactored so as the GUC table and its C declaration
use the same values.  This makes everything consistent with other
places, backend_flush_after, bgwriter_flush_after, port,
checkpoint_flush_after doing so already, for example.

Extracted from a larger patch by Peter Smith.  The spots updated in the
modules are from me.

Author: Peter Smith, Michael Paquier
Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

2 years agoUnder has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.
Noah Misch [Sat, 29 Oct 2022 17:42:16 +0000 (10:42 -0700)]
Under has_wal_read_bug, skip recovery/t/032_relfilenode_reuse.pl.

Per buildfarm member kittiwake.  Back-patch to v15, where this test
first appeared.

Discussion: https://postgr.es/m/20220116210241.GC756210@rfd.leadboat.com

2 years agoUse Limit instead of Unique to implement DISTINCT, when possible
David Rowley [Fri, 28 Oct 2022 10:04:38 +0000 (23:04 +1300)]
Use Limit instead of Unique to implement DISTINCT, when possible

When all of the query's DISTINCT pathkeys have been marked as redundant
due to EquivalenceClasses existing which contain constants, we can just
implement the DISTINCT operation on a query by just limiting the number of
returned rows to 1 instead of performing a Unique on all of the matching
(duplicate) rows.

This applies in cases such as:

SELECT DISTINCT col,col2 FROM tab WHERE col = 1 AND col2 = 10;

If there are any matching rows, then they must all be {1,10}.  There's no
point in fetching all of those and running a Unique operator on them to
leave only a single row.  Here we effectively just find the first row and
then stop.  We are obviously unable to apply this optimization if either
the col = 1 or col2 = 10 were missing from the WHERE clause or if there
were any additional columns in the SELECT clause.

Such queries are probably not all that common, but detecting when we can
apply this optimization amounts to checking if the distinct_pathkeys are
NULL, which is very cheap indeed.

Nothing is done here to check if the query already has a LIMIT clause.  If
it does then the plan may end up with 2 Limits nodes.  There's no harm in
that and it's probably not worth the complexity to unify them into a
single Limit node.

Author: David Rowley
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/CAApHDvqS0j8RUWRUSgCAXxOqnYjHUXmKwspRj4GzVfOO25ByHA@mail.gmail.com
Discussion: https://postgr.es/m/MEYPR01MB7101CD5DA0A07C9DE2B74850A4239@MEYPR01MB7101.ausprd01.prod.outlook.com

2 years agoRemove AssertArg and AssertState
Peter Eisentraut [Fri, 28 Oct 2022 07:19:06 +0000 (09:19 +0200)]
Remove AssertArg and AssertState

These don't offer anything over plain Assert, and their usage had
already been declared obsolescent.

Author: Nathan Bossart <nathandbossart@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://www.postgresql.org/message-id/20221009210148.GA900071@nathanxps13

2 years agoAllow nodeSort to perform Datum sorts for byref types
David Rowley [Thu, 27 Oct 2022 20:25:12 +0000 (09:25 +1300)]
Allow nodeSort to perform Datum sorts for byref types

Here we add a new 'copy' parameter to tuplesort_getdatum so that we can
instruct the function not to datumCopy() byref Datums before returning.

Similar to 91e9e89dc, this can provide significant performance
improvements in nodeSort when sorting by a single byref column and the
sort's targetlist contains only that column.

This allows us to re-enable Datum sorts for byref types which was disabled
in 3a5817695 due to a reported memory leak.

Additionally, here we slightly optimize DISTINCT aggregates so that we no
longer perform any datumCopy() when we find the current value not to be
distinct from the previous value.  Previously the code would always take a
copy of the most recent Datum and pfree the previous value, even when the
values were the same.  Testing shows a small but noticeable performance
increase when aggregate transitions are skipped due to the current
transition value being the same as the prior one.

Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvqS6wC5U==k9Hd26E4EQXH3QR67-T4=Q1rQ36NGvjfVSg@mail.gmail.com
Discussion: https://postgr.es/m/CAApHDvqHonfe9G1cVaKeHbDx70R_zCrM3qP2AGXpGrieSKGnhA@mail.gmail.com

2 years agoAvoid making commutatively-duplicate clauses in EquivalenceClasses.
Tom Lane [Thu, 27 Oct 2022 18:42:18 +0000 (14:42 -0400)]
Avoid making commutatively-duplicate clauses in EquivalenceClasses.

When we decide we need to make a derived clause equating a.x and
b.y, we already will re-use a previously-made clause "a.x = b.y".
But we might instead have "b.y = a.x", which is perfectly usable
because equivclass.c has never promised anything about the
operand order in clauses it builds.  Saving construction of a
new RestrictInfo doesn't matter all that much in itself --- but
because we cache selectivity estimates and so on per-RestrictInfo,
there's a possibility of saving a fair amount of duplicative
effort downstream.

Hence, check for commutative matches as well as direct ones when
seeing if we have a pre-existing clause.  This changes the visible
clause order in several regression test cases, but they're all
clearly-insignificant changes.

Checking for the reverse operand order is simple enough, but
if we wanted to check for operator OID match we'd need to call
get_commutator here, which is not so cheap.  I concluded that
we don't really need the operator check anyway, so I just
removed it.  It's unlikely that an opfamily contains more than
one applicable operator for a given pair of operand datatypes;
and if it does they had better give the same answers, so there
seems little need to insist that we use exactly the one
select_equality_operator chose.

Using the current core regression suite as a test case, I see
this change reducing the number of new join clauses built by
create_join_clause from 9673 to 5142 (out of 26652 calls).
So not quite 50% savings, but pretty close to it.

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

2 years agoMove pg_pwritev_with_retry() to src/common/file_utils.c
Michael Paquier [Thu, 27 Oct 2022 05:39:42 +0000 (14:39 +0900)]
Move pg_pwritev_with_retry() to src/common/file_utils.c

This commit moves pg_pwritev_with_retry(), a convenience wrapper of
pg_writev() able to handle partial writes, to common/file_utils.c so
that the frontend code is able to use it.  A first use-case targetted
for this routine is pg_basebackup and pg_receivewal, for the
zero-padding of a newly-initialized WAL segment.  This is used currently
in the backend when the GUC wal_init_zero is enabled (default).

Author: Bharath Rupireddy
Reviewed-by: Nathan Bossart, Thomas Munro
Discussion: https://postgr.es/m/CALj2ACUq7nAb7=bJNbK3yYmp-SZhJcXFR_pLk8un6XgDzDF3OA@mail.gmail.com

2 years agoAdd some tests to check the SQL functions of control file
Michael Paquier [Thu, 27 Oct 2022 00:58:44 +0000 (09:58 +0900)]
Add some tests to check the SQL functions of control file

As the recent commit 05d4cbf (reverted after as a448e49) has proved,
there is zero coverage for the four SQL functions that can scan the
control file data:
- pg_control_checkpoint()
- pg_control_init()
- pg_control_recovery()
- pg_control_system()

This commit adds a minimal coverage for these functions, checking that
their execution is able to complete.  This would have been enough to
catch the problems introduced in the commit mentioned above.  More
checks could be done for each individual fields, but it is unclear
whether this would be better than the other checks in place in the
backend code.

Per discussion with Bharath Rupireddy.

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

2 years agoAdd rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings
Michael Paquier [Wed, 26 Oct 2022 06:22:15 +0000 (15:22 +0900)]
Add rule_number to pg_hba_file_rules and map_number to pg_ident_file_mappings

These numbers are strictly-monotone identifiers assigned to each rule
of pg_hba_file_rules and each map of pg_ident_file_mappings when loading
the HBA and ident configuration files, indicating the order in which
they are checked at authentication time, until a match is found.

With only one file loaded currently, this is equivalent to the line
numbers assigned to the entries loaded if one wants to know their order,
but this becomes mandatory once the inclusion of external files is
added to the HBA and ident files to be able to know in which order the
rules and/or maps are applied at authentication.  Note that NULL is used
when a HBA or ident entry cannot be parsed or validated, aka when an
error exists, contrary to the line number.

Bump catalog version.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud

2 years agoFix variable assignment thinko in hba.c
Michael Paquier [Wed, 26 Oct 2022 03:57:40 +0000 (12:57 +0900)]
Fix variable assignment thinko in hba.c

The intention behind 1b73d0b was to limit the use of TokenizedAuthLine,
but I have fat-fingered one location in parse_hba_line() when creating
the HbaLine, where this should use the local variable and not the value
coming from TokenizedAuthLine.  This logic is the exactly the same, but
let's be clean about all that on consistency grounds.

Reported-by: Julien Rouhaud
Discussion: https://postgr.es/m/20221026032730.k3sib5krgm7l6njk@jrouhaud

2 years agoRefactor code handling the names of files loaded in hba.c
Michael Paquier [Wed, 26 Oct 2022 02:36:21 +0000 (11:36 +0900)]
Refactor code handling the names of files loaded in hba.c

This has the advantage to limit the presence of the GUC values
hba_file and ident_file to the code paths where these files are loaded,
easing the introduction of an upcoming feature aimed at adding inclusion
logic for files and directories in HBA and ident files.

Note that this needs the addition of the source file name to HbaLine, in
addition to the line number, which is something needed by the backend in
two places of auth.c (authentication failure details and auth_id log
when log_connections is enabled).

While on it, adjust a log generated on authentication failure to report
the name of the actual HBA file on which the connection attempt matched,
where the line number and the raw line written in the HBA file were
already included.  This was previously hardcoded as pg_hba.conf, which
would be incorrect when a custom value is used at postmaster startup for
the GUC hba_file.

Extracted from a larger patch by the same author.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud

2 years agoDoc/improve confusing, inefficient tests to locate CTID variable.
Tom Lane [Tue, 25 Oct 2022 21:35:19 +0000 (17:35 -0400)]
Doc/improve confusing, inefficient tests to locate CTID variable.

The IsCTIDVar() tests in nodeTidscan.c and nodeTidrangescan.c
look buggy at first sight: they aren't checking that the varno
matches the table to be scanned.  Actually they're safe because
any Var in a scan-level qual must be for the correct table ...
but if we're depending on that, it's pretty pointless to verify
varlevelsup.  (Besides which, varlevelsup is *always* zero at
execution, since we've flattened the rangetable long since.)

Remove the useless varlevelsup check, and instead add some
commentary explaining why we don't need to check varno.

Noted while fooling with a planner change that causes the order
of "t1.ctid = t2.ctid" to change in some tidscan.sql tests;
I was briefly fooled into thinking there was a live bug here.

2 years agoUpdate outdated comment for TransactionIdSetTreeStatus
Heikki Linnakangas [Tue, 25 Oct 2022 19:43:52 +0000 (21:43 +0200)]
Update outdated comment for TransactionIdSetTreeStatus

Commit 06da3c570f changed the way subtransactions are marked as
SUBCOMMITTED, but the example it included actually documented the old
way. Update it.

Author: Japin Li
Discussion: https://www.postgresql.org/message-id/MEYP282MB16690BC96DFBE08CC857E1E3B6319%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

2 years agoClean up some GUC declarations and comments
Michael Paquier [Tue, 25 Oct 2022 05:06:07 +0000 (14:06 +0900)]
Clean up some GUC declarations and comments

This adjusts a few things for GUCs related to logical replication,
replication slots and WAL senders, in the shape of incorrect comments
and values inconsistent with their initial default value.

Author: Peter Smith
Reviewed-by: Nathan Bossart, Tom Lane, Justin Pryzby
Discussion: https://postgr.es/m/CAHut+PtHE0XSfjjRQ6D4v7+dqzCw=d+1a64ujra4EX8aoc_Z+w@mail.gmail.com

2 years agoFix unlink() for STATUS_DELETE_PENDING on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:26:03 +0000 (15:26 +1300)]
Fix unlink() for STATUS_DELETE_PENDING on Windows.

Commit f357233c assumed that it was OK to return ENOENT directly if
lstat() failed that way.  If we got STATUS_DELETE_PENDING while trying
to unlink a file that we had already unlinked successfully once before
but someone else still had open (on a kernel version that has "pending"
unlinks by default), then we would no longer reach the retry loop in
pgunlink().  That loop claims to be only for handling sharing violations
(a different phenomenon), but the errno is the same.

Restore that behavior with an explicit check, to see if it fixes the
occasional 'directory not empty' failures seen in the pg_upgrade tests
on CI.  Further improvements are possible with proposed upgrades to
modern Windows APIs that would replace this convoluted code.

Reported-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20220920013122.GA31833%40telsasoft.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix stat() for recursive junction points on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:24:41 +0000 (15:24 +1300)]
Fix stat() for recursive junction points on Windows.

Commit c5cb8f3b supposed that we'd only ever have to follow one junction
point in stat(), because we don't construct longer chains of them ourselves.
When examining a parent directory supplied by the user, we should really be
able to cope with longer chains, just in case someone has their system
set up that way.  Choose an arbitrary cap of 8, to match the minimum
acceptable value of SYMLOOP_MAX in POSIX.

Previously I'd avoided reporting ELOOP thinking Windows didn't have it,
but it turns out that it does, so we can use the proper error number.

Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Discussion: https://postgr.es/m/CA%2BhUKGJ7JDGWYFt9%3D-TyJiRRy5q9TtPfqeKkneWDr1XPU1%2Biqw%40mail.gmail.com
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix readlink() for non-PostgreSQL junction points on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:21:42 +0000 (15:21 +1300)]
Fix readlink() for non-PostgreSQL junction points on Windows.

Since commit c5cb8f3b taught stat() to follow symlinks, and since initdb
uses pg_mkdir_p(), and that examines parent directories, our humble
readlink() implementation can now be exposed to junction points not of
PostgreSQL origin.  Those might be corrupted by our naive path mangling,
which doesn't really understand NT paths in general.

Simply decline to transform paths that don't look like a drive absolute
path.  That means that readlink() returns the NT path directly when
checking a parent directory of PGDATA that happen to point to a drive
using "rooted" format.  That  works for the purposes of our stat()
emulation.

Reported-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Reviewed-by: Roman Zharkov <r.zharkov@postgrespro.ru>
Discussion: https://postgr.es/m/4590c37927d7b8ee84f9855d83229018%40postgrespro.ru
Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix lstat() for broken junction points on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:20:00 +0000 (15:20 +1300)]
Fix lstat() for broken junction points on Windows.

When using junction points to emulate symlinks on Windows, one edge case
was not handled correctly by commit c5cb8f3b: if a junction point is
broken (pointing to a non-existent path), we'd report ENOENT.  This
doesn't break any known use case, but was noticed while developing a
test suite for these functions and is fixed here for completeness.

Also add translation ERROR_CANT_RESOLVE_FILENAME -> ENOENT, as that is
one of the errors Windows can report for some kinds of broken paths.

Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com

2 years agoFix readlink() return value on Windows.
Thomas Munro [Tue, 25 Oct 2022 02:13:52 +0000 (15:13 +1300)]
Fix readlink() return value on Windows.

Ancient bug noticed while working on a test suite for these functions.

Discussion: https://postgr.es/m/CA%2BhUKG%2BajSQ_8eu2AogTncOnZ5me2D-Cn66iN_-wZnRjLN%2Bicg%40mail.gmail.com