The new test added by commit 68a59f9e9 disables the subscription and
manually drops the associated replication slot. However, since
disabling the subsubscription doesn't wait for a walsender to release
the replication slot and exit, pg_drop_replication_slot() could
fail. Avoid failure by adding a wait for the replication slot to
become inactive.
Reported-by: Hou Zhijie, as per buildfarm Reviewed-by: Hou Zhijie
Discussion: https://postgr.es/m/OS0PR01MB571682316378379AA34854F694E9A%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Backpatch-through: 15
Extend the PG_TEST_EXTRA documentation to mention resource intensive
tests as well. The previous wording only mentioned special software
and security in the main paragraph, with resource usage listed on one
of the tests in the list.
Backpatch to v15 where f47ed79cc8 added wal_consistenct_checking as
a PG_TEST_EXTRA target.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/CAN55FZ0OthTuBdiNkaX2BvxuHdK4Y1MVEb8_uEuD1yHMPmT9Og@mail.gmail.com
Backpatch-through: 15
Michael Paquier [Thu, 7 Sep 2023 05:12:25 +0000 (14:12 +0900)]
pg_basebackup: Generate valid temporary slot names under PQbackendPID()
pgbouncer can cause PQbackendPID() to return negative values due to it
filling be_pid with random bytes (even these days pid_max can only be
set up to 2^22 on 64b machines on Linux, for example, so this cannot
happen with normal PID numbers). When this happens, pg_basebackup may
generate a temporary slot name that may not be accepted by the parser,
leading to spurious failures, like:
pg_basebackup: error: could not send replication command
ERROR: replication slot name "pg_basebackup_-1201966863" contains
invalid character
This commit fixes that problem by formatting the result from
PQbackendPID() as an unsigned integer when creating the temporary
replication slot name, so as the invalid character is gone and the
command can be parsed.
Thomas Munro [Wed, 6 Sep 2023 23:47:42 +0000 (11:47 +1200)]
Disable 031_recovery_conflict.pl in 15 and 16.
This test fails due to known bugs in the test and the server. Those
will be fixed in master shortly and possibly back-patched a bit later,
but in the meantime it is unhelpful for package maintainers if the tests
randomly fail, and it's not a good time to make complex changes in 16.
This had already been done for older branches prior to 15's release.
Now we're about to release 16, and Debian's test builds are regularly
failing on one architecture, so let's do the same for 15 and 16.
Reported-by: Christoph Berg <myon@debian.org> Reported-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CALj2ACVr8au2J_9D88UfRCi0JdWhyQDDxAcSVav0B0irx9nXEg%40mail.gmail.com
Michael Paquier [Mon, 4 Sep 2023 05:55:49 +0000 (14:55 +0900)]
Fix out-of-bound read in gtsvector_picksplit()
This could lead to an imprecise choice when splitting an index page of a
GiST index on a tsvector, deciding which entries should remain on the
old page and which entries should move to a new page.
This is wrong since tsearch2 has been moved into core with commit 140d4ebcb46e, so backpatch all the way down. This error has been
spotted by valgrind.
Author: Alexander Lakhin
Discussion: https://postgr.es/m/17950-6c80a8d2b94ec695@postgresql.org
Backpatch-through: 11
Michael Paquier [Sun, 3 Sep 2023 23:04:40 +0000 (08:04 +0900)]
Fix handling of shared statistics with dropped databases
Dropping a database while a connection is attempted on it was able to
lead to the presence of valid database entries in shared statistics.
The issue is that MyDatabaseId was getting set too early than it should,
as, if the connection attempted on the dropped database fails when
renamed or dropped, the shutdown callback of the shared statistics would
finish by re-inserting a correct entry related to the database already
dropped.
As analyzed by the bug reporters, this issue could lead to phantom
entries in the database list maintained by the autovacuum launcher
(in rebuild_database_list()) if the database dropped was part of the
database list when it was still valid. After the database was dropped,
it would remain the highest on the list of databases to considered by
the autovacuum worker as things to process. This would prevent
autovacuum jobs to happen on all the other databases still present.
The commit fixes this issue by delaying setting MyDatabaseId until the
database existence has been re-checked with the second scan on
pg_database after getting a shared lock on it, and by switching
pgstat_update_dbstats() so as nothing happens if MyDatabaseId is not
valid.
Issue introduced by 5891c7a8ed8f, so backpatch down to 15.
Reported-by: Will Mortensen, Jacob Speidel Analyzed-by: Will Mortensen, Jacob Speidel
Author: Andres Freund
Discussion: https://postgr.es/m/17973-bca1f7d5c14f601e@postgresql.org
Backpatch-through: 15
The comment in heapgettup_advance_block() says that it reports the
scan position before checking for end of scan, but that didn't match
the code. The code was refactored in commit 7ae0ab0ad9, which
inadvertently changed the order of the check and reporting. Change it
back.
This caused a few regression test failures with a small shared_buffers
setting like 10 MB. The 'portals' and 'cluster' tests perform seqscans
that are large enough that sync seqscans kick in. When the sync scan
position is not updated at end of scan, the next seq scan doesn't
start at the beginning of the table, and the test queries are
sensitive to that.
Reviewed-by: Melanie Plageman, David Rowley
Discussion: https://www.postgresql.org/message-id/6f991389-ae22-d844-a9d8-9aceb7c01a9a@iki.fi
Backpatch-through: 16
Nathan Bossart [Wed, 30 Aug 2023 21:47:20 +0000 (14:47 -0700)]
Use actual backend IDs in pg_stat_get_backend_subxact().
Unlike the other pg_stat_get_backend* functions,
pg_stat_get_backend_subxact() looks up the backend entry by using
its integer argument as a 1-based index in an internal array. The
other functions look for the entry with the matching session
backend ID. These numbers often match, but that isn't reliably
true.
This commit resolves this discrepancy by introducing
pgstat_get_local_beentry_by_backend_id() and using it in
pg_stat_get_backend_subxact(). We cannot use
pgstat_get_beentry_by_backend_id() because it returns a
PgBackendStatus, which lacks the locally computed additions
available in LocalPgBackendStatus that are required by
pg_stat_get_backend_subxact().
Author: Ian Barwick Reviewed-by: Sami Imseih, Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/CAB8KJ%3Dj-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few%40mail.gmail.com
Backpatch-through: 16
Nathan Bossart [Wed, 30 Aug 2023 21:47:14 +0000 (14:47 -0700)]
Rename some support functions for pgstat* views.
Presently, pgstat_fetch_stat_beentry() accepts a session's backend
ID as its argument, and pgstat_fetch_stat_local_beentry() accepts a
1-based index in an internal array as its argument. The former is
typically used wherever a user must provide a backend ID, and the
latter is usually used internally when looping over all entries in
the array. This difference was first introduced by d7e39d72ca.
Before that commit, both functions accepted a 1-based index to the
internal array.
This commit renames these two functions to make it clear whether
they use the backend ID or the 1-based index to look up the entry.
This is preparatory work for a follow-up change that will introduce
a function for looking up a LocalPgBackendStatus using a backend
ID.
Reviewed-by: Ian Barwick, Sami Imseih, Michael Paquier, Robert Haas
Discussion: https://postgr.es/m/CAB8KJ%3Dj-ACb3H4L9a_b3ZG3iCYDW5aEu3WsPAzkm2S7JzS1Few%40mail.gmail.com
Backpatch-through: 16
Michael Paquier [Tue, 29 Aug 2023 23:03:48 +0000 (08:03 +0900)]
Avoid possible overflow with ltsGetFreeBlock() in logtape.c
nFreeBlocks, defined as a long, stores the number of free blocks in a
logical tape. ltsGetFreeBlock() has been using an int to store the
value of nFreeBlocks, which could lead to overflows on platforms where
long and int are not the same size (in short everything except Windows
where long is 4 bytes).
The problematic intermediate variable is switched to be a long instead
of an int.
Issue introduced by c02fdc9223015, so backpatch down to 13.
Author: Ranier vilela Reviewed-by: Peter Geoghegan, David Rowley
Discussion: https://postgr.es/m/CAEudQApLDWCBR_xmwNjGBrDo+f+S4E87x3s7-+hoaKqYdtC4JQ@mail.gmail.com
Backpatch-through: 13
Peter Eisentraut [Tue, 29 Aug 2023 13:15:54 +0000 (15:15 +0200)]
Rename logical_replication_mode to debug_logical_replication_streaming
The logical_replication_mode GUC is intended for testing and debugging
purposes, but its current name may be misleading and encourage users to make
unnecessary changes.
To avoid confusion, renaming the GUC to a less misleading name
debug_logical_replication_streaming that casual users are less likely to mistakenly
assume needs to be modified in a regular logical replication setup.
Author: Hou Zhijie <houzj.fnst@cn.fujitsu.com> Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/d672d774-c44b-6fec-f993-793e744f169a%40eisentraut.org
While there are numerous instances of using "power of 2" in the code,
translated user-facing messages use "power of two". Fix two instances
which used "power of 2" instead.
After commit b0bea38705, syslogger prints 63 warnings about failing to
close a listen socket at postmaster startup. That's because the
syslogger process forks before the ListenSockets array is initialized,
so ClosePostmasterPorts() calls "close(0)" 64 times. The first call
succeeds, because fd 0 is stdin.
This has been like this since commit 9a86f03b4e in version 13, which
moved the SysLogger_Start() call to before initializing ListenSockets.
We just didn't notice until commit b0bea38705 added the LOG message.
Reported by Michael Paquier and Jeff Janes.
Author: Michael Paquier
Discussion: https://www.postgresql.org/message-id/ZOvvuQe0rdj2slA9%40paquier.xyz
Discussion: https://www.postgresql.org/message-id/ZO0fgDwVw2SUJiZx@paquier.xyz#482670177eb4eaf4c9f03c1eed963e5f
Backpatch-through: 13
Nathan Bossart [Thu, 24 Aug 2023 17:13:31 +0000 (10:13 -0700)]
pg_upgrade: Bump MESSAGE_WIDTH.
Commit 7b378237aa added a status message to pg_upgrade that is 60
characters wide. Since the MESSAGE_WIDTH macro is currently set to
60, there is no space between this new status message and the "ok"
or "failed" indicator appended when the step completes. To fix
this problem, this commit increases the value of MESSAGE_WIDTH to
62.
Suggested-by: Bharath Rupireddy Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/CALj2ACVVvk1cYLtWVxHv%3DZ1Ubq%3DUES9fhKbUU4c9k4W%2BfEDnbw%40mail.gmail.com
Backpatch-through: 16
Tom Lane [Thu, 24 Aug 2023 16:02:40 +0000 (12:02 -0400)]
Avoid unnecessary plancache revalidation of utility statements.
Revalidation of a plancache entry (after a cache invalidation event)
requires acquiring a snapshot. Normally that is harmless, but not
if the cached statement is one that needs to run without acquiring a
snapshot. We were already aware of that for TransactionStmts,
but for some reason hadn't extrapolated to the other statements that
PlannedStmtRequiresSnapshot() knows mustn't set a snapshot. This can
lead to unexpected failures of commands such as SET TRANSACTION
ISOLATION LEVEL. We can fix it in the same way, by excluding those
command types from revalidation.
However, we can do even better than that: there is no need to
revalidate for any statement type for which parse analysis, rewrite,
and plan steps do nothing interesting, which is nearly all utility
commands. To mechanize this, invent a parser function
stmt_requires_parse_analysis() that tells whether parse analysis does
anything beyond wrapping a CMD_UTILITY Query around the raw parse
tree. If that's what it does, then rewrite and plan will just
skip the Query, so that it is not possible for the same raw parse
tree to produce a different plan tree after cache invalidation.
stmt_requires_parse_analysis() is basically equivalent to the
existing function analyze_requires_snapshot(), except that for
obscure reasons that function omits ReturnStmt and CallStmt.
It is unclear whether those were oversights or intentional.
I have not been able to demonstrate a bug from not acquiring a
snapshot while analyzing these commands, but at best it seems mighty
fragile. It seems safer to acquire a snapshot for parse analysis of
these commands too, which allows making stmt_requires_parse_analysis
and analyze_requires_snapshot equivalent.
In passing this fixes a second bug, which is that ResetPlanCache
would exclude ReturnStmts and CallStmts from revalidation.
That's surely *not* safe, since they contain parsable expressions.
Per bug #18059 from Pavel Kulakov. Back-patch to all supported
branches.
Amit Kapila [Thu, 24 Aug 2023 09:21:57 +0000 (14:51 +0530)]
Fix the error message when failing to restore the snapshot.
The SnapBuildRestoreContents() used a const value in the error message to
indicate the size in bytes it was expecting to read from the serialized
snapshot file. Fix it by reporting the size that was actually passed.
Author: Hou Zhijie Reviewed-by: Amit Kapila
Backpatch-through: 16
Discussion: http://postgr.es/m/OS0PR01MB5716D408364F7DF32221C08D941FA@OS0PR01MB5716.jpnprd01.prod.outlook.com
David Rowley [Wed, 23 Aug 2023 22:33:48 +0000 (10:33 +1200)]
Meson: check for pg_config_paths.h left over from make
The meson build scripts attempt to find files left over from configure
and fail, mentioning that "make maintainer-clean" should be run to remove
these. This seems to have been done for files generated from configure.
pg_config_paths.h is generated during the actual make build, so seems to
have been missed. This would result in compilation using the wrong
pg_config_paths.h file.
Here we just add this file to generated_sources_ac so that meson errors
out if pg_config_paths.h exists.
Likely this wasn't noticed before because make maintainer-clean will
remove pg_config_paths.h, however, people using the MSVC build scripts
are more likely to run into issues and they have to manually remove
these files and pg_config_paths.h wasn't listed as a conflicting file to
remove in the meson log.
Backpatch-through: 16, where meson support was added
Discussion: https://postgr.es/m/CAApHDvqjYOxZfmLKAOWKFEE7LOr9_E6UA6YNmx9r8nxStcS3gg@mail.gmail.com
Andres Freund [Wed, 23 Aug 2023 22:15:28 +0000 (15:15 -0700)]
ci: Make compute resources for CI configurable
See prior commit for an explanation for the goal of the change and why it had
to be split into two commits.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 15-, where CI support was added
Andres Freund [Wed, 23 Aug 2023 22:15:28 +0000 (15:15 -0700)]
ci: Prepare to make compute resources for CI configurable
cirrus-ci will soon restrict the amount of free resources every user gets (as
have many other CI providers). For most users of CI that should not be an
issue. But e.g. for cfbot it will be an issue.
To allow configuring different resources on a per-repository basis, introduce
infrastructure for overriding the task execution environment. Unfortunately
this is not entirely trivial, as yaml anchors have to be defined before their
use, and cirrus-ci only allows injecting additional contents at the end of
.cirrus.yml.
To deal with that, move the definition of the CI tasks to
.cirrus.tasks.yml. The main .cirrus.yml is loaded first, then, if defined, the
file referenced by the REPO_CI_CONFIG_GIT_URL variable, will be added,
followed by the contents of .cirrus.tasks.yml. That allows
REPO_CI_CONFIG_GIT_URL to override the yaml anchors defined in .cirrus.yml.
Unfortunately git's default merge / rebase strategy does not handle copied
files, just renamed ones. To avoid painful rebasing over this change, this
commit just renames .cirrus.yml to .cirrus.tasks.yml, without adding a new
.cirrus.yml. That's done in the followup commit, which moves the relevant
portion of .cirrus.tasks.yml to .cirrus.yml. Until that is done,
REPO_CI_CONFIG_GIT_URL does not fully work.
The subsequent commit adds documentation for how to configure custom compute
resources to src/tools/ci/README
Reviewed-by: Daniel Gustafsson <daniel@yesql.se> Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 15-, where CI support was added
Andres Freund [Wed, 23 Aug 2023 19:29:50 +0000 (12:29 -0700)]
ci: Use VMs for SanityCheck and CompilerWarnings
The main reason for this change is to reduce different ways of executing
tasks, making it easier to use custom compute resources for cfbot. A secondary
benefit is that the tasks seem slightly faster this way, apparently the
increased startup overhead is outweighed by reduced runtime overhead.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 15-, where CI support was added
Andres Freund [Wed, 23 Aug 2023 19:27:40 +0000 (12:27 -0700)]
ci: Move execution method of tasks into yaml templates
This is done in preparation for making the compute resources for CI
configurable. It also looks cleaner.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 15-, where CI support was added
Andres Freund [Wed, 23 Aug 2023 19:27:29 +0000 (12:27 -0700)]
ci: Don't specify amount of memory
The number of CPUs is the cost-determining factor. Most instance types that
run tests have more memory/core than what we specified, there's no real
benefit in wasting that.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 15-, where CI support was added
Fix _bt_allequalimage() call within critical section.
_bt_allequalimage() does complicated things, so it's not OK to call it
in a critical section. Per buildfarm failure on 'prion', which uses
-DRELCACHE_FORCE_RELEASE -DCATCACHE_FORCE_RELEASE options.
Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9@iki.fi
Backpatch-through: 16, like commit ccadf73163 that introduced this
Use the buffer cache when initializing an unlogged index.
Some of the ambuildempty functions used smgrwrite() directly, followed
by smgrimmedsync(). A few small problems with that:
Firstly, one is supposed to use smgrextend() when extending a
relation, not smgrwrite(). It doesn't make much difference in
production builds. smgrextend() updates the relation size cache, so
you miss that, but that's harmless because we never use the cached
relation size of an init fork. But if you compile with
CHECK_WRITE_VS_EXTEND, you get an assertion failure.
Secondly, the smgrwrite() calls were performed before WAL-logging, so
the page image written to disk had 0/0 as the LSN, not the LSN of the
WAL record. That's also harmless in practice, but seems sloppy.
Thirdly, it's better to use the buffer cache, because then you don't
need to smgrimmedsync() the relation to disk, which adds latency.
Bypassing the cache makes sense for bulk operations like index
creation, but not when you're just initializing an empty index.
Creation of unlogged tables is hardly performance bottleneck in any
real world applications, but nevertheless.
Backpatch to v16, but no further. These issues should be harmless in
practice, so better to not rock the boat in older branches.
Reviewed-by: Robert Haas
Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9@iki.fi
doc: Replace list of drivers and PLs with wiki link
The list of external language drivers and procedural languages was
never complete or exhaustive, and rather than attempting to manage
it the content has migrated to the wiki. This replaces the tables
altogether with links to the wiki as we regularly get requests for
adding various projects, which we reject without any clear policy
for why or how the content should be managed.
The threads linked to below are the most recent discussions about
this, the archives contain many more.
Backpatch to all supported branches since the list on the wiki
applies to all branches.
Thomas Munro [Wed, 23 Aug 2023 00:10:18 +0000 (12:10 +1200)]
ExtendBufferedWhat -> BufferManagerRelation.
Commit 31966b15 invented a way for functions dealing with relation
extension to accept a Relation in online code and an SMgrRelation in
recovery code. It seems highly likely that future bufmgr.c interfaces
will face the same problem, and need to do something similar.
Generalize the names so that each interface doesn't have to re-invent
the wheel.
Back-patch to 16. Since extension AM authors might start using the
constructor macros once 16 ships, we agreed to do the rename in 16
rather than waiting for 17.
Reviewed-by: Peter Geoghegan <pg@bowt.ie> Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKG%2B6tLD2BhpRWycEoti6LVLyQq457UL4ticP5xd8LqHySA%40mail.gmail.com
Andrew Dunstan [Tue, 22 Aug 2023 15:57:08 +0000 (11:57 -0400)]
Cache by-reference missing values in a long lived context
Attribute missing values might be needed past the lifetime of the tuple
descriptors from which they are extracted. To avoid possibly using
pointers for by-reference values which might thus be left dangling, we
cache a datumCopy'd version of the datum in the TopMemoryContext. Since
we first search for the value this only needs to be done once per
session for any such value.
Original complaint from Tom Lane, idea for mitigation by Andrew Dunstan,
tweaked by Tom Lane.
Backpatch to version 11 where missing values were introduced.
Michael Paquier [Mon, 21 Aug 2023 04:33:04 +0000 (13:33 +0900)]
Fix pg_stat_reset_single_table_counters() for shared relations
This commit fixes the function of $subject for shared relations. This
feature has been added by e042678. Unfortunately, this new behavior got
removed by 5891c7a when moving statistics to shared memory.
Andres Freund [Sat, 19 Aug 2023 19:40:45 +0000 (12:40 -0700)]
ci: macos: use cached macports install
A significant chunk of the time on the macos CI task is spent installing
packages using homebrew. The downloads of the packages are cached, but the
installation needs to happen every time. We can't cache the whole homebrew
installation, because it is too large due to pre-installed packages.
Speed this up by installing packages using macports and caching the
installation as .dmg. That's a lot faster than unpacking a tarball.
In addition, don't install llvm - it wasn't enabled when building, so it's
just a waste of time/space.
This substantially speeds up the mac CI time, both in the cold cache and in
the warm cache case (the latter from ~1m20s to ~5s).
It doesn't seem great to have diverging sources of packages for CI between
branches, so backpatch to 15 (where CI was added).
Discussion: https://postgr.es/m/20230805202539.r3umyamsnctysdc7@awork3.anarazel.de
Backpatch: 15-, where CI was added
Peter Eisentraut [Fri, 18 Aug 2023 05:41:14 +0000 (07:41 +0200)]
Remove dubious warning message from SQL/JSON functions
There was a warning that FORMAT JSON has no effect on json/jsonb
types, which is true, but it's not clear why we should issue a warning
about it. The SQL standard does not say anything about this, which
should generally govern the behavior here. So remove it.
Thomas Munro [Mon, 14 Aug 2023 22:20:11 +0000 (10:20 +1200)]
De-pessimize ConditionVariableCancelSleep().
Commit b91dd9de was concerned with a theoretical problem with our
non-atomic condition variable operations. If you stop sleeping, and
then cancel the sleep in a separate step, you might be signaled in
between, and that could be lost. That doesn't matter for callers of
ConditionVariableBroadcast(), but callers of ConditionVariableSignal()
might be upset if a signal went missing like this.
Commit bc971f4025c interacted badly with that logic, because it doesn't
use ConditionVariableSleep(), which would normally put us back in the
wait list. ConditionVariableCancelSleep() would be confused and think
we'd received an extra signal, and try to forward it to another backend,
resulting in wakeup storms.
New idea: ConditionVariableCancelSleep() can just return true if we've
been signaled. Hypothetical users of ConditionVariableSignal() would
then still have a way to deal with rare lost signals if they are
concerned about that problem.
Reported-by: Tomas Vondra <tomas.vondra@enterprisedb.com> Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/2840876b-4cfe-240f-0a7e-29ffd66711e7%40enterprisedb.com
Andres Freund [Mon, 14 Aug 2023 16:54:03 +0000 (09:54 -0700)]
hio: Take number of prior relation extensions into account
The new relation extension logic, introduced in 00d1e02be24, could lead to
slowdowns in some scenarios. E.g., when loading narrow rows into a table using
COPY, the caller of RelationGetBufferForTuple() will only request a small
number of pages. Without concurrency, we just extended using pwritev() in that
case. However, if there is *some* concurrency, we switched between extending
by a small number of pages and a larger number of pages, depending on the
number of waiters for the relation extension logic. However, some
filesystems, XFS in particular, do not perform well when switching between
extending files using fallocate() and pwritev().
To avoid that issue, remember the number of prior relation extensions in
BulkInsertState and extend more aggressively if there were prior relation
extensions. That not just avoids the aforementioned slowdown, but also leads
to noticeable performance gains in other situations, primarily due to
extending more aggressively when there is no concurrency. I should have done
it this way from the get go.
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CAD21AoDvDmUQeJtZrau1ovnT_smN940=Kp6mszNGK3bq9yRN6g@mail.gmail.com
Backpatch: 16-, where the new relation extension code was added
Andres Freund [Sat, 12 Aug 2023 20:09:45 +0000 (13:09 -0700)]
ci: macos: Remove use of -Dsegsize_blocks=6
The option causes a measurable slowdown. Macos is, by far, the most expensive
platform for CI, therefore it doesn't make sense to run such a test there.
d3b111e3205 used a small segment size for two tasks, one with autoconf, one
with meson. In hindsight that is a bit overkill, it's unlikely that the option
would silently break. Thus don't move the -Dsegsize_blocks=6, just remove
it. I did however change the autoconf test to use 6 instead of 8 blocks, as
long as we allow it, a non-power-of-two test seems like a good idea.
While at it, add a comment explaining why we use a small segment size for CI.
Author: Andres Freund <andres@anarazel.de> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 16-, where d3b111e3205 introduced the use of -Dsegsize_blocks=6
Andres Freund [Sat, 12 Aug 2023 20:06:04 +0000 (13:06 -0700)]
ci: macos: Remove use of -DRANDOMIZE_ALLOCATED_MEMORY
RANDOMIZE_ALLOCATED_MEMORY causes a measurable slowdown. Macos is, by far, the
most expensive platform for CI, therefore it doesn't make sense to run such a
test there.
Ubsan and asan on linux should detect most of the the cases of uninitialized
memory, so it doesn't really seem worth using -DRANDOMIZE_ALLOCATED_MEMORY in
another instance type.
Author: Andres Freund <andres@anarazel.de> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de
Backpatch: 16-, where 89d16b635 added the use of -DRANDOMIZE_ALLOCATED_MEMORY
Noah Misch [Sat, 12 Aug 2023 21:37:05 +0000 (14:37 -0700)]
Fix off-by-one in XLogRecordMaxSize check.
pg_logical_emit_message(false, '_', repeat('x', 1069547465)) failed with
self-contradictory message "WAL record would be 1069547520 bytes (of
maximum 1069547520 bytes)". There's no particular benefit from allowing
or denying one byte in either direction; XLogRecordMaxSize could rise a
few megabytes without trouble. Hence, this is just for cleanliness.
Back-patch to v16, where this check first appeared.
The fix itself is fine, but the test revealed other problems related
to parallel query that are not easily fixable. Remove the test for
now to fix the buildfarm.
Alvaro Herrera [Thu, 10 Aug 2023 10:04:07 +0000 (12:04 +0200)]
Document RelationGetIndexAttrBitmap better
Commit 19d8e2308bc5 changed the list of set-of-columns that can be
returned by RelationGetIndexAttrBitmap, but didn't update its
"documentation". That was pretty hard to read already, so rewrite to
make it more comprehensible, adding the missing values while at it.
Backpatch to 16, like that commit.
Discussion: https://postgr.es/m/20230809091155.7c7f3gttjk3dj4ze@alvherre.pgsql Reviewed-by: Tomas Vondra <tomas.vondra@enterprisedb.com>
Noah Misch [Mon, 7 Aug 2023 13:05:56 +0000 (06:05 -0700)]
Reject substituting extension schemas or owners matching ["$'\].
Substituting such values in extension scripts facilitated SQL injection
when @extowner@, @extschema@, or @extschema:...@ appeared inside a
quoting construct (dollar quoting, '', or ""). No bundled extension was
vulnerable. Vulnerable uses do appear in a documentation example and in
non-bundled extensions. Hence, the attack prerequisite was an
administrator having installed files of a vulnerable, trusted,
non-bundled extension. Subject to that prerequisite, this enabled an
attacker having database-level CREATE privilege to execute arbitrary
code as the bootstrap superuser. By blocking this attack in the core
server, there's no need to modify individual extensions. Back-patch to
v11 (all supported versions).
Reported by Micah Gate, Valerie Woolard, Tim Carey-Smith, and Christoph
Berg.
David Rowley [Mon, 7 Aug 2023 10:14:54 +0000 (22:14 +1200)]
Don't Memoize lateral joins with volatile join conditions
The use of Memoize was already disabled in normal joins when the join
conditions had volatile functions per the code in
match_opclause_to_indexcol(). Ordinarily, the parameterization for the
inner side of a nested loop will be an Index Scan or at least eventually
lead to an index scan (perhaps nested several joins deep). However, for
lateral joins, that's not the case and seq scans can be parameterized
too, so we can't rely on match_opclause_to_indexcol().
Here we explicitly check the parameterization for volatile functions and
don't consider the generation of a Memoize path when such functions
are present.
Author: Richard Guo
Discussion: https://postgr.es/m/CAMbWs49nHFnHbpepLsv_yF3qkpCS4BdB-v8HoJVv8_=Oat0u_w@mail.gmail.com
Backpatch-through: 14, where Memoize was introduced
Dean Rasheed [Mon, 7 Aug 2023 08:27:09 +0000 (09:27 +0100)]
Fix RLS policy usage in MERGE.
If MERGE executes an UPDATE action on a table with row-level security,
the code incorrectly applied the WITH CHECK clauses from the target
table's INSERT policies to new rows, instead of the clauses from the
table's UPDATE policies. In addition, it failed to check new rows
against the target table's SELECT policies, if SELECT permissions were
required (likely to always be the case).
In addition, if MERGE executes a DO NOTHING action for matched rows,
the code incorrectly applied the USING clauses from the target table's
DELETE policies to existing target tuples. These policies were applied
as checks that would throw an error, if they did not pass.
Fix this, so that a MERGE UPDATE action applies the same RLS policies
as a plain UPDATE query with a WHERE clause, and a DO NOTHING action
does not apply any RLS checks (other than adding clauses from SELECT
policies to the join).
Andres Freund [Tue, 1 Aug 2023 18:22:03 +0000 (11:22 -0700)]
Fix pg_stat_io buffer reuse test instability
The stats regression test attempts to ensure that Buffer Access Strategy
"reuses" are being counted in pg_stat_io by vacuuming a table which is larger
than the size of the strategy ring. However, when shared buffers are in
sufficiently high demand, another backend could evict one of the blocks in the
strategy ring before the first backend has a chance to reuse the buffer. The
backend using the strategy would then evict another shared buffer and add that
buffer to the strategy ring. This counts as an eviction and not a reuse in
pg_stat_io. Count both evictions and reuses in the test to ensure it does not
fail incorrectly.
Reported-by: Jeff Davis <pgsql@j-davis.com>,
Author: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Alexander Lakhin <exclusion@gmail.com> Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAAKRu_bNG27AxG9TdPtwsL6wg8AWbVckjmTL2t1HF=miDQuNtw@mail.gmail.com
David Rowley [Wed, 2 Aug 2023 00:06:08 +0000 (12:06 +1200)]
Fix performance regression in pg_strtointNN_safe functions
Between 6fcda9aba and 1b6f632a3, the pg_strtoint functions became quite
a bit slower in v16, despite efforts in 6b423ec67 to speed these up.
Since the majority of cases for these functions will only contain
base-10 digits, perhaps prefixed by a '-', it makes sense to have a
special case for this and just fall back on the more complex version
which processes hex, octal, binary and underscores if the fast path
version fails to parse the string.
While we're here, update the header comments for these functions to
mention that hex, octal and binary formats along with underscore
separators are now supported.
Author: Andres Freund, David Rowley Reported-by: Masahiko Sawada Reviewed-by: Dean Rasheed, John Naylor
Discussion: https://postgr.es/m/CAD21AoDvDmUQeJtZrau1ovnT_smN940%3DKp6mszNGK3bq9yRN6g%40mail.gmail.com
Backpatch-through: 16, where 6fcda9aba and 1b6f632a3 were added
Reported-by: Alexander Kozhemyakin
Bug: #18035
Discussion: https://postgr.es/m/18035-64af5cdcb5adf2a9@postgresql.org
Backpatch-through: 12, where SQL/JSON path was added.
Disallow replacing joins with scans in problematic cases.
Commit e7cb7ee14, which introduced the infrastructure for FDWs and
custom scan providers to replace joins with scans, failed to add support
handling of pseudoconstant quals assigned to replaced joins in
createplan.c, leading to an incorrect plan without a gating Result node
when postgres_fdw replaced a join with such a qual.
To fix, we could add the support by 1) modifying the ForeignPath and
CustomPath structs to store the list of RestrictInfo nodes to apply to
the join, as in JoinPaths, if they represent foreign and custom scans
replacing a join with a scan, and by 2) modifying create_scan_plan() in
createplan.c to use that list in that case, instead of the
baserestrictinfo list, to get pseudoconstant quals assigned to the join;
but #1 would cause an ABI break. So fix by modifying the infrastructure
to just disallow replacing joins with such quals.
Back-patch to all supported branches.
Reported by Nishant Sharma. Patch by me, reviewed by Nishant Sharma and
Richard Guo.
Tom Lane [Thu, 27 Jul 2023 15:56:35 +0000 (11:56 -0400)]
Eliminate fixed token-length limit in hba.c.
Historically, hba.c limited tokens in the authentication configuration
files (pg_hba.conf and pg_ident.conf) to less than 256 bytes. We have
seen a few reports of this limit causing problems; notably, for
moderately-complex LDAP configurations. Let's get rid of the fixed
limit by using a StringInfo instead of a fixed-size buffer.
This actually takes less code than before, since we can get rid of
a nontrivial error recovery stanza. It's doubtless a hair slower,
but parsing the content of the HBA files should in no way be
performance-critical.
Although this is a pretty straightforward patch, it doesn't seem
worth the risk to back-patch given the small number of complaints
to date. In released branches, we'll just raise MAX_TOKEN to
ameliorate the problem.
projects / thirdparty / postgresql.git / log
