Re: Parallel Seq Scan

On Wed, Sep 23, 2015 at 6:42 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>
> > > On Thu, Sep 17, 2015 at 4:44 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
wrote:
>
> > 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
> field...)
> Sorry, I cannot understand the sentence of funcid population. It seems to
me
> funcid is displayed as-is, and _readFuncExpr() does nothing special...?
>

Here I am referring to operator's funcid (refer function _readOpExpr()).
It is
hard-coded to InvalidOid during read. Currently fix_opfuncids() is used to
fill the funcid for expressions, now I am not sure if it can be used for
fdw_private data (I have tried it, but it was failing, may be it is not at
all
required to fix any funcid for it) or other fields in Foreign Scan.

I have observed that in out functions, we output fdw_private field which
indicates that ideally we should be able to read it.

>
> 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);
> >
> > And similarly for READ_OID_ARRAY, READ_BOOL_ARRAY, READ_INT_ARRAY.
> >
> Like this?
>
https://github.com/kaigai/sepgsql/blob/readfuncs/src/backend/nodes/readfuncs.c#L133
>
> 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.
>

Yes, I have also noticed the same and thought of sending a patch which I
have done just before sending this mail.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com