]> granicus.if.org Git - postgresql/commitdiff
Fix missing calls to table_finish_bulk_insert during COPY, take 2
authorDavid Rowley <drowley@postgresql.org>
Wed, 10 Jul 2019 04:03:04 +0000 (16:03 +1200)
committerDavid Rowley <drowley@postgresql.org>
Wed, 10 Jul 2019 04:03:04 +0000 (16:03 +1200)
86b85044e abstracted calls to heap functions in COPY FROM to support a
generic table AM.  However, when performing a copy into a partitioned
table, this commit neglected to call table_finish_bulk_insert for each
partition.  Before 86b85044e, when we always called the heap functions,
there was no need to call heapam_finish_bulk_insert for partitions since
it only did any work when performing a copy without WAL.  For partitioned
tables, this was unsupported anyway, so there was no issue.  With
pluggable storage, we can't make any assumptions about what the table AM
might want to do in its equivalent function, so we'd better ensure we
always call table_finish_bulk_insert each partition that's received a row.

For now, we make the table_finish_bulk_insert call whenever we evict a
CopyMultiInsertBuffer out of the CopyMultiInsertInfo.  This does mean
that it's possible that we call table_finish_bulk_insert multiple times
per partition, which is not a problem other than being an inefficiency.
Improving this requires a more invasive patch, so let's leave that for
another day.

This also changes things so that we no longer needlessly call
table_finish_bulk_insert when performing a COPY FROM for a non-partitioned
table when not using multi-inserts.

Reported-by: Robert Haas
Backpatch-through: 12
Discussion: https://postgr.es/m/CA+TgmoYK=6BpxiJ0tN-p9wtH0BTAfbdxzHhwou0mdud4+BkYuQ@mail.gmail.com

src/backend/commands/copy.c

index f1161f0fee16d26142fd1abb323756722d00fc39..4f04d122c3041ebfd6df07e685defc66a275d153 100644 (file)
@@ -2518,7 +2518,8 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo,
  * The buffer must be flushed before cleanup.
  */
 static inline void
-CopyMultiInsertBufferCleanup(CopyMultiInsertBuffer *buffer)
+CopyMultiInsertBufferCleanup(CopyMultiInsertInfo *miinfo,
+                                                        CopyMultiInsertBuffer *buffer)
 {
        int                     i;
 
@@ -2534,6 +2535,9 @@ CopyMultiInsertBufferCleanup(CopyMultiInsertBuffer *buffer)
        for (i = 0; i < MAX_BUFFERED_TUPLES && buffer->slots[i] != NULL; i++)
                ExecDropSingleTupleTableSlot(buffer->slots[i]);
 
+       table_finish_bulk_insert(buffer->resultRelInfo->ri_RelationDesc,
+                                                        miinfo->ti_options);
+
        pfree(buffer);
 }
 
@@ -2585,7 +2589,7 @@ CopyMultiInsertInfoFlush(CopyMultiInsertInfo *miinfo, ResultRelInfo *curr_rri)
                        buffer = (CopyMultiInsertBuffer *) linitial(miinfo->multiInsertBuffers);
                }
 
-               CopyMultiInsertBufferCleanup(buffer);
+               CopyMultiInsertBufferCleanup(miinfo, buffer);
                miinfo->multiInsertBuffers = list_delete_first(miinfo->multiInsertBuffers);
        }
 }
@@ -2599,7 +2603,7 @@ CopyMultiInsertInfoCleanup(CopyMultiInsertInfo *miinfo)
        ListCell   *lc;
 
        foreach(lc, miinfo->multiInsertBuffers)
-               CopyMultiInsertBufferCleanup(lfirst(lc));
+               CopyMultiInsertBufferCleanup(miinfo, lfirst(lc));
 
        list_free(miinfo->multiInsertBuffers);
 }
@@ -3321,9 +3325,6 @@ CopyFrom(CopyState cstate)
        {
                if (!CopyMultiInsertInfoIsEmpty(&multiInsertInfo))
                        CopyMultiInsertInfoFlush(&multiInsertInfo, NULL);
-
-               /* Tear down the multi-insert buffer data */
-               CopyMultiInsertInfoCleanup(&multiInsertInfo);
        }
 
        /* Done, clean up */
@@ -3355,6 +3356,10 @@ CopyFrom(CopyState cstate)
                target_resultRelInfo->ri_FdwRoutine->EndForeignInsert(estate,
                                                                                                                          target_resultRelInfo);
 
+       /* Tear down the multi-insert buffer data */
+       if (insertMethod != CIM_SINGLE)
+               CopyMultiInsertInfoCleanup(&multiInsertInfo);
+
        ExecCloseIndices(target_resultRelInfo);
 
        /* Close all the partitioned tables, leaf partitions, and their indices */
@@ -3366,8 +3371,6 @@ CopyFrom(CopyState cstate)
 
        FreeExecutorState(estate);
 
-       table_finish_bulk_insert(cstate->rel, ti_options);
-
        return processed;
 }