> > On Tue, Mar 14, 2023 at 7:07 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > > * I decided to initialize QueryDesc.planstate even in the cases where
> > > ExecInitNode() traversal is aborted in the middle on detecting
> > > CachedPlan invalidation such that it points to a partially initialized
> > > PlanState tree. My earlier thinking that each PlanState node need not
> > > be visited for resource cleanup in such cases was naive after all. To
> > > that end, I've fixed the ExecEndNode() subroutines of all Plan node
> > > types to account for potentially uninitialized fields. There are a
> > > couple of cases where I'm a bit doubtful though. In
> > > ExecEndCustomScan(), there's no indication in CustomScanState whether
> > > it's OK to call EndCustomScan() when BeginCustomScan() may not have
> > > been called. For ForeignScanState, I've assumed that
> > > ForeignScanState.fdw_state being set can be used as a marker that
> > > BeginForeignScan would have been called, though maybe that's not a
> > > solid assumption.
It seems I hadn't noted in the ExecEndNode()'s comment that all node
types' recursive subroutines need to handle the change made by this
patch that the corresponding ExecInitNode() subroutine may now return
early without having initialized all state struct fields.
Also noted in the documentation for CustomScan and ForeignScan that
the Begin*Scan callback may not have been called at all, so the
End*Scan should handle that gracefully.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
