This commit fixes an oversight introduced in c61a2f5841, where COPY TO
would attempt to do encoding conversions even if the encodings of the
client and the server matched for multi-byte encodings. All conversions
go through pg_any_to_server() that makes the conversion a no-op when the
encodings of the client and the server match, even for multi-byte
encodings.
The logic was fine, but setting CopyToStateData->need_transcoding would
cause strlen() to be called for nothing for each attribute of all the
rows copied, and that was showing high in some profiles (more attributes
make that easier to reach). This change improves the runtime of some
worst-case COPY TO queries by 15%~ (number present at least here).
This is a performance improvement, so no backpatch is done out of
caution as this is not a regression.
Reported-by: Andres Freund
Analyzed-by: Andres Freund
Author: Michael Paquier
Reviewed-by: Heikki Linnakangas
Discussion: https://postgr.es/m/20240206020504.edijzczkgd25ek6z@awork3.anarazel.de
The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid. However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.
This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.
Backpatch to all supported versions.
Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12
Commit b0f0a9432d backpatched some code from upstream DocBook XSL to
our customization layer. It turned out that this failed to work with
anything but the latest DocBook XSL upstream version (1.79.*), because
the backpatched code references an XSLT parameter (autolink.index.see)
that is not defined in earlier versions (because the feature it is
used for did not exist yet).
There is no way in XSLT to test whether a parameter is declared before
the stylesheet processor tries and fails to access it. So the
possibilities to fix this would be to either remove the code that uses
the parameter (and thus give up on the feature it is used for) or
declare the parameter in our customization layer. The latter seems
easier, and with a few more lines of code we can backport the entire
autolink.index.see feature, so let's do that. (If we didn't, then
with older stylesheets the parameter will appear as on, but it won't
actually do anything, because of the way the stylesheets are laid out,
so it's less confusing to just make it work.)
With this, the documentation build should work again with docbook-xsl
versions 1.77.*, 1.78.*, and 1.79.* (which already worked before).
Version 1.76.1 already didn't work before all this, so was not
considered here.
Reported-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/9077b779-a9f8-09c8-6e85-da1ebfba15af@eisentraut.org
Various int variables were compared to macros that are of type size_t,
which caused -Wsign-compare warnings in cpluspluscheck. Change those
to size_t, which also better describes their purpose.
Per report from Peter Eisentraut
Discussion: https://postgr.es/m/486847dc-6de5-464a-938e-bac98ec2438b%40eisentraut.org
Commit a4fd3aa719 moved setup_cancel_handler out of psql and
exporeted it as a global function. While pg_dump isn't using
the header it's exported in, having a conflicting name still
risks causing confusion when grepping the code for callsites,
so rename the static function in pg_dump to avoid this.
Author: Yugo Nagata <nagata@sraoss.co.jp>
Discussion: https://postgr.es/m/20240126094245.cf6718cc659273765f3ab69a@sraoss.co.jp
Cover scram_iterations, which was added in commit b577743000. While
at it, turn the list into a <simplelist> with 2 columns, which is much
nicer to read.
In master, remove mentions of antediluvian versions before which some
parameters were not reported.
Noticed while investigating a question by Maiquel Grassi.
Backpatch to 16.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl>
Discussion: https://postgr.es/m/202401301236.mc5ebrohhtsd@alvherre.pgsql
A comment in grouping_planner() claimed that the PlannerInfo
upper_targets array was not used in core code. However, the code that
generated the paths for the UPPERREL_PARTIAL_DISTINCT rel made that
comment untrue.
Here we adjust the create_distinct_paths() function signature to pass
down the PathTarget the same as is done for create_grouping_paths(),
thus making the aforementioned comment true again.
In passing adjust the order of the upper_targets[] assignments. These
seem to be following the reverse enum order apart from
UPPERREL_PARTIAL_DISTINCT.
Also, update the header comment for generate_gather_paths() to mention
the function is also used to create gather paths for partial distinct
paths.
Author: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com
Commit 861f86beea used REGBUF_NO_CHANGE at one of the places in the hash
index to register the clean buffers but forgot to avoid setting LSN in
that case.
Reported-by: Michael Paquier
Author: Kuroda Hayato
Reviewed-by: Amit Kapila, Michael Paquier
Discussion: https://postgr.es/m/ZbyVVG_7eW3YD5-A@paquier.xyz
Following are a few clean-ups related to failover option support in slots:
1. Improve the documentation in create_subscription.sgml.
2. Remove the spurious blank line in subscriptioncmds.c.
3. Remove the NOTICE for alter_replication_slot in subscriptioncmds.c as
we would sometimes print it even when nothing has changed. One can find
the change by enabling log_replication_commands on the publisher.
4. Optimize ReplicationSlotAlter() function to prevent disk flushing when
the slot's data remains unchanged.
Author: Hou Zhijie
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
Discussion: https://postgr.es/m/OS0PR01MB57164904651FB588A518E98894472@OS0PR01MB5716.jpnprd01.prod.outlook.com
This reverts commit 2889fd23be, following a discussion with Andres
Freund as this callback, being called once per attribute when sending a
relation's row, can involve a lot of indirect function calls (more
attributes to deal with means more impact). The effects of a dispatch
at this level would become more visible when improving the per-row code
execution of COPY TO, impacting future potential performance
improvements.
Discussion: https://postgr.es/m/20240206014125.qofww7ew3dx3v3uk@awork3.anarazel.de
The new concurrency model proposed for slru.c to improve performance
does not include any single lock that would coordinate processes
doing concurrent reads/writes on SlruShared->latest_page_number.
We can instead use atomic reads and writes for that variable.
Author: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Andrey M. Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAFiTN-vzDvNz=ExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A@mail.gmail.com
The standalone functions fasthash{32,64} use length for two purposes:
how many bytes to hash, and how to perturb the internal seed.
Developers using the incremental interface may not know the length
ahead of time (e.g. for C strings). In this case, it's advised to
pass length to the finalizer, but initialization still needed some
length up front, in the form of a placeholder macro.
Separate the concerns by having the standalone functions perturb the
internal seed themselves from their own length parameter, allowing
to remove "len" from fasthash_init(), as well as the placeholder macro.
Discussion: https://postgr.es/m/CANWCAZbTUk2LOyhsFo33gjLyLAHZ7ucXCi5K9u%3D%2BPtnTShDKtw%40mail.gmail.com
The pg_stat_io and pg_stat_copy_progress view docs spelled "I/O" as "IO"
or even "io" in some places when not referring to literal names or
string values.
Author: Dagfinn Ilmari Mannsåker
Discussion: https://postgr.es/m/87fry6lx5s.fsf@wibble.ilmari.org
When assertions are disabled, the built SQL statement is invalid and
you get a "syntax error". So this isn't a serious problem, but let's
avoid the assertion failure.
Backpatch to all supported versions.
Reviewed-by: Noah Misch
The internal commands in REFRESH MATERIALIZED VIEW CONCURRENTLY are
correctly executed in SECURITY_RESTRICTED_OPERATION mode, except for
creating the temporary "diff" table, because you cannot create
temporary tables in SRO mode. But creating the temporary "diff" table
is a pretty complex CTAS command that selects from another temporary
table created earlier in the command. If you can cajole that CTAS
command to execute code defined by the table owner, the table owner
can run code with the privileges of the user running the REFRESH
command.
The proof-of-concept reported to the security team relied on CREATE
RULE to convert the internally-built temp table to a view. That's not
possible since commit b23cd185fd, and I was not able to find a
different way to turn the SELECT on the temp table into code
execution, so as far as I know this is only exploitable in v15 and
below. That's a fiddly assumption though, so apply this patch to
master and all stable versions.
Thanks to Pedro Gallegos for the report.
Security: CVE-2023-5869
Reviewed-by: Noah Misch
This patch provides support for regular (non-replication) connections in
libpqrcv_connect(). This can be used to execute SQL statements on the
primary server without starting a walsender.
A new API libpqrcv_get_dbname_from_conninfo() is also added to extract the
database name from the given connection-info.
Note that this patch doesn't change any existing functionality but later
patches implementing the slot synchronization will use this functionality
to connect to the primary server to fetch required slot information.
Author: Shveta Malik, Hou Zhijie, Ajin Cherian
Reviewed-by: Peter Smith, Bertrand Drouvot, Dilip Kumar, Masahiko Sawada, Nisha Moond, Kuroda Hayato, Amit Kapila
Discussion: https://postgr.es/m/514f6f2f-6833-4539-39f1-96cd1e011f23@enterprisedb.com
These routines are used by the text and CSV formats to send an output
representation of a string, applying quotes if required. This is
similar to 95fb5b4902, reducing the number of "if" branches that need
to be checked on a per-row basis when sending representation of fields
in text or CSV mode.
While on it, this simplifies the signature of CopyAttributeOutCSV() as
it is possible to know that an attribute is alone on a line thanks to
CopyToState. Headers should not use quotes, even if forced at query
level.
Extracted from a larger patch by the same author.
Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com
CopyReadAttributes{CSV,Text}() are used to parse lines for text and CSV
format. This reduces the number of "if" branches that need to be
checked when parsing fields in CSV and text mode when dealing with a
COPY FROM, something that can become more noticeable with more
attributes and more lines to process.
Extracted from a larger patch by the same author.
Author: Sutou Kouhei
Discussion: https://postgr.es/m/20231204.153548.2126325458835528809.kou@clear-code.com
MINIMUM_VERSION_FOR_WAL_SUMMARIES is used to check if the directory
pg_wal/summaries/ should be created in a base backup, but its condition
was reversed: pg_wal/summaries/ would be created when taking base
backups from backends of v16 and older versions, but it should be
created in base backups taken from backends of v17 and newer versions.
Author: Artur Zakirov
Reviewed-by: Yugo Nagata, Nazir Bilal Yavuz
Discussion: https://postgr.es/m/CAKNkYnzkkQ0gb_ZmLTY0r2-qV1q6imXgcCWxdA6UoA6yJkujGg@mail.gmail.com
A ResourceOwnerEnlarge() call was missing. That led to an error:
ERROR: ResourceOwnerRemember called but array was full
and an assertion failure, if you tried to extend a temp relation again
after a failure. Alexander's test case used running out of disk space
to trigger the original failure.
This bug was introduced in the large ResourceOwner rewrite commit
b8bff07daa. Before that, the UnpinLocalBuffer() call guaranteed that
the subsequent PinLocalBuffer() will succeed, but after the rewrite,
releasing an old resource doesn't guarantee that there is space for a
new one.
Add a comment explaining why the UnpinBuffer + PinBuffer calls in
BufferAlloc(), with no ResourceOwnerEnlarge() in between, are safe.
Reported-by: Alexander Lakhin
Discussion: https://www.postgresql.org/message-id/dc574fea-c83e-a600-08cd-10881762e4fa@gmail.com
Here we adjust the partial path generation for parallel DISTINCT queries
to add Sort nodes on top of any unsorted partial distinct paths.
This increases the likelihood of the planner pushing a Sort below a Gather
Merge which enables the final phase of the parallel distinct to be
implemented using a Unique node in more cases.
Sorting the partial distinct paths is particularly useful when the
DISTINCT query has an ORDER BY and LIMIT clause as this can allow cheaper
plans by having the workers Hash Aggregate then Sort before feeding the
results into the Gather Merge. The non-parallel portion of the plan then
becomes very cheap as it leaves only Unique and Limit to do in the leader
process.
Author: Richard Guo
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAMbWs48u9VoVOouJsys1qOaC9WVGVmBa+wT1dx8KvxF5GPzezA@mail.gmail.com
An OS crash could leave PG_VERSION empty or missing. The same symptom
appeared in a backup by block device snapshot, taken after the next
checkpoint and before the OS flushes the PG_VERSION blocks. Device
snapshots are not a documented backup method, however. Back-patch to
v15, where commit 9c08aea6a3 introduced
STRATEGY=WAL_LOG and made it the default.
Discussion: https://postgr.es/m/20240130195003.0a.nmisch@google.com
Restoring a base backup taken in the middle of CreateDirAndVersionFile()
or write_relmap_file() would lose the function's effects. The symptom
was absence of the database directory, PG_VERSION file, or
pg_filenode.map. If missing the directory, recovery would fail. Either
missing file would not fail recovery but would render the new database
unusable. Fix CreateDirAndVersionFile() with the transam/README "action
first and then write a WAL entry" strategy. That has a side benefit of
moving filesystem mutations out of a critical section, reducing the ways
to PANIC. Fix the write_relmap_file() call with a lock acquisition, so
it interacts with checkpoints like non-CREATE DATABASE calls do.
Back-patch to v15, where commit 9c08aea6a3
introduced STRATEGY=WAL_LOG and made it the default.
Discussion: https://postgr.es/m/20240130195003.0a.nmisch@google.com
DST law changes in Ittoqqortoormiit, Greenland (America/Scoresbysund),
Kazakhstan (Asia/Almaty and Asia/Qostanay) and Palestine; as well as
updates for the Antarctic stations Casey and Vostok.
Historical corrections for Vietnam, Toronto, and Miquelon.
Further fallout from commit 6ee26c6a4b. To keep code in sync and avoid
issues on older releases with different package names, simply use the
unqualified name like many other places in our code.
The latest buildfarm failures show that after the insert, we don't
actually wait long enough for WAL summarization to catch up, apparently
because the on disk state gets updated before the in-memory state, and
so by checking the on disk state to see whether we're caught up and then
the in-memory state to see where exactly how far we've progressed, we
can, if unlucky, derive an older value of summarized_lsn, messing up
the rest of the test.
Attempt to fix this by using pg_available_wal_summaries() everywhere in
the test and pg_get_wal_summarizer_state() nowhere.
Per buildfarm.
Role option management was changed in Postgres 16. This patch improves
the docs around these changes, including CREATE ROLE's INHERIT option,
inheritance handling, and grant's ability to change role options.
Discussion: https://postgr.es/m/Zab9GiV63EENDcWG@momjian.us
Co-authored-by: David G. Johnston
Backpatch-through: 16
The Parameters subsection had an extra TRIGGER in the grammar
for DISABLE/ENABLE which is incorrect. Backpatch down to all
supported versions since it's been like this all along.
Discussion: https://postgr.es/m/0AFB171E-7E78-4A90-A140-46AB270212CA@yesql.se
Backpatch-through: v12
The openssl command for displaying the DN of a client certificate was
using --subject and not the single-dash option -subject. While recent
versions of openssl handles double dash options, earlier does not so
fix by using just -subject (which is per the openssl documentation).
Backpatch to v14 where this was introduced.
Reported-by: konkove@gmail.com
Discussion: https://postgr.es/m/170672168899.666.10442618407194498217@wrigleys.postgresql.org
Backpatch-through: v14
This impacts the statistics retrieved in transactions for the following
views when updating the value of stats_fetch_consistency, leading to
behaviors contrary to what is documented since 605994651b as an update
of this parameter should discard all statistics snapshot data:
- pg_stat_archiver
- pg_stat_bgwriter
- pg_stat_checkpointer
- pg_stat_io
- pg_stat_slru
- pg_stat_wal
For example, updating stats_fetch_consistency from "snapshot" to "cache"
in a transaction did not re-fetch any fresh data, using data cached from
the time when "snapshot" was in use.
Author: Shinya Kato
Discussion: https://postgr.es/m/d77fc5190d4dbe1738d77231488e768b@oss.nttdata.com
Backpatch-through: 15
When building libpq there is a check to ensure that we're not
linking against code that calls exit(). This check is using a
heuristic grep with exclusions for known false positives. The
Threadsanitizer library instrumentation for function exits is
named such that it triggers the check, so add an exclusion.
This fix is only applied to the Makefile since the meson build
files don't yet have this check. Adding the check to meson is
outside the scope of this patch though.
Reported-by: Roman Lozko <lozko.roma@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAEhC_BmNGKgj2wKArH2EAU11BsaHYgLnrRFJGRm5Vs8WJzyiQA@mail.gmail.com
Michael Paquier
Peter Eisentraut
Alexander Korotkov
Daniel Gustafsson
Masahiko Sawada
John Naylor
Nathan Bossart
Alvaro Herrera
David Rowley
Amit Kapila
Heikki Linnakangas
Tom Lane
Noah Misch
Andrew Dunstan
Robert Haas
Bruce Momjian