This commit fixes failures with "tuple already updated by self" when
listing twice the same role and in a DROP ROLE query.
This is an oversight in 6566133c5f, that has introduced a two-phase
logic in DropRole() where dependencies of all the roles to drop are
removed in a first phase, with the roles themselves removed from
pg_authid in a second phase.
The code is simplified to not rely on a List of ObjectAddress built in
the first phase used to remove the pg_authid entries in the second
phase, switching to a list of OIDs. Duplicated OIDs can be simply
avoided in the first phase thanks to that. Using ObjectAddress was not
necessary for the roles as they are not used for anything specific to
dependency.c, building all the ObjectAddress in the List with
AuthIdRelationId as class ID.
In 15 and older versions, where a single phase is used, DROP ROLE with
duplicated role names would fail on "role \"blah\" does not exist" for
the second entry after the CCI() done by the first deletion. This is
not really incorrect, but it does not seem worth changing based on a
lack of complaints.
Reported-by: Alexander Lakhin
Reviewed-by: Tender Wang
Discussion: https://postgr.es/m/18310-1eb233c5908189c8@postgresql.org
Backpatch-through: 16
Both drongo and fairywren seem not to like a new regression test added
by 2cca95e17. These machines show a different number of actual rows in
the EXPLAIN ANALYZE output. Since the number of actual rows is divided by
the number of loops, I suspect this might be due to some platform
dependant rounding behavior as the total row count is 5 and the number of
loops is 2. drongo and fairywren seem to be calculating that 5.0 / 2.0 is
3, whereas most other machines think the answer is 2.
Here we tweak the test query's WHERE clause so it's 4.0 / 2.0 instead.
There shouldn't be too much wiggle room for platform dependant-behavior to
be a factor with those numbers.
Reported-by: Tom Lane
Discussion: https://postgr.es/m/1035225.1706301718%40sss.pgh.pa.us
Oversight in 2489d76c4. Preliminary analysis suggests that the
problem may be unreachable --- but if we did have instances of
the same column with different varnullingrels, we'd surely need
to treat them as different Params.
Discussion: https://postgr.es/m/412552.1706203379@sss.pgh.pa.us
We perform addition of the days field of an interval via
arithmetic on the Julian-date representation of the timestamp's date.
This step is subject to int32 overflow, and we also should not let
the Julian date become very negative, for fear of weird results from
j2date. (In the timestamptz case, allow a Julian date of -1 to pass,
since it might convert back to zero after timezone rotation.)
The additions of the months and microseconds fields could also
overflow, of course. However, I believe we need no additional
checks there; the existing range checks should catch such cases.
The difficulty here is that j2date's magic modular arithmetic could
produce something that looks like it's in-range.
Per bug #18313 from Christian Maurer. This has been wrong for
a long time, so back-patch to all supported branches.
Discussion: https://postgr.es/m/18313-64d2c8952d81e84b@postgresql.org
The tests in 002_blocks.pl are failing in the buildfarm from time to
time, but we don't know how to reproduce the failure elsewhere. The
most obvious explanation seems to be the unexpected disappearance of a
WAL summary file, so bump up the logging level in
RemoveWalSummaryIfOlderThan to try to help us spot such problems, and
print the cutoff time in addition to the removed filename. Also
adjust 002_blocks.pl to dump out a directory listing of the relevant
directory at various points.
This patch should be reverted once we sort out what's happening here.
Patch by me, reviewed by Nathan Bossart, who also reported the issue.
Discussion: http://postgr.es/m/20240124170846.GA2643050@nathanxps13
lazy_scan_prune() and lazy_scan_noprune() update the freespace map
with identical conditions; combine them. This consolidation is easier
now that cb970240f1 moved visibility map
updates into lazy_scan_prune().
While combining the FSM updates, simplify the logic for calling
lazy_scan_new_or_empty() and lazy_scan_noprune().
Also update a few comemnts in this part of the code to make them,
hopefully, clearer.
Melanie Plageman and Robert Haas
Discussion: https://postgr.es/m/CA%2BTgmoaLTvipm%3Dxx4rJLr07m908PCu%3DQH3uCjD1UOn8YaEuO2g%40mail.gmail.com
- Separate function to merge a child attribute into matching inherited
attribute: The logic to merge a child attribute into matching
inherited attribute in MergeAttribute() is only applicable to
regular inheritance child. The code is isolated and coherent enough
that it can be separated into a function of its own.
- Separate function to merge next parent attribute: Partitions inherit
from only a single parent. The logic to merge an attribute from the
next parent into the corresponding attribute inherited from previous
parents in MergeAttribute() is only applicable to regular
inheritance children. This code is isolated enough that it can be
separate into a function by itself.
These separations makes MergeAttribute() more readable by making it
easier to follow high level logic without getting entangled into
details.
Author: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
This commit changes the order of reindex on a relation so as a toast
relation is processed before its main relation.
The original order, where a rebuild was first done for the indexes on
the main table, could be a problem in the event of a corruption of a
toast index, because, as scans of a toast index may be required to
rebuild the indexes on the main relation, this could lead to failures
with REINDEX TABLE without being able to fix anything.
Rebuilding corrupted toast indexes before this change was possible but
troublesome, as it was necessary to issue a REINDEX on the toast
relation first, followed by a REINDEX on the main relation. Changing
the order of these operations should make things easier when rebuilding
corrupted indexes, as toast indexes would be rebuilt before they are
used for the indexes on the main relation.
Per request from Richard Vesely.
Author: Gurjeet Singh
Reviewed-by: Nathan Bossart, Michael Paquier
Discussion: https://postgr.es/m/18016-2bd9b549b1fe49b3@postgresql.org
It was possible when determining the cache keys for a Memoize path that
if the same expr appeared twice in the parameterized path's ppi_clauses
and/or in the Nested Loop's inner relation's lateral_vars. If this
happened the Memoize node's cache keys would contain duplicates. This
isn't a problem for correctness, all it means is that the cache lookups
will be suboptimal due to having redundant work to do on every hash table
lookup and insert.
Here we adjust paraminfo_get_equal_hashops() to look for duplicates and
ignore them when we find them.
Author: David Rowley
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/422277.1706207562%40sss.pgh.pa.us
It was possible in cases where we had a LATERAL joined subquery that
when the same Var is mentioned in both the lateral references and in the
outer Vars of the scan clauses that the given Var wouldn't be assigned
to the same NestLoopParam.
This could cause issues in Memoize as the cache key would reference the
Var for the scan clauses but when the parameter for the lateral references
changed some code in Memoize would see that some other parameter had
changed that's not part of the cache key and end up purging the entire
cache as a result, thinking the cache had become stale. This could
result in a Nested Loop -> Memoize plan being quite inefficient as, in
the worst case, the cache purging could result in never getting a cache
hit. In no cases could this problem lead to incorrect query results.
Here we switch the order of operations so that we create NestLoopParam
for the lateral references first before doing replace_nestloop_params().
replace_nestloop_params() will find and reuse the existing NestLoopParam
in cases where the Var exists in both locations.
Author: Richard Guo
Reviewed-by: Tom Lane, David Rowley
Discussion: https://postgr.es/m/CAMbWs48XHJEK1Q1CzAQ7L9sTANTs9W1cepXu8%3DKc0quUL%2Btg4Q%40mail.gmail.com
This reverts commit 2197d06224, following a discussion over a Coverity
report where issues like the "Billion laugh attack" could cause the
backend to waste CPU and memory even if a client applied checks on the
size of the data given in input, and libxml2 does not offer guarantees
that input limits are respected under XML_PARSE_HUGE.
Discussion: https://postgr.es/m/ZbHlgrPLtBZyr_QW@paquier.xyz
Formerly, these were only supported in to_char(), but there seems
little reason for that restriction. We should at least have enough
support to permit round-tripping the output of to_char().
In that spirit, TZ accepts either zone abbreviations or numeric
(HH or HH:MM) offsets, which are the cases that to_char() can output.
In an ideal world we'd make it take full zone names too, but
that seems like it'd introduce an unreasonable amount of ambiguity,
since the rules for POSIX-spec zone names are so lax.
OF is a subset of this, accepting only HH or HH:MM.
One small benefit of this improvement is that we can simplify
jsonpath's executeDateTimeMethod function, which no longer needs
to consider the HH and HH:MM cases separately. Moreover, letting
it accept zone abbreviations means it will accept "Z" to mean UTC,
which is emitted by JSON.stringify() for example.
Patch by me, reviewed by Aleksander Alekseev and Daniel Gustafsson
Discussion: https://postgr.es/m/1681086.1686673242@sss.pgh.pa.us
Remove a buggy and unnecessary test, along with an unnecessary pstrdup()
and a line of dead code.
Per report, diagnosis and fix from Tom Lane
Discussion: https://postgr.es/m/439811.1706211069@sss.pgh.pa.us
Clarify the behavior of jsonpath operators and functions by
describing their two different modes of operation explicitly.
In addition to the SQL-spec behavior, where a path returns
a list of matching items, we have a "predicate check" form
that always returns a single boolean result. That was mentioned
in only one place, but it seems better to annotate each operator
and function as to which form(s) it takes. Also improve the
examples by converting them into actual executable SQL with
results, and do a bunch of incidental wordsmithing.
David Wheeler, reviewed by Erik Wienhold, Jian He, and myself
Discussion: https://postgr.es/m/7262A188-59CA-4A8A-AAD7-83D4FF0B9758@justatheory.com
This commit implements ithe jsonpath .bigint(), .boolean(),
.date(), .decimal([precision [, scale]]), .integer(), .number(),
.string(), .time(), .time_tz(), .timestamp(), and .timestamp_tz()
methods.
.bigint() converts the given JSON string or a numeric value to
the bigint type representation.
.boolean() converts the given JSON string, numeric, or boolean
value to the boolean type representation. In the numeric case, only
integers are allowed. We use the parse_bool() backend function
to convert a string to a bool.
.decimal([precision [, scale]]) converts the given JSON string
or a numeric value to the numeric type representation. If precision
and scale are provided for .decimal(), then it is converted to the
equivalent numeric typmod and applied to the numeric number.
.integer() and .number() convert the given JSON string or a
numeric value to the int4 and numeric type representation.
.string() uses the datatype's output function to convert numeric
and various date/time types to the string representation.
The JSON string representing a valid date/time is converted to the
specific date or time type representation using jsonpath .date(),
.time(), .time_tz(), .timestamp(), .timestamp_tz() methods. The
changes use the infrastructure of the .datetime() method and perform
the datatype conversion as appropriate. Unlike the .datetime()
method, none of these methods accept a format template and use ISO
DateTime format instead. However, except for .date(), the
date/time related methods take an optional precision to adjust the
fractional seconds.
Jeevan Chalke, reviewed by Peter Eisentraut and Andrew Dunstan.
MergeAttributes() has a loop to merge multiple inheritance parents
into a column column definition. The merged-so-far definition is
stored in a ColumnDef node. If we have to merge columns from multiple
inheritance parents (if the name matches), then we have to check
whether various column properties (type, collation, etc.) match. The
current code extracts the pg_attribute value of the
currently-considered inheritance parent and compares it against the
merged-so-far ColumnDef value. If the currently considered column
doesn't match any previously inherited column, we make a new ColumnDef
node from the pg_attribute information and add it to the column list.
This patch rearranges this so that we create the ColumnDef node first
in either case. Then the code that checks the column properties for
compatibility compares ColumnDef against ColumnDef (instead of
ColumnDef against pg_attribute). This makes the code more symmetric
and easier to follow. Also, later in MergeAttributes(), there is a
similar but separate loop that merges the new local column definition
with the combination of the inheritance parents established in the
first loop. That comparison is already ColumnDef-vs-ColumnDef. With
this change, both of these can use similar-looking logic. (A future
project might be to extract these two sets of code into a common
routine that encodes all the knowledge of whether two column
definitions are compatible. But this isn't currently straightforward
because we want to give different error messages in the two cases.)
Furthermore, by avoiding the use of Form_pg_attribute here, we make it
easier to make changes in the pg_attribute layout without having to
worry about the local needs of tablecmds.c.
Because MergeAttributes() is hugely long, it's sometimes hard to know
where in the function you are currently looking. To help with that, I
also renamed some variables to make it clearer where you are currently
looking. The first look is "prev" vs. "new", the second loop is "inh"
vs. "new".
Reviewed-by: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/52a125e4-ff9a-95f5-9f61-b87cf447e4da@eisentraut.org
This is a mostly unused tool, but I discovered while nosing around the
Makefile that it hasn't been kept in line with other changes. Fix it.
Backpatching doesn't appear to be necessary.
Discussion: https://postgr.es/m/202401241114.ied53jcich72@alvherre.pgsql
Three LOG messages are added in the recovery code paths, providing
information that can be useful to track corruption issues depending on
the state of the cluster, telling that:
- Recovery has started from a backup_label.
- Recovery is restarting from a backup start LSN, without a
backup_label.
- Recovery has completed from a backup.
Author: Andres Freund
Reviewed-by: David Steele, Laurenz Albe, Michael Paquier
Discussion: https://postgr.es/m/20231117041811.vz4vgkthwjnwp2pp@awork3.anarazel.de
This commit adds the failover property to the replication slot. The
failover property indicates whether the slot will be synced to the standby
servers, enabling the resumption of corresponding logical replication
after failover. But note that this commit does not yet include the
capability to sync the replication slot; the subsequent commits will add
that capability.
A new optional parameter 'failover' is added to the
pg_create_logical_replication_slot() function. We will also enable to set
'failover' option for slots via the subscription commands in the
subsequent commits.
The value of the 'failover' flag is displayed as part of
pg_replication_slots view.
Author: Hou Zhijie, Shveta Malik, 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
Because of commit 1bdd54e662, the code of HandleWalWriterInterrupts()
became the same as HandleMainLoopInterrupts(). So this commit removes
HandleWalWriterInterrupts() and makes walwriter use
HandleMainLoopInterrupts() for improved code simplicity.
Author: Fujii Masao
Reviewed-by: Bharath Rupireddy, Nathan Bossart
Discussion: https://postgr.es/m/CAHGQGwHUtwCsB4DnqFLiMiVzjcA=zmeCKf9_pgQM-yJopydatw@mail.gmail.com
9e2d870119 enabled the COPY command to skip malformed data, however
there was no visibility into how many tuples were actually skipped
during the COPY FROM.
This commit adds a new "tuples_skipped" column to
pg_stat_progress_copy view to report the number of tuples that were
skipped because they contain malformed data.
Bump catalog version.
Author: Atsushi Torikoshi
Reviewed-by: Masahiko Sawada
Discussion: https://postgr.es/m/d12fd8c99adcae2744212cb23feff6ed%40oss.nttdata.com
Add WITHOUT OVERLAPS clause to PRIMARY KEY and UNIQUE constraints.
These are backed by GiST indexes instead of B-tree indexes, since they
are essentially exclusion constraints with = for the scalar parts of
the key and && for the temporal part.
Author: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: jian he <jian.universality@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com
Use C99 designated initializer syntax for array elements, instead of
writing the position in a comment. This is less verbose and much more
readable. Akin to cc15059634.
One disadvantage is that the BuiltinTrancheNames array now has a hole of
51 NULLs -- previously, the array elements were shifted 51 elements
downward to avoid this. This can be fixed by merging the
IndividualLWLockNames array into BuiltinTrancheNames, which would occupy
those 51 pointers, but because it requires some arguably ugly Meson
hackery, it's left for later.
Discussion: https://postgr.es/m/202401231025.gbv4nnte5fmm@alvherre.pgsql
This change adds a new meta-command called \syncpipeline to pgbench,
able to send a sync message without flushing using the new libpq
function PQsendPipelineSync().
This meta-command is available within a block made of \startpipeline and
\endpipeline.
Author: Anthonin Bonnefoy
Discussion: https://postgr.es/m/CAO6_XqpcNhW6LZHLF-2NpPzdTbyMm4-RVkr3+AP5cOKSm9hrWA@mail.gmail.com
This adds a Node *escontext parameter to it and a bunch of functions
downstream to it, replacing any ereport()s in that path by either
errsave() or ereturn() as appropriate. This also adds code to those
functions where necessary to return early upon encountering a soft
error.
The changes here are mainly intended to suppress errors in the
functions of jsonfuncs.c. Functions in any external modules, such as
arrayfuncs.c, that those functions may in turn call are not changed
here based on the assumption that the various checks in jsonfuncs.c
functions should ensure that only values that are structurally valid
get passed to the functions in those external modules. An exception
is made for domain_check() to allow handling domain constraint
violation errors softly.
For testing, this adds a function jsonb_populate_record_valid(),
which returns true if jsonb_populate_record() would finish without
causing an error for the provided JSON object, false otherwise. Note
that jsonb_populate_record() internally calls populate_record(),
which in turn uses populate_record_field().
Extracted from a much larger patch to add SQL/JSON query functions.
Author: Nikita Glukhov <n.gluhov@postgrespro.ru>
Author: Teodor Sigaev <teodor@sigaev.ru>
Author: Oleg Bartunov <obartunov@gmail.com>
Author: Alexander Korotkov <aekorotkov@gmail.com>
Author: Andrew Dunstan <andrew@dunslane.net>
Author: Amit Langote <amitlangote09@gmail.com>
Reviewers have included (in no particular order) Andres Freund,
Alexander Korotkov, Pavel Stehule, Andrew Alsup, Erik Rijkers,
Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson, Justin Pryzby,
Álvaro Herrera, Jian He, Peter Eisentraut
Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru
Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de
Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org
Discussion: https://postgr.es/m/CA+HiwqHROpf9e644D8BRqYvaAPmgBZVup-xKMDPk-nd4EpgzHw@mail.gmail.com
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
This adjusts the code for CoerceViaIO and CoerceToDomain expression
nodes to handle errors softly.
For CoerceViaIo, this adds a new ExprEvalStep opcode
EEOP_IOCOERCE_SAFE, which is implemented in the new accompanying
function ExecEvalCoerceViaIOSafe(). The only difference from
EEOP_IOCOERCE's inline implementation is that the input function
receives an ErrorSaveContext via the function's
FunctionCallInfo.context, which it can use to handle errors softly.
For CoerceToDomain, this simply entails replacing the ereport() in
ExecEvalConstraintNotNull() and ExecEvalConstraintCheck() by
errsave() passing it the ErrorSaveContext passed in the expression's
ExprEvalStep.
In both cases, the ErrorSaveContext to be used is passed by setting
ExprState.escontext to point to it before calling ExecInitExprRec()
on the expression tree whose errors are to be handled softly.
Note that there's no functional change as of this commit as no call
site of ExecInitExprRec() has been changed. This is intended for
implementing new SQL/JSON expression nodes in future commits.
Extracted from a much larger patch to add SQL/JSON query functions.
Author: Nikita Glukhov <n.gluhov@postgrespro.ru>
Author: Teodor Sigaev <teodor@sigaev.ru>
Author: Oleg Bartunov <obartunov@gmail.com>
Author: Alexander Korotkov <aekorotkov@gmail.com>
Author: Andrew Dunstan <andrew@dunslane.net>
Author: Amit Langote <amitlangote09@gmail.com>
Reviewers have included (in no particular order) Andres Freund,
Alexander Korotkov, Pavel Stehule, Andrew Alsup, Erik Rijkers,
Zihong Yu, Himanshu Upadhyaya, Daniel Gustafsson, Justin Pryzby,
Álvaro Herrera, Jian He, Peter Eisentraut
Discussion: https://postgr.es/m/cd0bb935-0158-78a7-08b5-904886deac4b@postgrespro.ru
Discussion: https://postgr.es/m/20220616233130.rparivafipt6doj3@alap3.anarazel.de
Discussion: https://postgr.es/m/abd9b83b-aa66-f230-3d6d-734817f0995d%40postgresql.org
Discussion: https://postgr.es/m/CA+HiwqHROpf9e644D8BRqYvaAPmgBZVup-xKMDPk-nd4EpgzHw@mail.gmail.com
Discussion: https://postgr.es/m/CA+HiwqE4XTdfb1nW=Ojoy_tQSRhYt-q_kb6i5d4xcKyrLC1Nbg@mail.gmail.com
This command, when used to add a column on a parent table with a complex
inheritance tree, tried to update multiple times the same tuple in
pg_attribute for a child table when incrementing attinhcount, causing
failures with "tuple already updated by self" because of a missing
CommandCounterIncrement() between two updates.
This exists for a rather long time, so backpatch all the way down.
Reported-by: Alexander Lakhin
Author: Tender Wang
Reviewed-by: Richard Guo
Discussion: https://postgr.es/m/18297-b04cd83a55b51e35@postgresql.org
Backpatch-through: 12
Commit abb0b4fc03 moved the shared state for autoprewarm to a
dynamic shared memory (DSM) segment, but it left apw_detach_shmem()
in the on_shmem_exit callback list for the autoprewarm leader
process. This is a problem because shmem_exit() detaches all the
DSM segments prior to calling the on_shmem_exit callbacks, thus
producing segfaults in the exit path for the autoprewarm leader
process.
To fix, move apw_detach_shmem() to the before_shmem_exit callback
list. This commit also adds a check to pg_prewarm's test that the
server shut down normally. It might be worth making this a common
check for all shutdowns in TAP tests, but that is left as a future
exercise.
Reported-by: Andres Freund
Reviewed-by: Andres Freund, Álvaro Herrera
Discussion: https://postgr.es/m/20240122204117.swton324xcoodnyi%40awork3.anarazel.de
AM was used throughout the documentation to denote Access Method, but
the acronym was not described. This adds an acronym entry as well as
a glossary term which the acronym links to. Each page which describe
AMs have the first occurrence with <acronym> markup.
Reported-by: alaa.attya91@gmail.com
Reviewed-by: Alvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/169974408805.398198.6927340566912872957@wrigleys.postgresql.org
portname is set to host_system, but host_system might still be changed
later in the file. As a consequence, platforms where host_system is
changed later wouldn't find some of their platform-specific files
(driven by portname), and the build would fail. Move the assignment
of portname further down after the last change of host_system (but
before further overriding assignments to portname).
Discussion: https://www.postgresql.org/message-id/flat/CAC7zN94TdsHhY88XkroJzSMx7E%3DBQpV9LKKjNSEnTM04ihoWCA%40mail.gmail.com
This reverts commit 728f86fec6.
The signal handling was a few bricks shy of a load in that commit,
which made the walreceiver non-responsive to SIGTERM while it was
waiting for the connection to be established. That prevented a standby
from being promoted.
Since it was non-essential refactoring, let's revert it to make v16
work the same as earlier releases. I reverted it in 'master' too, to
keep the branches in sync. The refactoring was a good idea as such,
but it needs a bit more work. Once we have developed a complete patch
with this issue fixed, let's re-apply that to 'master'.
Reported-by: Kyotaro Horiguchi
Backpatch-through: 16
Discussion: https://www.postgresql.org/message-id/20231231.200741.1078989336605759878.horikyota.ntt@gmail.com
This commit tweaks a couple of things in 035_standby_logical_decoding to
hopefully stabilize it:
- Autovacuum is now disabled, as it could hold a global xmin with a
transaction.
- Conflicts are generated with command sequences that removed rows (on
catalogs, shared or non-shared, or just plain relations) followed by a
VACUUM. This was unstable because these did not check that the horizon
moved between the SQL commands and the VACUUM. The logic is refactored
as follows, to ensure that VACUUM removes dead rows before testing for
slot invalidation on a standby (idea suggested by Andres Freund):
-- Grab the current horizon.
-- Launch SQL commands removing rows.
-- Check that the snapshot horizon has been updated.
-- Launch VACUUM on the relation whose rows have been removed by the
first step.
Note that there are still some issues because of standby snapshot WAL
records generated by the bgwriter, but this makes the test much more
stable.
Per reports from buildfarm members dikkop and skink, with analysis and
tests from Alexander Lakhin.
While on it, fix a couple of incorrect comments.
Author: Bertrand Drouvot
Reviewed-by: Alexander Lakhin, Michael Paquier
Discussion: https://postgr.es/m/OSZPR01MB6310ED3CEDB531BCEDBC6AF2FD479@OSZPR01MB6310.jpnprd01.prod.outlook.com
Discussion: https://postgr.es/m/bf67e076-b163-9ba3-4ade-b9fc51a3a8f6@gmail.com
Backpatch-through: 16
Until now PostgreSQL has not been very smart about optimizing away IS
NOT NULL base quals on columns defined as NOT NULL. The evaluation of
these needless quals adds overhead. Ordinarily, anyone who came
complaining about that would likely just have been told to not include
the qual in their query if it's not required. However, a recent bug
report indicates this might not always be possible.
Bug 17540 highlighted that when we optimize Min/Max aggregates the IS NOT
NULL qual that the planner adds to make the rewritten plan ignore NULLs
can cause issues with poor index choice. That particular case
demonstrated that other quals, especially ones where no statistics are
available to allow the planner a chance at estimating an approximate
selectivity for can result in poor index choice due to cheap startup paths
being prefered with LIMIT 1.
Here we take generic approach to fixing this by having the planner check
for NOT NULL columns and just have the planner remove these quals (when
they're not needed) for all queries, not just when optimizing Min/Max
aggregates.
Additionally, here we also detect IS NULL quals on a NOT NULL column and
transform that into a gating qual so that we don't have to perform the
scan at all. This also works for join relations when the Var is not
nullable by any outer join.
This also helps with the self-join removal work as it must replace
strict join quals with IS NOT NULL quals to ensure equivalence with the
original query.
Author: David Rowley, Richard Guo, Andy Fan
Reviewed-by: Richard Guo, David Rowley
Discussion: https://postgr.es/m/CAApHDvqg6XZDhYRPz0zgOcevSMo0d3vxA9DvHrZtKfqO30WTnw@mail.gmail.com
Discussion: https://postgr.es/m/17540-7aa1855ad5ec18b4%40postgresql.org
--enable-injection-points and -Dinjection_points=true are now given to
the commands triggered by the tasks where assertions are enabled,
providing coverage for injection points or any test using them.
Author: Heikki Linnakangas
Discussion: https://postgr.es/m/0615a424-b726-4157-afa7-4245629f9512@iki.fi