| Age | Commit message (Collapse) | Author |
|---|
|
The target failed, tested $PATH binaries, or tested a stale temporary
installation. Commit c66b438db62748000700c9b90b585e756dd54141 missed
this. Back-patch to v10 (all supported versions).
|
|
Suppress further attempts to read ahead in the WAL if we run out of
data, until the records already decoded have been replayed. This
restores the traditional behavior for continuous archive recovery, which
is to retry the failing restore_command only every 5 seconds. With the
coding in 5dc0418f, we would start retrying every time through the
recovery loop when our WAL decoding window hit the end of the current
segment and we tried to look ahead into a not-yet-available next file.
That was very slow.
Also change the no_readahead_until mechanism to use <= rather than <,
which seems more useful. Otherwise we'd either get one extra unwanted
retry of restore_command, or we'd need to add 1 to an LSN.
No change in behavior for regular streaming. That was already limited
by the flushedUpto variable, which won't be updated until we replay what
we have already.
Reported by Andres Freund while analyzing the failure of a TAP test on
build farm animal skink (investigation ongoing but probably due to
otherwise unrelated timing bugs triggered by this slowness magnified by
valgrind).
Discussion: https://postgr.es/m/20220409005910.alw46xqmmgny2sgr%40alap3.anarazel.de
|
|
Previously we didn't, which lead to an assertion failure when resetting
partially loaded statistics. This was encountered on the buildfarm, for
as-of-yet unknown reasons.
Ttighten up a validity check when reading the stats file, verifying 'E'
signals the end of the file (rather than just stopping reading). That's then
used in a test appending to the stats file that crashed before the fix in
pgstat_drop_all_entries().
Reported by buildfarm animals mylodon and kestrel, via Tom Lane.
Discussion: https://postgr.es/m/1656446.1650043715@sss.pgh.pa.us
|
|
This function gave the wrong answer when there's more than one
RegisteredSnapshots entry, whether or not any of them is the
CatalogSnapshot. This leads to assertion failure in some scenarios
involving fetching toasted data using a cursor. (As per discussion,
I'm dubious that this is the right contract to be enforcing at all;
but it surely doesn't help to be enforcing it incorrectly.)
Fetching toasted data using a cursor is evidently under-tested,
so add a test case too.
Per report from Erik Rijkers. This is new code, so no need for
back-patch.
Discussion: https://postgr.es/m/dc9dd229-ed30-6c62-4c41-d733ffff776b@xs4all.nl
|
|
This allows the newly added TAP tests to run.
|
|
Per buildfarm member mandrill. The test is new in v15, so no back-patch.
|
|
Per-backend global variables like VacuumPageHit are initialized once per
VACUUM command. This was missed by commit 49c9d9fc, which unified
VACUUM VERBOSE and autovacuum logging. As a result of that oversight,
incorrect values were shown when multiple relations were processed by a
single VACUUM VERBOSE command.
Relations that happened to be processed later on would show "buffer
usage:" values that incorrectly included buffer accesses made while
processing earlier unrelated relations. The same accesses were counted
multiple times.
To fix, take initial values for the tracker variables at the start of
heap_vacuum_rel(), and report delta values later on.
|
|
With a pre-v15 server, show NULL for the "ICU Locale" column,
matching what you see in v15 when the database locale isn't ICU.
The previous coding incorrectly repeated datcollate here.
(There's an unfinished discussion about whether to consolidate
these columns in \l output, but in any case we'd want this fix
for \l+ output.)
Euler Taveira, per report from Christoph Berg
Discussion: https://postgr.es/m/YlmIFCqu+TZSW4rB@msg.df7cb.de
|
|
ComputeXidHorizons (nee GetOldestXmin) thought that it could identify
walsenders by checking for proc->databaseId == 0. Perhaps that was
safe when the code was written, but it's been wrong at least since
autovacuum was invented. Background processes that aren't connected
to any particular database, such as the autovacuum launcher and
logical replication launcher, look like that too.
This imprecision is harmful because when such a process advertises an
xmin, the result is to hold back dead-tuple cleanup in all databases,
though it'd be sufficient to hold it back in shared catalogs (which
are the only relations such a process can access). Aside from being
generally inefficient, this has recently been seen to cause regression
test failures in the buildfarm, as a consequence of the logical
replication launcher's startup transaction preventing VACUUM from
marking pages of a user table as all-visible.
We only want that global hold-back effect for the case where a
walsender is advertising a hot standby feedback xmin. Therefore,
invent a new PGPROC flag that says that a process' xmin should be
considered globally, and check that instead of using the incorrect
databaseId == 0 test. Currently only a walsender sets that flag,
and only if it is not connected to any particular database. (This is
for bug-compatibility with the undocumented behavior of the existing
code, namely that feedback sent by a client who has connected to a
particular database would not be applied globally. I'm not sure this
is a great definition; however, such a client is capable of issuing
plain SQL commands, and I don't think we want xmins advertised for
such commands to be applied globally. Perhaps this could do with
refinement later.)
While at it, I rewrote the comment in ComputeXidHorizons, and
re-ordered the commented-upon if-tests, to make them match up
for intelligibility's sake.
This is arguably a back-patchable bug fix, but given the lack of
complaints I think it prudent to let it age awhile in HEAD first.
Discussion: https://postgr.es/m/1346227.1649887693@sss.pgh.pa.us
|
|
Be consistent about the lines that VACUUM VERBOSE outputs by including
an "index scan not needed: " line for completely empty tables. This
makes the output more readable, especially with multiple distinct VACUUM
operations processed by the same VACUUM command. It's also more
consistent; even empty tables can use the failsafe, which wasn't
reported in the standard way until now.
Follow-up to commit 6e20f460, which taught VACUUM VERBOSE to be more
consistent about reporting on scanned pages with empty tables.
|
|
The age of OldestXmin (a.k.a. "removable cutoff") when VACUUM ends often
indicates the approximate number of XIDs consumed while VACUUM ran.
However, there is at least one important exception: the cutoff could be
held back by a snapshot that was acquired before our VACUUM even began.
Successive VACUUM operations may even use exactly the same old cutoff in
extreme cases involving long held snapshots.
The log messages that described how removable cutoff aged (which were
added by commit 872770fd) created the impression that we were reporting
on how VACUUM's usable cutoff advanced while VACUUM ran, which was
misleading in these extreme cases. Fix by using a more general wording.
Per gripe from Tom Lane.
In passing, relocate related instrumentation code for clarity.
Author: Peter Geoghegan <pg@bowt.ie>
Discussion: https://postgr.es/m/1643035.1650035653@sss.pgh.pa.us
|
|
create_index.sql."
This reverts commit 5bb2b6abc8d6cf120a814317816e4384bcbb9c1e.
Not needed anymore.
|
|
These are to keep Coverity happy. In one case remove a redundant NULL
check, and in another explicitly ignore a function result that is already
known.
|
|
When not loading stats at startup (i.e. pgstat_discard_stats() getting
called), reset timestamps of fixed numbered stats would be left at
0. Oversight in 5891c7a8ed8.
Instead use pgstat_reset_after_failure() and add tests verifying that
fixed-numbered reset timestamps are set appropriately.
Reported-By: "David G. Johnston" <david.g.johnston@gmail.com>
Discussion: https://postgr.es/m/CAKFQuwamFuaQHKdhcMt4Gbw5+Hca2UE741B8gOOXoA=TtAd2Yw@mail.gmail.com
|
|
If a partitioned table has partitions owned by roles other than the
owner of the partitioned table, don't include them in the to-be-
clustered list. This is similar to what VACUUM FULL does (except we do
it sooner, because there is no reason to postpone it). Add a simple
test to verify that only owned partitions are clustered.
While at it, change memory context switch-and-back to occur once per
partition instead of outside of the loop.
Author: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/20220411140609.GF26620@telsasoft.com
|
|
This is to gather some more evidence about why buildfarm member wrasse
is failing. We should revert it (or at least scale it way back) once
that's resolved.
Discussion: https://postgr.es/m/1346227.1649887693@sss.pgh.pa.us
|
|
Fix the grammar in two, and add a hint to one.
|
|
Commit 1a36bc9dba8 conained some logic that was a little opaque and
could have involved a NULL dereference, as complained about by Coverity.
Make the logic more transparent and in doing so avoid the NULL
dereference.
|
|
Only 1 of 3 of these changes appear to be handled by pgindent. That change
is new to v15. The remaining two appear to be left alone by pgindent. The
exact reason for that is not 100% clear to me. It seems related to the
fact that it's a line that contains *only* a single line comment and no
actual code. It does not seem worth investigating this in too much
detail. In any case, these do not conform to our usual practices, so fix
them.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com
|
|
Input for these should be case-insensitive, but was not completely
so. Comparing to the similar queries for timezone names, I realized
that we'd missed forcing the comparison pattern to lower-case.
With that, it behaves as I expect.
While here, flatten the sub-selects in these queries; I don't
find that those add any readability.
Discussion: https://postgr.es/m/3369130.1649348542@sss.pgh.pa.us
|
|
Define "parameters with non-default settings" as being those that
not only have pg_settings.source different from 'default', but
also have a current value different from the hard-wired boot_val.
Adding the latter restriction removes a number of not-very-interesting
cases where the active setting is chosen by initdb but in practice
tends to be the same all the time.
Per discussion with Jonathan Katz.
Discussion: https://postgr.es/m/YlFQLzlPi4QD0wSi@msg.df7cb.de
|
|
heap_fetch() used to have a "keep_buf" parameter that told it to return
ownership of the buffer pin to the caller after finding that the
requested tuple TID exists but is invisible to the specified snapshot.
This was thoughtlessly removed in commit 5db6df0c0, which broke
heapam_tuple_lock() (formerly EvalPlanQualFetch) because that function
needs to do more accesses to the tuple even if it's invisible. The net
effect is that we would continue to touch the page for a microsecond or
two after releasing pin on the buffer. Usually no harm would result;
but if a different session decided to defragment the page concurrently,
we could see garbage data and mistakenly conclude that there's no newer
tuple version to chain up to. (It's hard to say whether this has
happened in the field. The bug was actually found thanks to a later
change that allowed valgrind to detect accesses to non-pinned buffers.)
The most reasonable way to fix this is to reintroduce keep_buf,
although I made it behave slightly differently: buffer ownership
is passed back only if there is a valid tuple at the requested TID.
In HEAD, we can just add the parameter back to heap_fetch().
To avoid an API break in the back branches, introduce an additional
function heap_fetch_extended() in those branches.
In HEAD there is an additional, less obvious API change: tuple->t_data
will be set to NULL in all cases where buffer ownership is not returned,
in particular when the tuple exists but fails the time qual (and
!keep_buf). This is to defend against any other callers attempting to
access non-pinned buffers. We concluded that making that change in back
branches would be more likely to introduce problems than cure any.
In passing, remove a comment about heap_fetch that was obsoleted by
9a8ee1dc6.
Per bug #17462 from Daniil Anisimov. Back-patch to v12 where the bug
was introduced.
Discussion: https://postgr.es/m/17462-9c98a0f00df9bd36@postgresql.org
|
|
These are useless and distracting. We wouldn't have written the code
with them to begin with, so there's no reason to keep them.
Author: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com
Discussion: https://postgr.es/m/attachment/133167/0016-Extraneous-blank-lines.patch
|
|
There was a small buglet in commit 52e4f0cd472d whereby a tuple acquired
from cache was not released, giving rise to WARNING messages; fix that.
While at it, restructure the code a bit on stylistic grounds.
Author: Hou zj <houzj.fnst@fujitsu.com>
Reported-by: Peter Smith <smithpb2250@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Discussion: https://postgr.es/m/CAHut+PvKTyhTBtYCQsP6Ph7=o-oWRSX+v+PXXLXp81-o2bazig@mail.gmail.com
|
|
Commit f4fb45d15c misguidedly tried to free some state during aggregate
finalization for json_objectagg. This resulted in attempts to access
freed memory, especially when the function is used as a window function.
Commit 4eb9798879 attempted to ameliorate that, but in fact it should
just be ripped out, which is done here. Also add some regression tests
for json_objectagg in various flavors as a window function.
Original report from Jaime Casanova, diagnosis by Andres Freund.
Discussion: https://postgr.es/m/YkfeMNYRCGhySKyg@ahch-to
|
|
|
|
The last usage of this argument in this routine can be tracked down to
7e2f9062, aka 11 years ago. Getting rid of this argument can also be an
advantage for extensions calling check_index_is_clusterable(), as it
removes any need to worry about the meaning of what a recheck would be
at this level.
Author: Justin Pryzby
Discussion: https://postgr.es/m/20220411140609.GF26620@telsasoft.com
|
|
Since babbbb5 and the introduction of LZ4 in pg_receivewal, the
compression of the WAL archived is controlled by two options:
- --compression-method with "gzip", "none" or "lz4" as possible value.
- --compress=N to specify a compression level. This includes a
backward-incompatible change where a value of 0 leads to a failure
instead of no compression enforced.
This commit takes advantage of a4b5754 and 3603f7c to rework the
compression options of pg_receivewal, as of:
- The removal of --compression-method.
- The extenction of --compress to use the same grammar as pg_basebackup,
with a METHOD:DETAIL format, where a METHOD is "gzip", "none" or "lz4"
and a DETAIL is a comma-separated list of options, the only keyword
supported is now "level" to control the compression level. If only an
integer is specified as value of this option, "none" is implied on 0
and "gzip" is implied otherwise. This brings back --compress to be
backward-compatible with ~14, while still supporting LZ4.
This has also the advantage of centralizing the set of checks used by
pg_receivewal to validate its compression options.
Author: Michael Paquier
Reviewed-by: Robert Haas, Georgios Kokolatos
Discussion: https://postgr.es/m/YlPQGNAAa04raObK@paquier.xyz
|
|
This reverts commits 0147fc7, 4567596, aa64f23, and 5ecd018.
There is no longer agreement that introducing this function
was the right way to address the problem. The consensus now
seems to favor trying to make a correct value for MaxBackends
available to mdules executing their _PG_init() functions.
Nathan Bossart
Discussion: http://postgr.es/m/20220323045229.i23skfscdbvrsuxa@jrouhaud
|
|
Enforce __pg_log_level message filtering centrally in logging.c,
instead of relying on the calling macros to do it. This is more
reliable (e.g. it works correctly for direct calls to pg_log_generic)
and it saves a percent or so of total code size because we get rid of
so many duplicate checks of __pg_log_level.
This does mean that argument expressions in a logging macro will be
evaluated even if we end up not printing anything. That seems of
little concern for INFO and higher levels as those messages are printed
by default, and most of our frontend programs don't even offer a way to
turn them off. I left the unlikely() checks in place for DEBUG
messages, though.
Discussion: https://postgr.es/m/3993549.1649449609@sss.pgh.pa.us
|
|
|
|
as done in e58136069687b9cf29c27281e227ac397d72141d
|
|
Add an assert to make this very explicit, as well as a code comment.
The former should silence Coverity complaining about this.
Introduced by 7103ebb7aae8.
Reported-by: Ranier Vilela
Discussion: https://postgr.es/m/CAEudQAqTTAOzXiYybab+1DQOb3ZUuK99=p_KD+yrRFhcDbd0jg@mail.gmail.com
|
|
The same structure, with the same set of elements (for none, lz4, gzip
and zstd), exists in compression.h, so let's make use of the centralized
version instead of duplicating things. Some of the variables used
previously for WalCompressionMethod are renamed to stick better with the
new structure and routine names.
WalCompressionMethod was leading to some confusion in walmethods.c, as
it was sometimes used to refer to some data unrelated to WAL.
Reported-by: Robert Haas
Author: Michael Paquier
Reviewed-by: Robert Haas, Georgios Kokolatos
Discussion: https://postgr.es/m/YlPQGNAAa04raObK@paquier.xyz
|
|
We were setting MERGE source subplan's targetlist by expanding the
individual attributes of the source relation completely, early in the
parse analysis phase. This failed to work when the condition of an
action included a whole-row reference, causing setrefs.c to error out
with
ERROR: variable not found in subplan target lists
because at that point there is nothing to resolve the whole-row
reference with. We can fix this by having preprocess_targetlist expand
the source targetlist for Vars required from the source rel by all
actions. Moreover, by using this expansion mechanism we can do away
with the targetlist expansion in transformMergeStmt, which is good
because then we no longer pull in columns that aren't needed for
anything.
Add a test case for the problem.
While at it, remove some redundant code in preprocess_targetlist():
MERGE was doing separately what is already being done for UPDATE/DELETE,
so we can just rely on the latter and remove the former. (The handling
of inherited rels was different for MERGE, but that was a no-longer-
necessary hack.)
Fix outdated, related comments for fix_join_expr also.
Author: Richard Guo <guofenglinux@gmail.com>
Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reported-by: Joe Wildish <joe@lateraljoin.com>
Discussion: https://postgr.es/m/fab3b90a-914d-46a9-beb0-df011ee39ee5@www.fastmail.com
|
|
Compression option handling (level, algorithm or even workers) can be
used across several parts of the system and not only base backups.
Structures, objects and routines are renamed in consequence, to remove
the concept of base backups from this part of the code making this
change straight-forward.
pg_receivewal, that has gained support for LZ4 since babbbb5, will make
use of this infrastructure for its set of compression options, bringing
more consistency with pg_basebackup. This cleanup needs to be done
before releasing a beta of 15. pg_dump is a potential future target, as
well, and adding more compression options to it may happen in 16~.
Author: Michael Paquier
Reviewed-by: Robert Haas, Georgios Kokolatos
Discussion: https://postgr.es/m/YlPQGNAAa04raObK@paquier.xyz
|
|
All but a few existing callers assume without checking that this
function succeeds. While it probably will, that's a poor excuse for
not checking. Let's make it return void and instead throw an error
if it doesn't find the block reference. Callers that actually need
to handle the no-such-block case must now use the underlying function
XLogRecGetBlockTagExtended.
In addition to being a bit less error-prone, this should also serve
to suppress some Coverity complaints about XLogRecGetBlockRefInfo.
While at it, clean up some inconsistency about use of the
XLogRecHasBlockRef macro: make XLogRecGetBlockTagExtended use
that instead of open-coding the same condition, and avoid calling
XLogRecHasBlockRef twice in relevant code paths. (That is,
calling XLogRecHasBlockRef followed by XLogRecGetBlockTag is now
deprecated: use XLogRecGetBlockTagExtended instead.)
Patch HEAD only; this doesn't seem to have enough value to consider
a back-branch API break.
Discussion: https://postgr.es/m/425039.1649701221@sss.pgh.pa.us
|
|
Remove comment block about how heap page vacuuming used to set tuples
with storage to LP_UNUSED in a rare edge case that can no longer happen
following commit 8523492d4e. The comments seem unnecessary now, since
it's now generally clear that heap vacuuming only applies to LP_DEAD
items from VACUUM's first heap pass following more recent work from
commits 12b5ade902 and 4f8d9d1217.
|
|
As of commit 39969e2a1, no caller of do_pg_backup_start() passes NULL
for labelfile or tblspcmapfile, nor is it plausible that any would
do so in the future. Remove the code that coped with that case,
as (a) it's dead and (b) it causes Coverity to bleat about possibly
leaked storage.
While here, do some janitorial work on the function's header comment.
|
|
\dconfig without an argument originally printed all parameters,
but it seems more useful to print only those parameters with
non-default settings. You can easily get the show-everything
behavior with "\dconfig *", but that output is unwieldy and
seems unlikely to be wanted very often.
Per suggestion from Christoph Berg.
Discussion: https://postgr.es/m/YlFQLzlPi4QD0wSi@msg.df7cb.de
|
|
With nowait passed as false, pgstat_lock_entry() must return true
so there's no need to check its result. Coverity seems unconvinced
of this, so whack it upside the head with a (void) cast.
|
|
This has no practical effect, since this code doesn't actually need to
distinguish EOF (-1) from \0377; but it silences a Coverity complaint.
|
|
As before, the defaults are similar to gcc's default appearance.
|
|
The existing coding resulted in touching every copyright-containing
file in the tree, even if it was already up to date. That doesn't
matter much for the annual run, but it's an annoyance if you try
to use the script for mop-up at the close of a devel cycle, as
I just did.
Discussion: https://postgr.es/m/266030.1649685473@sss.pgh.pa.us
|
|
Author: Justin Pryzby
Discussion: https://postgr.es/m/20220411020336.GB26620@telsasoft.com
|
|
0ad8032 and 4e34747 are at the origin of that. Julien has found the one
in parse_jsontable.c, while I have spotted the rest.
Author: Julien Rouhaud, Michael Paquier
Discussion: https://postgr.es/m/20220411060838.ftnzyvflpwu6f74w@jrouhaud
|
|
|
|
Commit 4a7e964fc6 made pgperlsyncheck fail, but apparently nobody
noticed, although the buildfarm module that does more or less the same
thing was modified. So fix the in-core test. I will look at unifying the
two sets of tests so we avoid a future mismatch.
|
|
These apply to traces from Test::More functions such as ok(), is(),
diag() and note(). Output from other sources (e.g. external programs
such a initdb) is not affected. The elapsed time is the time since the
last such trace (or the beginning of the test in the first case). Times
and timestamps are at millisecond precision.
Discussion: https://postgr.es/m/20220401172150.rsycz4lrn7ewruil@alap3.anarazel.de
|
|
|
