Re: Fix missing EvalPlanQual recheck for TID scans
От | Sophie Alpert |
---|---|
Тема | Re: Fix missing EvalPlanQual recheck for TID scans |
Дата | |
Msg-id | 2186d776-d833-497d-819c-28234a3a7ff0@app.fastmail.com обсуждение исходный текст |
Ответ на | Re: Fix missing EvalPlanQual recheck for TID scans (David Rowley <dgrowleyml@gmail.com>) |
Список | pgsql-hackers |
Updated patch attached. On Sun, Sep 7, 2025 at 9:51 PM, David Rowley <dgrowleyml@gmail.com> 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 thesemantics when the min or max is NULL, I'm setting trss_mintid/trss_maxtid to have invalid ItemPointers as a sentinelin the cases where TidRangeEval returns false. I added a ItemPointerIsValid assertion given that it's now more relevantto correctness but I can remove it if it feels superfluous. Let me know if there is a more idiomatic way to treatthis. > 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
Вложения
В списке pgsql-hackers по дате отправления: