From d4b42e5215fae85e2ae473ba4957705f4c9861e9 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 18 Aug 2017 13:01:05 -0400 Subject: [PATCH] Fix interaction of triggers, partitioning, and EXPLAIN ANALYZE. Add a new EState member es_leaf_result_relations, so that the trigger code knows about ResultRelInfos created by tuple routing. Also make sure ExplainPrintTriggers knows about partition-related ResultRelInfos. Etsuro Fujita, reviewed by Amit Langote Discussion: http://postgr.es/m/57163e18-8e56-da83-337a-22f2c0008051@lab.ntt.co.jp --- src/backend/commands/copy.c | 110 +++++++++++++------------ src/backend/commands/explain.c | 15 +++- src/backend/executor/execMain.c | 45 +++++++--- src/backend/executor/execUtils.c | 5 ++ src/backend/executor/nodeModifyTable.c | 1 + src/include/executor/executor.h | 1 + src/include/nodes/execnodes.h | 3 + 7 files changed, 115 insertions(+), 65 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index a258965c20..375a25fbcf 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -1415,59 +1415,6 @@ BeginCopy(ParseState *pstate, (errcode(ERRCODE_UNDEFINED_COLUMN), errmsg("table \"%s\" does not have OIDs", RelationGetRelationName(cstate->rel)))); - - /* - * If there are any triggers with transition tables on the named - * relation, we need to be prepared to capture transition tuples. - */ - cstate->transition_capture = MakeTransitionCaptureState(rel->trigdesc); - - /* Initialize state for CopyFrom tuple routing. */ - if (is_from && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) - { - PartitionDispatch *partition_dispatch_info; - ResultRelInfo *partitions; - TupleConversionMap **partition_tupconv_maps; - TupleTableSlot *partition_tuple_slot; - int num_parted, - num_partitions; - - ExecSetupPartitionTupleRouting(rel, - 1, - &partition_dispatch_info, - &partitions, - &partition_tupconv_maps, - &partition_tuple_slot, - &num_parted, &num_partitions); - cstate->partition_dispatch_info = partition_dispatch_info; - cstate->num_dispatch = num_parted; - cstate->partitions = partitions; - cstate->num_partitions = num_partitions; - cstate->partition_tupconv_maps = partition_tupconv_maps; - cstate->partition_tuple_slot = partition_tuple_slot; - - /* - * If we are capturing transition tuples, they may need to be - * converted from partition format back to partitioned table - * format (this is only ever necessary if a BEFORE trigger - * modifies the tuple). - */ - if (cstate->transition_capture != NULL) - { - int i; - - cstate->transition_tupconv_maps = (TupleConversionMap **) - palloc0(sizeof(TupleConversionMap *) * - cstate->num_partitions); - for (i = 0; i < cstate->num_partitions; ++i) - { - cstate->transition_tupconv_maps[i] = - convert_tuples_by_name(RelationGetDescr(cstate->partitions[i].ri_RelationDesc), - RelationGetDescr(rel), - gettext_noop("could not convert row type")); - } - } - } } else { @@ -2482,6 +2429,63 @@ CopyFrom(CopyState cstate) /* Triggers might need a slot as well */ estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate); + /* + * If there are any triggers with transition tables on the named relation, + * we need to be prepared to capture transition tuples. + */ + cstate->transition_capture = + MakeTransitionCaptureState(cstate->rel->trigdesc); + + /* + * If the named relation is a partitioned table, initialize state for + * CopyFrom tuple routing. + */ + if (cstate->rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) + { + PartitionDispatch *partition_dispatch_info; + ResultRelInfo *partitions; + TupleConversionMap **partition_tupconv_maps; + TupleTableSlot *partition_tuple_slot; + int num_parted, + num_partitions; + + ExecSetupPartitionTupleRouting(cstate->rel, + 1, + estate, + &partition_dispatch_info, + &partitions, + &partition_tupconv_maps, + &partition_tuple_slot, + &num_parted, &num_partitions); + cstate->partition_dispatch_info = partition_dispatch_info; + cstate->num_dispatch = num_parted; + cstate->partitions = partitions; + cstate->num_partitions = num_partitions; + cstate->partition_tupconv_maps = partition_tupconv_maps; + cstate->partition_tuple_slot = partition_tuple_slot; + + /* + * If we are capturing transition tuples, they may need to be + * converted from partition format back to partitioned table format + * (this is only ever necessary if a BEFORE trigger modifies the + * tuple). + */ + if (cstate->transition_capture != NULL) + { + int i; + + cstate->transition_tupconv_maps = (TupleConversionMap **) + palloc0(sizeof(TupleConversionMap *) * cstate->num_partitions); + for (i = 0; i < cstate->num_partitions; ++i) + { + cstate->transition_tupconv_maps[i] = + convert_tuples_by_name(RelationGetDescr(cstate->partitions[i].ri_RelationDesc), + RelationGetDescr(cstate->rel), + gettext_noop("could not convert row type")); + } + } + } + /* * It's more efficient to prepare a bunch of tuples for insertion, and * insert them in one heap_multi_insert() call, than call heap_insert() diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 7648201218..953e74d73c 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -656,17 +656,30 @@ ExplainPrintTriggers(ExplainState *es, QueryDesc *queryDesc) ResultRelInfo *rInfo; bool show_relname; int numrels = queryDesc->estate->es_num_result_relations; + int numrootrels = queryDesc->estate->es_num_root_result_relations; + List *leafrels = queryDesc->estate->es_leaf_result_relations; List *targrels = queryDesc->estate->es_trig_target_relations; int nr; ListCell *l; ExplainOpenGroup("Triggers", "Triggers", false, es); - show_relname = (numrels > 1 || targrels != NIL); + show_relname = (numrels > 1 || numrootrels > 0 || + leafrels != NIL || targrels != NIL); rInfo = queryDesc->estate->es_result_relations; for (nr = 0; nr < numrels; rInfo++, nr++) report_triggers(rInfo, show_relname, es); + rInfo = queryDesc->estate->es_root_result_relations; + for (nr = 0; nr < numrootrels; rInfo++, nr++) + report_triggers(rInfo, show_relname, es); + + foreach(l, leafrels) + { + rInfo = (ResultRelInfo *) lfirst(l); + report_triggers(rInfo, show_relname, es); + } + foreach(l, targrels) { rInfo = (ResultRelInfo *) lfirst(l); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 74071eede6..4582a3caa0 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1365,16 +1365,18 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, * * Get a ResultRelInfo for a trigger target relation. Most of the time, * triggers are fired on one of the result relations of the query, and so - * we can just return a member of the es_result_relations array. (Note: in - * self-join situations there might be multiple members with the same OID; - * if so it doesn't matter which one we pick.) However, it is sometimes - * necessary to fire triggers on other relations; this happens mainly when an - * RI update trigger queues additional triggers on other relations, which will - * be processed in the context of the outer query. For efficiency's sake, - * we want to have a ResultRelInfo for those triggers too; that can avoid - * repeated re-opening of the relation. (It also provides a way for EXPLAIN - * ANALYZE to report the runtimes of such triggers.) So we make additional - * ResultRelInfo's as needed, and save them in es_trig_target_relations. + * we can just return a member of the es_result_relations array, the + * es_root_result_relations array (if any), or the es_leaf_result_relations + * list (if any). (Note: in self-join situations there might be multiple + * members with the same OID; if so it doesn't matter which one we pick.) + * However, it is sometimes necessary to fire triggers on other relations; + * this happens mainly when an RI update trigger queues additional triggers + * on other relations, which will be processed in the context of the outer + * query. For efficiency's sake, we want to have a ResultRelInfo for those + * triggers too; that can avoid repeated re-opening of the relation. (It + * also provides a way for EXPLAIN ANALYZE to report the runtimes of such + * triggers.) So we make additional ResultRelInfo's as needed, and save them + * in es_trig_target_relations. */ ResultRelInfo * ExecGetTriggerResultRel(EState *estate, Oid relid) @@ -1395,6 +1397,23 @@ ExecGetTriggerResultRel(EState *estate, Oid relid) rInfo++; nr--; } + /* Second, search through the root result relations, if any */ + rInfo = estate->es_root_result_relations; + nr = estate->es_num_root_result_relations; + while (nr > 0) + { + if (RelationGetRelid(rInfo->ri_RelationDesc) == relid) + return rInfo; + rInfo++; + nr--; + } + /* Third, search through the leaf result relations, if any */ + foreach(l, estate->es_leaf_result_relations) + { + rInfo = (ResultRelInfo *) lfirst(l); + if (RelationGetRelid(rInfo->ri_RelationDesc) == relid) + return rInfo; + } /* Nope, but maybe we already made an extra ResultRelInfo for it */ foreach(l, estate->es_trig_target_relations) { @@ -3238,6 +3257,7 @@ EvalPlanQualEnd(EPQState *epqstate) void ExecSetupPartitionTupleRouting(Relation rel, Index resultRTindex, + EState *estate, PartitionDispatch **pd, ResultRelInfo **partitions, TupleConversionMap ***tup_conv_maps, @@ -3301,7 +3321,10 @@ ExecSetupPartitionTupleRouting(Relation rel, partrel, resultRTindex, rel, - 0); + estate->es_instrument); + + estate->es_leaf_result_relations = + lappend(estate->es_leaf_result_relations, leaf_part_rri); /* * Open partition indices (remember we do not support ON CONFLICT in diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 25772fc603..c398846879 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -115,6 +115,11 @@ CreateExecutorState(void) estate->es_num_result_relations = 0; estate->es_result_relation_info = NULL; + estate->es_root_result_relations = NULL; + estate->es_num_root_result_relations = 0; + + estate->es_leaf_result_relations = NIL; + estate->es_trig_target_relations = NIL; estate->es_trig_tuple_slot = NULL; estate->es_trig_oldtup_slot = NULL; diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 36b2b43bc6..70a6b847a0 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1919,6 +1919,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) ExecSetupPartitionTupleRouting(rel, node->nominalRelation, + estate, &partition_dispatch_info, &partitions, &partition_tupconv_maps, diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 60326f9d03..eacbea3c36 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -208,6 +208,7 @@ extern void EvalPlanQualSetTuple(EPQState *epqstate, Index rti, extern HeapTuple EvalPlanQualGetTuple(EPQState *epqstate, Index rti); extern void ExecSetupPartitionTupleRouting(Relation rel, Index resultRTindex, + EState *estate, PartitionDispatch **pd, ResultRelInfo **partitions, TupleConversionMap ***tup_conv_maps, diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 577499465d..3272c4b315 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -452,6 +452,9 @@ typedef struct EState ResultRelInfo *es_root_result_relations; /* array of ResultRelInfos */ int es_num_root_result_relations; /* length of the array */ + /* Info about leaf partitions of partitioned table(s) for insert queries: */ + List *es_leaf_result_relations; /* List of ResultRelInfos */ + /* Stuff used for firing triggers: */ List *es_trig_target_relations; /* trigger-only ResultRelInfos */ TupleTableSlot *es_trig_tuple_slot; /* for trigger output tuples */ -- 2.40.0