Updated patch attached. On Sun, Sep 7, 2025 at 9:51 PM, David Rowley <[email protected]> wrote: > 1. This part is quite annoying: > > + if (node->tss_TidList == NULL) > + TidListEval(node); > > Of course, it's required since ExecReScanTidScan() wiped out that list.
Given that EPQ uses separate PlanState, I've left this as is. > 2. For TidRangeRecheck, I don't see why this part is needed: > > + if (!TidRangeEval(node)) > + return false; > > The TID range is preserved already and shouldn't need to be recalculated. I've added a new trss_boundsInitialized flag such that we calculate the range once per EPQ rescan. In order to preserve the semantics when the min or max is NULL, I'm setting trss_mintid/trss_maxtid to have invalid ItemPointers as a sentinel in the cases where TidRangeEval returns false. I added a ItemPointerIsValid assertion given that it's now more relevant to correctness but I can remove it if it feels superfluous. Let me know if there is a more idiomatic way to treat this. > 3. In TidRecheck(), can you make "match" an "ItemPointer" and do: > match = (ItemPointer) bsearch(...); > 4. Can you put this comment above the "if"? > 5. Can you make TidRangeRecheck() look like this? > 6. For the tests. It should be ok to make the Tid range scan test do > ctid BETWEEN '(0,1)' AND '(0,1)'. I feel this is more aligned to the > TID Range Scan version of what you're doing in the TID Scan test. All done. Do let me know if other changes would be helpful. Sophie
0001-Fix-missing-EvalPlanQual-recheck-for-TID-scans.v2.patch
Description: Binary data
