From 98a6f077cd3b694683ec0e4a3250c040cc33cb39 Mon Sep 17 00:00:00 2001 From: Andrey Lepikhov Date: Mon, 14 Dec 2020 11:29:03 +0500 Subject: [PATCH 1/2] Move multi-insert decision logic into executor When 0d5f05cde introduced support for using multi-insert mode when copying into partitioned tables, it introduced single variable of enum type CopyInsertMethod shared across all potential target relations (partitions) that, along with some target relation properties, dictated whether to engage multi-insert mode for a given target relation. Move that decision logic into InitResultRelInfo which now sets a new boolean field ri_usesMultiInsert of ResultRelInfo when a target relation is first initialized. That prevents repeated computation of the same information in some cases, especially for partitions, and the new arrangement results in slightly more readability. --- src/backend/commands/copyfrom.c | 142 ++++++----------------- src/backend/executor/execMain.c | 52 +++++++++ src/backend/executor/execPartition.c | 7 ++ src/include/commands/copyfrom_internal.h | 10 -- src/include/executor/execPartition.h | 2 + src/include/executor/executor.h | 2 + src/include/nodes/execnodes.h | 8 +- 7 files changed, 108 insertions(+), 115 deletions(-) diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 1b14e9a6eb..6d4f6cb80d 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -535,12 +535,10 @@ CopyFrom(CopyFromState cstate) CommandId mycid = GetCurrentCommandId(true); int ti_options = 0; /* start with default options for insert */ BulkInsertState bistate = NULL; - CopyInsertMethod insertMethod; CopyMultiInsertInfo multiInsertInfo = {0}; /* pacify compiler */ uint64 processed = 0; bool has_before_insert_row_trig; bool has_instead_insert_row_trig; - bool leafpart_use_multi_insert = false; Assert(cstate->rel); Assert(list_length(cstate->range_table) == 1); @@ -650,6 +648,30 @@ CopyFrom(CopyFromState cstate) resultRelInfo = target_resultRelInfo = makeNode(ResultRelInfo); ExecInitResultRelation(estate, resultRelInfo, 1); + Assert(target_resultRelInfo->ri_usesMultiInsert == false); + + /* + * It's generally more efficient to prepare a bunch of tuples for + * insertion, and insert them in bulk, for example, with one + * table_multi_insert() call than call table_tuple_insert() separately for + * every tuple. However, there are a number of reasons why we might not be + * able to do this. For example, if there any volatile expressions in the + * table's default values or in the statement's WHERE clause, which may + * query the table we are inserting into, buffering tuples might produce + * wrong results. Also, the relation we are trying to insert into itself + * may not be amenable to buffered inserts. + * + * Note: For partitions, this flag is set considering the target table's + * flag that is being set here and partition's own properties which are + * checked by calling ExecSetRelationUsesMultiInsert(). It does not matter + * whether partitions have any volatile default expressions as we use the + * defaults from the target of the COPY command. + */ + if (!cstate->volatile_defexprs && + !contain_volatile_functions(cstate->whereClause)) + target_resultRelInfo->ri_usesMultiInsert = + ExecSetRelationUsesMultiInsert(target_resultRelInfo, NULL); + /* Verify the named relation is a valid target for INSERT */ CheckValidResultRel(resultRelInfo, CMD_INSERT); @@ -665,6 +687,10 @@ CopyFrom(CopyFromState cstate) mtstate->operation = CMD_INSERT; mtstate->resultRelInfo = resultRelInfo; + /* + * Init copying process into foreign table. Initialization of copying into + * foreign partitions will be done later. + */ if (resultRelInfo->ri_FdwRoutine != NULL && resultRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL) resultRelInfo->ri_FdwRoutine->BeginForeignInsert(mtstate, @@ -697,83 +723,9 @@ CopyFrom(CopyFromState cstate) cstate->qualexpr = ExecInitQual(castNode(List, cstate->whereClause), &mtstate->ps); - /* - * It's generally more efficient to prepare a bunch of tuples for - * insertion, and insert them in one table_multi_insert() call, than call - * table_tuple_insert() separately for every tuple. However, there are a - * number of reasons why we might not be able to do this. These are - * explained below. - */ - if (resultRelInfo->ri_TrigDesc != NULL && - (resultRelInfo->ri_TrigDesc->trig_insert_before_row || - resultRelInfo->ri_TrigDesc->trig_insert_instead_row)) - { - /* - * Can't support multi-inserts when there are any BEFORE/INSTEAD OF - * triggers on the table. Such triggers might query the table we're - * inserting into and act differently if the tuples that have already - * been processed and prepared for insertion are not there. - */ - insertMethod = CIM_SINGLE; - } - else if (proute != NULL && resultRelInfo->ri_TrigDesc != NULL && - resultRelInfo->ri_TrigDesc->trig_insert_new_table) - { - /* - * For partitioned tables we can't support multi-inserts when there - * are any statement level insert triggers. It might be possible to - * allow partitioned tables with such triggers in the future, but for - * now, CopyMultiInsertInfoFlush expects that any before row insert - * and statement level insert triggers are on the same relation. - */ - insertMethod = CIM_SINGLE; - } - else if (resultRelInfo->ri_FdwRoutine != NULL || - cstate->volatile_defexprs) - { - /* - * Can't support multi-inserts to foreign tables or if there are any - * volatile default expressions in the table. Similarly to the - * trigger case above, such expressions may query the table we're - * inserting into. - * - * Note: It does not matter if any partitions have any volatile - * default expressions as we use the defaults from the target of the - * COPY command. - */ - insertMethod = CIM_SINGLE; - } - else if (contain_volatile_functions(cstate->whereClause)) - { - /* - * Can't support multi-inserts if there are any volatile function - * expressions in WHERE clause. Similarly to the trigger case above, - * such expressions may query the table we're inserting into. - */ - insertMethod = CIM_SINGLE; - } - else - { - /* - * For partitioned tables, we may still be able to perform bulk - * inserts. However, the possibility of this depends on which types - * of triggers exist on the partition. We must disable bulk inserts - * if the partition is a foreign table or it has any before row insert - * or insert instead triggers (same as we checked above for the parent - * table). Since the partition's resultRelInfos are initialized only - * when we actually need to insert the first tuple into them, we must - * have the intermediate insert method of CIM_MULTI_CONDITIONAL to - * flag that we must later determine if we can use bulk-inserts for - * the partition being inserted into. - */ - if (proute) - insertMethod = CIM_MULTI_CONDITIONAL; - else - insertMethod = CIM_MULTI; - + if (resultRelInfo->ri_usesMultiInsert) CopyMultiInsertInfoInit(&multiInsertInfo, resultRelInfo, cstate, estate, mycid, ti_options); - } /* * If not using batch mode (which allocates slots as needed) set up a @@ -781,7 +733,7 @@ CopyFrom(CopyFromState cstate) * one, even if we might batch insert, to read the tuple in the root * partition's form. */ - if (insertMethod == CIM_SINGLE || insertMethod == CIM_MULTI_CONDITIONAL) + if (!resultRelInfo->ri_usesMultiInsert || proute) { singleslot = table_slot_create(resultRelInfo->ri_RelationDesc, &estate->es_tupleTable); @@ -824,7 +776,7 @@ CopyFrom(CopyFromState cstate) ResetPerTupleExprContext(estate); /* select slot to (initially) load row into */ - if (insertMethod == CIM_SINGLE || proute) + if (!target_resultRelInfo->ri_usesMultiInsert || proute) { myslot = singleslot; Assert(myslot != NULL); @@ -832,7 +784,6 @@ CopyFrom(CopyFromState cstate) else { Assert(resultRelInfo == target_resultRelInfo); - Assert(insertMethod == CIM_MULTI); myslot = CopyMultiInsertInfoNextFreeSlot(&multiInsertInfo, resultRelInfo); @@ -891,24 +842,14 @@ CopyFrom(CopyFromState cstate) has_instead_insert_row_trig = (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_instead_row); - /* - * Disable multi-inserts when the partition has BEFORE/INSTEAD - * OF triggers, or if the partition is a foreign partition. - */ - leafpart_use_multi_insert = insertMethod == CIM_MULTI_CONDITIONAL && - !has_before_insert_row_trig && - !has_instead_insert_row_trig && - resultRelInfo->ri_FdwRoutine == NULL; - /* Set the multi-insert buffer to use for this partition. */ - if (leafpart_use_multi_insert) + if (resultRelInfo->ri_usesMultiInsert) { if (resultRelInfo->ri_CopyMultiInsertBuffer == NULL) CopyMultiInsertInfoSetupBuffer(&multiInsertInfo, resultRelInfo); } - else if (insertMethod == CIM_MULTI_CONDITIONAL && - !CopyMultiInsertInfoIsEmpty(&multiInsertInfo)) + else if (!CopyMultiInsertInfoIsEmpty(&multiInsertInfo)) { /* * Flush pending inserts if this partition can't use @@ -938,7 +879,7 @@ CopyFrom(CopyFromState cstate) * rowtype. */ map = resultRelInfo->ri_RootToPartitionMap; - if (insertMethod == CIM_SINGLE || !leafpart_use_multi_insert) + if (!resultRelInfo->ri_usesMultiInsert) { /* non batch insert */ if (map != NULL) @@ -957,9 +898,6 @@ CopyFrom(CopyFromState cstate) */ TupleTableSlot *batchslot; - /* no other path available for partitioned table */ - Assert(insertMethod == CIM_MULTI_CONDITIONAL); - batchslot = CopyMultiInsertInfoNextFreeSlot(&multiInsertInfo, resultRelInfo); @@ -1031,7 +969,7 @@ CopyFrom(CopyFromState cstate) ExecPartitionCheck(resultRelInfo, myslot, estate, true); /* Store the slot in the multi-insert buffer, when enabled. */ - if (insertMethod == CIM_MULTI || leafpart_use_multi_insert) + if (resultRelInfo->ri_usesMultiInsert) { /* * The slot previously might point into the per-tuple @@ -1107,11 +1045,8 @@ CopyFrom(CopyFromState cstate) } /* Flush any remaining buffered tuples */ - if (insertMethod != CIM_SINGLE) - { - if (!CopyMultiInsertInfoIsEmpty(&multiInsertInfo)) - CopyMultiInsertInfoFlush(&multiInsertInfo, NULL); - } + if (!CopyMultiInsertInfoIsEmpty(&multiInsertInfo)) + CopyMultiInsertInfoFlush(&multiInsertInfo, NULL); /* Done, clean up */ error_context_stack = errcallback.previous; @@ -1143,8 +1078,7 @@ CopyFrom(CopyFromState cstate) target_resultRelInfo); /* Tear down the multi-insert buffer data */ - if (insertMethod != CIM_SINGLE) - CopyMultiInsertInfoCleanup(&multiInsertInfo); + CopyMultiInsertInfoCleanup(&multiInsertInfo); /* Close all the partitioned tables, leaf partitions, and their indices */ if (proute) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 7179f589f9..9809c03a8e 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1248,6 +1248,58 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, resultRelInfo->ri_PartitionTupleSlot = NULL; /* ditto */ resultRelInfo->ri_ChildToRootMap = NULL; resultRelInfo->ri_CopyMultiInsertBuffer = NULL; + + /* Define multi-insert mode possibility later if needed */ + resultRelInfo->ri_usesMultiInsert = false; +} + +/* + * ExecSetRelationUsesMultiInsert + * Does this relation allow caller to use multi-insert mode when + * inserting rows into it? + */ +bool +ExecSetRelationUsesMultiInsert(const ResultRelInfo *rri, + const ResultRelInfo *partition_root) +{ + Assert(rri->ri_usesMultiInsert == false); + + /* + * If a partition's root parent isn't allowed to use it, neither is the + * partition. + */ + if (partition_root && !partition_root->ri_usesMultiInsert) + return false; + + /* + * Can't support multi-inserts when there are any BEFORE/INSTEAD OF + * triggers on the table. Such triggers might query the table we're + * inserting into and act differently if the tuples that have already + * been processed and prepared for insertion are not there. + */ + if (rri->ri_TrigDesc != NULL && + (rri->ri_TrigDesc->trig_insert_before_row || + rri->ri_TrigDesc->trig_insert_instead_row)) + return false; + + /* + * For partitioned tables we can't support multi-inserts when there are + * any statement level insert triggers. It might be possible to allow + * partitioned tables with such triggers in the future, but for now, + * CopyMultiInsertInfoFlush expects that any before row insert and + * statement level insert triggers are on the same relation. + */ + if (rri->ri_RelationDesc->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && + rri->ri_TrigDesc != NULL && + rri->ri_TrigDesc->trig_insert_new_table) + return false; + + /* Foreign tables don't support multi-inserts. */ + if (rri->ri_FdwRoutine != NULL) + return false; + + /* OK, caller can use multi-insert on this relation. */ + return true; } /* diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 86594bd056..5a201dfbfa 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -587,6 +587,13 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, rootrel, estate->es_instrument); + /* + * Use multi-insert mode if the condition checking passes for the + * parent and its child. + */ + leaf_part_rri->ri_usesMultiInsert = + ExecSetRelationUsesMultiInsert(leaf_part_rri, rootResultRelInfo); + /* * Verify result relation is a valid target for an INSERT. An UPDATE of a * partition-key becomes a DELETE+INSERT operation, so this check is still diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h index c15ea803c3..7a948f7e63 100644 --- a/src/include/commands/copyfrom_internal.h +++ b/src/include/commands/copyfrom_internal.h @@ -40,16 +40,6 @@ typedef enum EolType EOL_CRNL } EolType; -/* - * Represents the heap insert method to be used during COPY FROM. - */ -typedef enum CopyInsertMethod -{ - CIM_SINGLE, /* use table_tuple_insert or fdw routine */ - CIM_MULTI, /* always use table_multi_insert */ - CIM_MULTI_CONDITIONAL /* use table_multi_insert only if valid */ -} CopyInsertMethod; - /* * This struct contains all the state variables used throughout a COPY FROM * operation. diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h index 473c4cd84f..87b6693fe6 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -118,6 +118,8 @@ extern ResultRelInfo *ExecFindPartition(ModifyTableState *mtstate, PartitionTupleRouting *proute, TupleTableSlot *slot, EState *estate); +extern bool checkMultiInsertMode(const ResultRelInfo *rri, + const ResultRelInfo *parent); extern void ExecCleanupTupleRouting(ModifyTableState *mtstate, PartitionTupleRouting *proute); extern PartitionPruneState *ExecCreatePartitionPruneState(PlanState *planstate, diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 0c48d2a519..ad6eccc028 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -193,6 +193,8 @@ extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Index resultRelationIndex, Relation partition_root, int instrument_options); +extern bool ExecSetRelationUsesMultiInsert(const ResultRelInfo *rri, + const ResultRelInfo *partition_root); extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid); extern void ExecConstraints(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 61ba4c3666..b6f4f2626b 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -498,7 +498,13 @@ typedef struct ResultRelInfo */ TupleConversionMap *ri_ChildToRootMap; - /* for use by copyfrom.c when performing multi-inserts */ + /* + * The following fields are currently only relevant to copyfrom.c. + * True if okay to use multi-insert on this relation + */ + bool ri_usesMultiInsert; + + /* Buffer allocated to this relation when using multi-insert mode */ struct CopyMultiInsertBuffer *ri_CopyMultiInsertBuffer; } ResultRelInfo; -- 2.25.1