The leak would show up when using batch inserts with foreign tables
included in a partition tree, as the slots used in the batch were not
reset once processed. In order to fix this problem, some
ExecClearTuple() are added to clean up the slots used once a batch is
filled and processed, mapping with the number of slots currently in use
as tracked by the counter ri_NumSlots.
This buffer refcount leak has been introduced in b676ac4 with the
addition of the executor facility to improve bulk inserts for FDWs, so
backpatch down to 14.
Alexander has provided the patch (slightly modified by me). The test
for postgres_fdw comes from me, based on the test case that the author
has sent in the report.
Author: Alexander Pyhalov
Discussion: https://postgr.es/m/b035780a740efd38dc30790c76927255@postgrespro.ru
Backpatch-through: 14
vacuum_defer_cleanup_age was introduced before hot_standby_feedback and
replication slots existed. It is hard to use reasonably - commonly it will
either be set too low (not preventing recovery conflicts, while still causing
some bloat), or too high (causing a lot of bloat). The alternatives do not
have that issue.
That on its own might not be sufficient reason to remove
vacuum_defer_cleanup_age, but it also complicates computation of xid
horizons. See e.g. the bug fixed in be504a3e97. It also is untested.
This commit removes TransactionIdRetreatSafely(), as there are no users
anymore. There might be potential future users, hence noting that here.
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/20230317230930.nhsgk3qfk7f4axls@awork3.anarazel.de
Commit 04fe805a1 modified plpgsql so that datatype casts make use of
expressions cached by plancache.c, in place of older code where these
expression trees were managed by plpgsql itself. However, I (tgl)
forgot that we use a separate, shorter-lived cast info hashtable in
DO blocks. The new mechanism thus resulted in session-lifespan
leakage of the plancache data once a DO block containing one or more
casts terminated. To fix, split the cast hash table into two parts,
one that tracks only the plancache's CachedExpressions and one that
tracks the expression state trees generated from them. DO blocks need
their own expression state trees and hence their own version of the
second hash table, but there's no reason they can't share the
CachedExpressions with regular plpgsql functions.
Per report from Ajit Awekar. Back-patch to v12 where the issue
was introduced.
Ajit Awekar and Tom Lane
Discussion: https://postgr.es/m/CAHv6PyrNaqdvyWUspzd3txYQguFTBSnhx+m6tS06TnM+KWc_LQ@mail.gmail.com
The name of this function suggests that it ought to reparent R/W
expanded objects to be children of the persistent aggcontext, instead
of copying them. In fact it does no such thing, and if you try to
make it do so you will see multiple regression failures. Rename it
to the less-misleading ExecAggCopyTransValue, and add commentary
about why that attractive-sounding optimization won't work. Also
adjust comments at call sites, some of which were describing logic
that has since been moved into ExecAggCopyTransValue.
Discussion: https://postgr.es/m/3004282.1681930251@sss.pgh.pa.us
Commit 6df7a9698b accidentally included two identical prototypes for
default_multirange_selectivi() and commit 086cf1458c added a break;
statement where one was already present, thus duplicating it. While
there is no bug caused by this, fix by removing the duplicated lines
as they provide no value.
Backpatch the fix for duplicate prototypes to v14 and the duplicate
break statement fix to all supported branches to avoid backpatching
hazards due to the removal.
Reported-by: Anton Voloshin <a.voloshin@postgrespro.ru>
Discussion: https://postgr.es/m/0e69cb60-0176-f6d0-7e15-6478b7d85724@postgrespro.ru
We need to call them only when validate == true.
Backpatch to 13, where opclass options were introduced.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/2656633.1681831542%40sss.pgh.pa.us
Reviewed-by: Tom Lane, Pavel Borisov
Backpatch-through: 13
For regex escape sequences, just test directly for the relevant ASCII
characters rather than using locale-sensitive character
classification.
This fixes an assertion failure when a locale considers a non-ASCII
character, such as "൧", to be a digit.
Reported-by: Richard Guo
Discussion: https://postgr.es/m/CAMbWs49Q6UoKGeT8pBkMtJGJd+16CBFZaaWUk9Du+2ERE5g_YA@mail.gmail.com
Backpatch-through: 11
The errormessage for an incorrect require_auth method wasn't using the
common "invalid %s value" errormessage which lessens the burden on our
translators. Fix by changing to that format to make use of existing
translations and to make error messages consistent in wording.
Reported and fixed by Gurjeet Singh with some tweaking by myself.
Author: Gurjeet Singh <gurjeet@singh.im>
Discussion: https://postgr.es/m/CABwTF4Xu3g9zohJ9obu8m7MKbf8g63NgpRDjwqPHQgAtB+Gb8Q@mail.gmail.com
a9c70b46 added the statistics view pg_stat_io which contained columns
"io_context" and "io_object". Given that the columns are in the
pg_stat_io view, the "io" prefix is somewhat redundant, so remove it.
The code variables referring to these fields are kept unchanged so as
they can keep their context about I/O.
Bump catalog version.
Author: Melanie Plageman
Reviewed-by: Kyotaro Horiguchi, Fabrízio de Royes Mello
Discussion: https://postgr.es/m/CAAKRu_aAQoJWrvT2BYYQvJChFKra_O-5ra3jhzKJZqWsTR1CPQ@mail.gmail.com
We've long used "--strip-unneeded" for shared libraries but plain
"-x" for static libraries when stripping symbols with GNU strip.
There doesn't seem to be any really good reason for that though,
since --strip-unneeded produces smaller output (as "-x" alone
does not remove debug symbols). Moreover it seems that
llvm-strip, although it identifies as GNU strip, misbehaves when
given "-x" for this purpose. It's unclear whether that's
intentional or a bug in llvm-strip, but in any case it seems like
changing to use --strip-unneeded in all cases should be a win.
Note that this doesn't change our behavior when dealing with
non-GNU strip.
Per gripes from Ed Maste and Palle Girgensohn. Back-patch,
in case anyone wants to use llvm-strip with stable branches.
Discussion: https://postgr.es/m/17898-5308d09543463266@postgresql.org
Discussion: https://postgr.es/m/20230420153338.bbj2g5jiyy3afhjz@awork3.anarazel.de
Commit 7d71d3dd0 introduced finer grained updates of autovacuum option
changes by increasing the frequency of reading the configuration file.
The debug logging of cost parameter was however changed such that some
initial values weren't logged. Fix by changing logging to use the old
frequency of logging regardless of them changing.
Also avoid taking a log for rendering the log message unless the set
loglevel is such that the log entry will be emitted.
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoBS7o6Ljt_vfqPQPf67AhzKu3fR0iqk8B=vVYczMugKMQ@mail.gmail.com
list_copy_head() given an empty List would crash from trying to
dereference the List to obtain its length. Since NIL is how we represent
an empty List, we should just be returning another empty List in this
case.
list_copy_head() is new to v16, so let's fix it now before too many people
start coding around the buggy NIL behavior.
Reported-by: Miroslav Bendik
Discussion: https://postgr.es/m/CAPoEpV02WhawuWnmnKet6BqU63bEu7oec0pJc=nKMtPsHMzTXQ@mail.gmail.com
The "lev" name that appeared in NEWROOT nbtree record desc output was
inconsistent with the symbol name from the underlying C struct. It was
also inconsistent with nbtdesc output for other nearby record types with
similar level fields.
Standardize on "level" to make everything consistent.
Follow-up to commit 1c453cfd.
Recent enhancements to rmgr desc routines that made the output summarize
certain block data (added by commits 7d8219a4 and 1c453cfd) dealt with
records that lack relevant block data (and so have nothing to give a
more detailed summary of) by testing !DecodedBkpBlock.has_image. As a
result, more detailed descriptions of block data were not output when
wal_consistency_checking was enabled.
This bug affected records with summarizable block data that also
happened to have an FPI that the REDO routine isn't supposed to apply
(FPIs used for consistency checking purposes only). The presence of
such an FPI was incorrectly taken to indicate the absence of block data.
To fix, test DecodedBkpBlock.has_data, not !DecodedBkpBlock.has_image.
This is the exact condition that we care about, not an inexact proxy.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/CAH2-Wzm5Sc9cBg1qWV_cEBfLNJCrW9FjS-SoHVt8FLA7Ldn8yg@mail.gmail.com
The error message for a missing or invalid system CA when using
sslrootcert=system differs based on the OpenSSL version used.
In OpenSSL 1.0.1-3.0 it is reported as SSL Error, with varying
degrees of helpfulness in the error message. With OpenSSL 3.1 it
is reported as an SSL SYSCALL error with "Undefined error" as
the error message. This fix pulls out the particular error in
OpenSSL 3.1 as a certificate verify error in order to help the
user better figure out what happened, and to keep the ssl test
working. While there is no evidence that extracing the errors
will clobber errno, this adds a guard against that regardless
to also make the consistent with how we handle OpenSSL errors
elsewhere. It also memorizes the output from OpenSSL 3.0 in
the test in cases where the system CA isn't responding.
Reported-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/c39be3c5-c1a5-1e33-1024-16f527e251a4@enterprisedb.com
DST law changes in Egypt, Greenland, Morocco, and Palestine.
When observing Moscow time, Europe/Kirov and Europe/Volgograd now
use the abbreviations MSK/MSD instead of numeric abbreviations,
for consistency with other timezones observing Moscow time.
Also, America/Yellowknife is no longer distinct from America/Edmonton;
this affects some pre-1948 timestamps in that area.
_bt_dedup_pass()'s heapRel argument hasn't been needed or used since
commit cf2acaf4dc made deleting any existing LP_DEAD index tuples the
caller's responsibility.
Utils.pm has a BEGIN block that editorializes on the locale-related
environment variables, primarily in order to stabilize the behavior
of child programs. It turns out that if the calling test script
has already done "use locale", this fails to affect the behavior
of Perl itself, causing locale behavior to be different between
Perl and child programs. That breaks commit cd82e5c79's attempt
to deal with locale-specific behavior in psql.
To fix, we just need to call setlocale() to redo the calculation
of locale.
Per report from Aleksander Alekseev. No back-patch for now, since
there are no locale-dependent TAP tests in prior branches, and
I'm not yet convinced that this won't have side-effects of its own.
Discussion: https://postgr.es/m/CAJ7c6TO9KpYYxoVVseWEQB5KtjWDkt8NfyAeKPcHoe2Jq+ykpw@mail.gmail.com
Commit c6f2f01611 purported to make
this work, but problems remained. In a plain-format backup, the
files from an in-place tablespace got included in the tar file for
the main tablespace, which is wrong but it's not clear that it
has any user-visible consequences. In a tar-format backup, the
TABLESPACE_MAP option is used, and so we never iterated over
pg_tblspc and thus never backed up the in-place tablespaces
anywhere at all.
To fix this, reverse the changes in that commit, so that when we scan
pg_tblspc during a backup, we create tablespaceinfo objects even for
in-place tablespaces. We set the field that would normally contain the
absolute pathname to the relative path pg_tblspc/${TSOID}, and that's
good enough to make basebackup.c happy without any further changes.
However, pg_basebackup needs a couple of adjustments to make it work.
First, it needs to understand that a relative path for a tablespace
means it's an in-place tablespace. Second, it needs to tolerate the
situation where restoring the main tablespace tries to create
pg_tblspc or a subdirectory and finds that it already exists, because
we restore user-defined tablespaces before the main tablespace.
Since in-place tablespaces are only intended for use in development
and testing, no back-patch.
Patch by me, reviewed by Thomas Munro and Michael Paquier.
Discussion: http://postgr.es/m/CA+TgmobwvbEp+fLq2PykMYzizcvuNv0a7gPMJtxOTMOuuRLMHg@mail.gmail.com
When compiled with -C ORACLE, ecpg_get_data() had a one-off issue where
it would incorrectly store the null terminator byte to str[-1] when
varcharsize is 0, which is something that can happen when using SQLDA.
This would eat 1 byte from the previous field stored, corrupting the
results generated.
All the callers of ecpg_get_data() estimate and allocate enough storage
for the data received, and the fix of this commit relies on this
assumption. Note that this maps to the case where no padding or
truncation is required.
This issue has been introduced by 3b7ab43 with the Oracle compatibility
option, so backpatch down to v11.
Author: Kyotaro Horiguchi
Discussion: https://postgr.es/m/20230410.173500.440060475837236886.horikyota.ntt@gmail.com
Backpatch-through: 11
This fixes many spelling mistakes in comments, but a few references to
invalid parameter names, function names and option names too in comments
and also some in string constants
Also, fix an #undef that was undefining the incorrect definition
Author: Alexander Lakhin
Reviewed-by: Justin Pryzby
Discussion: https://postgr.es/m/d5f68d19-c0fc-91a9-118d-7c6a5a3f5fad@gmail.com
If the last few pages in the specified range are empty (all zero),
then log_newpage_range() could try to emit an empty WAL record
containing no FPIs. This at least upsets an Assert in
ReserveXLogInsertLocation, and might perhaps have bad real-world
consequences in non-assert builds.
This has been broken since log_newpage_range() was introduced,
but the case was hard if not impossible to hit before commit 3d6a98457
decided it was okay to leave VM and FSM pages intentionally zero.
Nonetheless, it seems prudent to back-patch. log_newpage_range()
was added in v12 but later back-patched, so this affects all
supported branches.
Matthias van de Meent, per report from Justin Pryzby
Discussion: https://postgr.es/m/ZD1daibg4RF50IOj@telsasoft.com
The nbtree VACUUM WAL record stores its page offset number payload in
blk 0 (just like the closely related nbtree DELETE WAL record). Commit
ebd551f5 fixed a similar issue with the DELETE WAL record, but missed
this one.
We have two existing conventions for long options: either alphabetical
among short options, or all long options after all the short options.
But the convention apparently used here, next to a functionally
related option, is not one of them.
The finalfunc might return a read-write expanded object. If we
de-duplicate multiple call sites for the aggregate, any function(s)
receiving the aggregate result earlier could alter or destroy the
value that reaches the ones called later. This is a brown-paper-bag
bug in commit 42b746d4c, because we actually considered the need
for read-only-ness but failed to realize that it applied to the case
with a finalfunc as well as the case without.
Per report from Justin Pryzby. New error in HEAD,
no need for back-patch.
Discussion: https://postgr.es/m/ZDm5TuKsh3tzoEjz@telsasoft.com
This addresses various deficiencies in the documentation for VACUUM and
ANALYZE's BUFFER_USEAGE_LIMIT docs.
Here we declare "size" in the syntax synopsis for VACUUM and ANALYZE's
BUFFER_USAGE_LIMIT option and then define exactly what values can be
specified for it in the section for that below.
Also, fix the incorrect ordering of vacuumdb options both in the documents
and in vacuumdb's --help output. These should be in alphabetical order.
In passing also add the minimum/maximum range for the BUFFER_USAGE_LIMIT
option. These will also serve as example values that can be modified and
used.
Reported-by: Peter Eisentraut
Discussion: https://postgr.es/m/16845cb1-b228-e157-f293-5892bced9253@enterprisedb.com
Commit 3e310d837 taught isAssignmentIndirectionExpr() to look through
CoerceToDomain nodes. That's not sufficient, because since commit
04fe805a1 it's been possible for the planner to simplify
CoerceToDomain to RelabelType when the domain has no constraints
to enforce. So we need to look through RelabelType too.
Per bug #17897 from Alexander Lakhin. Although 3e310d837 was
back-patched to v11, it seems sufficient to apply this change
to v12 and later, since 04fe805a1 came in in v12.
Dmitry Dolgov
Discussion: https://postgr.es/m/17897-4216c546c3874044@postgresql.org
For some reason I had not implemented RBM_ZERO_AND_CLEANUP_LOCK support in
ExtendBufferedRelTo(), likely thinking it not being reachable. But it is
reachable, e.g. when replaying a WAL record for a page in a relation that
subsequently is truncated (likely only reachable when doing crash recovery or
PITR, not during ongoing streaming replication).
As now all of the RBM_* modes are supported, remove assertions checking mode.
As we had no test coverage for this scenario, add a new TAP test. There's
plenty more that ought to be tested in this area...
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Reviewed-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/392271.1681238924%40sss.pgh.pa.us
Discussion: https://postgr.es/m/0b5eb82b-cb99-e0a4-b932-3dc60e2e3926@gmail.com
