Commit Graph

54304 Commits

Author SHA1 Message Date
Alvaro Herrera e44dae07f9
BRIN: mask BRIN_EVACUATE_PAGE for WAL consistency checking
That bit is unlogged and therefore it's wrong to consider it in WAL page
comparison.

Add a test that tickles the case, as branch testing technology allows.

This has been a problem ever since wal consistency checking was
introduced (commit a507b86900 for pg10), so backpatch to all supported
branches.

Author: 王海洋 (Haiyang Wang) <wanghaiyang.001@bytedance.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/CACciXAD2UvLMOhc4jX9VvOKt7DtYLr3OYRBhvOZ-jRxtzc_7Jg@mail.gmail.com
Discussion: https://postgr.es/m/CACciXADOfErX9Bx0nzE_SkdfXr6Bbpo5R=v_B6MUTEYW4ya+cg@mail.gmail.com
2022-08-05 18:00:17 +02:00
Noah Misch d263ced225 Be more prescriptive in new doc about re-archiving of same WAL file.
Nathan Bossart, reviewed by David Steele.

Discussion: https://postgr.es/m/20220731065156.GA3694276@rfd.leadboat.com
2022-08-05 08:30:55 -07:00
Alvaro Herrera 90a4b64134
regress: fix test instability
Having additional triggers in a test table made the ORDER BY clauses in
old queries underspecified.  Add another column there for stability.

Per sporadic buildfarm pink.
2022-08-05 11:55:52 +02:00
John Naylor 6b41a1579b Simplify coding style of is_valid_ascii()
Calculate end of input rather than maintaining length,
per prior suggestion from Heikki Linnakangas. In passing,
use more natural language in a comment.

Discussion: https://www.postgresql.org/message-id/b4648cc2-5e9c-c93a-52cc-51e5c658a4f6%40iki.fi
2022-08-05 16:50:22 +07:00
Etsuro Fujita 82593b9a3d postgres_fdw: Disable batch insertion when there are WCO constraints.
When inserting a view referencing a foreign table that has WITH CHECK
OPTION constraints, in single-insert mode postgres_fdw retrieves the
data that was actually inserted on the remote side so that the WITH
CHECK OPTION constraints are enforced with the data locally, but in
batch-insert mode it cannot currently retrieve the data (except for the
row first inserted through the view), resulting in enforcing the WITH
CHECK OPTION constraints with the data passed from the core (except for
the first-inserted row), which led to incorrect results when inserting
into a view referencing a foreign table in which a remote BEFORE ROW
INSERT trigger changes the rows inserted through the view so that they
violate the view's WITH CHECK OPTION constraint.  Also, the query
inserting into the view caused an assertion failure in assert-enabled
builds.

Fix these by disabling batch insertion when inserting into such a view.

Back-patch to v14 where batch insertion was added.

Discussion: https://postgr.es/m/CAPmGK17LpbTZs4m4a_6THP54UBeK9fHvX8aVVA%2BC6yEZDZwQcg%40mail.gmail.com
2022-08-05 17:15:00 +09:00
Alvaro Herrera ec0925c22a
Fix ENABLE/DISABLE TRIGGER to handle recursion correctly
Using ATSimpleRecursion() in ATPrepCmd() to do so as bbb927b4db did is
not correct, because ATPrepCmd() can't distinguish between triggers that
may be cloned and those that may not, so would wrongly try to recurse
for the latter category of triggers.

So this commit restores the code in EnableDisableTrigger() that
86f575948c had added to do the recursion, which would do it only for
triggers that may be cloned, that is, row-level triggers.  This also
changes tablecmds.c such that ATExecCmd() is able to pass the value of
ONLY flag down to EnableDisableTrigger() using its new 'recurse'
parameter.

This also fixes what seems like an oversight of 86f575948c that the
recursion to partition triggers would only occur if EnableDisableTrigger()
had actually changed the trigger.  It is more apt to recurse to inspect
partition triggers even if the parent's trigger didn't need to be
changed: only then can we be certain that all descendants share the same
state afterwards.

Backpatch all the way back to 11, like bbb927b4db.  Care is taken not
to break ABI compatibility (and that no catversion bump is needed.)

Co-authored-by: Amit Langote <amitlangote09@gmail.com>
Reviewed-by: Dmitry Koval <d.koval@postgrespro.ru>
Discussion: https://postgr.es/m/CA+HiwqG-cZT3XzGAnEgZQLoQbyfJApVwOTQaCaas1mhpf+4V5A@mail.gmail.com
2022-08-05 09:47:26 +02:00
Thomas Munro d2e150831a Remove configure probe for fdatasync.
fdatasync() is in SUSv2, and all targeted Unix systems have it.  We have
a replacement function for Windows.

We retain the probe for the function declaration, which allows us to
supply the mysteriously missing declaration for macOS, and also for
Windows.  No need to keep a HAVE_FDATASYNC macro around.

Also rename src/port/fdatasync.c to win32fdatasync.c since it's only for
Windows.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
Discussion: https://postgr.es/m/CA%2BhUKGJZJVO%3DiX%2Beb-PXi2_XS9ZRqnn_4URh0NUQOwt6-_51xQ%40mail.gmail.com
2022-08-05 16:37:38 +12:00
Thomas Munro 623cc67347 Remove configure probe for clock_gettime.
clock_gettime() is in SUSv2 and all targeted Unix systems have it.
Remove a chunk of fallback code for old Unix is no longer reachable on
modern systems, and untested as of the retirement of build farm animal
prairiedog.

There is no need to retain a HAVE_CLOCK_GETTIME macro here, because it
is already used in a context with Unix and Windows code paths.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 16:37:11 +12:00
Peter Geoghegan 92f375056c Fix nbtree maximum item size macro.
Commit dd299df818, which made heap TID a tiebreaker nbtree index
column, introduced new rules on page space management to make suffix
truncation safe for v4+ indexes.  New pivot tuples (generated by suffix
truncation during leaf page splits) sometimes require dedicated extra
space at the end of a new leaf page high key/pivot to store a heap TID
using a special representation (a representation only used in pivot
tuples).

The definition of "1/3 of a page" was reduced by a single MAXALIGN()
quantum for v4 indexes to make sure that the final enlarged pivot tuple
always fit, even with a split point whose firstright tuple happened to
already be at the "1/3 of a page" limit (limit for non-pivot tuples).
Internal pages (which only contain pivot tuples) stuck with the original
"1/3 of a page" definition.  This scheme made it impossible for any page
split to fail to free enough space for its newitem, which is never okay.

The macro that determines whether non-pivot tuples exceed their "1/3 of
a leaf page" restriction was structured as if space was needed for all
three tuples during a leaf page split (the new pivot plus two very large
adjoining non-pivots that are separated by the split).  This was subtly
wrong, in that it accidentally relied on implementation details that
could (at least in theory) change in the future.

To fix, make the macro subtract a single MAXALIGN() quantum, once.  The
macro evaluates to exactly the same value as before in practice.  But it
no longer depends on the current layout of nbtree's special area struct.

No backpatch, since this isn't a live bug.

Author: Peter Geoghegan <pg@bowt.ie>
Reported-By: Robert Haas <robertmhaas@gmail.com>
Diagnosed-By: Robert Haas <robertmhaas@gmail.com>
Discussion: https://postgr.es/m/CA+Tgmoa7UBxivM7f6Ocx_qbq4=ky3uXc+WZNOBcVX+kvJvWOEA@mail.gmail.com
2022-08-04 20:55:02 -07:00
Thomas Munro a0dc827112 Simplify replacement code for preadv and pwritev.
preadv() and pwritev() are not standardized by POSIX, but appeared in
NetBSD in 1999 and were adopted by at least OpenBSD, FreeBSD,
DragonFlyBSD, Linux, AIX, illumos and macOS.  We don't use them much
yet, but an active proposal uses them heavily.

In 15, we had two replacement implementations for other OSes: one based
on lseek() + -v function if available for true vector I/O, and the other
based on a loop over p- function.

The former would be an obstacle to hypothetical future multi-threaded
code sharing file descriptors, while the latter would not, since commit
cf112c12.  Furthermore, the number of targeted systems that could
benefit from the former's potential upside has dwindled to just one
niche OS, since macOS added the functions and we de-supported HP-UX.
That doesn't seem like a good trade-off.

Therefore, drop the lseek()-based variant, and also the pg_ prefix now
that the file position portability hazard is gone.

At the time of writing, the only systems in our build farm that lack
native preadv/pwritev and thus use fallback code are:

 * Solaris (but not illumos)
 * macOS before release 11.0
 * Windows

With this commit, the above systems will now use the *same* fallback
code, the version that loops over pread()/pwrite().  Windows already
used that (though a later proposal may include true vector I/O for
Windows), so this decision really only affects Solaris, until it gets
around to adding these system calls.

Also remove some useless includes while here.

Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 14:04:02 +12:00
Michael Paquier 718fe0a14a Make consistent a couple of log messages when parsing HBA files
This commit adjusts two log messages:
- When a field in pg_ident.conf is not populated, report the line of the
configuration file in an error context message instead of the main
entry.
- When parsing pg_ident.conf and finding an invalid regexp, add some
information about the line of the configuration file involved within an
error context message.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
2022-08-05 09:50:27 +09:00
Michael Paquier 47ab1ac822 Use hba_file/ident_file GUCs rather than pg_hba.conf/pg_ident.conf in logs
This is particularly useful when log_min_messages is set to FATAL, so as
one can know which file was not getting loaded whether hba_file or
ident_file are set to some non-default values.  If using the default
values of these GUC parameters, the same reports are generated.

This commit changes the load (startup) and reload (SIGHUP) messages.

Author: Julien Rouhaud
Discussion: https://postgr.es/m/20220223045959.35ipdsvbxcstrhya@jrouhaud
2022-08-05 09:37:12 +09:00
David Rowley 53823a06be Fix failure to set correct operator in window run condition
This was a simple omission in 9d9c02ccd where the code didn't correctly
set the operator to use in the run condition OpExpr when the window
function was both monotonically increasing and decreasing.

Bug discovered by Julien Roze, although he did not report it.

Reported-by: Phil Florent
Discussion: https://postgr.es/m/PA4P191MB160009A09B9D0624359278CFBA9F9@PA4P191MB1600.EURP191.PROD.OUTLOOK.COM
Backpatch-through: 15, where 9d9c02ccd was added
2022-08-05 10:14:00 +12:00
Thomas Munro cf112c1220 Remove dead pread and pwrite replacement code.
pread() and pwrite() are in SUSv2, and all targeted Unix systems have
them.

Previously, we defined pg_pread and pg_pwrite to emulate these function
with lseek() on old Unixen.  The names with a pg_ prefix were a reminder
of a portability hazard: they might change the current file position.
That hazard is gone, so we can drop the prefixes.

Since the remaining replacement code is Windows-only, move it into
src/port/win32p{read,write}.c, and move the declarations into
src/include/port/win32_port.h.

No need for vestigial HAVE_PREAD, HAVE_PWRITE macros as they were only
used for declarations in port.h which have now moved into win32_port.h.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Greg Stark <stark@mit.edu>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:49:21 +12:00
Thomas Munro 71f5dc6dfb Remove dead setenv, unsetenv replacement code.
setenv() and unsetenv() are in SUSv3 and targeted Unix systems have
them.  We still need special code for these on Windows, but that doesn't
require a configure probe.

This marks the first time we require a SUSv3 (POSIX.1-2001) facility
(rather than SUSv2).  The replacement code removed here was not needed
on any targeted system or any known non-EOL'd Unix system, and was
therefore dead and untested.

No need for vestigial HAVE_SETENV and HAVE_UNSETENV macros, because we
provide a replacement for Windows, and we didn't previously test the
macros.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Greg Stark <stark@mit.edu>
Reviewed-by: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:38:36 +12:00
Thomas Munro b79ec732d2 Remove configure probes for poll and poll.h.
poll() and <poll.h> are in SUSv2 and all targeted Unix systems have
them.

Retain HAVE_POLL and HAVE_POLL_H macros for readability.  There's an
error in latch.c that is now unreachable (since we always have one of
WIN32 or HAVE_POLL defined), but that falls out of a decision to keep
using defined(HAVE_POLL) instead of !defined(WIN32) to guard the poll()
code.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:37:53 +12:00
Thomas Munro 5963c9a154 Remove configure probe for link.
link() is in SUSv2 and all targeted Unix systems have it.  We have
replacement code for Windows that doesn't require a configure probe.
Since only Windows needs it, rename src/port/link.c to win32link.c like
other similar things.

There is no need for a vestigial HAVE_LINK macro, because we expect all
Unix and, with our replacement function, Windows systems to have it, so
we didn't have any tests around link() usage.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:36:50 +12:00
Thomas Munro 2b1f580ee2 Remove configure probes for symlink/readlink, and dead code.
symlink() and readlink() are in SUSv2 and all targeted Unix systems have
them.  We have partial emulation on Windows.  Code that raised runtime
errors on systems without it has been dead for years, so we can remove
that and also references to such systems in the documentation.

Define HAVE_READLINK and HAVE_SYMLINK macros on Unix.  Our Windows
replacement functions based on junction points can't be used for
relative paths or for non-directories, so the macros can be used to
check for full symlink support.  The places that deal with tablespaces
can just use symlink functions without checking the macros.  (If they
did check the macros, they'd need to provide an #else branch with a
runtime or compile time error, and it'd be dead code.)

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:22:56 +12:00
Thomas Munro adeef67834 Remove configure probe for setsid.
setsid() is in SUSv2 and all targeted Unix systems have it.  Retain a
HAVE_SETSID macro, defined on Unix only.  That's easier to understand
than !defined(WIN32), for the optional code it governs.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:21:51 +12:00
Thomas Munro 098f4d813b Remove configure probe for shm_open.
shm_open() is in SUSv2 and all targeted Unix systems have it.

We retain a HAVE_SHM_OPEN macro, because it's clearer to readers than
something like !defined(WIN32).

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:19:36 +12:00
Thomas Munro bdb657edd6 Remove configure probe and related tests for getrlimit.
getrlimit() is in SUSv2 and all targeted systems have it.

Windows doesn't have it.  We could just use #ifndef WIN32, but for a
little more explanation about why we're making things conditional, let's
retain the HAVE_GETRLIMIT macro.  It's defined in port.h for Unix systems.

On systems that have it, it's not necessary to test for RLIMIT_CORE,
RLIMIT_STACK or RLIMIT_NOFILE macros, since SUSv2 requires those and all
targeted systems have them.  Also remove references to a pre-historic
alternative spelling of RLIMIT_NOFILE, and coding that seemed to believe
that Cygwin didn't have it.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:18:34 +12:00
Thomas Munro ca1e85513e Remove configure probe for dlopen, and refactor.
dlopen() is in SUSv2 and all targeted Unix systems have it.  We still
need replacement functions for Windows, but we don't need a configure
probe for that.

Since it's no longer needed by other operating systems, rename dlopen.c
to win32dlopen.c and move the declarations into win32_port.h.

Likewise, the macros RTLD_NOW and RTLD_GLOBAL now only need to be
defined on Windows, since all targeted Unix systems have 'em.

Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA+hUKGJ3LHeP9w5Fgzdr4G8AnEtJ=z=p6hGDEm4qYGEUX5B6fQ@mail.gmail.com
2022-08-05 09:12:45 +12:00
Robert Haas 87e22f675f Revert recent changes to 002_pg_upgrade.pl.
The test is proving to be unreliable in the buildfarm, and we neither
agree on how best to fix it nor have time to do so before the upcoming
release. So for now, put things back to the way they were before commit
d498e052b4.

Discussion: http://postgr.es/m/3628089.1659640252@sss.pgh.pa.us
2022-08-04 15:26:07 -04:00
Tom Lane d59383924c Fix check_exclusion_or_unique_constraint for UNIQUE NULLS NOT DISTINCT.
Adjusting this function was overlooked in commit 94aa7cc5f.  The only
visible symptom (so far) is that INSERT ... ON CONFLICT could go into
an endless loop when inserting a null that has a conflict.

Richard Guo and Tom Lane, per bug #17558 from Andrew Kesper

Discussion: https://postgr.es/m/17558-3f6599ffcf52fd4a@postgresql.org
2022-08-04 14:16:26 -04:00
Tom Lane 6ad86feecb Add CHECK_FOR_INTERRUPTS in ExecInsert's speculative insertion loop.
Ordinarily the functions called in this loop ought to have plenty
of CFIs themselves; but we've now seen a case where no such CFI is
reached, making the loop uninterruptible.  Even though that's from
a recently-introduced bug, it seems prudent to install a CFI at
the loop level in all branches.

Per discussion of bug #17558 from Andrew Kesper (an actual fix for
that bug will follow).

Discussion: https://postgr.es/m/17558-3f6599ffcf52fd4a@postgresql.org
2022-08-04 14:10:06 -04:00
Tom Lane cc11647991 Add proper regression test for the recent SRFs-in-pathkeys problem.
Remove the test case added by commit fac1b470a, which never actually
worked to expose the problem it claimed to test.  Replace it with
a case that does expose the problem, and also covers the SRF-not-
at-the-top deficiency repaired in 1aa8dad41.

Richard Guo, with some editorialization by me

Discussion: https://postgr.es/m/17564-c7472c2f90ef2da3@postgresql.org
2022-08-04 11:11:33 -04:00
Daniel Gustafsson f8f20203c2 Rephrase comments to make them clearer
The use of "we" when referring to the active backend might be
misunderstood, so rephrase to make it clearer who is performing
the actions discussed in the comment.

Author: Junwang Zhao <zhjwpku@gmail.com>
Reviewed-by: Erikjan Rijkers <er@xs4all.nl>
Reviewed-by: Robert Treat <rob@xzilla.net>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAEG8a3LRSMqkvjiURiJoSi4aGWORpiXUmUfQQK5PaD6WfPzu3w@mail.gmail.com
2022-08-04 16:30:06 +02:00
John Naylor bcabbfc6a9 Fix formatting and comment typos
Justin Pryzby

Discussion: https://www.postgresql.org/message-id/20220801181136.GJ15006%40telsasoft.com
2022-08-04 16:41:29 +07:00
John Naylor 9f08803828 Fix assorted doc typos
Erik Rijkers and Justin Pryzby

Backpatch to v14

Discussion: https://www.postgresql.org/message-id/b79bfeff-d0e3-29a3-2576-0e325848dede%40xs4all.nl
2022-08-04 16:09:15 +07:00
John Naylor f217d093fd Clarify DROP EXTENSION docs regarding explicitly dependent routines
Per suggestion from Robert Haas

Backpatch to v14

Discussion: https://www.postgresql.org/message-id/CA%2BTgmoZ1QvHquYHLkMy1oHKqz4-E7QQctj6e0ocq_GP1B5%2B9bA%40mail.gmail.com
2022-08-04 15:48:37 +07:00
Michael Paquier 245e14e28b Fix inconsistent comments for some function declarations in headers
Some of the headers list a couple of function prototypes located in a
different file than what is referred to.  This fixes a couple of
places where this inconsistency exists.

Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4__RdcSNXPa7L62Ozvo_Q4LvT60o3Bnp8yrQ_m9y5CKvg@mail.gmail.com
2022-08-04 17:36:21 +09:00
John Naylor 56f2c7b58b Support SSE2 intrinsics where available
SSE2 vector instructions are part of the spec for the 64-bit x86
architecture. Until now we have relied on the compiler to autovectorize
in some limited situations, but some useful coding idioms can only be
expressed explicitly via compiler intrinsics. To this end, add a header
that defines USE_SSE2 where available. While x86-only for now, we can
add other architectures in the future. This will also be the intended
place for helper functions that use vector operations.

Reviewed by Nathan Bossart and Masahiko Sawada

Discussion: https://www.postgresql.org/message-id/CAFBsxsE2G_H_5Wbw%2BNOPm70-BK4xxKf86-mRzY%3DL2sLoQqM%2B-Q%40mail.gmail.com
2022-08-04 13:49:18 +07:00
Tom Lane 1aa8dad41f Fix incorrect tests for SRFs in relation_can_be_sorted_early().
Commit fac1b470a thought we could check for set-returning functions
by testing only the top-level node in an expression tree.  This is
wrong in itself, and to make matters worse it encouraged others
to make the same mistake, by exporting tlist.c's special-purpose
IS_SRF_CALL() as a widely-visible macro.  I can't find any evidence
that anyone's taken the bait, but it was only a matter of time.

Use expression_returns_set() instead, and stuff the IS_SRF_CALL()
genie back in its bottle, this time with a warning label.  I also
added a couple of cross-reference comments.

After a fair amount of fooling around, I've despaired of making
a robust test case that exposes the bug reliably, so no test case
here.  (Note that the test case added by fac1b470a is itself
broken, in that it doesn't notice if you remove the code change.
The repro given by the bug submitter currently doesn't fail either
in v15 or HEAD, though I suspect that may indicate an unrelated bug.)

Per bug #17564 from Martijn van Oosterhout.  Back-patch to v13,
as the faulty patch was.

Discussion: https://postgr.es/m/17564-c7472c2f90ef2da3@postgresql.org
2022-08-03 17:33:42 -04:00
Peter Geoghegan bc76f5ac48 Add another old commit to git-blame-ignore-revs. 2022-08-03 09:28:27 -07:00
Tom Lane 1da0850f0e Reduce test runtime of src/test/modules/snapshot_too_old.
The sto_using_cursor and sto_using_select tests were coded to exercise
every permutation of their test steps, but AFAICS there is no value in
exercising more than one.  This matters because each permutation costs
about six seconds, thanks to the "pg_sleep(6)".  Perhaps we could
reduce that, but the useless permutations seem worth getting rid of
in any case.  (Note that sto_using_hash_index got it right already.)

While here, clean up some other sloppiness such as an unused table.

This doesn't make too much difference in interactive testing, since the
wasted time is typically masked by parallelization with other tests.
However, the buildfarm runs this as a serial step, which means we can
expect to shave ~40 seconds from every buildfarm run.  That makes it
worth back-patching.

Discussion: https://postgr.es/m/2515192.1659454702@sss.pgh.pa.us
2022-08-03 11:14:55 -04:00
Amit Kapila 0c20dd33db Add wait_for_subscription_sync for TAP tests.
The TAP tests for logical replication in src/test/subscription are using
the following code in many places to make sure that the subscription is
synchronized with the publisher:

  $node_publisher->wait_for_catchup('tap_sub');
  $node_subscriber->poll_query_until('postgres',
    qq[SELECT count(1) = 0
       FROM pg_subscription_rel
       WHERE srsubstate NOT IN ('r', 's')]);

The new function wait_for_subscription_sync() can be used to replace the
above code. This eliminates duplicated code and makes it easier to write
future tests.

Author: Masahiko Sawada
Reviewed by: Amit Kapila, Shi yu
Discussion: https://postgr.es/m/CAD21AoC-fvAkaKHa4t1urupwL8xbAcWRePeETvshvy80f6WV1A@mail.gmail.com
2022-08-03 15:31:17 +05:30
Tom Lane c67c2e2a29 Be more wary about 32-bit integer overflow in pg_stat_statements.
We've heard a couple of reports of people having trouble with
multi-gigabyte-sized query-texts files.  It occurred to me that on
32-bit platforms, there could be an issue with integer overflow
of calculations associated with the total query text size.
Address that with several changes:

1. Limit pg_stat_statements.max to INT_MAX / 2 not INT_MAX.
The hashtable code will bound it to that anyway unless "long"
is 64 bits.  We still need overflow guards on its use, but
this helps.

2. Add a check to prevent extending the query-texts file to
more than MaxAllocHugeSize.  If it got that big, qtext_load_file
would certainly fail, so there's not much point in allowing it.
Without this, we'd need to consider whether extent, query_offset,
and related variables shouldn't be off_t not size_t.

3. Adjust the comparisons in need_gc_qtexts() to be done in 64-bit
arithmetic on all platforms.  It appears possible that under duress
those multiplications could overflow 32 bits, yielding a false
conclusion that we need to garbage-collect the texts file, which
could lead to repeatedly garbage-collecting after every hash table
insertion.

Per report from Bruno da Silva.  I'm not convinced that these
issues fully explain his problem; there may be some other bug that's
contributing to the query-texts file becoming so large in the first
place.  But it did get that big, so #2 is a reasonable defense,
and #3 could explain the reported performance difficulties.

(See also commit 8bbe4cbd9, which addressed some related bugs.
The second Discussion: link is the thread that led up to that.)

This issue is old, and is primarily a problem for old platforms,
so back-patch.

Discussion: https://postgr.es/m/CAB+Nuk93fL1Q9eLOCotvLP07g7RAv4vbdrkm0cVQohDVMpAb9A@mail.gmail.com
Discussion: https://postgr.es/m/5601D354.5000703@BlueTreble.com
2022-08-02 18:05:38 -04:00
David Rowley 9fc1776dda Remove unused fields from ExprEvalStep
These were added recently by 1349d2790.

Reported-by: Zhihong Yu
Discussion: https://postgr.es/m/CALNJ-vTi+YDuAWKp4Z_Dv=mrz=aq81qTg0D7wzc8y7rS_+i_cw@mail.gmail.com
2022-08-03 09:46:02 +12:00
Tom Lane ec62ce55a8 Change type "char"'s I/O format for non-ASCII characters.
Previously, a byte with the high bit set was just transmitted
as-is by charin() and charout().  This is problematic if the
database encoding is multibyte, because the result of charout()
won't be validly encoded, which breaks various stuff that
expects all text strings to be validly encoded.  We've
previously decided to enforce encoding validity rather than try
to individually harden each place that might have a problem with
such strings, so it's time to do something about "char".

To fix, represent high-bit-set characters as \ooo (backslash
and three octal digits), following the ancient "escape" format
for bytea.  charin() will continue to accept the old way as well,
though that is only reachable in single-byte encodings.

Add some test cases just so there is coverage for this code.
We'll otherwise leave this question undocumented as it was before,
because we don't really want to encourage end-user use of "char".

For the moment, back-patch into v15 so that this change appears
in 15beta3.  If there's not great pushback we should consider
absorbing this change into the older branches.

Discussion: https://postgr.es/m/2318797.1638558730@sss.pgh.pa.us
2022-08-02 10:29:35 -04:00
David Rowley 1349d2790b Improve performance of ORDER BY / DISTINCT aggregates
ORDER BY / DISTINCT aggreagtes have, since implemented in Postgres, been
executed by always performing a sort in nodeAgg.c to sort the tuples in
the current group into the correct order before calling the transition
function on the sorted tuples.  This was not great as often there might be
an index that could have provided pre-sorted input and allowed the
transition functions to be called as the rows come in, rather than having
to store them in a tuplestore in order to sort them once all the tuples
for the group have arrived.

Here we change the planner so it requests a path with a sort order which
supports the most amount of ORDER BY / DISTINCT aggregate functions and
add new code to the executor to allow it to support the processing of
ORDER BY / DISTINCT aggregates where the tuples are already sorted in the
correct order.

Since there can be many ORDER BY / DISTINCT aggregates in any given query
level, it's very possible that we can't find an order that suits all of
these aggregates.  The sort order that the planner chooses is simply the
one that suits the most aggregate functions.  We take the most strictly
sorted variation of each order and see how many aggregate functions can
use that, then we try again with the order of the remaining aggregates to
see if another order would suit more aggregate functions.  For example:

SELECT agg(a ORDER BY a),agg2(a ORDER BY a,b) ...

would request the sort order to be {a, b} because {a} is a subset of the
sort order of {a,b}, but;

SELECT agg(a ORDER BY a),agg2(a ORDER BY c) ...

would just pick a plan ordered by {a} (we give precedence to aggregates
which are earlier in the targetlist).

SELECT agg(a ORDER BY a),agg2(a ORDER BY b),agg3(a ORDER BY b) ...

would choose to order by {b} since two aggregates suit that vs just one
that requires input ordered by {a}.

Author: David Rowley
Reviewed-by: Ronan Dunklau, James Coleman, Ranier Vilela, Richard Guo, Tom Lane
Discussion: https://postgr.es/m/CAApHDvpHzfo92%3DR4W0%2BxVua3BUYCKMckWAmo-2t_KiXN-wYH%3Dw%40mail.gmail.com
2022-08-02 23:11:45 +12:00
Michael Paquier a69959fab2 doc: Fix typos in protocol.sgml
Author: Ekaterina Kiryanova
Discussion: https://postgr.es/m/745414e7-efb2-a6ae-5b83-fcbdf35aabc8@postgrespro.ru
Backpatch-through: 15
2022-08-02 19:55:59 +09:00
Amit Kapila 6b24d3f9cc Move common catalog cache access routines to lsyscache.c
In passing, move pg_relation_is_publishable next to similar functions.

Suggested-by: Alvaro Herrera
Author: Amit Kapila
Reviewed-by: Hou Zhijie
Discussion: https://postgr.es/m/CAHut+PupQ5UW9A9ut0Yjt21J9tHhx958z5L0k8-9hTYf_NYqxA@mail.gmail.com
2022-08-02 10:47:22 +05:30
John Naylor c689baa158 Fix comment in pg_db_role_setting.h
Noted by Japin Li

Discussion: https://www.postgresql.org/message-id/MEYP282MB16691ACEDBC94161CF4BA1CCB69A9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
2022-08-02 11:49:37 +07:00
Amit Kapila 7bf91ec0f3 Remove duplicated wait for subscription sync from 007_ddl.pl.
An oversight in 8f2e2bbf14.

Author: Masahiko Sawada
Reviewed by: Amit Kapila
Backpatch-through: 15, where it was introduced
Discussion: https://postgr.es/m/CAD21AoC-fvAkaKHa4t1urupwL8xbAcWRePeETvshvy80f6WV1A@mail.gmail.com
2022-08-02 09:30:46 +05:30
Tom Lane 418ec32072 Add a regression test for contrib/tcn.
Just whittling down the list of contrib modules with zero coverage.

Discussion: https://postgr.es/m/909667.1659222591@sss.pgh.pa.us
2022-08-01 19:18:50 -04:00
David Rowley b592422095 Relax overly strict rules in select_outer_pathkeys_for_merge()
The select_outer_pathkeys_for_merge function made an attempt to build the
merge join pathkeys in the same order as query_pathkeys.  This was done as
it may have led to no sort being required for an ORDER BY or GROUP BY
clause in the upper planner.  However, this restriction seems overly
strict as it required that we match the query_pathkeys entirely or we
don't bother putting the merge join pathkeys in that order.

Here we relax this rule so that we use a prefix of the query_pathkeys
providing that prefix matches all of the join quals.  This may provide the
upper planner with partially sorted input which will allow the use of
incremental sorts instead of full sorts.

Author: David Rowley
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/CAApHDvrtZu0PHVfDPFM4Yx3jNR2Wuwosv+T2zqa7LrhhBr2rRg@mail.gmail.com
2022-08-02 11:02:46 +12:00
Tom Lane 2865b4060a Add a regression test for contrib/pg_prewarm.
We had a little bit of coverage here thanks to e2f65f425,
but not enough; notably, autoprewarm wasn't exercised at all.

Dong Wook Lee, with help from Julien Rouhaud and myself

Discussion: https://postgr.es/m/20220629053812.mifmdrch5iuasg2s@home-desktop
2022-08-01 17:59:44 -04:00
David Rowley 3592e0ff98 Have ExecFindPartition cache the last found partition
Here we add code which detects when ExecFindPartition() continually finds
the same partition and add a caching layer to improve partition lookup
performance for such cases.

Both RANGE and LIST partitioned tables traditionally require a binary
search for the set of Datums that a partition needs to be found for. This
binary search is commonly visible in profiles when bulk loading into a
partitioned table.  Here we aim to reduce the overhead of bulk-loading
into partitioned tables for cases where many consecutive tuples belong to
the same partition and make the performance of this operation closer to
what it is with a traditional non-partitioned table.

When we find the same partition 16 times in a row, the next search will
result in us simply just checking if the current set of values belongs to
the last found partition.  For LIST partitioning we record the index into
the PartitionBoundInfo's datum array.  This allows us to check if the
current Datum is the same as the Datum that was last looked up.  This
means if any given LIST partition supports storing multiple different
Datum values, then the caching only works when we find the same value as
we did the last time.  For RANGE partitioning we simply check if the given
Datums are in the same range as the previously found partition.

We store the details of the cached partition in PartitionDesc (i.e.
relcache) so that the cached values are maintained over multiple
statements.

No caching is done for HASH partitions.  The majority of the cost in HASH
partition lookups are in the hashing function(s), which would also have to
be executed if we were to try to do caching for HASH partitioned tables.
Since most of the cost is already incurred, we just don't bother.  We also
don't do any caching for LIST partitions when we continually find the
values being looked up belong to the DEFAULT partition.  We've no
corresponding index in the PartitionBoundInfo's datum array for this case.
We also don't cache when we find the given values match to a LIST
partitioned table's NULL partition.  This is so cheap that there's no
point in doing any caching for this.  We also don't cache for a RANGE
partitioned table's DEFAULT partition.

There have been a number of different patches submitted to improve
partition lookups. Hou, Zhijie submitted a patch to detect when the value
belonging to the partition key column(s) were constant and added code to
cache the partition in that case.  Amit Langote then implemented an idea
suggested by me to remember the last found partition and start to check if
the current values work for that partition.  The final patch here was
written by me and was done by taking many of the ideas I liked from the
patches in the thread and redesigning other aspects.

Discussion: https://postgr.es/m/OS0PR01MB571649B27E912EA6CC4EEF03942D9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Author: Amit Langote, Hou Zhijie, David Rowley
Reviewed-by: Amit Langote, Hou Zhijie
2022-08-02 09:55:27 +12:00
Tom Lane 83f1793d60 Check maximum number of columns in function RTEs, too.
I thought commit fd96d14d9 had plugged all the holes of this sort,
but no, function RTEs could produce oversize tuples too, either
via long coldeflists or just from multiple functions in one RTE.
(I'm pretty sure the other variants of base RTEs aren't a problem,
because they ultimately refer to either a table or a sub-SELECT,
whose widths are enforced elsewhere.  But we explicitly allow join
RTEs to be overwidth, as long as you don't try to form their
tuple result.)

Per further discussion of bug #17561.  As before, patch all branches.

Discussion: https://postgr.es/m/17561-80350151b9ad2ad4@postgresql.org
2022-08-01 12:22:35 -04:00
Michael Paquier 8b1ec7d295 Fix error reporting after ioctl() call with pg_upgrade --clone
errno was not reported correctly after attempting to clone a file,
leading to incorrect error reports.  While scanning through the code, I
have not noticed any similar mistakes.

Error introduced in 3a769d8.

Author: Justin Pryzby
Discussion: https://postgr.es/m/20220731134135.GY15006@telsasoft.com
Backpatch-through: 12
2022-08-01 16:38:23 +09:00