Commit 85c17f6 mistakenly declared a variable storing catalog_xmin as
XLogRecPtr, even though catalog_xmin is a TransactionId.
This caused no functional issue, but the type was clearly incorrect.
Therefore, this commit fixes it to use the correct type TransactionId
instead, and backpatch to v17 where the issue was introduced.
Author: Imran Zaheer <imran.zhir@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CA+UBfa=mNeLt-4BFjEP4tqdDsnq+oMqqPr7fd9Wji2_9YXmQdA@mail.gmail.com
There previously were a number of issues:
- We'd upload the cache even if we already had a high hit rate. That means we
churn through the available cache space very quickly.
For this we now check if the cache hit ratio is already high, and skip
uploading a new cache in that case.
- We'd generate per-branch caches, even if master's already would suffice,
because the branch doesn't change much
This is solved indirectly by the above.
- The cache key allowed prefix matches based on the branch,
e.g. master-pending would always use master's branch
Replace the cache key element separator of - with :, which is not a valid
part of a branch name.
- When rebasing a feature branch, we'd start with just that branch's cache,
rather than also having the newer cache of master available
This is solved by downloading by master's and the feature branch's cache,
simply overlaying both. That's possible because ccache is content addressed.
- The size of a cache would increase to the max, even though there likely will
be no benefit from old cache entries.
Address this by explicitly evicting old data and also recompressing the
cache before uploading it.
In my testing this utilizes the available cache space (10GB for personal
accounts) much more effectively than before.
The not entirely trivial determination of whether it's worth uploading a cache
entry is moved to a python script. I first had it as shell, but that gets
awkward. This way it'd also be more viable to use ccache for msvc at some
point.
The per-job redundancies are a bit annoying. There's a way around that, by
using composite actions, but I think that might be harder to understand,
without all that much of an improvement.
Reviewed-by: Nazir Bilal Yavuz <byavuz81@gmail.com>
Discussion: https://postgr.es/m/7eugqon2ilnaq6yimtq7prtl5wlia43mhpmwlydzlw4u4wonaz@hh2fagz5bjuu
Strings built by this function are not supposed to be subject to
NLS translation, but commit 6566133c5 missed that memo, so that
object identities like "membership of role %s in role %s" were
translated.
Previously, outlen was miscalculated if case_sensitive was false and
str_tolower() changed the byte length of the string. If outlen was too
large, pnstrdup() would stop at the NUL terminator, preventing
overrun. But if outlen was too small, it would cause truncation.
Fix by just removing outlen. It was only used in a single site, which
could just as well use pstrdup().
Discussion: https://postgre.es/m/1101e1a3afbbabb503317069c40374b82e6f4cac.camel@j-davis.com
Reviewed-by: Tristan Partin <tristan@partin.io>
Backpatch-through: 14
This reverts commit a0b6ef29a5, along with
its follow-up 2e123e3c2b ("Silence compiler
warning from older compilers"), which only adjusted code introduced by
the former.
The change failed with an empty table and an invalid default, and the
best way to deal with that will involve an addition to the TAM API, so
it's not ready for relese 19 now.
Discussion: https://postgr.es/m/7033D663-DDB4-4B35-922C-F33DE53B1502@gmail.com
Commit 0c8e082fba changed the time at which MyBackendType is assigned,
breaking a careful choreography in syslogger to decide when to write
messages to its own log files. Fix by flipping a boolean at the
(approximate) location where previously MyBackendType was set, instead
of depending on MyBackendType directly.
Author: Álvaro Herrera <alvherre@kurilemu.de>
Reported-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Euler Taveira <euler@eulerto.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/ahP-JT4ZRPyobnLb@paquier.xyz
ExecForPortionOfLeftovers() assumed that any result relation with
ri_RootResultRelInfo should reinsert temporal leftovers through the
root relation. That is correct for partitioned tables, where tuple
routing is needed, but it is wrong for plain inheritance.
When UPDATE/DELETE FOR PORTION OF is run on an inheritance parent and
a child row is split, the leftover rows must be inserted back into the
child relation. Reinserting through the parent can lose child-only
columns and place the leftover rows in the wrong relation.
Fix this by distinguishing partitioned-table routing from plain
inheritance. For partitioned tables, keep using the root leftover
slot and insert through the root relation. For plain inheritance
children, use a leftover slot matching the child relation and insert
directly into the child. Also keep translating the application-time
column attno for child relations, so multiple-inheritance cases with
different attribute numbers are handled correctly.
Added an ExecInitForPortionOf function to set up the ForPortionOfState
for each child table, which keeps most of these decisions localized
instead of spread out through ExecForPortionOfLeftovers. Incidentally
clarified a comment about the rangetype stored in ForPortionOfState.
Add regression tests for UPDATE and DELETE FOR PORTION OF on
inheritance children, including a multiple-inheritance case where the
range column has a different attnum in the parent and child.
Author: jian he <jian.universality@gmail.com>
Co-authored-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/4245F94D-84F1-4E05-BF81-C458A6CF9901%40gmail.com
The operators for array_eq, record_eq, range_eq, and multirange_eq
are all marked oprcanhash, but there's a pitfall: their hash functions
can fail at runtime if the contained type(s) are not hashable.
Therefore, the planner has to check hashability of the contained types
before deciding it can use hashing in these cases. Not every place
had gotten this memo, and noplace at all had considered the issue
for ranges or multiranges. In particular we could attempt to use
hashing for a ScalarArrayOpExpr on a container type when it won't
actually work, leading to "could not identify a hash function ..."
runtime failures.
For the most part we should fix this in the lookup functions provided
by lsyscache.c, to wit get_op_hash_functions and op_hashjoinable.
But there's a problem: get_op_hash_functions is not passed the input
data type it would need to check. We mustn't change the API of that
exported function in a back-patched fix, and even if we wanted to,
its call sites in the executor mostly don't have easy access to the
required data type OID. Fortunately, the executor call sites don't
actually need fixing, because it's expected that the planner verified
hashability before building a plan that requires it. Therefore,
leave get_op_hash_functions as-is and invent a wrapper function
get_op_hash_functions_ext that does the additional checking needed
in the planner's uses.
We also need to fix hash_ok_operator (extending the fix in 647889667).
While at it, neaten up a couple of places in lookup_type_cache where
relevant code for multirange cases was written differently from the
code for other container types.
Note: while this touches pg_operator.dat, it's only to add oid_symbol
macros. So there's no on-disk data change and no need for a
catversion bump.
Reported-by: Andrei Lepikhov <lepihov@gmail.com>
Author: Andrei Lepikhov <lepihov@gmail.com>
Co-authored-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/ed221f95-f09b-4a9c-b05b-e1fed621ec87@gmail.com
Backpatch-through: 14
Previously, ecpg accepted multiple descriptor header items in GET DESCRIPTOR
and SET DESCRIPTOR, but generated broken C code when they were used.
Although the grammar allowed this syntax, the implementation did not actually
support it.
This commit tightens the ecpg grammar so the header form of GET/SET DESCRIPTOR
accepts only a single header item, matching the implementation and preventing
generation of broken C code.
Also update the documentation synopsis accordingly.
Backpatch to all supported versions.
Author: Masashi Kamura <kamura.masashi@fujitsu.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: Lakshmi G <lakshmigcdac@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/OS9PR01MB13174AD7D1829D0644B6BB90E9447A@OS9PR01MB13174.jpnprd01.prod.outlook.com
Backpatch-through: 14
pg_createsubscriber rejected duplicate --publication values while parsing
command-line options, even when the duplicate names referred to
publications in different databases. Since publication names are
database-local objects, the same name is perfectly valid across multiple
databases.
This restriction was not a practical problem before commit 85ddcc2f4c,
which added support for reusing pre-existing publications. After that
change, users who have identically-named publications in multiple
databases (a common convention) could not use the feature without renaming
their publications.
The analogous restriction on --subscription names is intentionally kept as
they are reused as replication slot names, which are cluster-global, so
allowing duplicate subscription names without additional guards could
cause a slot-name collision. That work is left for a future release.
Author: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Shlok Kyal <shlok.kyal.oss@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/B08A7C89-B3DE-4C1D-A671-32AD8BAB7E22@gmail.com
When called from a parallel worker, this function calls initStringInfo()
and pq_beginmessage(), causing a StringInfo allocation to happen twice.
pq_endmessage() frees only the second allocation, with each call leaking
~1 kB into the per-worker memory context. This could cause a few
hundred megabytes worth of memory to pile up until the worker exits (the
message allocations happen in the parallel worker context), with the
situation being worse the longer a parallel worker runs.
Oversight in f1889729dd.
Author: Baji Shaik <baji.pgdev@gmail.com>
Reviewed-by: Sami Imseih <samimseih@gmail.com>
Reviewed-by: Tristan Partin <tristan@partin.io>
Discussion: https://postgr.es/m/CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com
Backpatch-through: 17
When a table's columns are narrower than the record header line, the
expanded aligned format produced misaligned output because the data
column width was not adjusted to match the record header width, leading
to output like:
+-[ RECORD 1 ]-+
| a | 10 |
| b | 20 |
+---+----+
This commit adjusts the output so as the column width match with the
header line, giving:
+-[ RECORD 1 ]-+
| a | 10 |
| b | 20 |
+---+----------+
Author: Pavel Stehule <pavel.stehule@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAFj8pRCzGpsr9zTHbtTd4mGh2YPJqOEgLgt8JLiopuYA9_1xGw@mail.gmail.com
Backpatch-through: 14
In AlterSubscription(), when the SET clause includes both
retain_dead_tuples and origin options, the origin branch was using
assignment (=) rather than bitwise-or assignment (|=) when setting
check_pub_rdt. This meant that if retain_dead_tuples had already set the
flag to true in the same command, the origin branch would silently
overwrite it. As a result, the publisher-side retain_dead_tuples check
could be incorrectly skipped.
Fix by changing the assignment to |= so that the flag accumulates across
both option branches within the same ALTER SUBSCRIPTION command.
Author: SATYANARAYANA NARLAPURAM <satyanarlapuram@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDfe7WPOhVGKzv83ZB+BmXM88r=KPQn1sa_ZXMMChcNo=A@mail.gmail.com
TupleDescFinalize() failed to take into account virtual generated
columns, which are always stored as NULL in tuples. TupleDescFinalize()
didn't check for this, and that could result in attcacheoff being set for
and beyond virtual generated columns. Also, the TupleDesc's
firstNonGuaranteedAttr could also be set incorrectly, which could result
in the tuple deformation function deforming without checking for NULLs,
and deforming using incorrectly cached offsets.
This could result in tuples being deformed incorrectly, which could
result in incorrect results, ERRORs or possibly a crash.
This has been broken since c456e39113.
Author: Chao Li <li.evan.chao@gmail.com>
Reported-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: ChangAo Chen <cca5507@qq.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/A4BC563C-0CA3-4EF3-952A-EA41F9E5BF1E%40gmail.com
The message sent to pg_stat_activity when waiting for transactions to
end was long enough that it could get truncated when the xid included
in the string grew large. Fix by rewording to avoid truncation, using
a similar message in the logical replication code as inspiration.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: ChangAo Chen <cca5507@qq.com>
Discussion: https://postgr.es/m/tencent_47CA9CE2F91FF4D81D875E162AD69AA4900A@qq.com
When rewriting a GRAPH_TABLE into a subquery,
replace_property_refs_mutator() bumps levelsup of lateral references
by one so that they reference outside the subquery. This works for
path patterns that result in only one path query. Patterns that
produce multiple path queries are rewritten as a UNION of path
queries. Since every path query becomes a subquery of the UNION
statement, the levelsup of lateral references in those path queries
need an additional bump. This adjustment was missing resulting in the
lateral references being interpreted as references in the UNION query
itself. This caused different symptoms like a crash in
create_lateral_join_info() or error "plan should not reference
subplan's variable". The symptoms varied depending on the number of
RangeTblEntries in the UNION statement.
This commit adds the missing adjustment.
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Author: Satya Narlapuram <satyanarlapuram@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAHg%2BQDfnLzsgjaQ_CiKSpP4JH3MKOiwoawEcCzXa9uYr45yiWw%40mail.gmail.com
getObjectTypeDescription() and getObjectIdentityParts() do not handle
objects in pg_propgraph_element_label and pg_propgraph_label_property
catalogs. These functions when called for handling DDL that affects
these objects cause an "unsupported object class" error. An error is
reported when these functions are called via pg_identify_object() and
pg_identify_object_as_address() with objects from the said catalogs.
The objects in these catalogs do not have a (user-given) name but they
can be manipulated individually through ALTER PROPERTY GRAPH
sub-commands. Hence they need to be accessible to the event triggers.
Handle these catalogs in the respective functions.
Reported-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/aej1DkLwhyZWmtxJ@bdtpg
The subwalrcvtimeout column was added by commit fb80f38, but the
column-level privileges on pg_subscription were not updated. As a
result, non-superusers cannot read the column, unlike the other
publicly readable pg_subscription columns.
This commit grants SELECT privilege on subwalrcvtimeout to PUBLIC.
Bump catalog version.
Author: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CABdArM4uA=6nA0BunJwudiEoY1BcWUS_oj_2pkEq_d-YdiBJhw@mail.gmail.com
The NFC recomposition incorrectly included TBASE as a valid T syllable,
which is incorrect based on the Unicode specification (TBASE is one
below the start of the range, range beginning at U+11A8).
This would cause the TBASE to be silently swallowed in the
normalization, leading to an incorrect result.
A couple of regression tests are added to check more patterns with
Hangul recomposition and decomposition, on top of a test to check the
problem with TBASE. Diego has submitted the code fix, and I have
written the tests.
Author: Diego Frias <mail@dzfrias.dev>
Co-authored-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/B92ED640-7D4A-4505-B09F-3548F58CBB16@dzfrias.dev
Backpatch-through: 14
Cirrus CI, which the project used for CI until now, has shut down on June 1,
2026. Replace it with GitHub Actions. GitHub Actions was selected because it
has unlimited runner time for public repositories.
The GitHub Actions based CI currently covers:
- SanityCheck
- Linux - Autoconf
- Linux - Meson, (32-bit and 64-bit)
- macOS - Meson
- Windows (Visual Studio + Meson and MinGW + Meson)
- CompilerWarnings
BSD coverage is left for later, as it requires more work.
Note that, for performance reasons, use of address sanitizer was moved to the
Linux - Meson (64-bit) task.
While Actions workflows in new forks are disabled by default, existing forks
that pull new changes into the repository will automatically start running
CI. That may not be desired. There however is no way native to Actions to
prevent this.
To avoid that, each repository that wants real CI to run needs to explicitly
opt into doing so, by creating the 'PG_CI_ENABLED' repository variable with
the value 1.
To make that less confusing, emit a summary whenever we skip running CI, with
a message explaining how to enable CI.
The remaining cirrus-ci support will be removed in a subsequent commit, to
make review easier.
Back-branches will be updated later, after being sure that workflow runs
correctly on master.
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Author: Jelte Fennema-Nio <postgres@jeltef.nl>
Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Discussion: https://postgr.es/m/3ydjipcr7kbss57nvi67noplncqhesl5eyb6wgol4ccjxynspv%40yatlykpribmm
This commit addresses two related issues:
tsvector_filter() assumed it could print an incorrect weight value
with %c. This could result in an invalidly-encoded error message
if the database encoding is multibyte and the char value has its
high bit set. Weight values that are ASCII control characters
could render illegibly too. Fix by printing such values in octal
(\ooo), similarly to how charout() would render them.
tsvector_setweight() and tsvector_setweight_by_filter() reported
the same unrecognized-weight error condition with elog(), as though
it were an internal error. That'd not translate, would produce an
unwanted XX000 SQLSTATE code, and also reported the bad value as a
decimal integer which seems unhelpful. Fix by refactoring so that
all three functions share one copy of the code that interprets a
weight argument.
The invalid-encoding aspect seems to me (tgl) to justify
back-patching.
Author: Ewan Young <kdbase.hack@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAON2xHNaeLAUzRCXL5AmXLcXaSE_gWAVjWQRmLzc_oZ=1_Vf4Q@mail.gmail.com
Backpatch-through: 14
Like 8f1791c61, this fixes a case of implicitly casting away
const by not treating the result of strrchr() on a const pointer
as const. This was missed at the time because the machines
reporting those warnings weren't building with --with-llvm.
While here, clean up another infelicity: in the probably-
impossible case that the input string contains only one dot,
this function would call pnstrdup() with a length of -1
and thereby emit a module name equal to the function name.
It seems to me we should emit modname = NULL instead.
Also remove a useless Assert and two redundant assignments.
Back-patch, as 8f1791c61 was, so that users of back branches
don't see this warning when building with late-model gcc.
Reported-by: hubert depesz lubaczewski <depesz@depesz.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/aiGNJ89PBqvq2Yyz@depesz.com
Backpatch-through: 14
Various code assumed this was true already, and usually it is.
However, it emerges that in a "universal" (multi-architecture)
macOS build, configure will define USE_AVX2_WITH_RUNTIME_CHECK
if the build host is x86_64, and then the arm64 half of the
build fails.
Ideally we'd get pg_config.h to define this symbol conditionally
depending on defined(__x86_64__), but I don't see any way to
persuade Autoconf to do that. Instead, clean up the mess by
#undef'ing it again in c.h for not-x86_64 builds.
For consistency I made c.h also #undef the USE_AVX512... symbols.
Those are not actively broken, but it seems only happenstance
that configure's tests for them fail in a universal build.
Down the road we may have occasion to add more #undef's here.
This problem is new in v19, so no need for back-patch.
Reported-by: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>
Reported-by: Tobias Bussmann <t.bussmann@gmx.net>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/15574903-87C9-478A-B2D7-CC8F4C275DBB@gmx.net
It seems like the SQL standard does require this after all, and it
makes sense because these columns get changed. (This is not to be
confused with *not* requiring INSERT permission to add the temporal
leftovers.)
Adding the column to RTEPermissionInfo->updatedCols also fixes a
couple outstanding bugs from other (non-permission) features using
that bitmapset to detect changes: GENERATED columns and UPDATE OF
triggers. This patch includes test cases to exercise those scenarios,
including on partitioned tables.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CAHg%2BQDcd%3Dt69gLf9yQexO07EJ2mx0Z70NFHo6h94X1EDA%3DhM0g%40mail.gmail.com
When we insert temporal leftovers after an UPDATE FOR PORTION OF, we
must make a new copy of the tuple before each insert. Otherwise, if
an insert trigger assigns to attributes of NEW, the second leftover
sees those changes.
Author: Sergei Patiakin <sergei.patiakin@enterprisedb.com>
Reviewed-by: Paul A Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CANE55rCqcse_pwXBMWhbj3_7XROb8Dks6%3DOLFmKy3bO3zDsCsg%40mail.gmail.com
When "servicefile" is specified in a connection string but the requested
service is not found in this file, libpq falls back to a default file,
pg_service.conf in PGSYSCONFDIR. The "servicefile" option was missing
an update in this case, causing an incorrect value to be reported,
impacting SERVICEFILE in psql, for example.
An extra update of the option is added in this commit, based on the
actual file where the service name has been found. A test is added to
cover this problem.
Oversight in 092f3c63ef.
Author: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/59DD1652-4EB5-449B-8DE3-959F06561A96@gmail.com
getIndexes() declared indAttNames and nindAttNames in the outer
per-table loop, so the names collected for an index on expressions
were carried over to the next plain index in the same table.
This is an internal inconsistency rather than a user-facing bug.
dumpRelationStats_dumper() only walks indexes that have pg_statistic
rows, and ANALYZE only creates those for indexes with expressions,
so the second index in the affected pair is not visited and the stale
array is never consulted.
Fix by moving the two variables into the inner per-index loop so each
iteration starts with a clean slate.
Author: Maksim Melnikov <m.melnikov@postgrespro.ru>
Reviewed-by: Alexander Korotkov <aekorotkov@gmail.com>
Discussion: https://postgr.es/m/be5fc489-587e-421f-bbb8-adb43cfd50f4@postgrespro.ru
Backpatch-through: 17
When releasing an ephemeral replication slot, ReplicationSlotRelease()
drops the slot via ReplicationSlotDropAcquired().
However, after dropping the slot, ReplicationSlotRelease() continued
to use its local "slot" pointer, which still referenced the dropped
slot's former shared-memory entry. It could then update fields such as
effective_xmin in that entry.
Once an ephemeral slot has been dropped (via ReplicationSlotDropAcquired()),
its slot array entry can be reused immediately by another backend
creating a new slot. As a result, those updates could corrupt
the state of an unrelated replication slot.
Fix by skipping those shared-memory updates for phemeral slots and
performing them only for non-ephemeral slots, whose shared-memory
entries remain valid after release.
Backpatch to all supported versions.
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Masao Fujii <masao.fujii@gmail.com>
Reviewed-by: Srinath Reddy Sadipiralla <srinath2133@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/TY4PR01MB177184FF9EE916F577E1F554194082@TY4PR01MB17718.jpnprd01.prod.outlook.com
Backpatch-through: 14
As with corr() and regr_r2(), improve regr_intercept()'s final
function to cope with overflow/underflow in the final calculation.
Here, instead of using sqrt(), we use frexp() and ldexp() to recover,
if an overflow or underflow is detected, so that the multiplication
and division steps operate on normalised mantissas, and cannot
overflow or underflow.
As with 6498287696, and the previous commit improving regr_r2(), this
is arguably a bug fix, but given the lack of prior complaints, refrain
from back-patching.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Chengpeng Yan <chengpeng_yan@outlook.com>
Discussion: https://postgr.es/m/33E01656-BB3B-46E9-A41F-24A01A7C35F4@outlook.com
Commit 6498287696 improved corr()'s final function to cope with
overflow/underflow in the final calculation, and clamped its result to
[-1, 1] in case of roundoff error. Improve regr_r2() in a similar way,
clamping its result to [0, 1].
Arguably this is a bug fix, but given the lack of prior complaints,
refrain from back-patching, as we did with 6498287696.
Reported-by: Chengpeng Yan <chengpeng_yan@outlook.com>
Author: Chengpeng Yan <chengpeng_yan@outlook.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/33E01656-BB3B-46E9-A41F-24A01A7C35F4@outlook.com
Commit 6429e5b77 changed vacuumdb to process partitioned tables when
--analyze-only or --analyze-in-stages is specified, matching the
documented behavior of analyzing regular tables, partitioned tables,
and materialized views.
Later, commit c4067383cb accidentally changed this behavior so that
partitioned tables were processed for --analyze-only, but skipped for
--analyze-in-stages.
As a result, vacuumdb --analyze-in-stages no longer matched its
documentation. Partitioned tables were skipped, leaving parent-table
statistics outdated even though users running vacuumdb
--analyze-in-stages after pg_upgrade, restore, or bulk loading would
expect statistics to be rebuilt for the database.
Fix this issue by making --analyze-in-stages process partitioned
tables again.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/EDFF0AFB-050F-4FBF-8D4F-B44DC454D957@gmail.com
The code failed to initialize the second isnull argument passed to
FunctionCallInvoke(). This is harmless for existing in-core extended
hash support functions, since FunctionCallInvoke() does not use the
value (note that all the in-core extended hash functions are strict),
examining only the argument values. However, extension-provided
extended hash functions could be affected if they inspect
PG_ARGISNULL(1).
Oversight in 01e658fa74.
Author: Man Zeng <zengman@halodbtech.com>
Discussion: https://postgr.es/m/tencent_7818173C01E01836109848C3@qq.com
Backpatch-through: 14
Eager aggregation pushes a partial aggregate down to a base or join
relation, to be finalized after that relation is joined with the rest
of the query. eager_aggregation_possible_for_relation() already
refuses to do this for a relation on the nullable side of an outer
join, but it failed to also refuse it for a relation on the inner side
of a semijoin or antijoin.
Such a join does not emit its inner rows, so a partial aggregate
computed on the inner side does not survive the join and cannot be
combined by the final aggregation. This can happen only for an
aggregate that references no table column, such as count(*): it is
considered computable on any relation, including the inner one,
whereas an aggregate that references a column is anchored to the outer
side and never reaches the inner relation.
The existing outer-join check did not catch this because it consults
nulling_relids, which only tracks joins that null-extend their inner
side. Semijoins and antijoins formed from EXISTS, IN, NOT EXISTS, or
NOT IN sublinks do not null-extend and carry no ojrelid, so they are
invisible to that check.
Fix by additionally rejecting any relation that includes inner-side
relations of a semijoin or antijoin but not the join's outer side.
Pushing a partial aggregate to the outer side of such a join, grouped
by the join key, remains valid and is still allowed.
Reported-by: Radim Marek <radim@boringsql.com>
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/CAJgoLk+d_P5sKrx-SZt01Acm_j0QnWn6aKJzFJ=waRu_3C8AoQ@mail.gmail.com
The check for window functions (point 4) guarded on the wrong bit: it
tested UNSAFE_NOTIN_DISTINCTON_CLAUSE while setting
UNSAFE_NOTIN_PARTITIONBY_CLAUSE. Each check in this loop guards on
the same bit it is about to set, as an idempotency optimization, since
unsafeFlags[] is accumulated across the arms of a set operation and
there is no point recomputing a column's status once its bit is
present.
This is not a live bug. When UNSAFE_NOTIN_PARTITIONBY_CLAUSE is
already set but UNSAFE_NOTIN_DISTINCTON_CLAUSE is not, the guard fails
to skip targetIsInAllPartitionLists() and recomputes it, but setting
the same bit again changes nothing. When
UNSAFE_NOTIN_DISTINCTON_CLAUSE is already set, point 4 is skipped and
UNSAFE_NOTIN_PARTITIONBY_CLAUSE is left unset; but such a column is
already unsafe for pushdown via UNSAFE_NOTIN_DISTINCTON_CLAUSE, so the
outcome is unchanged.
To fix, test UNSAFE_NOTIN_PARTITIONBY_CLAUSE, matching the bit being
set and the pattern of the surrounding checks.
Back-patch to v15, where the buggy check was introduced.
Author: Richard Guo <guofenglinux@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAMbWs49Q_xnF_P2QSUyDzJ34MnrO7dh-cUAaK2HJPgSgh88NcA@mail.gmail.com
Backpatch-through: 15
When an error is raised while processing a Sync message in a pipeline,
like a deferred constraint violation, the error was not associated with
the piped command and was not counted in available_results. This caused
assertion failures in discardAbortedPipelineResults(), keeping an
incorrect state at pipeline exit, because the code assumed that the
number of available and requested results would always be positive,
expecting all the counters to be 0 at the end of a pipeline.
This commit switches discardAbortedPipelineResults() and
ExecQueryAndProcessResults() to take a softer approach when consuming
and draining the results after an error. If there are still piped syncs
in the pipeline when it ends, we now attempt to consume them before
leaving the pipeline mode.
Alexander has been able to reach two assertion failures through his
testing. While investigating more this issue, I have bumped into two
more. Most of these cases are covered by the regression tests added in
this commit, plus some cases with mixes of pipelines, deferred errors
and results fetched. Some of the tests discussed (like the backend
termination one) could not be included in this commit but have been
tested manually. Another test scenario discussed involved the injection
of an error state in the backend, that was able to trick libpq
internally and put its queue out of sync. This scenario is not going to
happen in practice, but if we were to do something about it we would
need to make libpq understand that it needs to fail in some cases but
not block.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Author: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/19494-97a86d84fee71c47@postgresql.org
Backpatch-through: 18
The documentation says that when extension_control_path is set to an
empty string, the default '$system' path is still assumed. However,
get_extension_control_directories() added the system extension directory
with a NULL macro in that case. As a result, pg_available_extensions
could expose the expanded system directory path instead of reporting
'$system' as the location.
Record the implicitly-added system directory with the '$system' macro, so
pg_available_extensions reports the documented symbolic location and does
not leak the actual system path.
Update the extension_control_path TAP test to check the reported location
directly.
Author: Chao Li <lic@highgo.com>
Reviewed-by: Lu Feng <fnlo1995@gmail.com>
Reviewed-by: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/357C774A-ECE9-4455-B641-315205D4D9A1@gmail.com
CopyToJsonOneRow() sent the output of composite_to_json() directly
via CopySendData() without encoding conversion. The text and CSV
paths convert per-attribute via pg_server_to_any() when
need_transcoding is true, but the JSON path skipped this entirely.
This meant COPY ... TO ... WITH (FORMAT json, ENCODING 'LATIN1') on
a UTF-8 server silently produced UTF-8 output, and COPY TO STDOUT
with a non-UTF-8 client_encoding would send unconverted bytes to
the client.
Apply pg_server_to_any() to the whole JSON buffer after
composite_to_json() returns, converting to the requested file
encoding when it differs from the server encoding. Tests cover
both the explicit ENCODING option and the implicit case where
file_encoding is inherited from client_encoding.
Introduced by 7dadd38cda (json format for COPY TO).
Author: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Discussion: https://postgr.es/m/CAJTYsWX-jsLzxGRAb-dWnEpGYRPbDYHwce8LctVE92LiDfM2Jw@mail.gmail.com
During REPACK CONCURRENTLY, logical decoding keeps replication
slot.restart_lsn pinned behind the oldest running transaction, which is
often the long-lived REPACK transaction itself. As a result, old WAL
segments are retained longer than necessary.
This commit advances the replication slot each time WAL insertion
crosses a segment boundary, so obsolete WAL files can be recycled while
REPACK is still running.
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Antonin Houska <ah@cybertec.at>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/TY4PR01MB17718B44164522D0798F8E898940A2@TY4PR01MB17718.jpnprd01.prod.outlook.com
OpenSSL 4.0.0 changed some parameters and returnvalues to const, so
we need to update our declarations and subsequently cast away const-
ness from a few callsites to make libpq build without warnings. This
is tested with OpenSSL 1.1.1 through 4.0.0 as well as with LibreSSL.
No functional change is introduced, this commit only allows postgres
to be compiled against OpenSSL 4.0.0 without warnings.
There is also an errormessage change in OpenSSL 4.0.0 which needed
to be covered by our testharness.
This will be backpatched to all supported branches since they are
all equally likely to be built against OpenSSL 4.0.0 as it becomes
available in distributions. Backpatching will be done once it has
been in master for a few days without issues.
Author: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/066B07BB-85FA-487C-BE8C-40F791CFC3C4@yesql.se
Backpatch-through: 14
The launcher and worker for enabling/disabling checksums were named
"datachecksum worker|launcher" but using the plural form makes more
sense given the underlying GUC name data_checksums.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/20260528.121622.1662808269492494574.horikyota.ntt@gmail.com
The datachecksums launcher was using int for storing a PID, but
the correct type is pid_t (which is defined to be a signed int).
Author: Tomas Vondra <tomas@vondra.me>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://postgr.es/m/f1281cf3-89a3-4936-9bc5-2a5a6291229f@vondra.me
When a standby is promoted to primary during checksum enabling when the
state is inprogress-on, the standby shall revert the state to off since
checksums weren't fully enabled at the time of the crash. Consider the
following scenario:
1) primary/standby cluster has checksums off
2) primary starts enabling checksums
3) primary moves to inprogress-on
4) standby receives that and moves to inprogress-on too
5) primary crashes
6) standby gets promoted, and does the StartupXLOG thing
7) standby moves from inprogress-on back to off
Any processes in the standby need to be informed at step 6 to change
state with a procsignalbarrier, else they will stay in inprogress-on
while new backends will see the state as off. StartupXLOG failed to
emit a procsignalbarrier which caused inconsistent state in the node
promoted to primary.
Fixed by emitting a procsignalbarrier during promotion, and adding a
new test for this scenario.
Author: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Tomas Vondra <tomas@vondra.me>
Discussion: https://postgr.es/m/f1281cf3-89a3-4936-9bc5-2a5a6291229f@vondra.me
The existing logic computed an updated replicationSlotMinLSN from all
slots' restart_lsn only when catalog_xmin also advanced. This is not a
problem in normal (non-repack) cases, because catalog_xmin changes
pretty frequently, so the recomputation is triggered frequently enough.
However, REPACK does not currently change its catalog snapshot, so that
doesn't work very well if no other replication slot is being used.
(After this commit, we still don't recycle WAL properly for REPACK,
because its background worker is not advancing its restart_lsn either;
that will be fixed in a separate commit. However, this preexisting
problem in older code is logically separate from that one.)
Author: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/TY4PR01MB17718B44164522D0798F8E898940A2@TY4PR01MB17718.jpnprd01.prod.outlook.com
