From 282f08bfab695b84baa4f311623ee8bba8ef1776 Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 13 Dec 2016 15:07:41 +0900 Subject: [PATCH 4/7] Fix a bug of insertion into an internal partition. Since implicit partition constraints are not inherited, an internal partition's constraint was not being enforced when targeted directly. So, include such constraint when setting up leaf partition result relations for tuple-routing. InitResultRelInfo()'s API changes with this. Instead of passing a boolean telling whether or not to load the partition constraint, callers now need to pass the exact constraint expression to use as ri_PartitionCheck or NIL. --- src/backend/commands/copy.c | 7 +++++- src/backend/commands/tablecmds.c | 2 +- src/backend/executor/execMain.c | 43 ++++++++++++++++++++++++++++++------ src/include/executor/executor.h | 3 +-- src/test/regress/expected/insert.out | 4 ++++ src/test/regress/sql/insert.sql | 3 +++ 6 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index d5901651db..a0eb4241e2 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2294,6 +2294,7 @@ CopyFrom(CopyState cstate) uint64 processed = 0; bool useHeapMultiInsert; int nBufferedTuples = 0; + List *partcheck = NIL; #define MAX_BUFFERED_TUPLES 1000 HeapTuple *bufferedTuples = NULL; /* initialize to silence warning */ @@ -2410,6 +2411,10 @@ CopyFrom(CopyState cstate) hi_options |= HEAP_INSERT_FROZEN; } + /* Don't forget the partition constraints */ + if (cstate->rel->rd_rel->relispartition) + partcheck = RelationGetPartitionQual(cstate->rel); + /* * We need a ResultRelInfo so we can use the regular executor's * index-entry-making machinery. (There used to be a huge amount of code @@ -2419,7 +2424,7 @@ CopyFrom(CopyState cstate) InitResultRelInfo(resultRelInfo, cstate->rel, 1, /* dummy rangetable index */ - true, /* do load partition check expression */ + partcheck, 0); ExecOpenIndices(resultRelInfo, false); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d2e4cfc365..4bd4ec4e8c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1323,7 +1323,7 @@ ExecuteTruncate(TruncateStmt *stmt) InitResultRelInfo(resultRelInfo, rel, 0, /* dummy rangetable index */ - false, + NIL, 0); resultRelInfo++; } diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b3cedadce4..5627378a35 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -821,13 +821,19 @@ InitPlan(QueryDesc *queryDesc, int eflags) Index resultRelationIndex = lfirst_int(l); Oid resultRelationOid; Relation resultRelation; + List *partcheck = NIL; resultRelationOid = getrelid(resultRelationIndex, rangeTable); resultRelation = heap_open(resultRelationOid, RowExclusiveLock); + + /* Don't forget the partition constraint */ + if (resultRelation->rd_rel->relispartition) + partcheck = RelationGetPartitionQual(resultRelation); + InitResultRelInfo(resultRelInfo, resultRelation, resultRelationIndex, - true, + partcheck, estate->es_instrument); resultRelInfo++; } @@ -1217,7 +1223,7 @@ void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, - bool load_partition_check, + List *partition_check, int instrument_options) { MemSet(resultRelInfo, 0, sizeof(ResultRelInfo)); @@ -1255,9 +1261,7 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo, resultRelInfo->ri_ConstraintExprs = NULL; resultRelInfo->ri_junkFilter = NULL; resultRelInfo->ri_projectReturning = NULL; - if (load_partition_check) - resultRelInfo->ri_PartitionCheck = - RelationGetPartitionQual(resultRelationDesc); + resultRelInfo->ri_PartitionCheck = partition_check; } /* @@ -1284,6 +1288,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid) ListCell *l; Relation rel; MemoryContext oldcontext; + List *partcheck = NIL; /* First, search through the query result relations */ rInfo = estate->es_result_relations; @@ -1312,6 +1317,10 @@ ExecGetTriggerResultRel(EState *estate, Oid relid) */ rel = heap_open(relid, NoLock); + /* Don't forget the partition constraint */ + if (rel->rd_rel->relispartition) + partcheck = RelationGetPartitionQual(rel); + /* * Make the new entry in the right context. */ @@ -1320,7 +1329,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid) InitResultRelInfo(rInfo, rel, 0, /* dummy rangetable index */ - true, + partcheck, estate->es_instrument); estate->es_trig_target_relations = lappend(estate->es_trig_target_relations, rInfo); @@ -3032,6 +3041,7 @@ ExecSetupPartitionTupleRouting(Relation rel, ListCell *cell; int i; ResultRelInfo *leaf_part_rri; + List *partcheck = NIL; /* Get the tuple-routing information and lock partitions */ *pd = RelationGetPartitionDispatchInfo(rel, RowExclusiveLock, num_parted, @@ -3042,12 +3052,22 @@ ExecSetupPartitionTupleRouting(Relation rel, *tup_conv_maps = (TupleConversionMap **) palloc0(*num_partitions * sizeof(TupleConversionMap *)); + /* + * If the main target rel is a partition, ExecConstraints() as applied to + * each leaf partition must consider its partition constraint, because + * unlike explicit constraints, an implicit partition constraint is not + * inherited. + */ + if (rel->rd_rel->relispartition) + partcheck = RelationGetPartitionQual(rel); + leaf_part_rri = *partitions; i = 0; foreach(cell, leaf_parts) { Relation partrel; TupleDesc part_tupdesc; + List *my_check = NIL; /* * We locked all the partitions above including the leaf partitions. @@ -3069,10 +3089,19 @@ ExecSetupPartitionTupleRouting(Relation rel, (*tup_conv_maps)[i] = convert_tuples_by_name(tupDesc, part_tupdesc, gettext_noop("could not convert row type")); + /* + * This is the parent's partition constraint, so any Vars in + * it bear the its attribute numbers. We must switch them to + * the leaf partition's, which is possible with the + * reverse_map's attribute-number map. + */ + if (partcheck) + my_check = map_partition_varattnos(partcheck, partrel, rel); + InitResultRelInfo(leaf_part_rri, partrel, 1, /* dummy */ - false, + my_check, 0); /* Open partition indices */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index bd1bc6bb6e..8bcc876809 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -73,7 +73,6 @@ #define ExecEvalExpr(expr, econtext, isNull, isDone) \ ((*(expr)->evalfunc) (expr, econtext, isNull, isDone)) - /* Hook for plugins to get control in ExecutorStart() */ typedef void (*ExecutorStart_hook_type) (QueryDesc *queryDesc, int eflags); extern PGDLLIMPORT ExecutorStart_hook_type ExecutorStart_hook; @@ -189,7 +188,7 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, - bool load_partition_check, + List *partition_check, int instrument_options); extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid); extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids); diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out index 561cefa3c4..95a7c4da7a 100644 --- a/src/test/regress/expected/insert.out +++ b/src/test/regress/expected/insert.out @@ -285,6 +285,10 @@ select tableoid::regclass, * from list_parted; part_ee_ff2 | EE | 10 (8 rows) +-- fail due to partition constraint failure +insert into part_ee_ff values ('gg', 1); +ERROR: new row for relation "part_ee_ff1" violates partition constraint +DETAIL: Failing row contains (gg, 1). -- cleanup drop table range_parted cascade; NOTICE: drop cascades to 4 other objects diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql index 846bb5897a..77577682ac 100644 --- a/src/test/regress/sql/insert.sql +++ b/src/test/regress/sql/insert.sql @@ -167,6 +167,9 @@ insert into list_parted values ('EE', 1); insert into part_ee_ff values ('EE', 10); select tableoid::regclass, * from list_parted; +-- fail due to partition constraint failure +insert into part_ee_ff values ('gg', 1); + -- cleanup drop table range_parted cascade; drop table list_parted cascade; -- 2.11.0