On Mon, Jan 18, 2021 at 4:13 PM David Geier <da...@swarm64.com> wrote: > > Hi hackers, > > While working with cursors that reference plans with CustomScanStates > nodes, I encountered a segfault which originates from > search_plan_tree(). The query plan is the result of a simple SELECT > statement into which I inject a Custom Scan node at the root to do some > post-processing before returning rows. This plan is referenced by a > second plan with a Tid Scan which originates from a query of the form > DELETE FROM foo WHERE CURRENT OF my_cursor; > > search_plan_tree() assumes that > CustomScanState::ScanState::ss_currentRelation is never NULL. In my > understanding that only holds for CustomScanState nodes which are at the > bottom of the plan and actually read from a relation. CustomScanState > nodes which are not at the bottom don't have ss_currentRelation set. I > believe for such nodes, instead search_plan_tree() should recurse into > CustomScanState::custom_ps. > > I attached a patch. Any thoughts?
I don't have any comments about your patch as such, but ForeignScan is similar to CustomScan. ForeignScan also can leave ss_currentRelation NULL if it represents join between two foreign tables. So either ForeignScan has the same problem as CustomScan (it's just above CustomScan case in search_plan_tree()) or it's handling it in some other way. In the first case we may want to fix that too in the same manner (not necessarily in the same patch) and the in the later case CustomScan can handle it the same way. Said that, I didn't notice any field in ForeignScan which is parallel to custom_ps, so what you are proposing is still needed. -- Best Wishes, Ashutosh Bapat