12.04.2023 15:00, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17893
>
> The following script:
> ...
>
> triggers an assertion failure:
I've come to this assertion failure again, this time from another side, so I
have one reason more to move forward to the issue fix.
First, I've found that two instances of "additional check for
transaction-snapshot mode RI updates" in head_update() and heap_delete()
are not covered by the existing regression tests at all, so I would like to
propose an addition for the fk-snapshot test (see attached). It increases
coverage and causes the assertion failures in both functions.
Second, it looks like Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID))
was added to make sure that tmfd->xmax = HeapTupleHeaderGetUpdateXid(tp.t_data);
(which ends up with just returning t_xmax) returns valid xmax. Really, in the
problematic case we get zero in tmfd->xmax. But ExecDelete(), ExecUpdate() do:
...
switch (result)
...
case TM_Updated:
...
if (IsolationUsesXactSnapshot())
ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
errmsg("could not serialize access due to concurrent update")));
before any processing of tmfd. So it seems like the invalid xmax is not
an issue in this case.
Thus, maybe
Assert((!(tp.t_data->t_infomask & HEAP_XMAX_INVALID))
should be changed to
Assert(!(tp.t_data->t_infomask & HEAP_XMAX_INVALID) ||
(result == TM_Updated && crosscheck != InvalidSnapshot));
Third, with this change applied I immediately got a failure of the next
assert in heap_delete():
Assert(!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
which was introduced by 5db6df0c0.
(By the way, I've noticed a typo brought by that commit:
t_xmax, and, if possible, and, if possible, t_cmax
)
It's not clear to me yet, which anomalous condition that Assert should
detect, specifically in heap_update(), inside this branch:
/* Perform additional check for transaction-snapshot mode RI updates */
if (!HeapTupleSatisfiesVisibility(&oldtup, crosscheck, buffer))
{
result = TM_Updated;
Assert(!ItemPointerEquals(&oldtup.t_self, &oldtup.t_data->t_ctid));
}
Andres, could you shed some light on this, please?
Best regards,
Alexander
