Re: Custom Scans and private data

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