Re: Logical Replication WIP

On 2016-09-14 13:20:02 -0500, Peter Eisentraut wrote:
> On 9/14/16 11:21 AM, Andres Freund wrote:
> >> + ExecInsert(NULL, /* mtstate is only used for onconflict handling which we don't support atm */
> >> > + remoteslot,
> >> > + remoteslot,
> >> > + NIL,
> >> > + ONCONFLICT_NONE,
> >> > + estate,
> >> > + false);
> > I have *severe* doubts about just using the (newly) exposed functions
> > 1:1 here.
>
> It is a valid concern, but what is the alternative? ExecInsert() and
> the others appear to do exactly the right things that are required.

They're actually a lot more heavyweight than what's required. If you
e.g. do a large COPY on the source side, we create a single executor
state (if at all), and then insert the rows using lower level
routines. And that's *vastly* faster, than going through all the setup
costs here for each row.

> Are your concerns mainly philosophical about calling into internal
> executor code, or do you have technical concerns that this will not do
> the right thing in some cases?

Well, not about it being wrong in the sene of returning wrong results,
but wrong in the sense of not even remotely being able to keep up in
common cases.

Andres