Andres Freund [Tue, 26 Nov 2024 17:20:59 +0000 (12:20 -0500)]
Distinguish between AcquireExternalFD and epoll_create1 / kqueue failing
The error messages in CreateWaitEventSet() made it hard to know whether the
syscall or AcquireExternalFD() failed. This is particularly relevant because
AcquireExternalFD() imposes a lower limit than what would cause syscalls fail
with EMFILE.
I did not change the message in libpqsrv_connect_prepare(), which is the one
other use of AcquireExternalFD() in our codebase, as the error message already
is less ambiguous.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/xjjx7r4xa7beixuu4qtkdhnwdbchrrpo3gaeb3jsbinvvdiat5@cwjw55mna5of
Peter Eisentraut [Tue, 26 Nov 2024 17:06:08 +0000 (18:06 +0100)]
meson: Build pgevent as shared_module rather than shared_library
This matches the behavior of the makefiles and the old MSVC build
system. The main effect is that the build result gets installed into
pkglibdir rather than bindir. The documentation says to locate the
library in pkglibdir, so this makes the code match the documentation
again.
Reviewed-by: Ryohei Takahashi (Fujitsu) <r.takahashi_2@fujitsu.com>
Discussion: https://www.postgresql.org/message-id/flat/TY3PR01MB118912125614599641CA881B782522%40TY3PR01MB11891.jpnprd01.prod.outlook.com
Álvaro Herrera [Tue, 26 Nov 2024 16:10:07 +0000 (17:10 +0100)]
Clean up newlines following left parentheses
Most came in during the 17 cycle, so backpatch there. Some
(particularly reorderbuffer.h) are very old, but backpatching doesn't
seem useful.
Like commits
c9d297751959,
c4f113e8fef9.
Peter Eisentraut [Tue, 26 Nov 2024 07:25:23 +0000 (08:25 +0100)]
Improve InitShmemAccess() prototype
The code comment said, 'the argument should be declared "PGShmemHeader
*seghdr", but we use void to avoid having to include ipc.h in
shmem.h.' We can achieve the original goal with a struct forward
declaration. (ipc.h was also not the correct header file.)
Discussion: https://www.postgresql.org/message-id/flat/cnthxg2eekacrejyeonuhiaezc7vd7o2uowlsbenxqfkjwgvwj@qgzu6eoqrglb
Richard Guo [Tue, 26 Nov 2024 02:12:57 +0000 (11:12 +0900)]
Fix test case from
a8ccf4e93
Commit
a8ccf4e93 uses the same table name "distinct_tbl" in both
select_distinct.sql and select_distinct_on.sql, which could cause
conflicts when these two test scripts are run in parallel.
Fix by renaming the table in select_distinct_on.sql to
"distinct_on_tbl".
Per buildfarm (via Tom Lane)
Discussion: https://postgr.es/m/
1572004.
1732583549@sss.pgh.pa.us
Michael Paquier [Tue, 26 Nov 2024 00:45:34 +0000 (09:45 +0900)]
pg_amcheck: Use CppAsString2() for relkind and relpersistence in queries
This utility has been using hardcoded values for relkind and
relpersistence in its queries generated. These queries are switched to
use CppAsString2() instead, with the values fetched directly from the
header of pg_class. This has the advantage of making the code more
self-documented, as it becomes unnecessary to look at a header for the
meaning of a value.
There should be no functional changes; the queries are generated the
same way as before this commit.
Reviewed-by: Nathan Bossart, Daniel Gustafsson, Álvaro Herrera, Karina
Litskevich
Discussion: https://postgr.es/m/ZxIvemDk0Ob1RGwh@paquier.xyz
Richard Guo [Tue, 26 Nov 2024 00:27:53 +0000 (09:27 +0900)]
Remove dead code in get_param_path_clause_serials()
The function get_param_path_clause_serials() is used to get the set of
pushed-down clauses enforced within a parameterized Path. Since we
don't currently support parameterized MergeAppend paths, and it
doesn't look like that is going to change anytime soon (as explained
in the comments for generate_orderedappend_paths), we don't need to
consider MergeAppendPath in this function.
This change won't make any measurable difference in performance; it's
just for clarity's sake.
Author: Richard Guo
Reviewed-by: Andrei Lepikhov
Discussion: https://postgr.es/m/CAMbWs4_Puie4DQ2ODvjQB_3CxYkUODnrJm8jn_ObMAcrjYNW7Q@mail.gmail.com
Richard Guo [Tue, 26 Nov 2024 00:25:18 +0000 (09:25 +0900)]
Reordering DISTINCT keys to match input path's pathkeys
The ordering of DISTINCT items is semantically insignificant, so we
can reorder them as needed. In fact, in the parser, we absorb the
sorting semantics of the sortClause as much as possible into the
distinctClause, ensuring that one clause is a prefix of the other.
This can help avoid a possible need to re-sort.
In this commit, we attempt to adjust the DISTINCT keys to match the
input path's pathkeys. This can likewise help avoid re-sorting, or
allow us to use incremental-sort to save efforts.
For DISTINCT ON expressions, the parser already ensures that they
match the initial ORDER BY expressions. When reordering the DISTINCT
keys, we must ensure that the resulting pathkey list matches the
initial distinctClause pathkeys.
This introduces a new GUC, enable_distinct_reordering, which allows
the optimization to be disabled if needed.
Author: Richard Guo
Reviewed-by: Andrei Lepikhov
Discussion: https://postgr.es/m/CAMbWs48dR26cCcX0f=8bja2JKQPcU64136kHk=xekHT9xschiQ@mail.gmail.com
Tom Lane [Mon, 25 Nov 2024 23:08:58 +0000 (18:08 -0500)]
Fix NULLIF()'s handling of read-write expanded objects.
If passed a read-write expanded object pointer, the EEOP_NULLIF
code would hand that same pointer to the equality function
and then (unless equality was reported) also return the same
pointer as its value. This is no good, because a function that
receives a read-write expanded object pointer is fully entitled
to scribble on or even delete the object, thus corrupting the
NULLIF output. (This problem is likely unobservable with the
equality functions provided in core Postgres, but it's easy to
demonstrate with one coded in plpgsql.)
To fix, make sure the pointer passed to the equality function
is read-only. We can still return the original read-write
pointer as the NULLIF result, allowing optimization of later
operations.
Per bug #18722 from Alexander Lakhin. This has been wrong
since we invented expanded objects, so back-patch to all
supported branches.
Discussion: https://postgr.es/m/18722-
fd9e645448cc78b4@postgresql.org
Noah Misch [Mon, 25 Nov 2024 22:42:35 +0000 (14:42 -0800)]
Avoid "you don't own a lock of type ExclusiveLock" in GRANT TABLESPACE.
This WARNING appeared because SearchSysCacheLocked1() read
cc_relisshared before catcache initialization, when the field is false
unconditionally. On the basis of reading false there, it constructed a
locktag as though pg_tablespace weren't relisshared. Only shared
catalogs could be affected, and only GRANT TABLESPACE was affected in
practice. SearchSysCacheLocked1() callers use one other shared-relation
syscache, DATABASEOID. DATABASEOID is initialized by the end of
CheckMyDatabase(), making the problem unreachable for pg_database.
Back-patch to v13 (all supported versions). This has no known impact
before v16, where ExecGrant_common() first appeared. Earlier branches
avoid trouble by having a separate ExecGrant_Tablespace() that doesn't
use LOCKTAG_TUPLE. However, leaving this unfixed in v15 could ensnare a
future back-patch of a SearchSysCacheLocked1() call.
Reported by Aya Iwata.
Discussion: https://postgr.es/m/OS7PR01MB11964507B5548245A7EE54E70EA212@OS7PR01MB11964.jpnprd01.prod.outlook.com
Nathan Bossart [Mon, 25 Nov 2024 22:36:37 +0000 (16:36 -0600)]
pg_dump: Add dumpSchema and dumpData derivative flags.
Various parts of pg_dump consult the --schema-only and --data-only
options to determine whether to run a section of code. While this
is simple enough for two mutually-exclusive options, it will become
progressively more complicated as more options are added. In
anticipation of that, this commit introduces new internal flags
called dumpSchema and dumpData, which are derivatives of
--schema-only and --data-only. This commit also removes the
schemaOnly and dataOnly members from the dump/restore options
structs to prevent their use elsewhere.
Note that this change neither adds new user-facing command-line
options nor changes the existing --schema-only and --data-only
options.
Author: Corey Huinker
Reviewed-by: Jeff Davis
Discussion: https://postgr.es/m/CADkLM%3DcQgghMJOS8EcAVBwRO4s1dUVtxGZv5gLPfZkQ1nL1gzA%40mail.gmail.com
Thomas Munro [Mon, 25 Nov 2024 22:29:31 +0000 (11:29 +1300)]
Clean up <stdbool.h> reference in meson.build.
Commit
bc5a4dfc accidentally left a check for <stdbool.h> in
meson.build's header_checks. Synchronize with configure, which no
longer defines HAVE_STDBOOL_H.
There is still a reference to <stdbool.h> in an earlier test to see if
we need -std=c99 to get C99 features, like autoconf 2.69's
AC_PROG_CC_C99. (Therefore the test remove by this commit was
tautological since day one: you'd have copped "C compiler does not
support C99" before making it this far.)
Back-patch to 16, where meson begins.
Tom Lane [Mon, 25 Nov 2024 17:50:17 +0000 (12:50 -0500)]
Update configure probes for CFLAGS needed for ARM CRC instructions.
On ARM platforms where the baseline CPU target lacks CRC instructions,
we need to supply a -march flag to persuade the compiler to compile
such instructions. It turns out that our existing choice of
"-march=armv8-a+crc" has not worked for some time, because recent gcc
will interpret that as selecting software floating point, and then
will spit up if the platform requires hard-float ABI, as most do
nowadays. The end result was to silently fall back to software CRC,
which isn't very desirable since in practice almost all currently
produced ARM chips do have hardware CRC.
We can fix this by using "-march=armv8-a+crc+simd" to enable the
correct ABI choice. (This has no impact on the code actually
generated, since neither of the files we compile with this flag
does any floating-point stuff, let alone SIMD.) Keep the test for
"-march=armv8-a+crc" since that's required for soft-float ABI,
but try that second since most platforms we're likely to build on
use hard-float.
Since this isn't working as-intended on the last several years'
worth of gcc releases, back-patch to all supported branches.
Discussion: https://postgr.es/m/
4496616.iHFcN1HehY@portable-bastien
Tom Lane [Mon, 25 Nov 2024 16:53:26 +0000 (11:53 -0500)]
Support runtime CRC feature probing on NetBSD/ARM using sysctl().
Commit
aac831caf left this as a to-do; here's code to do it.
Like the previous patch, this is HEAD-only for now.
Discussion: https://postgr.es/m/
4496616.iHFcN1HehY@portable-bastien
Peter Eisentraut [Mon, 25 Nov 2024 07:03:16 +0000 (08:03 +0100)]
Add support for Tcl 9
Tcl 9 changed several API functions to take Tcl_Size, which is
ptrdiff_t, instead of int, for 64-bit enablement. We have to change a
few local variables to be compatible with that. We also provide a
fallback typedef of Tcl_Size for older Tcl versions.
The affected variables are used for quantities that will not approach
values beyond the range of int, so this doesn't change any
functionality.
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://www.postgresql.org/message-id/flat/
bce0fe54-75b4-438e-b42b-
8e84bc7c0e9c%40eisentraut.org
Thomas Munro [Mon, 25 Nov 2024 00:11:28 +0000 (13:11 +1300)]
Assume that <stdbool.h> conforms to the C standard.
Previously we checked "for <stdbool.h> that conforms to C99" using
autoconf's AC_HEADER_STDBOOL macro. We've required C99 since PostgreSQL
12, so the test was redundant, and under C23 it was broken: autoconf
2.69's implementation doesn't understand C23's new empty header (the
macros it's looking for went away, replaced by language keywords).
Later autoconf versions fixed that, but let's just remove the
anachronistic test.
HAVE_STDBOOL_H and HAVE__BOOL will no longer be defined, but they
weren't directly tested in core or likely extensions (except in 11, see
below). PG_USE_STDBOOL (or USE_STDBOOL in 11 and 12) is still defined
when sizeof(bool) is 1, which should be true on all modern systems.
Otherwise we define our own bool type and values of size 1, which would
fail to compile under C23 as revealed by the broken test. (We'll
probably clean that dead code up in master, but here we want a minimal
back-patchable change.)
This came to our attention when GCC 15 recently started using using C23
by default and failed to compile the replacement code, as reported by
Sam James and build farm animal alligator.
Back-patch to all supported releases, and then two older versions that
also know about <stdbool.h>, per the recently-out-of-support policy[1].
12 requires C99 so it's much like the supported releases, but 11 only
assumes C89 so it now uses AC_CHECK_HEADERS instead of the overly picky
AC_HEADER_STDBOOL. (I could find no discussion of which historical
systems had <stdbool.h> but failed the conformance test; if they ever
existed, they surely aren't relevant to that policy's goals.)
[1] https://wiki.postgresql.org/wiki/Committing_checklist#Policies
Reported-by: Sam James <sam@gentoo.org>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org> (master version)
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (approach)
Discussion: https://www.postgresql.org/message-id/flat/87o72eo9iu.fsf%40gentoo.org
Alexander Korotkov [Mon, 25 Nov 2024 07:05:26 +0000 (09:05 +0200)]
Remove the wrong assertion from match_orclause_to_indexcol()
Obviously, the constant could be zero. Also, add the relevant check to
regression tests.
Reported-by: Richard Guo
Discussion: https://postgr.es/m/CAMbWs4-siKJdtWhcbqk4Y-xG12do2Ckm1qw672GNsSnDqL9FQg%40mail.gmail.com
Amit Kapila [Mon, 25 Nov 2024 05:42:32 +0000 (11:12 +0530)]
Doc: Clarify the `inactive_since` field description.
Updated to specify that it represents the exact time a slot became
inactive, rather than the period of inactivity.
Reported-by: Peter Smith
Author: Bruce Momjian, Nisha Moond
Reviewed-by: Amit Kapila, Peter Smith
Backpatch-through: 17
Discussion: https://postgr.es/m/CAHut+PuvsyA5v8y7rYoY9mkDQzUhwaESM05yCByTMaDoRh30tA@mail.gmail.com
Michael Paquier [Mon, 25 Nov 2024 00:43:16 +0000 (09:43 +0900)]
Simplify some SPI tests of PL/Python
These tests relied on both next() and __next__(), but only the former is
needed since Python 2 support has been removed, so let's simplify a bit
the tests.
Author: Erik Wienhold
Discussion: https://postgr.es/m/
173209043143.
2092749.
13692266486972491694@wrigleys.postgresql.org
Michael Paquier [Mon, 25 Nov 2024 00:15:25 +0000 (09:15 +0900)]
doc: Fix example with __next__() in PL/Python function
Per PEP 3114, iterator.next() has been renamed to iterator.__next__(),
and one example in the documentation still used next(). This caused the
example provided to fail the function creation since Python 2 is not
supported anymore since
19252e8ec93.
Author: Erik Wienhold
Discussion: https://postgr.es/m/
173209043143.
2092749.
13692266486972491694@wrigleys.postgresql.org
Backpatch-through: 15
Noah Misch [Sun, 24 Nov 2024 20:49:53 +0000 (12:49 -0800)]
Test "options=-crole=" and "ALTER DATABASE SET role".
Commit
7b88529f4363994450bd4cd3c172006a8a77e222 fixed a regression
spanning these features, but it didn't test them. It did test code
paths sufficient for their present implementations, so no back-patch.
Reported by Matthew Woodcraft.
Discussion: https://postgr.es/m/87iksnsbhx.fsf@golux.woodcraft.me.uk
Alexander Korotkov [Sat, 23 Nov 2024 23:41:45 +0000 (01:41 +0200)]
Teach bitmap path generation about transforming OR-clauses to SAOP's
When optimizer generates bitmap paths, it considers breaking OR-clause
arguments one-by-one. But now, a group of similar OR-clauses can be
transformed into SAOP during index matching. So, bitmap paths should
keep up.
This commit teaches bitmap paths generation machinery to group similar
OR-clauses into dedicated RestrictInfos. Those RestrictInfos are considered
both to match index as a whole (as SAOP), or to match as a set of individual
OR-clause argument one-by-one (the old way).
Therefore, bitmap path generation will takes advantage of OR-clauses to SAOP's
transformation. The old way of handling them is also considered. So, there
shouldn't be planning regression.
Discussion: https://postgr.es/m/CAPpHfdu5iQOjF93vGbjidsQkhHvY2NSm29duENYH_cbhC6x%2BMg%40mail.gmail.com
Author: Alexander Korotkov, Andrey Lepikhov
Reviewed-by: Alena Rybakina, Andrei Lepikhov, Jian he, Robert Haas
Reviewed-by: Peter Geoghegan
Alexander Korotkov [Sat, 23 Nov 2024 23:40:20 +0000 (01:40 +0200)]
Transform OR-clauses to SAOP's during index matching
This commit makes match_clause_to_indexcol() match
"(indexkey op C1) OR (indexkey op C2) ... (indexkey op CN)" expression
to the index while transforming it into "indexkey op ANY(ARRAY[C1, C2, ...])"
(ScalarArrayOpExpr node).
This transformation allows handling long OR-clauses with single IndexScan
avoiding diving them into a slower BitmapOr.
We currently restrict Ci to be either Const or Param to apply this
transformation only when it's clearly beneficial. However, in the future,
we might switch to a liberal understanding of constants, as it is in other
cases.
Discussion: https://postgr.es/m/
567ED6CA.
2040504%40sigaev.ru
Author: Alena Rybakina, Andrey Lepikhov, Alexander Korotkov
Reviewed-by: Peter Geoghegan, Ranier Vilela, Alexander Korotkov, Robert Haas
Reviewed-by: Jian He, Tom Lane, Nikolay Shaplov
Jeff Davis [Fri, 22 Nov 2024 20:07:46 +0000 (12:07 -0800)]
Disallow modifying statistics on system columns.
Reported-by: Heikki Linnakangas
Discussion: https://postgr.es/m/
df3e1c41-4e6c-40ad-9636-
98deefe488cd@iki.fi
Nathan Bossart [Fri, 22 Nov 2024 18:41:57 +0000 (12:41 -0600)]
Add INT64_HEX_FORMAT and UINT64_HEX_FORMAT to c.h.
Like INT64_FORMAT and UINT64_FORMAT, these macros produce format
strings for 64-bit integers. However, INT64_HEX_FORMAT and
UINT64_HEX_FORMAT generate the output in hexadecimal instead of
decimal. Besides introducing these macros, this commit makes use
of them in several places. This was originally intended to be part
of commit
5d6187d2a2, but I left it out because I felt there was a
nonzero chance that back-patching these new macros into c.h could
cause problems with third-party code. We tend to be less cautious
with such changes in new major versions.
Note that UINT64_HEX_FORMAT was originally added in commit
ee1b30f128, but it was placed in test_radixtree.c, so it wasn't
widely available. This commit moves UINT64_HEX_FORMAT to c.h.
Discussion: https://postgr.es/m/ZwQvtUbPKaaRQezd%40nathan
Nathan Bossart [Fri, 22 Nov 2024 18:17:35 +0000 (12:17 -0600)]
Add a couple of recent commits to .git-blame-ignore-revs.
Heikki Linnakangas [Fri, 22 Nov 2024 15:43:04 +0000 (17:43 +0200)]
Make the memory layout of Port struct independent of USE_OPENSSL
Commit
d39a49c1e4 added new fields to the struct, but missed the "keep
these last" comment on the previous fields. Add placeholder variables
so that the offsets of the fields are the same whether you build with
USE_OPENSSL or not. This is a courtesy to extensions that might peek
at the fields, to make the ABI the same regardless of the options used
to build PostgreSQL.
In reality, I don't expect any extensions to look at the 'raw_buf'
fields. Firstly, they are new in v17, so no one's written such
extensions yet. Secondly, extensions should have no business poking at
those fields anyway. Nevertheless, fix this properly on 'master'. On
v17, we mustn't change the memory layout, so just fix the comments.
Author: Jacob Champion
Discussion: https://www.postgresql.org/message-id/raw/CAOYmi%2BmKVJNzn5_TD_MK%3DhqO64r_w8Gb0FHCLk0oAkW-PJv8jQ@mail.gmail.com
Heikki Linnakangas [Fri, 22 Nov 2024 14:28:24 +0000 (16:28 +0200)]
Fix data loss when restarting the bulk_write facility
If a user started a bulk write operation on a fork with existing data
to append data in bulk, the bulk_write machinery would zero out all
previously written pages up to the last page written by the new
bulk_write operation.
This is not an issue for PostgreSQL itself, because we never use the
bulk_write facility on a non-empty fork. But there are use cases where
it makes sense. TimescaleDB extension is known to do that to merge
partitions, for example.
Backpatch to v17, where the bulk_write machinery was introduced.
Author: Matthias van de Meent <boekewurm+postgres@gmail.com>
Reported-By: Erik Nordström <erik@timescale.com>
Reviewed-by: Erik Nordström <erik@timescale.com>
Discussion: https://www.postgresql.org/message-id/CACAa4VJ%2BQY4pY7M0ECq29uGkrOygikYtao1UG9yCDFosxaps9g@mail.gmail.com
Thomas Munro [Fri, 22 Nov 2024 03:45:19 +0000 (16:45 +1300)]
Use auxv to check for CRC32 instructions on ARM.
Previously we probed for CRC32 instructions by testing if they caused
SIGILL. Some have expressed doubts about that technique, the Linux
documentation advises not to use it, and it's not exactly beautiful.
Now that more operating systems expose CPU features to userspace via the
ELF loader in approximately the same way, let's use that instead.
This is expected to work on Linux, FreeBSD and recent OpenBSD.
OpenBSD/ARM has not been tested and is not present in our build farm,
but the API matches FreeBSD.
On macOS, compilers use a more recent baseline ISA so the runtime test
mechanism isn't reached. (A similar situation is expected for
Windows/ARM when that port lands.)
On NetBSD, runtime feature probing is lost for armv8-a builds. It looks
potentially doable with sysctl following the example of the cpuctl
program; patches are welcome.
No back-patch for now, since we don't have any evidence of actual
breakage from the previous technique.
Suggested-by: Bastien Roucariès <rouca@debian.org>
Discussion: https://postgr.es/m/
4496616.iHFcN1HehY%40portable-bastien
Michael Paquier [Fri, 22 Nov 2024 05:04:21 +0000 (14:04 +0900)]
psql: Fix category of \parse in output of --help=commands and \?
\parse was listed under the category "Connection", which was incorrect.
Let's move it to "General" like the other meta-commands of the same type
(\bind, \bind_named and \close).
Oversight in commit
d55322b0da60.
Discussion: https://postgr.es/m/Zz_x-NEKNeeRlAVc@paquier.xyz
Michael Paquier [Fri, 22 Nov 2024 03:17:37 +0000 (12:17 +0900)]
psql: Include \pset xheader_width in --help=commands|variables
psql's --help was missed the description of the \pset variable
xheader_width, that should be listed when using \? or --help=commands,
and described for --help=variables.
Oversight in
a45388d6e098.
Author: Pavel Luzanov
Discussion: https://postgr.es/m/
1e3e06d6-0807-4e62-a9f6-
c11481e6eb10@postgrespro.ru
Backpatch-through: 16
Thomas Munro [Fri, 22 Nov 2024 01:53:21 +0000 (14:53 +1300)]
jit: Use -mno-outline-atomics for bitcode on ARM.
If the executable's .o files were produced by a compiler (probably gcc)
not using -moutline-atomics, and the corresponding .bc files were
produced by clang using -moutline-atomics (probably by default), then
the generated bitcode functions would have the target attribute
"+outline-atomics", and could fail at runtime when inlined. If the
target ISA at bitcode generation time was armv8-a (the most conservative
aarch64 target, no LSE), then LLVM IR atomic instructions would generate
calls to functions in libgcc.a or libclang_rt.*.a that switch between
LL/SC and faster LSE instructions depending on a runtime AT_HWCAP check.
Since the corresponding .o files didn't need those functions, they
wouldn't have been included in the executable, and resolution would
fail.
At least Debian and Ubuntu are known to ship gcc and clang compilers
that target armv8-a but differ on the use of outline atomics by default.
Fix, by suppressing the outline atomics attribute in bitcode explicitly.
Inline LL/SC instructions will be generated for atomic operations in
bitcode built for armv8-a. Only configure scripts are adjusted for now,
because the meson build system doesn't generate bitcode yet.
This doesn't seem to be a new phenomenon, so real cases of functions
using atomics that are inlined by JIT must be rare in the wild given how
long it took for a bug report to arrive. The reported case could be
reduced to:
postgres=# set jit_inline_above_cost = 0;
SET
postgres=# set jit_above_cost = 0;
SET
postgres=# select pg_last_wal_receive_lsn();
WARNING: failed to resolve name __aarch64_swp4_acq_rel
FATAL: fatal llvm error: Program used external function
'__aarch64_swp4_acq_rel' which could not be resolved!
The change doesn't affect non-ARM systems or later target ISAs.
Back-patch to all supported releases.
Reported-by: Alexander Kozhemyakin <a.kozhemyakin@postgrespro.ru>
Discussion: https://postgr.es/m/18610-
37bf303f904fede3%40postgresql.org
Michael Paquier [Fri, 22 Nov 2024 01:12:26 +0000 (10:12 +0900)]
Add write_to_file to PgStat_KindInfo for pgstats kinds
This new field controls if entries of a stats kind should be written or
not to the on-disk pgstats file when shutting down an instance. This
affects both fixed and variable-numbered kinds.
This is useful for custom statistics by itself, and a patch is under
discussion to add a new builtin stats kind where the write of the stats
is not necessary. All the built-in stats kinds, as well as the two
custom stats kinds in the test module injection_points, set this flag to
"true" for now, so as stats entries are written to the on-disk pgstats
file.
Author: Bertrand Drouvot
Reviewed-by: Nazir Bilal Yavuz
Discussion: https://postgr.es/m/Zz7T47nHwYgeYwOe@ip-10-97-1-34.eu-west-3.compute.internal
Bruce Momjian [Thu, 21 Nov 2024 22:14:33 +0000 (17:14 -0500)]
doc: clarify how logical replication takes its initial snapshot
Reported-by: Koen De Groote
Discussion: https://postgr.es/m/
171606613152.686.
7693963105919927503@wrigleys.postgresql.org
Backpatch-through: master
Peter Eisentraut [Thu, 21 Nov 2024 20:40:17 +0000 (21:40 +0100)]
pgindent run
for commit
79b575d3bc0
Álvaro Herrera [Thu, 21 Nov 2024 16:04:26 +0000 (17:04 +0100)]
Fix newly introduced 010_keep_recycled_wals.pl
It failed to set the archive_command as it desired because of a syntax
problem. Oversight in commit
90bcc7c2db1d.
This bug doesn't cause the test to fail, because the test only checks
pg_rewind's output messages, not the actual outcome (and the outcome in
both cases is that the file is kept, not deleted). But in either case
the message about the file being kept is there, so it's hard to get
excited about doing much more.
Reported-by: Antonin Houska <ah@cybertec.at>
Author: Alexander Kukushkin <cyberdemn@gmail.com>
Discussion: https://postgr.es/m/7822.
1732167825@antos
Álvaro Herrera [Thu, 21 Nov 2024 15:54:36 +0000 (16:54 +0100)]
Fix outdated bit in README.tuplock
Apparently this information has been outdated since first committed,
because we adopted a different implementation during development per
reviews and this detail was not updated in the README.
This has been wrong since commit
0ac5ad5134f2 introduced the file in
2013. Backpatch to all live branches.
Reported-by: Will Mortensen <will@extrahop.com>
Discussion: https://postgr.es/m/CAMpnoC6yEQ=c0Rdq-J7uRedrP7Zo9UMp6VZyP23QMT68n06cvA@mail.gmail.com
Peter Eisentraut [Thu, 21 Nov 2024 12:50:18 +0000 (13:50 +0100)]
Fix ALTER TABLE / REPLICA IDENTITY for temporal tables
REPLICA IDENTITY USING INDEX did not accept a GiST index. This should
be allowed when used as a temporal primary key.
Author: Paul Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/
04579cbf-b134-45e1-8f2d-
8c54c849c1ee@illuminatedcomputing.com
Álvaro Herrera [Thu, 21 Nov 2024 09:54:30 +0000 (10:54 +0100)]
Unify repetitive error messages
Michael Paquier [Thu, 21 Nov 2024 06:14:02 +0000 (15:14 +0900)]
Fix memory leak in pgoutput for the WAL sender
RelationSyncCache, the hash table in charge of tracking the relation
schemas sent through pgoutput, was forgetting to free the TupleDesc
associated to the two slots used to store the new and old tuples,
causing some memory to be leaked each time a relation is invalidated
when the slots of an existing relation entry are cleaned up.
This is rather hard to notice as the bloat is pretty minimal, but a
long-running WAL sender would be in trouble over time depending on the
workload. sysbench has proved to be pretty good at showing the problem,
coupled with some memory monitoring of the WAL sender.
Issue introduced in
52e4f0cd472d, that has added row filters for tables
logically replicated.
Author: Boyu Yang
Reviewed-by: Michael Paquier, Hou Zhijie
Discussion: https://postgr.es/m/DM3PR84MB3442E14B340E553313B5C816E3252@DM3PR84MB3442.NAMPRD84.PROD.OUTLOOK.COM
Backpatch-through: 15
Bruce Momjian [Wed, 20 Nov 2024 22:09:17 +0000 (17:09 -0500)]
More logically order libpq func. includes, e.g., group GUC vals
Reported-by: David Zhang
Discussion: https://postgr.es/m/
65909efe-97c6-4863-af4e-
21eb5a26dd1e@highgo.ca
Co-authored-by: David Zhang
Backpatch-through: master
Bruce Momjian [Wed, 20 Nov 2024 22:03:45 +0000 (17:03 -0500)]
doc: clarify that jsonb_path_match() returns an SQL boolean
Not a JSON boolean. Also clarify that other predicate check expressions
functions return a JSON boolean, not an SQL boolean.
Reported-by: jian he
Discussion: https://postgr.es/m/CACJufxH7tP1NXCHN1bUBXcEB=dv7-qE+ZjB3UxwK6Em+9Qzb9Q@mail.gmail.com
Backpatch-through: 17
Bruce Momjian [Wed, 20 Nov 2024 19:48:31 +0000 (14:48 -0500)]
clarify --no-comments option in --help and SGML files
The previous commit,
b38bac26e20, missed these cases for dump/restore.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/
3495698.
1731968093@sss.pgh.pa.us
Backpatch-through: master
Peter Geoghegan [Wed, 20 Nov 2024 18:37:08 +0000 (13:37 -0500)]
Refine nbtree = redundancy preprocessing comment.
Spell out how a = key associated with a SAOP array renders a > key
against the same index column redundant at the relevant point inside
_bt_preprocess_keys.
Follow-up to commit
5bf748b8.
Tom Lane [Wed, 20 Nov 2024 17:03:47 +0000 (12:03 -0500)]
Avoid assertion failure if a setop leaf query contains setops.
Ordinarily transformSetOperationTree will collect all UNION/
INTERSECT/EXCEPT steps into the setOperations tree of the topmost
Query, so that leaf queries do not contain any setOperations.
However, it cannot thus flatten a subquery that also contains
WITH, ORDER BY, FOR UPDATE, or LIMIT. I (tgl) forgot that in
commit
07b4c48b6 and wrote an assertion in rule deparsing that
a leaf's setOperations would always be empty.
If it were nonempty then we would want to parenthesize the subquery
to ensure that the output represents the setop nesting correctly
(e.g. UNION below INTERSECT had better get parenthesized). So
rather than just removing the faulty Assert, let's change it into
an additional case to check to decide whether to add parens. We
don't expect that the additional case will ever fire, but it's
cheap insurance.
Man Zeng and Tom Lane
Discussion: https://postgr.es/m/tencent_7ABF9B1F23B0C77606FC5FE3@qq.com
Fujii Masao [Wed, 20 Nov 2024 14:53:19 +0000 (23:53 +0900)]
file_fdw: Add REJECT_LIMIT option to file_fdw.
Commit
4ac2a9bece introduced the REJECT_LIMIT option for the COPY
command. This commit extends the support for this option to file_fdw.
As well as REJECT_LIMIT option for COPY, this option limits
the maximum number of erroneous rows that can be skipped.
If the number of data type conversion errors exceeds this limit,
accessing the file_fdw foreign table will fail with an error,
even when on_error = 'ignore' is specified.
Since the CREATE/ALTER FOREIGN TABLE commands require foreign
table options to be single-quoted, this commit updates
defGetCopyRejectLimitOption() to handle also string value for them,
in addition to int64 value for COPY command option.
Author: Atsushi Torikoshi
Reviewed-by: Fujii Masao, Yugo Nagata, Kirill Reshke
Discussion: https://postgr.es/m/
bab68a9fc502b12693f0755b6f35f327@oss.nttdata.com
Michael Paquier [Wed, 20 Nov 2024 05:20:52 +0000 (14:20 +0900)]
doc: Fix section of functions age(xid) and mxid_age(xid)
In 17~, age(xid) and mxid_age(xid) were listed as deprecated. Based on
the discussion that led to
48b5aa3143, this is not intentional as this
could break many existing monitoring queries. Note that vacuumdb also
uses both of them.
In 16, both functions were listed under "Control Data Functions", which
is incorrect, so let's move them to the list of functions related to
transaction IDs and snapshots.
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/Zzr2zZFyeFKXWe8a@ip-10-97-1-34.eu-west-3.compute.internal
Discussion: https://postgr.es/m/
20231114013224.4z6oxa6p6va33rxr@awork3.anarazel.de
Backpatch-through: 16
Tom Lane [Tue, 19 Nov 2024 23:26:19 +0000 (18:26 -0500)]
Compare collations before merging UNION operations.
In the dim past we figured it was okay to ignore collations
when combining UNION set-operation nodes into a single N-way
UNION operation. I believe that was fine at the time, but
it stopped being fine when we added nondeterministic collations:
the semantics of distinct-ness are affected by those. v17 made
it even less fine by allowing per-child sorting operations to
be merged via MergeAppend, although I think we accidentally
avoided any live bug from that.
Add a check that collations match before deciding that two
UNION nodes are equivalent. I also failed to resist the
temptation to comment plan_union_children() a little better.
Back-patch to all supported branches (v13 now), since they
all have nondeterministic collations.
Discussion: https://postgr.es/m/
3605568.
1731970579@sss.pgh.pa.us
Fujii Masao [Tue, 19 Nov 2024 17:00:50 +0000 (02:00 +0900)]
Improve error message for database object stats manipulation functions.
Previously, database object statistics manipulation functions like
pg_set_relation_stats() reported unclear error and hint messages
when executed during recovery. These messages were "internal",
making it difficult for users to understand the issue:
ERROR: cannot acquire lock mode ShareUpdateExclusiveLock on database objects while recovery is in progress
HINT: Only RowExclusiveLock or less can be acquired on database objects during recovery.
This commit updates the error handling so that, if these functions
are called during recovery, they produce clearer messages:
ERROR: recovery is in progress
HINT: Statistics cannot be modified during recovery.
The related documentation has also been updated to explicitly
clarify that these functions are not available during recovery.
Author: Fujii Masao
Reviewed-by: Heikki Linnakangas, Maxim Orlov
Discussion: https://postgr.es/m/
6d313829-5f56-4a28-ae4b-
bd01bf1ae791@oss.nttdata.com
Michael Paquier [Tue, 19 Nov 2024 04:27:42 +0000 (13:27 +0900)]
libpq: Improve error message when parsing URI parameters and keywords
The error message showing up when parameters or keywords include too
many whitespaces was "trailing data found", which was confusing because
there was no hint about what was actually wrong.
Issue introduced in
430ce189fc45, hence there is no need for a
backpatch.
Author: Yushi Ogiwara
Reviewed-by: Fujii Masao, Tom Lane, Bruce Momjian
Discussion: https://postgr.es/m/
645bd22a53c4da8a1bc7e1e52d9d3b52@oss.nttdata.com
Bruce Momjian [Mon, 18 Nov 2024 21:30:33 +0000 (16:30 -0500)]
doc: clarify pg_dump --no-comments meaning as SQL comments
Discussion: https://postgr.es/m/ZyjdAjEsXbFPkD3t@momjian.us
Backpatch-through: master
Bruce Momjian [Mon, 18 Nov 2024 20:34:59 +0000 (15:34 -0500)]
doc: clarify text about combining row-level policies
Reported-by: splarv@ya.ru
Discussion: https://postgr.es/m/
173045909386.700.
9231055113418242392@wrigleys.postgresql.org
Backpatch-through: master
Peter Geoghegan [Mon, 18 Nov 2024 18:35:28 +0000 (13:35 -0500)]
nbtree: consistently use minoff variable.
This was arguably an oversight in commit
29b64d1de7, which moved this
code from nbtutils.c to its nbtsearch.c caller.
Michael Paquier [Mon, 18 Nov 2024 04:41:10 +0000 (13:41 +0900)]
Improve some code format in gist.c
Author: Tender Wang
Discussion: https://postgr.es/m/CAHewXNmD=K7XmsHq=L1SyyzZYvwU4oaMG9EKSSMe4OrXfykLzg@mail.gmail.com
Michael Paquier [Mon, 18 Nov 2024 02:44:11 +0000 (11:44 +0900)]
Use pg_memory_is_all_zeros() in PageIsVerifiedExtended()
Relying on pg_memory_is_all_zeros(), which would apply SIMD instructions
when dealing with an aligned page, is proving to be at least three times
faster than the original size_t-based comparisons when checking if a
BLCKSZ page is full of zeros. Note that PageIsVerifiedExtended() is
called each time a page is read from disk, and making it faster is a
good thing.
Author: Bertrand Drouvot
Discussion: https://postgr.es/m/CAApHDvq7P-JgFhgtxUPqhavG-qSDVUhyWaEX9M8_MNorFEijZA@mail.gmail.com
Michael Paquier [Mon, 18 Nov 2024 01:08:20 +0000 (10:08 +0900)]
Optimize pg_memory_is_all_zeros() in memutils.h
pg_memory_is_all_zeros() is currently implemented to do only a
byte-per-byte comparison. While being sufficient for its existing
callers for pgstats entries, it could lead to performance regressions
should it be used for larger memory areas, like 8kB blocks, or even
future commits.
This commit optimizes the implementation of this function to be more
efficient for larger sizes, written in a way so that compilers can
optimize the code. This is portable across 32b and 64b architectures.
The implementation handles three cases, depending on the size of the
input provided:
* If less than sizeof(size_t), do a simple byte-by-byte comparison.
* If between sizeof(size_t) and (sizeof(size_t) * 8 - 1):
** Phase 1: byte-by-byte comparison, until the pointer is aligned.
** Phase 2: size_t comparisons, with aligned pointers, up to the last
aligned location possible.
** Phase 3: byte-by-byte comparison, until the end location.
* If more than (sizeof(size_t) * 8) bytes, this is the same as case 2
except that an additional phase is placed between Phase 1 and Phase 2,
with 8 * sizeof(size_t) comparisons using bitwise OR, to encourage
compilers to use SIMD instructions if available.
The last improvement proves to be at least 3 times faster than the
size_t comparisons, which is something currently used for the all-zero
page check in PageIsVerifiedExtended().
The optimization tricks that would encourage the use of SIMD
instructions have been suggested by David Rowley.
Author: Bertrand Drouvot
Reviewed-by: Michael Paquier, Ranier Vilela
Discussion: https://postgr.es/m/CAApHDvq7P-JgFhgtxUPqhavG-qSDVUhyWaEX9M8_MNorFEijZA@mail.gmail.com
Noah Misch [Sat, 16 Nov 2024 04:39:56 +0000 (20:39 -0800)]
Fix per-session activation of ALTER {ROLE|DATABASE} SET role.
After commit
5a2fed911a85ed6d8a015a6bafe3a0d9a69334ae, the catalog state
resulting from these commands ceased to affect sessions. Restore the
longstanding behavior, which is like beginning the session with a SET
ROLE command. If cherry-picking the CVE-2024-10978 fixes, default to
including this, too. (This fixes an unintended side effect of fixing
CVE-2024-10978.) Back-patch to v12, like that commit. The release team
decided to include v12, despite the original intent to halt v12 commits
earlier this week.
Tom Lane and Noah Misch. Reported by Etienne LAFARGE.
Discussion: https://postgr.es/m/CADOZwSb0UsEr4_UTFXC5k7=fyyK8uKXekucd+-uuGjJsGBfxgw@mail.gmail.com
Masahiko Sawada [Sat, 16 Nov 2024 01:06:11 +0000 (17:06 -0800)]
Fix a possibility of logical replication slot's restart_lsn going backwards.
Previously LogicalIncreaseRestartDecodingForSlot() accidentally
accepted any LSN as the candidate_lsn and candidate_valid after the
restart_lsn of the replication slot was updated, so it potentially
caused the restart_lsn to move backwards.
A scenario where this could happen in logical replication is: after a
logical replication restart, based on previous candidate_lsn and
candidate_valid values in memory, the restart_lsn advances upon
receiving a subscriber acknowledgment. Then, logical decoding restarts
from an older point, setting candidate_lsn and candidate_valid based
on an old RUNNING_XACTS record. Subsequent subscriber acknowledgments
then update the restart_lsn to an LSN older than the current value.
In the reported case, after WAL files were removed by a checkpoint,
the retreated restart_lsn prevented logical replication from
restarting due to missing WAL segments.
This change essentially modifies the 'if' condition to 'else if'
condition within the function. The previous code had an asymmetry in
this regard compared to LogicalIncreaseXminForSlot(), which does
almost the same thing for different fields.
The WAL removal issue was reported by Hubert Depesz Lubaczewski.
Backpatch to all supported versions, since the bug exists since 9.4
where logical decoding was introduced.
Reviewed-by: Tomas Vondra, Ashutosh Bapat, Amit Kapila
Discussion: https://postgr.es/m/Yz2hivgyjS1RfMKs%40depesz.com
Discussion: https://postgr.es/m/
85fff40e-148b-4e86-b921-
b4b846289132%40vondra.me
Backpatch-through: 13
Tom Lane [Fri, 15 Nov 2024 23:23:38 +0000 (18:23 -0500)]
Avoid assertion due to disconnected NFA sub-graphs in regex parsing.
In commit
08c0d6ad6 which introduced "rainbow" arcs in regex NFAs,
I didn't think terribly hard about what to do when creating the color
complement of a rainbow arc. Clearly, the complement cannot match any
characters, and I took the easy way out by just not building any arcs
at all in the complement arc set. That mostly works, but Nikolay
Shaplov found a case where it doesn't: if we decide to delete that
sub-NFA later because it's inside a "{0}" quantifier, delsub()
suffered an assertion failure. That's because delsub() relies on
the target sub-NFA being fully connected. That was always true
before, and the best fix seems to be to restore that property.
Hence, invent a new arc type CANTMATCH that can be generated in
place of an empty color complement, and drop it again later when we
start NFA optimization. (At that point we don't need to do delsub()
any more, and besides there are other cases where NFA optimization can
lead to disconnected subgraphs.)
It appears that this bug has no consequences in a non-assert-enabled
build: there will be some transiently leaked NFA states/arcs, but
they'll get cleaned up eventually. Still, we don't like assertion
failures, so back-patch to v14 where rainbow arcs were introduced.
Per bug #18708 from Nikolay Shaplov.
Discussion: https://postgr.es/m/18708-
f94f2599c9d2c005@postgresql.org
Fujii Masao [Fri, 15 Nov 2024 16:59:33 +0000 (01:59 +0900)]
Remove unnecessary backslash from CopyFrom() code.
Commit
4ac2a9bece accidentally added an unnecessary backslash
to CopyFrom() code. This commit removes it.
Author: Yugo Nagata
Reviewed-by: Tender Wang
Discussion: https://postgr.es/m/
20241112114609.
4175a2e175282edd1463dbc6@sraoss.co.jp
Peter Eisentraut [Fri, 15 Nov 2024 13:52:28 +0000 (14:52 +0100)]
Fix collation handling for foreign keys
Allowing foreign keys where the referenced and the referencing columns
have collations with different notions of equality is problematic.
This can only happen when using nondeterministic collations, for
example, if the referencing column is case-insensitive and the
referenced column is not, or vice versa. It does not happen if both
collations are deterministic.
To show one example:
CREATE COLLATION case_insensitive (provider = icu, deterministic = false, locale = 'und-u-ks-level2');
CREATE TABLE pktable (x text COLLATE "C" PRIMARY KEY);
CREATE TABLE fktable (x text COLLATE case_insensitive REFERENCES pktable ON UPDATE CASCADE ON DELETE CASCADE);
INSERT INTO pktable VALUES ('A'), ('a');
INSERT INTO fktable VALUES ('A');
BEGIN; DELETE FROM pktable WHERE x = 'a'; TABLE fktable; ROLLBACK;
BEGIN; DELETE FROM pktable WHERE x = 'A'; TABLE fktable; ROLLBACK;
Both of these DELETE statements delete the one row from fktable. So
this means that one row from fktable references two rows in pktable,
which should not happen. (That's why a primary key or unique
constraint is required on pktable.)
When nondeterministic collations were implemented, the SQL standard
available to yours truly said that referential integrity checks should
be performed with the collation of the referenced column, and so
that's how we implemented it. But this turned out to be a mistake in
the SQL standard, for the same reasons as above, that was later
(SQL:2016) fixed to require both collations to be the same. So that's
what we are aiming for here.
We don't have to be quite so strict. We can allow different
collations if they are both deterministic. This is also good for
backward compatibility.
So the new rule is that the collations either have to be the same or
both deterministic. Or in other words, if one of them is
nondeterministic, then both have to be the same.
Users upgrading from before that have affected setups will need to
make changes to their schemas (i.e., change one or both collations in
affected foreign-key relationships) before the upgrade will succeed.
Some of the nice test cases for the previous situation in
collate.icu.utf8.sql are now obsolete. They are changed to just check
the error checking of the new rule. Note that collate.sql already
contained a test for foreign keys with different deterministic
collations.
A bunch of code in ri_triggers.c that added a COLLATE clause to
enforce the referenced column's collation can be removed, because both
columns now have to have the same notion of equality, so it doesn't
matter which one to use.
Reported-by: Paul Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
78d824e0-b21e-480d-a252-
e4b84bc2c24b@illuminatedcomputing.com
Álvaro Herrera [Fri, 15 Nov 2024 11:53:12 +0000 (12:53 +0100)]
Avoid deleting critical WAL segments during pg_rewind
Previously, in unlucky cases, it was possible for pg_rewind to remove
certain WAL segments from the rewound demoted primary. In particular
this happens if those files have been marked for archival (i.e., their
.ready files were created) but not yet archived; the newly promoted node
no longer has such files because of them having been recycled, but they
are likely critical for recovery in the demoted node. If pg_rewind
removes them, recovery is not possible anymore.
Fix this by maintaining a hash table of files in this situation in the
scan that looks for a checkpoint, which the decide_file_actions phase
can consult so that it knows to preserve them.
Backpatch to 14. The problem also exists in 13, but that branch was not
blessed with commit
eb00f1d4bf96, so this patch is difficult to apply
there. Users of older releases will just have to continue to be extra
careful when rewinding.
Co-authored-by: Полина Бунгина (Polina Bungina) <bungina@gmail.com>
Co-authored-by: Alexander Kukushkin <cyberdemn@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Discussion: https://postgr.es/m/CAAtGL4AhzmBRsEsaDdz7065T+k+BscNadfTqP1NcPmsqwA5HBw@mail.gmail.com
Peter Eisentraut [Fri, 15 Nov 2024 09:53:54 +0000 (10:53 +0100)]
Proper object locking for GRANT/REVOKE
Refactor objectNamesToOids() to use get_object_address() internally if
possible. Not only does this save a lot of code, it also allows us to
use the object locking provided by get_object_address() for
GRANT/REVOKE. There was previously a code comment that complained
about the lack of locking in objectNamesToOids(), which is now fixed.
The check in ExecGrant_Type_check() is obsolete because
get_object_address_type() already does the same check.
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/
bf72b82c-124d-4efa-a484-
bb928e9494e4@eisentraut.org
Heikki Linnakangas [Fri, 15 Nov 2024 08:06:36 +0000 (10:06 +0200)]
jit: Stop emitting some unnecessary instructions
In EEOP_BOOL_AND_STEP* and EEOP_BOOL_OR_STEP*, we emitted pointlesss
store instructions to store to resnull/resvalue values that were just
loaded from the same fields in the previous instructions. They will
surely get optimized away by LLVM if any optimizations are enabled,
but it's better to not emit them in the first place. In
EEOP_BOOL_NOT_STEP, similar story with resnull.
In EEOP_NULLIF, when it returns NULL, there was also a redundant store
to resvalue just after storing a 0 to it. The value of resvalue
doesn't matter when resnull is set, so in fact even storing the 0 is
unnecessary, but I kept that because we tend to do that for general
tidiness.
Author: Xing Guo <higuoxing@gmail.com>
Reviewed-by: Andreas Karlsson <andreas@proxel.se>
Discussion: https://www.postgresql.org/message-id/CACpMh%2BC%3Dg13WdvzLRSponsVWGgxwDSMzQWM4Gz0heOyaA0-N6g@mail.gmail.com
Peter Eisentraut [Fri, 15 Nov 2024 07:42:59 +0000 (08:42 +0100)]
Add an assertion in get_object_address()
Some places declared a Relation before calling get_object_address()
only to assert that the relation is NULL after the call.
The new assertion allows passing NULL as the relation argument at
those places making the code cleaner and easier to understand.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://www.postgresql.org/message-id/ZzG34eNrT83W/Orz@ip-10-97-1-34.eu-west-3.compute.internal
Michael Paquier [Fri, 15 Nov 2024 02:31:58 +0000 (11:31 +0900)]
Fix race conditions with drop of reused pgstats entries
This fixes a set of race conditions with cumulative statistics where a
shared stats entry could be dropped while it should still be valid in
the event when it is reused: an entry may refer to a different object
but requires the same hash key. This can happen with various stats
kinds, like:
- Replication slots that compute internally an index number, for
different slot names.
- Stats kinds that use an OID in the object key, where a wraparound
causes the same key to be used if an OID is used for the same object.
- As of PostgreSQL 18, custom pgstats kinds could also be an issue,
depending on their implementation.
This issue is fixed by introducing a counter called "generation" in the
shared entries via PgStatShared_HashEntry, initialized at 0 when an
entry is created and incremented when the same entry is reused, to avoid
concurrent issues on drop because of other backends still holding a
reference to it. This "generation" is copied to the local copy that a
backend holds when looking at an object, then cross-checked with the
shared entry to make sure that the entry is not dropped even if its
"refcount" justifies that if it has been reused.
This problem could show up when a backend shuts down and needs to
discard any entries it still holds, causing statistics to be removed
when they should not, or even an assertion failure. Another report
involved a failure in a standby after an OID wraparound, where the
startup process would FATAL on a "can only drop stats once", stopping
recovery abruptly. The buildfarm has been sporadically complaining
about the problem, as well, but the window is hard to reach with the
in-core tests.
Note that the issue can be reproduced easily by adding a sleep before
dshash_find() in pgstat_release_entry_ref() to enlarge the problematic
window while repeating test_decoding's isolation test oldest_xmin a
couple of times, for example, as pointed out by Alexander Lakhin.
Reported-by: Alexander Lakhin, Peter Smith
Author: Kyotaro Horiguchi, Michael Paquier
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/CAA4eK1KxuMVyAryz_Vk5yq3ejgKYcL6F45Hj9ZnMNBS-g+PuZg@mail.gmail.com
Discussion: https://postgr.es/m/17947-
b9554521ad963c9c@postgresql.org
Backpatch-through: 15
Heikki Linnakangas [Thu, 14 Nov 2024 14:12:32 +0000 (16:12 +0200)]
Pass MyPMChildSlot as an explicit argument to child process
All the other global variables passed from postmaster to child have
the same value in all the processes, while MyPMChildSlot is more like
a parameter to each child process.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
a102f15f-eac4-4ff2-af02-
f9ff209ec66f@iki.fi
Heikki Linnakangas [Thu, 14 Nov 2024 14:12:28 +0000 (16:12 +0200)]
Assign a child slot to every postmaster child process
Previously, only backends, autovacuum workers, and background workers
had an entry in the PMChildFlags array. With this commit, all
postmaster child processes, including all the aux processes, have an
entry. Dead-end backends still don't get an entry, though, and other
processes that don't touch shared memory will never mark their
PMChildFlags entry as active.
We now maintain separate freelists for different kinds of child
processes. That ensures that there are always slots available for
autovacuum and background workers. Previously, pre-authentication
backends could prevent autovacuum or background workers from starting
up, by using up all the slots.
The code to manage the slots in the postmaster process is in a new
pmchild.c source file. Because postmaster.c is just so large.
Assigning pmsignal slot numbers is now pmchild.c's responsibility.
This replaces the PMChildInUse array in pmsignal.c.
Some of the comments in postmaster.c still talked about the "stats
process", but that was removed in commit
5891c7a8ed. Fix those while
we're at it.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
a102f15f-eac4-4ff2-af02-
f9ff209ec66f@iki.fi
Heikki Linnakangas [Thu, 14 Nov 2024 14:12:04 +0000 (16:12 +0200)]
Kill dead-end children when there's nothing else left
Previously, the postmaster would never try to kill dead-end child
processes, even if there were no other processes left. A dead-end
backend will eventually exit, when authentication_timeout expires, but
if a dead-end backend is the only thing that's preventing the server
from shutting down, it seems better to kill it immediately. It's
particularly important, if there was a bug in the early startup code
that prevented a dead-end child from timing out and exiting normally.
Includes a test for that case where a dead-end backend previously
prevented the server from shutting down.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
a102f15f-eac4-4ff2-af02-
f9ff209ec66f@iki.fi
Heikki Linnakangas [Thu, 14 Nov 2024 14:06:16 +0000 (16:06 +0200)]
Replace postc's own backend type codes with BackendType
Introduce a separate BackendType for dead-end children, so that we
don't need a separate dead_end flag.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://www.postgresql.org/message-id/
a102f15f-eac4-4ff2-af02-
f9ff209ec66f@iki.fi
Peter Eisentraut [Thu, 14 Nov 2024 08:30:14 +0000 (09:30 +0100)]
Remove a useless cast to (void *) in hash_search() call
This pattern was previously cleaned up in
54a177a948b, but a new
instance snuck in around the same time in
31966b151e6.
Michael Paquier [Thu, 14 Nov 2024 04:23:11 +0000 (13:23 +0900)]
contrib/lo: Use SQL-standard function bodies
Author: Ronan Dunklau
Discussion: https://postgr.es/m/
3316564.aeNJFYEL58@aivenlaptop
Michael Paquier [Thu, 14 Nov 2024 04:10:36 +0000 (13:10 +0900)]
xml2: Add tests for functions xpath_nodeset() and xpath_list()
These two functions with their different argument lists have never been
tested in this module, so let's add something.
Author: Ronan Dunklau
Discussion: https://postgr.es/m/ZzMSJkiNZhimjXWx@paquier.xyz
Michael Paquier [Thu, 14 Nov 2024 03:24:00 +0000 (12:24 +0900)]
contrib/lo: Add test for function lo_oid()
Author: Ronan Dunklau
Discussion: https://postgr.es/m/ZzMSJkiNZhimjXWx@paquier.xyz
Peter Geoghegan [Wed, 13 Nov 2024 14:50:57 +0000 (09:50 -0500)]
Add nbtree amgettuple return item function.
This makes it easier to add precondition assertions. We now assert that
the last call to _bt_readpage succeeded, and that the current item index
is within the bounds of the currPos items array.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Discussion: https://postgr.es/m/CAH2-WznFkEs9K1PtNruti5JjawY-dwj+gkaEh_k1ZE+1xLLGkA@mail.gmail.com
Álvaro Herrera [Wed, 13 Nov 2024 10:06:44 +0000 (11:06 +0100)]
Fix pg_upgrade's cross-version tests when old < 18
Because in the 18 cycle we turned checksums on by default with commit
04bec894a04c, and pg_upgrade fails if the setting doesn't match in old
and new clusters, the built-in cross-version pg_upgrade test is failing
if the old version is older than 18. Fix the script so that it creates
the old cluster with checksums enabled (-k) in cross-version scenarios.
This went unnoticed because the buildfarm doesn't use the same test code
for cross-version testing.
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://postgr.es/m/
202411071838.7fgkb7uvavvz@alvherre.pgsql
Peter Eisentraut [Wed, 13 Nov 2024 09:29:31 +0000 (10:29 +0100)]
configure.ac: Remove useless AC_SUBST
No longer used since commit
805e431a386.
Peter Eisentraut [Wed, 13 Nov 2024 08:05:02 +0000 (09:05 +0100)]
doc: Update pg_constraint.conexclop docs for WITHOUT OVERLAPS
Fixup for commit
fc0438b4e80.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/
57ea0668-5205-426e-b934-
efc89f2186c2@illuminatedcomputing.com
Peter Eisentraut [Wed, 13 Nov 2024 07:53:08 +0000 (08:53 +0100)]
doc: Add PERIOD to ALTER TABLE reference docs
Commit
89f908a6d0a documented foreign keys with PERIOD in the CREATE
TABLE docs, but not in ALTER TABLE. This commit adds the new syntax
to the ALTER TABLE docs.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/
57ea0668-5205-426e-b934-
efc89f2186c2@illuminatedcomputing.com
Peter Eisentraut [Wed, 13 Nov 2024 07:51:23 +0000 (08:51 +0100)]
doc: Small improvement in CREATE TABLE / PERIOD documentation
Use placeholders that are more consistent and match the description
better. Fixup for commit
89f908a6d0a.
Peter Eisentraut [Wed, 13 Nov 2024 07:42:34 +0000 (08:42 +0100)]
doc: Add WITHOUT OVERLAPS to ALTER TABLE reference docs
Commit
fc0438b4e80 documented WITHOUT OVERLAPS in the CREATE TABLE
docs, but not in ALTER TABLE. This commit adds the new syntax to the
ALTER TABLE docs.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/
57ea0668-5205-426e-b934-
efc89f2186c2@illuminatedcomputing.com
Michael Paquier [Wed, 13 Nov 2024 04:58:09 +0000 (13:58 +0900)]
Fix comment in injection_point.c
InjectionPointEntry->name was described as a hash key, which was fine
when introduced in
d86d20f0ba79, but it is not now.
Oversight in
86db52a5062a, that has changed the way injection points are
stored in shared memory from a hash table to an array.
Backpatch-through: 17
Peter Geoghegan [Wed, 13 Nov 2024 03:09:00 +0000 (22:09 -0500)]
Fix obsolete nbtree page reuse FSM comment.
Oversight in commit
d088ba5a.
Peter Geoghegan [Wed, 13 Nov 2024 01:57:45 +0000 (20:57 -0500)]
Count contrib/bloom index scans in pgstat view.
Maintain the pg_stat_user_indexes.idx_scan pgstat counter during
contrib/Bloom index scans.
Oversight in commit
9ee014fc, which added the Bloom index contrib
module.
Author: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Reviewed-By: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/
c48839d881388ee401a01807c686004d@oss.nttdata.com
Backpatch: 13- (all supported branches).
Amit Langote [Tue, 12 Nov 2024 06:56:51 +0000 (15:56 +0900)]
Add missing word in comment
Discussion: https://postgr.es/m/CA+HiwqFgdp8=0_gi+DU0fPWZbg7qY3KZ_c1Wj1DEvzXC4BCnMQ@mail.gmail.com
Álvaro Herrera [Tue, 12 Nov 2024 10:35:43 +0000 (11:35 +0100)]
Silence compilers about extractNotNullColumn()
Multiple buildfarm animals warn that a newly added Assert() is
impossible to fail; remove it to avoid the noise. While at it, use
direct assignment to obtain the value we need, avoiding an unnecessary
memcpy().
(I decided to remove the "pfree" call for the detoasted short-datum;
because this is only used for DDL, it's not problematic to leak such a
small allocation.)
Noted by Tom Lane about
14e87ffa5c54.
Discussion: https://postgr.es/m/
3649828.
1731083171@sss.pgh.pa.us
Michael Paquier [Tue, 12 Nov 2024 08:28:03 +0000 (17:28 +0900)]
pg_freespacemap: Use SQL-standard function bodies
72a5b1fc8804 was the piece missing for the conversion of this module.
pg_freespace is bumped to 1.3, with its function pg_freespace(regclass)
converted to this new style.
There are other modules in the tree that need a similar treatment; these
will be handled later.
Author: Tom Lane
Reviewed-by: Ronan Dunklau
Discussion: https://postgr.es/m/
3395418.
1618352794@sss.pgh.pa.us
Alexander Korotkov [Mon, 11 Nov 2024 23:44:20 +0000 (01:44 +0200)]
Fix arrays comparison in CompareOpclassOptions()
The current code calls array_eq() and does not provide FmgrInfo. This commit
provides initialization of FmgrInfo and uses C collation as the safe option
for text comparison because we don't know anything about the semantics of
opclass options.
Backpatch to 13, where opclass options were introduced.
Reported-by: Nicolas Maus
Discussion: https://postgr.es/m/18692-
72ea398df3ec6712%40postgresql.org
Backpatch-through: 13
Tom Lane [Mon, 11 Nov 2024 22:05:53 +0000 (17:05 -0500)]
Parallel workers use AuthenticatedUserId for connection privilege checks.
Commit
5a2fed911 had an unexpected side-effect: the parallel worker
launched for the new test case would fail if it couldn't use a
superuser-reserved connection slot. The reason that test failed
while all our pre-existing ones worked is that the connection
privilege tests in InitPostgres had been based on the superuserness
of the leader's AuthenticatedUserId, but after the rearrangements
of
5a2fed911 we were testing the superuserness of CurrentUserId,
which the new test case deliberately made to be a non-superuser.
This all seems very accidental and probably not the behavior we really
want, but a security patch is no time to be redesigning things.
Pending some discussion about desirable semantics, hack it so that
InitPostgres continues to pay attention to the superuserness of
AuthenticatedUserId when starting a parallel worker.
Nathan Bossart and Tom Lane, per buildfarm member sawshark.
Security: CVE-2024-10978
Tom Lane [Mon, 11 Nov 2024 18:57:21 +0000 (13:57 -0500)]
Fix cross-version upgrade tests.
TestUpgradeXversion knows how to make the main regression database's
references to pg_regress.so be version-independent. But it doesn't
do that for plperl's database, so that the C function added by
commit
b7e3a52a8 is causing cross-version upgrade test failures.
Path of least resistance is to just drop the function at the end
of the new test.
In <= v14, also take the opportunity to clean up the generated
test files.
Security: CVE-2024-10979
Tom Lane [Mon, 11 Nov 2024 17:20:08 +0000 (12:20 -0500)]
Avoid bizarre meson behavior with backslashes in command arguments.
meson makes the backslashes in text2macro.pl's --strip argument
into forward slashes, effectively disabling comment stripping.
That hasn't caused us issues before, but it breaks the test case
for
b7e3a52a8. We don't really need the pattern to be adjustable,
so just hard-wire it into the script instead.
Context: https://github.com/mesonbuild/meson/issues/1564
Security: CVE-2024-10979
Tom Lane [Mon, 11 Nov 2024 15:29:54 +0000 (10:29 -0500)]
Fix improper interactions between session_authorization and role.
The SQL spec mandates that SET SESSION AUTHORIZATION implies
SET ROLE NONE. We tried to implement that within the lowest-level
functions that manipulate these settings, but that was a bad idea.
In particular, guc.c assumes that it doesn't matter in what order
it applies GUC variable updates, but that was not the case for these
two variables. This problem, compounded by some hackish attempts to
work around it, led to some security-grade issues:
* Rolling back a transaction that had done SET SESSION AUTHORIZATION
would revert to SET ROLE NONE, even if that had not been the previous
state, so that the effective user ID might now be different from what
it had been.
* The same for SET SESSION AUTHORIZATION in a function SET clause.
* If a parallel worker inspected current_setting('role'), it saw
"none" even when it should see something else.
Also, although the parallel worker startup code intended to cope
with the current role's pg_authid row having disappeared, its
implementation of that was incomplete so it would still fail.
Fix by fully separating the miscinit.c functions that assign
session_authorization from those that assign role. To implement the
spec's requirement, teach set_config_option itself to perform "SET
ROLE NONE" when it sets session_authorization. (This is undoubtedly
ugly, but the alternatives seem worse. In particular, there's no way
to do it within assign_session_authorization without incompatible
changes in the API for GUC assign hooks.) Also, improve
ParallelWorkerMain to directly set all the relevant user-ID variables
instead of relying on some of them to get set indirectly. That
allows us to survive not finding the pg_authid row during worker
startup.
In v16 and earlier, this includes back-patching
9987a7bf3 which
fixed a violation of GUC coding rules: SetSessionAuthorization
is not an appropriate place to be throwing errors from.
Security: CVE-2024-10978
Nathan Bossart [Mon, 11 Nov 2024 15:00:00 +0000 (09:00 -0600)]
Ensure cached plans are correctly marked as dependent on role.
If a CTE, subquery, sublink, security invoker view, or coercion
projection references a table with row-level security policies, we
neglected to mark the plan as potentially dependent on which role
is executing it. This could lead to later executions in the same
session returning or hiding rows that should have been hidden or
returned instead.
Reported-by: Wolfgang Walther
Reviewed-by: Noah Misch
Security: CVE-2024-10976
Backpatch-through: 12
Noah Misch [Mon, 11 Nov 2024 14:23:43 +0000 (06:23 -0800)]
Block environment variable mutations from trusted PL/Perl.
Many process environment variables (e.g. PATH), bypass the containment
expected of a trusted PL. Hence, trusted PLs must not offer features
that achieve setenv(). Otherwise, an attacker having USAGE privilege on
the language often can achieve arbitrary code execution, even if the
attacker lacks a database server operating system user.
To fix PL/Perl, replace trusted PL/Perl %ENV with a tied hash that just
replaces each modification attempt with a warning. Sites that reach
these warnings should evaluate the application-specific implications of
proceeding without the environment modification:
Can the application reasonably proceed without the modification?
If no, switch to plperlu or another approach.
If yes, the application should change the code to stop attempting
environment modifications. If that's too difficult, add "untie
%main::ENV" in any code executed before the warning. For example,
one might add it to the start of the affected function or even to
the plperl.on_plperl_init setting.
In passing, link to Perl's guidance about the Perl features behind the
security posture of PL/Perl.
Back-patch to v12 (all supported versions).
Andrew Dunstan and Noah Misch
Security: CVE-2024-10979
Amit Kapila [Mon, 11 Nov 2024 09:54:40 +0000 (15:24 +0530)]
Doc: Add links to clarify the max_replication_slots.
The GUC max_replication_slots has a different meaning for sending servers
and subscribers. Add cross-links in each section for easy reference.
Author: Tristan Partin
Discussion: https://postgr.es/m/D5FNEPMMFHFX.1OQBCML0TU5AH@partin.io
Michael Paquier [Mon, 11 Nov 2024 01:40:48 +0000 (10:40 +0900)]
Add two attributes to pg_stat_database for parallel workers activity
Two attributes are added to pg_stat_database:
* parallel_workers_to_launch, counting the total number of parallel
workers that were planned to be launched.
* parallel_workers_launched, counting the total number of parallel
workers actually launched.
The ratio of both fields can provide hints that there are not enough
slots available when launching parallel workers, also useful when
pg_stat_statements is not deployed on an instance (i.e.
cf54a2c00254).
This commit relies on
de3a2ea3b264, that has added two fields to EState,
that get incremented when executing Gather or GatherMerge nodes.
A test is added in select_parallel, where parallel workers are spawned.
Bump catalog version.
Author: Benoit Lobréau
Discussion: https://postgr.es/m/
783bc7f7-659a-42fa-99dd-
ee0565644e25@dalibo.com
Michael Paquier [Mon, 11 Nov 2024 01:19:52 +0000 (10:19 +0900)]
libpq: Bail out during SSL/GSS negotiation errors
This commit changes libpq so that errors reported by the backend during
the protocol negotiation for SSL and GSS are discarded by the client, as
these may include bytes that could be consumed by the client and write
arbitrary bytes to a client's terminal.
A failure with the SSL negotiation now leads to an error immediately
reported, without a retry on any other methods allowed, like a fallback
to a plaintext connection.
A failure with GSS discards the error message received, and we allow a
fallback as it may be possible that the error is caused by a connection
attempt with a pre-11 server, GSS encryption having been introduced in
v12. This was a problem only with v17 and newer versions; older
versions discard the error message already in this case, assuming a
failure caused by a lack of support for GSS encryption.
Author: Jacob Champion
Reviewed-by: Peter Eisentraut, Heikki Linnakangas, Michael Paquier
Security: CVE-2024-10977
Backpatch-through: 12
Michael Paquier [Mon, 11 Nov 2024 00:02:30 +0000 (09:02 +0900)]
pg_stat_statements: Avoid some locking during PGSS entry scans
A single PGSS entry's spinlock is used to be able to modify "counters"
without holding pgss->lock exclusively, as mentioned at the top of
pg_stat_statements.c and within pgssEntry.
Within a single pgssEntry, stats_since and minmax_stats_since are never
modified without holding pgss->lock exclusively, so there is no need to
hold an entry's spinlock when reading stats_since and
minmax_stats_since, as done when scanning all the PGSS entries for
function calls of pg_stat_statements().
This also restores the consistency between the code and the comments
about the entry's spinlock usage. This change is a performance
improvement (it can be argued that this is a logic bug), so there is no
need for a backpatch. This saves two instructions from being read while
holding an entry's spinlock.
Author: Karina Litskevich
Reviewed-by: Michael Paquier, wenhui qiu
Discussion: https://postgr.es/m/CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com
Thomas Munro [Sun, 10 Nov 2024 22:46:37 +0000 (11:46 +1300)]
jit: Remove obsolete LLVM version guard.
Commit
9044fc1d needed a version guard when back-patched, but it is
redundant in master as of commit
972c2cd2, and I accidentally left it
in there.
Nathan Bossart [Fri, 8 Nov 2024 22:11:08 +0000 (16:11 -0600)]
Fix sign-compare warnings in pg_iovec.h.
The code in question (pg_preadv() and pg_pwritev()) has been around
for a while, but commit
15c9ac3629 moved it to a header file. If
third-party code that includes this header file is built with
-Wsign-compare on a system without preadv() or pwritev(), warnings
ensue. This commit fixes said warnings by casting the result of
pg_pread()/pg_pwrite() to size_t, which should be safe because we
will have already checked for a negative value.
Author: Wolfgang Walther
Discussion: https://postgr.es/m/
16989737-1aa8-48fd-8dfe-
b7ada06509ab%40technowledgy.de
Backpatch-through: 17