Re: generic plans and "initial" pruning

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, David Rowley <dgrowleyml(at)gmail(dot)com>, Jacob Champion <jchampion(at)timescale(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: generic plans and "initial" pruning
Date: 2023-03-27 14:00:38
Message-ID: CA+HiwqHoZSM4A0HKoTERmp=_stQjpjmomgg=rCf_4x4qCpxbZA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> > On Tue, Mar 14, 2023 at 7:07 PM Amit Langote <amitlangote09(at)gmail(dot)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

Attachment Content-Type Size
v38-0001-Add-field-to-store-partitioned-relids-to-Append-.patch application/octet-stream 20.2 KB
v38-0003-Track-opened-range-table-relations-in-a-List-in-.patch application/octet-stream 2.3 KB
v38-0002-Move-AcquireExecutorLocks-s-responsibility-into-.patch application/octet-stream 134.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melanie Plageman 2023-03-27 14:01:03 Re: Memory leak from ExecutorState context?
Previous Message Peter Eisentraut 2023-03-27 13:55:27 Re: cataloguing NOT NULL constraints