Re: search_plan_tree(): handling of non-leaf CustomScanState nodes causes segfault
David Geier <david@swarm64.com>
From: David Geier <david@swarm64.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Date: 2021-01-19T07:41:20Z
Lists: pgsql-hackers
Hi, On 18.01.21 23:42, Tom Lane wrote: > David Geier<david@swarm64.com> writes: >> On 18.01.21 19:46, Tom Lane wrote: >>> Hm. I agree that we shouldn't simply assume that ss_currentRelation >>> isn't null. However, we cannot make search_plan_tree() descend >>> through non-leaf CustomScan nodes, because we don't know what processing >>> is involved there. We need to find a scan that is guaranteed to return >>> rows that are one-to-one with the cursor output. This is why the function >>> doesn't descend through join or aggregation nodes, and I see no argument >>> by which we should assume we know more about what a customscan node will >>> do than we know about those. >> That makes sense. Thanks for the explanation. > OK, cool. I was afraid you'd argue that you really needed your CustomScan > node to be transparent in such cases. We could imagine inventing an > additional custom-scan-provider callback to embed the necessary knowledge, > but I'd rather not add the complexity until someone has a use-case. I was thinking about that. Generally, having such possibility would be very useful. Unfortunately, I believe that in my specific case even having such functionality wouldn't allow for the query to work with CURRENT OF, because my CSP behaves similarly to a materialize node. My understanding is only such plans are supported which consist of nodes that guarantee that the tuple returned by the plan is the unmodified tuple a scan leaf node is currently positioned on? Still, if there's interest I would be happy to draft a patch. Instead of a separate CSP callback, we could also provide an additional flag like CUSTOMPATH_SUPPORT_CURRENT_OF. The advantage of the callback would be that we could delay the decision until execution time where potentially more information is available. >> I updated the patch to match your proposal. > WFM, will push in a bit. > > regards, tom lane Best regards, David Swarm64
Commits
-
Remove faulty support for MergeAppend plan with WHERE CURRENT OF.
- 0ab177bec1c5 9.5.25 landed
- fe8edbb8267a 9.6.21 landed
- fac54bd5e216 10.16 landed
- 794562d0770a 11.11 landed
- 6253159965d5 12.6 landed
- 188cd4f440ed 13.2 landed
- a0efda88a679 14.0 landed
-
Avoid crash with WHERE CURRENT OF and a custom scan plan.
- 33a46993f15a 9.5.25 landed
- ffbf1746354a 9.6.21 landed
- 5cd94de777e0 10.16 landed
- de622e677cfd 11.11 landed
- ba80489c66cf 12.6 landed
- f0f53195b51a 13.2 landed
- 60661bbf2dca 14.0 landed