Re: [POC] Fast COPY FROM command for the table with foreign partitions

On Wed, 17 Jun 2020 at 11:54, Andrey V. Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
wrote:

> On 6/15/20 10:26 AM, Ashutosh Bapat wrote:
> > Thanks Andrey for the patch. I am glad that the patch has taken care
> > of some corner cases already but there exist still more.
> >
> > COPY command constructed doesn't take care of dropped columns. There
> > is code in deparseAnalyzeSql which constructs list of columns for a
> > given foreign relation. 0002 patch attached here, moves that code to a
> > separate function and reuses it for COPY. If you find that code change
> > useful please include it in the main patch.
>
> Thanks, i included it.
>
> > 2. In the same case, if the foreign table declared locally didn't have
> > any non-dropped columns but the relation that it referred to on the
> > foreign server had some non-dropped columns, COPY command fails. I
> > added a test case for this in 0002 but haven't fixed it.
>
> I fixed it.
> This is very special corner case. The problem was that COPY FROM does
> not support semantics like the "INSERT INTO .. DEFAULT VALUES". To
> simplify the solution, i switched off bulk copying for this case.
>
> > I think this work is useful. Please add it to the next commitfest so
> > that it's tracked.
> Ok.
>

It looks like we call BeginForeignInsert and EndForeignInsert even though
actual copy is performed using BeginForeignCopy, ExecForeignCopy
and EndForeignCopy. BeginForeignInsert constructs the INSERT query which
looks unnecessary. Also some of the other PgFdwModifyState members are
initialized unnecessarily. It also gives an impression that we are using
INSERT underneath the copy. Instead a better way would be to
call BeginForeignCopy instead of BeginForeignInsert and EndForeignCopy
instead of EndForeignInsert, if we are going to use COPY protocol to copy
data to the foreign server. Corresponding postgres_fdw implementations need
to change in order to do that.

This isn't a full review. I will continue reviewing this patch further.
--
Best Wishes,
Ashutosh