Re: planstate_tree_walker oversight CustomScan - Mailing list pgsql-hackers

From Kouhei Kaigai
Subject Re: planstate_tree_walker oversight CustomScan
Date
Msg-id 9A28C8860F777E439AA12E8AEA7694F8011495EB@BPXM15GP.gisp.nec.co.jp
Whole thread Raw
In response to Re: planstate_tree_walker oversight CustomScan  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: planstate_tree_walker oversight CustomScan  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
> -----Original Message-----
> From: pgsql-hackers-owner@postgresql.org
> [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Robert Haas
> Sent: Wednesday, September 23, 2015 10:15 AM
> To: Kaigai Kouhei(海外 浩平)
> Cc: pgsql-hackers@postgresql.org
> Subject: Re: [HACKERS] planstate_tree_walker oversight CustomScan
> 
> On Mon, Sep 21, 2015 at 9:54 AM, Kouhei Kaigai <kaigai@ak.jp.nec.com> wrote:
> > The planstate_tree_walker() oversight custom_ps of CustomScanState;
> > that should be a list of underlying PlanState object if any.
> >
> > ExplainPreScanNode() treated ForeignScan and CustomScan in special
> > way (it is sufficient for ExplainPreScanNode() purpose), thus, it
> > didn't implement its recursive portion originally.
> >
> > The job of ExplainPreScanNode() is know all the relids involved
> > in a particular subquery execution. On the other hands, fs_relids
> > of ForeignScan and custom_relids of CustomScan informs a set of
> > relids to be scanned by this Scan node without recursive, so it
> > did not have recursive walks on the underlying sub-plans.
> >
> > However, planstate_tree_walker() will have different expectation.
> > It is general walker routine, thus, it is natural users to expect
> > the callback is also kicked towards the underlying planstate of
> > CustomScan (and ForeignScan; once EPQ recheck gets solved).
> >
> > The attached patch adds support of CustomScan on the walker.
> 
> Do you need to add something to ExplainPreScanNode for this as well,
> like if (scanrelid != 0) *rels_used = bms_add_member(*rels_used,
> scanrelid)?
>
The latest ExplainPreScanNode is sufficient. Regardless of scanrelid
(even if it is zero), fs_relids and custom_relids shall be set properly
to introduce which relations are represented by this ForeignScan and
CustomScan node. So, additional planstate_tree_walker() call might be
a bit redundant, but harmless.

The reason why ForeignScan/CustomScan node have these bitmap is, we
cannot guarantee they always have underlying scan node. For example,
ForeignScan that kicks remote join query will not have local underlying
scan node on the foreign tables to be involved.
So, we had to inform ExplainPreScanNode which relids are represented
by this Foreign/CustomScan node.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Parallel Seq Scan
Next
From: Robert Haas
Date:
Subject: Re: a funnel by any other name