Re: Fast COPY FROM based on batch insert

From: Andrey Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>
To: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, Zhihong Yu <zyu(at)yugabyte(dot)com>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, tanghy(dot)fnst(at)fujitsu(dot)com, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, houzj(dot)fnst(at)fujitsu(dot)com
Subject: Re: Fast COPY FROM based on batch insert
Date: 2022-10-11 06:06:57
Message-ID: e618cae7-b421-2e1f-d777-38946a669879@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 10/7/22 11:18, Etsuro Fujita wrote:
> On Tue, Sep 27, 2022 at 6:03 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
>> I will review the patch a bit more, but I feel that it is
>> in good shape.
>
> One thing I noticed is this bit added to CopyMultiInsertBufferFlush()
> to run triggers on the foreign table.
>
> + /* Run AFTER ROW INSERT triggers */
> + if (resultRelInfo->ri_TrigDesc != NULL &&
> + (resultRelInfo->ri_TrigDesc->trig_insert_after_row ||
> + resultRelInfo->ri_TrigDesc->trig_insert_new_table))
> + {
> + Oid relid =
> RelationGetRelid(resultRelInfo->ri_RelationDesc);
> +
> + for (i = 0; i < inserted; i++)
> + {
> + TupleTableSlot *slot = rslots[i];
> +
> + /*
> + * AFTER ROW Triggers might reference the tableoid column,
> + * so (re-)initialize tts_tableOid before evaluating them.
> + */
> + slot->tts_tableOid = relid;
> +
> + ExecARInsertTriggers(estate, resultRelInfo,
> + slot, NIL,
> + cstate->transition_capture);
> + }
> + }
>
> Since foreign tables cannot have transition tables, we have
> trig_insert_new_table=false. So I simplified the if test and added an
> assertion ensuring trig_insert_new_table=false. Attached is a new
> version of the patch. I tweaked some comments a bit as well. I think
> the patch is committable. So I plan on committing it next week if
> there are no objections.
I reviewed the patch one more time. Only one question: bistate and
ri_FdwRoutine are strongly bounded. Maybe to add some assertion on
(ri_FdwRoutine XOR bistate) ? Just to prevent possible errors in future.

--
Regards
Andrey Lepikhov
Postgres Professional

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-10-11 06:25:56 Re: PostgreSQL Logical decoding
Previous Message Michael Paquier 2022-10-11 05:49:00 Re: src/test/perl/PostgreSQL/Test/*.pm not installed