Re: Logical Replication WIP

On 14/09/16 20:50, Andres Freund wrote:
> 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.
>

I'd say in common case they will. I don't plan to use these forever btw,
but it's simplest to just use them in v1 IMHO instead of trying to
reinvent new versions of these that perform better but also behave
correctly (in terms of triggers and stuff for example).

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services