On 2015-08-26 00:55:48 +0000, Kouhei Kaigai wrote:
> As Tom pointed out, the primary reason why CustomScan required provider
> to save its private data on custom_exprs/custom_private were awareness
> of copyObject().
Well, a callback brings that with it as well. I do think it makes sense
to *allow* not to have a callback and rely on copyObject() to do the
work.
> In addition, custom_exprs are expected to link expression node to be
> processed in setrefs.c and subselect.c because core implementation
> cannot know which type of data is managed in private.
Right.
> Do you concern about custom_private only?
Yes, pretty much. There's very little chance you can expand the
expression tree with custom expressions and survive the experience. So
custom_exprs etc. makes sense.
> Even if we have extra
> callbacks like CopyObjectCustomScan() and TextReadCustomScan(),
> how do we care about the situation when core implementation needs to
> know the location of expression nodes? Is custom_exprs retained as is?
Yes.
> In the earlier version of CustomScan interface had individual
> callbacks on setrefs.c and subselect.c, however, its structure
> depended on the current implementation too much, then we moved
> to the implementation to have two individual private fields
> rather than callbacks on outfuncs.c.
I agree with that choice.
> On the other hands, I'm inclined to think TextOutCustomScan() might
> be a misdesign to support serialize/deserialize via readfuncs.c.
> http://www.postgresql.org/message-id/9A28C8860F777E439AA12E8AEA7694F80111D04F@BPXM15GP.gisp.nec.co.jp
> I think it shall be deprecated rather then enhancement.
Well, right now there's no support for reading/writing plans at all. But
if we add it, TextOutCustomScan() seems like a small problem in
comparison to others. CustomScan contains pointers, that's definitely
not something you can ship over the wire and expect to work. We'll
probably have to store a soname + function name instead.
More generally I rather doubt that it'll always make sense to
serialize/deserialize in a generic manner between different backends. It
very well can make sense to refer to backend-local state in a plan - you
need to be able to take that into account upon
serialization/deserialization. Consider e.g. a connection id for an FDW
that uses pooling.
Greetings,
Andres Freund
