Horton

Thread

Commits

  1. Update comment in ExecInsert() regarding batch insertion.

  2. Implement support for bulk inserts in postgres_fdw

  1. Obsolete comment in ExecInsert()

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2022-09-28T10:25:12Z

    Hi,

While reviewing the “Fast COPY FROM based on batch insert" patch, I
noticed this comment introduced in commit b663a4136:

            /*
             * If a certain number of tuples have already been accumulated, or
             * a tuple has come for a different relation than that for the
             * accumulated tuples, perform the batch insert
             */
            if (resultRelInfo->ri_NumSlots == resultRelInfo->ri_BatchSize)
            {
                ExecBatchInsert(mtstate, resultRelInfo,
                                resultRelInfo->ri_Slots,
                                resultRelInfo->ri_PlanSlots,
                                resultRelInfo->ri_NumSlots,
                                estate, canSetTag);
                resultRelInfo->ri_NumSlots = 0;
            }

I think the “or a tuple has come for a different relation than that
for the accumulated tuples" part in the comment is a leftover from an
earlier version of the patch [1].  As the code shows, we do not handle
that case anymore, so I think we should remove that part from the
comment.  Attached is a patch for that.

Best regards,
Etsuro Fujita

[1] https://www.postgresql.org/message-id/TYAPR01MB2990ECD1C68EA694DD0667E4FEE90%40TYAPR01MB2990.jpnprd01.prod.outlook.com

  2. Re: Obsolete comment in ExecInsert()

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-28T14:42:21Z

    Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> I think the “or a tuple has come for a different relation than that
> for the accumulated tuples" part in the comment is a leftover from an
> earlier version of the patch [1].  As the code shows, we do not handle
> that case anymore, so I think we should remove that part from the
> comment.  Attached is a patch for that.

+1, but what remains still seems awkwardly worded.  How about something
like "When we've reached the desired batch size, perform the insertion"?

			regards, tom lane

  3. Re: Obsolete comment in ExecInsert()

    Etsuro Fujita <etsuro.fujita@gmail.com> — 2022-09-29T08:10:09Z

    Hi,

On Wed, Sep 28, 2022 at 11:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Etsuro Fujita <etsuro.fujita@gmail.com> writes:
> > I think the “or a tuple has come for a different relation than that
> > for the accumulated tuples" part in the comment is a leftover from an
> > earlier version of the patch [1].  As the code shows, we do not handle
> > that case anymore, so I think we should remove that part from the
> > comment.  Attached is a patch for that.
>
> +1, but what remains still seems awkwardly worded.  How about something
> like "When we've reached the desired batch size, perform the insertion"?

+1 for that change.  Pushed that way.

Thanks for reviewing!

Best regards,
Etsuro Fujita