Re: Fix missing EvalPlanQual recheck for TID scans
Chao Li <li.evan.chao@gmail.com>
From: Chao Li <li.evan.chao@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>,
Sophie Alpert <pg@sophiebits.com>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2025-09-09T07:52:54Z
Lists: 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/
Commits
-
Add missing EPQ recheck for TID Range Scan
- f78a69034740 14.20 landed
- f00ad440a5b6 15.15 landed
- ba0203880a8f 16.11 landed
- 0fb06e893331 17.7 landed
- 78e6047dcea5 18.0 landed
- ac06ea8f7b6c 19 (unreleased) landed
-
Add missing EPQ recheck for TID Scan
- 940f3cd5df57 13.23 landed
- 2eb7ea97d8af 14.20 landed
- 005770203889 15.15 landed
- d6539f88b7c5 16.11 landed
- 3d939a9b1c72 17.7 landed
- bae6c74ba4e4 18.0 landed
- dee21ea6d617 19 (unreleased) landed
-
Repair problems with EvalPlanQual where target table is scanned as
- 6799a6ca21e5 7.2.1 cited
-
Tid access method feature from Hiroshi Inoue, Inoue@tpf.co.jp
- 6f9ff92cc0ff 7.1.1 cited