> > On Thu, Sep 17, 2015 at 4:44 PM, Robert Haas <robertmh...@gmail.com> wrote:
> > >
> > > I haven't studied the planner logic in enough detail yet to have a
> > > clear opinion on this.  But what I do think is that this is a very
> > > good reason why we should bite the bullet and add outfuncs/readfuncs
> > > support for all Plan nodes.  Otherwise, we're going to have to scan
> > > subplans for nodes we're not expecting to see there, which seems
> > > silly.  We eventually want to allow all of those nodes in the worker
> > > anyway.
> > >
> >
> > makes sense to me.  There are 39 plan nodes and it seems we have
> > support for all of them in outfuncs and needs to add for most of them
> > in readfuncs.
> >
> Attached patch (read_funcs_v1.patch) contains support for all the plan
> and other nodes (like SubPlan which could be required for worker) except
> CustomScan node.  CustomScan contains TextOutCustomScan and doesn't
> contain corresponding Read function pointer, we could add the support for
> same, but I am not sure if CustomScan is required to be passed to worker
> in near future, so I am leaving it for now.
Oh... I did exactly duplicated job a few days before.

Regarding of CustomScan node, I'd like to run on worker process as soon as
possible once it gets supported. I'm highly motivated.

Andres raised a related topic a few weeks before:

Here are two issues:

* How to reproduce "methods" pointer on another process. Extension may not be
  loaded via shared_preload_libraries.
-> One solution is to provide a pair of library and symbol name of the method
   table, instead of the pointer. I think it is a reasonable idea.

* How to treat additional output of TextOutCustomScan.
-> Here are two solutions. (1) Mark TextOutCustomScan as an obsolete callback,
   however, it still makes Andres concern because we need to form/deform private
   data for copyObject safe. (2) Add TextReadCustomScan (and 
   callback to process private fields.

> To verify the patch, I have done 2 things, first I have added elog to
> the newly supported read funcs and then in planner, I have used
> nodeToString and stringToNode on planned_stmt and then used the
> newly generated planned_stmt for further execution.  After making these
> changes, I have ran make check-world and ensures that it covers all the
> newly added nodes.
> Note, that as we don't populate funcid's in expressions during read, the
> same has to be updated by traversing the tree and updating in different
> expressions based on node type.  Attached patch (read_funcs_test_v1)
> contains the changes required for testing the patch.  I am not very sure
> about what do about some of the ForeignScan fields (fdw_private) in order
> to update the funcid as the data in those expressions could be FDW specific.
> This is anyway for test, so doesn't matter much, but the same will be
> required to support read of ForeignScan node by worker.
Because of interface contract, it is role of FDW driver to put nodes which
are safe to copyObject on fdw_exprs and fdw_private field. Unless FDW driver
does not violate, fdw_exprs and fdw_private shall be reproduced on the worker
side. (Of course, we cannot guarantee nobody has local pointer on private
Sorry, I cannot understand the sentence of funcid population. It seems to me
funcid is displayed as-is, and _readFuncExpr() does nothing special...?

Robert Haas said:
> I think it would be worth doing something like this:
> #define READ_ATTRNUMBER_ARRAY(fldname, len) \
>     token = pg_strtok(&length); \
>     local_node->fldname = readAttrNumberCols(len);
Like this?

I think outfuncs.c also have similar macro to centralize the format of array.
Actually, most of boolean array are displayed using booltostr(), however, only
_outMergeJoin() uses "%d" format to display boolean as integer.
It is a bit inconsistent manner.

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

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to