Hi Sophie,
Thanks for the fix. We do have a lot of applications that use ctid to
update/delete rows for performance considerations.
> On Sep 8, 2025, at 17:15, David Rowley <dgrowle...@gmail.com> wrote:
>
> On Mon, 8 Sept 2025 at 18:57, Sophie Alpert <p...@sophiebits.com> wrote:
>> I'm actually digging in more now and if I'm reading the code correctly,
>> EvalPlanQualStart calls ExecInitNode to create an entirely separate
>> PlanState tree for EPQ so I'm unsure if any of the state is carried over to
>> the Recheck calls?
>
> Oh, you're right. The EPQ executor start isn't the same as the normal
> executor state, so the recalc seems to be needed.
>
> David
>
Following the reproduce procedure that David provided, I tested and traced the
fix, basically it works for me.
However, I noticed that, when “TidRecheck()” is called, it is actually passed
with “epqstate->recheckplanstate” that is always newly created, which means we
repeatedly call “TidListEval(node)” and run “bsearch()” when there are multiple
row involved in the update. If update qual is just “ctid = <a single ctid>”,
that should fine. But if we do “update .. where ctid in (a list of ctid)”, then
duplicately call “TidListEval()” might not be a good thing.
As “TidRecheck(TidScanState *node, TupleTableSlot *slot)” gets the second
parameter “slot” with the updated slot, so I am thinking the other solution
could be that, we just check if the updated slot is visible to current
transaction. So I made a local change like this:
```
static bool
TidRecheck(TidScanState *node, TupleTableSlot *slot)
{
HeapTuple tuple;
bool visible;
if (node->tss_isCurrentOf)
/* WHERE CURRENT OF always intends to resolve to the latest tuple */
return true;
tuple = ExecCopySlotHeapTuple(slot);
visible = HeapTupleSatisfiesVisibility(tuple, GetActiveSnapshot(),
slot->tts_tableOid);
return visible;
}
```
This seems also work for me. WDYT?
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
