diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 025f922b4c..32ceddd4ba 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -2021,7 +2021,7 @@ postgresBeginForeignInsert(ModifyTableState *mtstate, PgFdwModifyState *fmstate; ModifyTable *plan = castNode(ModifyTable, mtstate->ps.plan); EState *estate = mtstate->ps.state; - Index resultRelation = resultRelInfo->ri_RangeTableIndex; + Index resultRelation; Relation rel = resultRelInfo->ri_RelationDesc; RangeTblEntry *rte; TupleDesc tupdesc = RelationGetDescr(rel); @@ -2072,7 +2072,8 @@ postgresBeginForeignInsert(ModifyTableState *mtstate, } /* - * If the foreign table is a partition, we need to create a new RTE + * If the foreign table is a partition that doesn't have a corresponding + * RTE entry, we need to create a new RTE * describing the foreign table for use by deparseInsertSql and * create_foreign_modify() below, after first copying the parent's RTE and * modifying some fields to describe the foreign partition to work on. @@ -2080,9 +2081,11 @@ postgresBeginForeignInsert(ModifyTableState *mtstate, * correspond to this partition if it is one of the UPDATE subplan target * rels; in that case, we can just use the existing RTE as-is. */ - rte = list_nth(estate->es_range_table, resultRelation - 1); - if (rte->relid != RelationGetRelid(rel)) + if (resultRelInfo->ri_RangeTableIndex == 0) { + ResultRelInfo *rootResultRelInfo = resultRelInfo->ri_RootResultRelInfo; + + rte = list_nth(estate->es_range_table, rootResultRelInfo->ri_RangeTableIndex - 1); rte = copyObject(rte); rte->relid = RelationGetRelid(rel); rte->relkind = RELKIND_FOREIGN_TABLE; @@ -2094,8 +2097,15 @@ postgresBeginForeignInsert(ModifyTableState *mtstate, * Vars contained in those expressions. */ if (plan && plan->operation == CMD_UPDATE && - resultRelation == plan->nominalRelation) + rootResultRelInfo->ri_RangeTableIndex == plan->nominalRelation) resultRelation = mtstate->resultRelInfo[0].ri_RangeTableIndex; + else + resultRelation = rootResultRelInfo->ri_RangeTableIndex; + } + else + { + resultRelation = resultRelInfo->ri_RangeTableIndex; + rte = list_nth(estate->es_range_table, resultRelation - 1); } /* Construct the SQL command string. */ diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c index a40382b146..c95961a63e 100644 --- a/src/backend/access/common/tupconvert.c +++ b/src/backend/access/common/tupconvert.c @@ -21,6 +21,7 @@ #include "postgres.h" #include "access/htup_details.h" +#include "access/sysattr.h" #include "access/tupconvert.h" #include "utils/builtins.h" @@ -401,6 +402,59 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map) return heap_form_tuple(map->outdesc, outvalues, outisnull); } +/* + * Perform conversion of bitmap of columns according to the map. + * + * The input and output bitmaps are offset by + * FirstLowInvalidHeapAttributeNumber to accommodate system cols, like the + * column-bitmaps in RangeTblEntry. + */ +Bitmapset * +execute_attr_map_cols(Bitmapset *in_cols, TupleConversionMap *map) +{ + AttrNumber *attrMap = map->attrMap; + int maplen = map->outdesc->natts; + Bitmapset *out_cols; + int out_attnum; + + /* fast path for the common trivial case */ + if (in_cols == NULL) + return NULL; + + /* + * For each output column, check which input column it corresponds to. + */ + out_cols = NULL; + + for (out_attnum = FirstLowInvalidHeapAttributeNumber + 1; + out_attnum <= maplen; + out_attnum++) + { + int in_attnum; + + if (out_attnum < 0) + { + /* System column. No mapping. */ + in_attnum = out_attnum; + } + else if (out_attnum == 0) + continue; + else + { + /* normal user column */ + in_attnum = attrMap[out_attnum - 1]; + + if (in_attnum == 0) + continue; + } + + if (bms_is_member(in_attnum - FirstLowInvalidHeapAttributeNumber, in_cols)) + out_cols = bms_add_member(out_cols, out_attnum - FirstLowInvalidHeapAttributeNumber); + } + + return out_cols; +} + /* * Free a TupleConversionMap structure. */ diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 121db3122c..231c390759 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2530,6 +2530,7 @@ CopyFrom(CopyState cstate) mtstate->ps.state = estate; mtstate->operation = CMD_INSERT; mtstate->resultRelInfo = estate->es_result_relations; + mtstate->rootResultRelInfo = estate->es_result_relations; if (resultRelInfo->ri_FdwRoutine != NULL && resultRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL) @@ -2557,7 +2558,7 @@ CopyFrom(CopyState cstate) PartitionTupleRouting *proute; proute = cstate->partition_tuple_routing = - ExecSetupPartitionTupleRouting(NULL, cstate->rel); + ExecSetupPartitionTupleRouting(NULL, resultRelInfo); /* * If we are capturing transition tuples, they may need to be diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 31966bca64..e9ba140808 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -3137,7 +3137,7 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors, /* Should we explicitly label target relations? */ labeltargets = (mtstate->mt_nplans > 1 || (mtstate->mt_nplans == 1 && - mtstate->resultRelInfo->ri_RangeTableIndex != node->nominalRelation)); + mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation)); if (labeltargets) ExplainOpenGroup("Target Tables", "Target Tables", false, es); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index acfb9b2614..d3d7bb036e 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -68,15 +68,6 @@ int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN; /* How many levels deep into trigger execution are we? */ static int MyTriggerDepth = 0; -/* - * Note that similar macros also exist in executor/execMain.c. There does not - * appear to be any good header to put them into, given the structures that - * they use, so we let them be duplicated. Be sure to update all if one needs - * to be changed, however. - */ -#define GetUpdatedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) - /* Local function prototypes */ static void ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid); static void SetTriggerFlags(TriggerDesc *trigdesc, Trigger *trigger); @@ -2892,7 +2883,7 @@ ExecBSUpdateTriggers(EState *estate, ResultRelInfo *relinfo) CMD_UPDATE)) return; - updatedCols = GetUpdatedColumns(relinfo, estate); + updatedCols = ExecGetUpdatedCols(relinfo, estate); LocTriggerData.type = T_TriggerData; LocTriggerData.tg_event = TRIGGER_EVENT_UPDATE | @@ -2938,10 +2929,13 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo, { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; + /* statement-level triggers operate on the parent table */ + Assert(relinfo->ri_RootResultRelInfo == NULL); + if (trigdesc && trigdesc->trig_update_after_statement) AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, false, NULL, NULL, NIL, - GetUpdatedColumns(relinfo, estate), + ExecGetUpdatedCols(relinfo, estate), transition_capture); } @@ -3007,7 +3001,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, LocTriggerData.tg_relation = relinfo->ri_RelationDesc; LocTriggerData.tg_oldtable = NULL; LocTriggerData.tg_newtable = NULL; - updatedCols = GetUpdatedColumns(relinfo, estate); + updatedCols = ExecGetUpdatedCols(relinfo, estate); for (i = 0; i < trigdesc->numtriggers; i++) { Trigger *trigger = &trigdesc->triggers[i]; @@ -3099,7 +3093,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, true, trigtuple, newtuple, recheckIndexes, - GetUpdatedColumns(relinfo, estate), + ExecGetUpdatedCols(relinfo, estate), transition_capture); if (trigtuple != fdw_trigtuple) heap_freetuple(trigtuple); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 8daf0fe933..f852ac4faa 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -102,17 +102,6 @@ static char *ExecBuildSlotValueDescription(Oid reloid, static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree); -/* - * Note that GetUpdatedColumns() also exists in commands/trigger.c. There does - * not appear to be any good header to put it into, given the structures that - * it uses, so we let them be duplicated. Be sure to update both if one needs - * to be changed, however. - */ -#define GetInsertedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->insertedCols) -#define GetUpdatedColumns(relinfo, estate) \ - (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->updatedCols) - /* end of local decls */ @@ -1302,7 +1291,7 @@ void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, - Relation partition_root, + ResultRelInfo *partition_root_rri, int instrument_options) { List *partition_check = NIL; @@ -1363,7 +1352,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, partition_check = RelationGetPartitionQual(resultRelationDesc); resultRelInfo->ri_PartitionCheck = partition_check; - resultRelInfo->ri_PartitionRoot = partition_root; + resultRelInfo->ri_RootResultRelInfo = partition_root_rri; resultRelInfo->ri_PartitionReadyForRouting = false; } @@ -1906,26 +1895,28 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate) { + Oid root_relid; Relation rel = resultRelInfo->ri_RelationDesc; Relation orig_rel = rel; TupleDesc tupdesc = RelationGetDescr(rel); char *val_desc; Bitmapset *modifiedCols; - Bitmapset *insertedCols; - Bitmapset *updatedCols; /* * Need to first convert the tuple to the root partitioned table's row * type. For details, check similar comments in ExecConstraints(). */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); - TupleDesc old_tupdesc = RelationGetDescr(rel); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; + TupleDesc old_tupdesc; TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + root_relid = RelationGetRelid(rootrel->ri_RelationDesc); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); + + old_tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -1936,12 +1927,18 @@ ExecPartitionCheckEmitError(ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + } + else + { + root_relid = RelationGetRelid(resultRelInfo->ri_RelationDesc); + tupdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc); + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); } - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); - val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), + val_desc = ExecBuildSlotValueDescription(root_relid, slot, tupdesc, modifiedCols, @@ -1972,8 +1969,6 @@ ExecConstraints(ResultRelInfo *resultRelInfo, TupleDesc tupdesc = RelationGetDescr(rel); TupleConstr *constr = tupdesc->constr; Bitmapset *modifiedCols; - Bitmapset *insertedCols; - Bitmapset *updatedCols; Assert(constr || resultRelInfo->ri_PartitionCheck); @@ -1999,13 +1994,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo, * rowtype so that val_desc shown error message matches the * input tuple. */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(orig_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -2016,11 +2011,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + rel = rootrel->ri_RelationDesc; } - - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); + else + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -2047,14 +2044,14 @@ ExecConstraints(ResultRelInfo *resultRelInfo, Relation orig_rel = rel; /* See the comment above. */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; TupleDesc old_tupdesc = RelationGetDescr(rel); TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -2065,11 +2062,13 @@ ExecConstraints(ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + rel = rootrel->ri_RelationDesc; } - - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); + else + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -2138,8 +2137,6 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, { char *val_desc; Bitmapset *modifiedCols; - Bitmapset *insertedCols; - Bitmapset *updatedCols; switch (wco->kind) { @@ -2154,14 +2151,14 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, */ case WCO_VIEW_CHECK: /* See the comment in ExecConstraints(). */ - if (resultRelInfo->ri_PartitionRoot) + if (resultRelInfo->ri_RootResultRelInfo) { HeapTuple tuple = ExecFetchSlotTuple(slot); + ResultRelInfo *rootrel = resultRelInfo->ri_RootResultRelInfo; TupleDesc old_tupdesc = RelationGetDescr(rel); TupleConversionMap *map; - rel = resultRelInfo->ri_PartitionRoot; - tupdesc = RelationGetDescr(rel); + tupdesc = RelationGetDescr(rootrel->ri_RelationDesc); /* a reverse map */ map = convert_tuples_by_name(old_tupdesc, tupdesc, gettext_noop("could not convert row type")); @@ -2172,11 +2169,13 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo, slot = MakeTupleTableSlot(tupdesc); ExecStoreTuple(tuple, slot, InvalidBuffer, false); } + modifiedCols = bms_union(ExecGetInsertedCols(rootrel, estate), + ExecGetUpdatedCols(rootrel, estate)); + rel = rootrel->ri_RelationDesc; } - - insertedCols = GetInsertedColumns(resultRelInfo, estate); - updatedCols = GetUpdatedColumns(resultRelInfo, estate); - modifiedCols = bms_union(insertedCols, updatedCols); + else + modifiedCols = bms_union(ExecGetInsertedCols(resultRelInfo, estate), + ExecGetUpdatedCols(resultRelInfo, estate)); val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), slot, tupdesc, @@ -2390,7 +2389,7 @@ ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo) * been modified, then we can use a weaker lock, allowing for better * concurrency. */ - updatedCols = GetUpdatedColumns(relinfo, estate); + updatedCols = ExecGetUpdatedCols(relinfo, estate); keyCols = RelationGetIndexAttrBitmap(relinfo->ri_RelationDesc, INDEX_ATTR_BITMAP_KEY); diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index dc58fd0a82..40b5d3717f 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -107,7 +107,7 @@ static void find_matching_subplans_recurse(PartitionPruningData *prunedata, * the partitions to route tuples to. See ExecPrepareTupleRouting. */ PartitionTupleRouting * -ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) +ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, ResultRelInfo *rootResultRelInfo) { List *leaf_parts; ListCell *cell; @@ -123,10 +123,12 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) * Get the information about the partition tree after locking all the * partitions. */ - (void) find_all_inheritors(RelationGetRelid(rel), RowExclusiveLock, NULL); + (void) find_all_inheritors(RelationGetRelid(rootResultRelInfo->ri_RelationDesc), + RowExclusiveLock, NULL); proute = (PartitionTupleRouting *) palloc0(sizeof(PartitionTupleRouting)); proute->partition_dispatch_info = - RelationGetPartitionDispatchInfo(rel, &proute->num_dispatch, + RelationGetPartitionDispatchInfo(rootResultRelInfo->ri_RelationDesc, + &proute->num_dispatch, &leaf_parts); proute->num_partitions = nparts = list_length(leaf_parts); proute->partitions = @@ -186,7 +188,7 @@ ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, Relation rel) * descriptor. When generating the per-subplan result rels, this * was not set. */ - leaf_part_rri->ri_PartitionRoot = rel; + leaf_part_rri->ri_RootResultRelInfo = rootResultRelInfo; /* Remember the subplan offset for this ResultRelInfo */ proute->subplan_partition_offsets[update_rri_index] = i; @@ -366,13 +368,13 @@ ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, */ ResultRelInfo * ExecInitPartitionInfo(ModifyTableState *mtstate, - ResultRelInfo *resultRelInfo, + ResultRelInfo *rootResultRelInfo, PartitionTupleRouting *proute, EState *estate, int partidx) { ModifyTable *node = (ModifyTable *) mtstate->ps.plan; - Relation rootrel = resultRelInfo->ri_RelationDesc, - partrel; + Relation partrel; + int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; Relation firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc; ResultRelInfo *leaf_part_rri; MemoryContext oldContext; @@ -394,8 +396,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, leaf_part_rri = makeNode(ResultRelInfo); InitResultRelInfo(leaf_part_rri, partrel, - node ? node->nominalRelation : 1, - rootrel, + 0, + rootResultRelInfo, estate->es_instrument); /* @@ -441,7 +443,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, List *wcoList; List *wcoExprs = NIL; ListCell *ll; - int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; /* * In the case of INSERT on a partitioned table, there is only one @@ -507,7 +508,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, TupleTableSlot *slot; ExprContext *econtext; List *returningList; - int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; /* See the comment above for WCO lists. */ Assert((node->operation == CMD_INSERT && @@ -568,7 +568,6 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, if (node && node->onConflictAction != ONCONFLICT_NONE) { TupleConversionMap *map = proute->parent_child_tupconv_maps[partidx]; - int firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex; TupleDesc partrelDesc = RelationGetDescr(partrel); ExprContext *econtext = mtstate->ps.ps_ExprContext; ListCell *lc; @@ -580,7 +579,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * list and searching for ancestry relationships to each index in the * ancestor table. */ - if (list_length(resultRelInfo->ri_onConflictArbiterIndexes) > 0) + if (list_length(rootResultRelInfo->ri_onConflictArbiterIndexes) > 0) { List *childIdxs; @@ -593,7 +592,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, ListCell *lc2; ancestors = get_partition_ancestors(childIdx); - foreach(lc2, resultRelInfo->ri_onConflictArbiterIndexes) + foreach(lc2, rootResultRelInfo->ri_onConflictArbiterIndexes) { if (list_member_oid(ancestors, lfirst_oid(lc2))) arbiterIndexes = lappend_oid(arbiterIndexes, childIdx); @@ -607,7 +606,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * (This shouldn't happen, since arbiter index selection should not * pick up an invalid index.) */ - if (list_length(resultRelInfo->ri_onConflictArbiterIndexes) != + if (list_length(rootResultRelInfo->ri_onConflictArbiterIndexes) != list_length(arbiterIndexes)) elog(ERROR, "invalid arbiter index list"); leaf_part_rri->ri_onConflictArbiterIndexes = arbiterIndexes; @@ -618,7 +617,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, if (node->onConflictAction == ONCONFLICT_UPDATE) { Assert(node->onConflictSet != NIL); - Assert(resultRelInfo->ri_onConflict != NULL); + Assert(rootResultRelInfo->ri_onConflict != NULL); /* * If the partition's tuple descriptor matches exactly the root @@ -627,7 +626,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * need to create state specific to this partition. */ if (map == NULL) - leaf_part_rri->ri_onConflict = resultRelInfo->ri_onConflict; + leaf_part_rri->ri_onConflict = rootResultRelInfo->ri_onConflict; else { List *onconflset; @@ -737,6 +736,7 @@ ExecInitRoutingInfo(ModifyTableState *mtstate, ResultRelInfo *partRelInfo, int partidx) { + ResultRelInfo *rootRelInfo = partRelInfo->ri_RootResultRelInfo; MemoryContext oldContext; /* @@ -749,7 +749,7 @@ ExecInitRoutingInfo(ModifyTableState *mtstate, * partition from the parent's type to the partition's. */ proute->parent_child_tupconv_maps[partidx] = - convert_tuples_by_name(RelationGetDescr(partRelInfo->ri_PartitionRoot), + convert_tuples_by_name(RelationGetDescr(rootRelInfo->ri_RelationDesc), RelationGetDescr(partRelInfo->ri_RelationDesc), gettext_noop("could not convert row type")); diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 5b3eaec80b..e91a6ebddd 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -45,6 +45,7 @@ #include "access/relscan.h" #include "access/transam.h" #include "executor/executor.h" +#include "executor/execPartition.h" #include "jit/jit.h" #include "mb/pg_wchar.h" #include "nodes/nodeFuncs.h" @@ -1068,3 +1069,66 @@ ExecCleanTargetListLength(List *targetlist) } return len; } + +/* Return a bitmap representing columns being inserted */ +Bitmapset * +ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate) +{ + /* + * The columns are stored in the range table entry. If this ResultRelInfo + * doesn't have an entry in the range table (i.e. if it represents a + * partition routing target), fetch the parent's RTE and map the columns + * to the order they are in the partition. + */ + if (relinfo->ri_RangeTableIndex != 0) + { + RangeTblEntry *rte = rt_fetch(relinfo->ri_RangeTableIndex, + estate->es_range_table); + + return rte->insertedCols; + } + else + { + ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo; + RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex, + estate->es_range_table); + TupleConversionMap *map; + + map = convert_tuples_by_name(RelationGetDescr(rootRelInfo->ri_RelationDesc), + RelationGetDescr(relinfo->ri_RelationDesc), + gettext_noop("could not convert row type")); + if (map != NULL) + return execute_attr_map_cols(rte->insertedCols, map); + else + return rte->insertedCols; + } +} + +/* Return a bitmap representing columns being updated */ +Bitmapset * +ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate) +{ + /* see ExecGetInsertedCols() */ + if (relinfo->ri_RangeTableIndex != 0) + { + RangeTblEntry *rte = rt_fetch(relinfo->ri_RangeTableIndex, + estate->es_range_table); + + return rte->updatedCols; + } + else + { + ResultRelInfo *rootRelInfo = relinfo->ri_RootResultRelInfo; + RangeTblEntry *rte = rt_fetch(rootRelInfo->ri_RangeTableIndex, + estate->es_range_table); + TupleConversionMap *map; + + map = convert_tuples_by_name(RelationGetDescr(rootRelInfo->ri_RelationDesc), + RelationGetDescr(relinfo->ri_RelationDesc), + gettext_noop("could not convert row type")); + if (map != NULL) + return execute_attr_map_cols(rte->updatedCols, map); + else + return rte->updatedCols; + } +} diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index c84171973c..a0fd6c15d2 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -403,7 +403,7 @@ ExecInsert(ModifyTableState *mtstate, * if there's no BR trigger defined on the partition. */ if (resultRelInfo->ri_PartitionCheck && - (resultRelInfo->ri_PartitionRoot == NULL || + (resultRelInfo->ri_RootResultRelInfo == NULL || (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_before_row))) ExecPartitionCheck(resultRelInfo, slot, estate, true); @@ -2324,7 +2324,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) estate->es_result_relation_info = saved_resultRelInfo; /* Get the target relation */ - rel = (getTargetResultRelInfo(mtstate))->ri_RelationDesc; + resultRelInfo = getTargetResultRelInfo(mtstate); + rel = resultRelInfo->ri_RelationDesc; /* * If it's not a partitioned table after all, UPDATE tuple routing should @@ -2340,7 +2341,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && (operation == CMD_INSERT || update_tuple_routing_needed)) mtstate->mt_partition_tuple_routing = - ExecSetupPartitionTupleRouting(mtstate, rel); + ExecSetupPartitionTupleRouting(mtstate, resultRelInfo); /* * Build state for collecting transition tuples. This requires having a diff --git a/src/include/access/tupconvert.h b/src/include/access/tupconvert.h index 66c0ed0882..9137c875b2 100644 --- a/src/include/access/tupconvert.h +++ b/src/include/access/tupconvert.h @@ -16,6 +16,7 @@ #include "access/htup.h" #include "access/tupdesc.h" +#include "nodes/bitmapset.h" typedef struct TupleConversionMap @@ -43,6 +44,7 @@ extern AttrNumber *convert_tuples_by_name_map(TupleDesc indesc, const char *msg); extern HeapTuple do_convert_tuple(HeapTuple tuple, TupleConversionMap *map); +extern Bitmapset *execute_attr_map_cols(Bitmapset *inbitmap, TupleConversionMap *map); extern void free_conversion_map(TupleConversionMap *map); diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h index 6c22901e8f..3f6f87f8a3 100644 --- a/src/include/executor/execPartition.h +++ b/src/include/executor/execPartition.h @@ -173,13 +173,13 @@ typedef struct PartitionPruneState } PartitionPruneState; extern PartitionTupleRouting *ExecSetupPartitionTupleRouting(ModifyTableState *mtstate, - Relation rel); + ResultRelInfo *rootResultRelInfo); extern int ExecFindPartition(ResultRelInfo *resultRelInfo, PartitionDispatch *pd, TupleTableSlot *slot, EState *estate); extern ResultRelInfo *ExecInitPartitionInfo(ModifyTableState *mtstate, - ResultRelInfo *resultRelInfo, + ResultRelInfo *rootResultRelInfo, PartitionTupleRouting *proute, EState *estate, int partidx); extern void ExecInitRoutingInfo(ModifyTableState *mtstate, diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 97b1c0f67d..e225294091 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -15,6 +15,7 @@ #define EXECUTOR_H #include "executor/execdesc.h" +#include "executor/execPartition.h" #include "nodes/parsenodes.h" #include "utils/memutils.h" @@ -184,7 +185,7 @@ extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation) extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, - Relation partition_root, + ResultRelInfo *partition_root_rri, int instrument_options); extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid); extern void ExecCleanUpTriggerState(EState *estate); @@ -544,6 +545,9 @@ extern Datum GetAttributeByNum(HeapTupleHeader tuple, AttrNumber attrno, extern int ExecTargetListLength(List *targetlist); extern int ExecCleanTargetListLength(List *targetlist); +extern Bitmapset *ExecGetInsertedCols(ResultRelInfo *relinfo, EState *estate); +extern Bitmapset *ExecGetUpdatedCols(ResultRelInfo *relinfo, EState *estate); + /* * prototypes from functions in execIndexing.c */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 092843ed80..77ada4377f 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -457,8 +457,13 @@ typedef struct ResultRelInfo /* partition check expression state */ ExprState *ri_PartitionCheckExpr; - /* relation descriptor for root partitioned table */ - Relation ri_PartitionRoot; + /* + * RootResultRelInfo gives the target relation mentioned in the query, if + * it's a partitioned table. It is not set if the target relation + * mentioned in the query is an inherited table, nor when tuple routing is + * not needed. + */ + struct ResultRelInfo *ri_RootResultRelInfo; /* true if ready for tuple routing */ bool ri_PartitionReadyForRouting; diff --git a/src/test/isolation/expected/tuplelock-partition.out b/src/test/isolation/expected/tuplelock-partition.out new file mode 100644 index 0000000000..dd6d37c577 --- /dev/null +++ b/src/test/isolation/expected/tuplelock-partition.out @@ -0,0 +1,20 @@ +Parsed test spec with 2 sessions + +starting permutation: s1b s1update_nokey s2locktuple s1c +step s1b: BEGIN; +step s1update_nokey: INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET col1 = 'x', col2 = 'y'; +step s2locktuple: SELECT * FROM parttab FOR KEY SHARE; +col1 key col2 + +a 1 b +step s1c: COMMIT; + +starting permutation: s1b s1update_key s2locktuple s1c +step s1b: BEGIN; +step s1update_key: INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET key=1; +step s2locktuple: SELECT * FROM parttab FOR KEY SHARE; +step s1c: COMMIT; +step s2locktuple: <... completed> +col1 key col2 + +a 1 b diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 96a5d4be75..4a16d42366 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -50,6 +50,7 @@ test: propagate-lock-delete test: tuplelock-conflict test: tuplelock-update test: tuplelock-upgrade-no-deadlock +test: tuplelock-partition test: freeze-the-dead test: nowait test: nowait-2 diff --git a/src/test/isolation/specs/tuplelock-partition.spec b/src/test/isolation/specs/tuplelock-partition.spec new file mode 100644 index 0000000000..9a585cb161 --- /dev/null +++ b/src/test/isolation/specs/tuplelock-partition.spec @@ -0,0 +1,32 @@ +# Test tuple locking on INSERT ON CONFLICT UPDATE on a partitioned table. + +setup +{ + DROP TABLE IF EXISTS parttab; + CREATE TABLE parttab (col1 text, key INTEGER PRIMARY KEY, col2 text) PARTITION BY LIST (key); + CREATE TABLE parttab1 (key INTEGER PRIMARY KEY, col1 text, col2 text); + CREATE TABLE parttab2 (key INTEGER PRIMARY KEY, col1 text, col2 text); + ALTER TABLE parttab ATTACH PARTITION parttab1 FOR VALUES IN (1); + ALTER TABLE parttab ATTACH PARTITION parttab2 FOR VALUES IN (2); + INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b'); +} + +teardown +{ + DROP TABLE parttab; +} + +session "s1" +step "s1b" { BEGIN; } +step "s1update_nokey" { INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET col1 = 'x', col2 = 'y'; } +step "s1update_key" { INSERT INTO parttab (key, col1, col2) VALUES (1, 'a', 'b') ON CONFLICT (key) DO UPDATE SET key=1; } +step "s1c" { COMMIT; } + +session "s2" +step "s2locktuple" { SELECT * FROM parttab FOR KEY SHARE; } + +# INSERT ON CONFLICT UPDATE, performs an UPDATE on non-key columns +permutation "s1b" "s1update_nokey" "s2locktuple" "s1c" + +# INSERT ON CONFLICT UPDATE, performs an UPDATE on key column +permutation "s1b" "s1update_key" "s2locktuple" "s1c" diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index ff4f26f6c5..478c2cdf01 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -612,6 +612,45 @@ ERROR: new row for relation "t1" violates check constraint "t1_c3_check" DETAIL: Failing row contains (c1, c3) = (1, 10). SET SESSION AUTHORIZATION regress_priv_user1; DROP TABLE t1; +-- check error reporting with column privs on a partitioned table +CREATE TABLE errtst(a text, b text NOT NULL, c text, secret1 text, secret2 text) PARTITION BY LIST (a); +CREATE TABLE errtst_part_1(secret2 text, c text, a text, b text NOT NULL, secret1 text); +CREATE TABLE errtst_part_2(secret1 text, secret2 text, a text, c text, b text NOT NULL); +ALTER TABLE errtst ATTACH PARTITION errtst_part_1 FOR VALUES IN ('aaa'); +ALTER TABLE errtst ATTACH PARTITION errtst_part_2 FOR VALUES IN ('aaaa'); +GRANT SELECT (a, b, c) ON TABLE errtst TO regress_priv_user2; +GRANT UPDATE (a, b, c) ON TABLE errtst TO regress_priv_user2; +GRANT INSERT (a, b, c) ON TABLE errtst TO regress_priv_user2; +INSERT INTO errtst_part_1 (a, b, c, secret1, secret2) +VALUES ('aaa', 'bbb', 'ccc', 'the body', 'is in the attic'); +SET SESSION AUTHORIZATION regress_priv_user2; +-- Perform a few updates that violate the NOT NULL constraint. Make sure +-- the error messages don't leak the secret fields. +-- simple insert. +INSERT INTO errtst (a, b) VALUES ('aaa', NULL); +ERROR: null value in column "b" violates not-null constraint +DETAIL: Failing row contains (a, b, c) = (aaa, null, null). +-- simple update. +UPDATE errtst SET b = NULL; +ERROR: null value in column "b" violates not-null constraint +DETAIL: Failing row contains (b) = (null). +-- partitioning key is updated, doesn't move the row. +UPDATE errtst SET a = 'aaa', b = NULL; +ERROR: null value in column "b" violates not-null constraint +DETAIL: Failing row contains (a, b, c) = (aaa, null, ccc). +-- row is moved to another partition. +UPDATE errtst SET a = 'aaaa', b = NULL; +ERROR: null value in column "b" violates not-null constraint +DETAIL: Failing row contains (a, b, c) = (aaaa, null, ccc). +-- row is moved to another partition. This differs from the previous case in +-- that the new partition is excluded by constraint exclusion, so its +-- ResultRelInfo is not created at ExecInitModifyTable, but needs to be +-- constructed on the fly when the updated tuple is routed to it. +UPDATE errtst SET a = 'aaaa', b = NULL WHERE a = 'aaa'; +ERROR: null value in column "b" violates not-null constraint +DETAIL: Failing row contains (a, b, c) = (aaaa, null, ccc). +SET SESSION AUTHORIZATION regress_priv_user1; +DROP TABLE errtst; -- test column-level privileges when involved with DELETE SET SESSION AUTHORIZATION regress_priv_user1; ALTER TABLE atest6 ADD COLUMN three integer; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index f592f40c7e..b19e7b3888 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -397,6 +397,44 @@ UPDATE t1 SET c3 = 10; -- fail, but see columns with SELECT rights, or being mod SET SESSION AUTHORIZATION regress_priv_user1; DROP TABLE t1; +-- check error reporting with column privs on a partitioned table +CREATE TABLE errtst(a text, b text NOT NULL, c text, secret1 text, secret2 text) PARTITION BY LIST (a); +CREATE TABLE errtst_part_1(secret2 text, c text, a text, b text NOT NULL, secret1 text); +CREATE TABLE errtst_part_2(secret1 text, secret2 text, a text, c text, b text NOT NULL); + +ALTER TABLE errtst ATTACH PARTITION errtst_part_1 FOR VALUES IN ('aaa'); +ALTER TABLE errtst ATTACH PARTITION errtst_part_2 FOR VALUES IN ('aaaa'); + +GRANT SELECT (a, b, c) ON TABLE errtst TO regress_priv_user2; +GRANT UPDATE (a, b, c) ON TABLE errtst TO regress_priv_user2; +GRANT INSERT (a, b, c) ON TABLE errtst TO regress_priv_user2; + +INSERT INTO errtst_part_1 (a, b, c, secret1, secret2) +VALUES ('aaa', 'bbb', 'ccc', 'the body', 'is in the attic'); + +SET SESSION AUTHORIZATION regress_priv_user2; + +-- Perform a few updates that violate the NOT NULL constraint. Make sure +-- the error messages don't leak the secret fields. + +-- simple insert. +INSERT INTO errtst (a, b) VALUES ('aaa', NULL); +-- simple update. +UPDATE errtst SET b = NULL; +-- partitioning key is updated, doesn't move the row. +UPDATE errtst SET a = 'aaa', b = NULL; +-- row is moved to another partition. +UPDATE errtst SET a = 'aaaa', b = NULL; + +-- row is moved to another partition. This differs from the previous case in +-- that the new partition is excluded by constraint exclusion, so its +-- ResultRelInfo is not created at ExecInitModifyTable, but needs to be +-- constructed on the fly when the updated tuple is routed to it. +UPDATE errtst SET a = 'aaaa', b = NULL WHERE a = 'aaa'; + +SET SESSION AUTHORIZATION regress_priv_user1; +DROP TABLE errtst; + -- test column-level privileges when involved with DELETE SET SESSION AUTHORIZATION regress_priv_user1; ALTER TABLE atest6 ADD COLUMN three integer;