Re: Fix missing EvalPlanQual recheck for TID scans - Mailing list pgsql-hackers

From Chao Li
Subject Re: Fix missing EvalPlanQual recheck for TID scans
Date
Msg-id CE7E2787-942B-481B-9340-B19267C45822@gmail.com
Whole thread Raw
In response to Re: Fix missing EvalPlanQual recheck for TID scans  (David Rowley <dgrowleyml@gmail.com>)
Responses Re: Fix missing EvalPlanQual recheck for TID scans
List pgsql-hackers
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 <dgrowleyml@gmail.com> wrote:

On Mon, 8 Sept 2025 at 18:57, Sophie Alpert <pg@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/




pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: [PATCH] Accept connections post recovery without waiting for RemoveOldXlogFiles
Next
From: Pierrick
Date:
Subject: Re: Only one version can be installed when using extension_control_path