On 2016/12/21 1:53, Robert Haas wrote:
> On Mon, Dec 19, 2016 at 10:59 PM, Robert Haas <robertmh...@gmail.com> wrote:
>> On Sun, Dec 18, 2016 at 10:00 PM, Amit Langote
>> <langote_amit...@lab.ntt.co.jp> wrote:
>>> Here are updated patches including the additional information.
>>
>> Thanks. Committed 0001. Will have to review the others when I'm less tired.
>
> 0002. Can you add a test case for the bug fixed by this patch?
Done (also see below).
> 0003. Loses equalTupleDescs() check and various cases where
> ExecOpenIndexes can be skipped. Isn't that bad?
I realized that as far as checking whether tuple conversion mapping is
required, the checks performed by convert_tuples_by_name() at the
beginning of the function following the comment /* Verify compatibility
and prepare attribute-number map */ are enough. If equalTupleDescs()
returned false (which it always does because tdtypeid are never the same
for passed in tuple descriptors), we would be repeating some of the tests
in convert_tuples_by_name() anyway.
As for the checks performed for ExecOpenIndices(), it seems would be
better to keep the following in place, so added back:
if (leaf_part_rri->ri_RelationDesc->rd_rel->relhasindex &&
leaf_part_rri->ri_IndexRelationDescs == NULL)
ExecOpenIndices(leaf_part_rri, false);
> Also, "We locked all
> the partitions above including the leaf partitions" should say "Caller
> must have locked all the partitions including the leaf partitions".
No, we do the locking in RelationGetPartitionDispatchInfo(), which is
called by ExecSetupPartitionTupleRouting() itself.
In ExecSetupPartitionTupleRouting() is the first time we lock all the
partitions.
> 0004. Unnecessary whitespace change in executor.h. Still don't
> understand why we need to hoist RelationGetPartitionQual() into the
> caller.
We only need to check a result relation's (ri_RelationDesc's) partition
constraint if we are inserting into the result relation directly. In case
of tuple-routing, we do not want to check the leaf partitions' partition
constraint, but if the direct target in that case is an internal
partition, we must check its partition constraint, which is same for all
leaf partitions in that sub-tree. It wouldn't be wrong per se to check
each leaf partition's constraint in that case, which includes the target
partitioned table's constraint as well, but that would inefficient due to
both having to retrieve the constraints and having ExecConstraints()
*unnecessarily* check it for every row.
If we keep doing RelationGetPartitionQual() in InitResultRelInfo()
controlled by a bool argument (load_partition_check), we cannot avoid the
above mentioned inefficiency if we want to fix this bug.
> 0005. Can you add a test case for the bug fixed by this patch?
Done, but...
Breaking changes into multiple commits/patches does not seem to work for
adding regression tests. So, I've combined multiple patches into a single
patch which is now patch 0002 in the attached set of patches. Its commit
message is very long now. To show an example of bugs that 0002 is meant for:
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
create table p11 (like p1);
alter table p11 drop a;
alter table p11 add a int;
alter table p11 drop a;
alter table p11 add a int not null;
# select attrelid::regclass, attname, attnum
from pg_attribute
where attnum > 0
and (attrelid = 'p'::regclass
or attrelid = 'p1'::regclass
or attrelid = 'p11'::regclass) and attname = 'a';
attrelid | attname | attnum
----------+---------+--------
p | a | 1
p1 | a | 2
p11 | a | 4
(3 rows)
alter table p1 attach partition p11 for values from (1) to (5);
alter table p attach partition p1 for values from (1, 1) to (1, 10);
-- the following is wrong
# insert into p11 (a, b) values (10, 4);
INSERT 0 1
-- wrong too (using the wrong TupleDesc after tuple routing)
# insert into p1 (a, b) values (10, 4);
ERROR: null value in column "a" violates not-null constraint
DETAIL: Failing row contains (4, null).
-- once we fix the wrong TupleDesc issue
# insert into p1 (a, b) values (10, 4);
INSERT 0 1
which is wrong because p1, as a partition of p, should not accept 10 for
a. But its partition constraint is not being applied to the leaf
partition p11 into which the tuple is routed (the bug).
Thanks,
Amit
>From d21ae74ae01e93f21df5b84ed097ebbc85e529dd Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Thu, 15 Dec 2016 15:56:27 +0900
Subject: [PATCH 1/3] Refactor tuple-routing setup code
It's unnecessarily repeated in copy.c and nodeModifyTable.c, which makes
it a burden to maintain. Should've been like this to begin with.
I moved the common code to ExecSetupPartitionTupleRouting() in execMain.c
that also houses ExecFindParttion() currently. Hmm, should there be a
new src/backend/executor/execPartition.c?
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
src/backend/commands/copy.c | 72 ++++++--------------------
src/backend/executor/execMain.c | 92 ++++++++++++++++++++++++++++++++++
src/backend/executor/nodeModifyTable.c | 76 ++++++----------------------
src/include/executor/executor.h | 5 ++
4 files changed, 127 insertions(+), 118 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 7a8da338f0..d5901651db 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1406,64 +1406,22 @@ BeginCopy(ParseState *pstate,
/* Initialize state for CopyFrom tuple routing. */
if (is_from && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
- List *leaf_parts;
- ListCell *cell;
- int i,
- num_parted;
- ResultRelInfo *leaf_part_rri;
-
- /* Get the tuple-routing information and lock partitions */
- cstate->partition_dispatch_info =
- RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
- &num_parted,
- &leaf_parts);
+ PartitionDispatch *partition_dispatch_info;
+ ResultRelInfo *partitions;
+ TupleConversionMap **partition_tupconv_maps;
+ int num_parted,
+ num_partitions;
+
+ ExecSetupPartitionTupleRouting(rel,
+ &partition_dispatch_info,
+ &partitions,
+ &partition_tupconv_maps,
+ &num_parted, &num_partitions);
+ cstate->partition_dispatch_info = partition_dispatch_info;
cstate->num_dispatch = num_parted;
- cstate->num_partitions = list_length(leaf_parts);
- cstate->partitions = (ResultRelInfo *)
- palloc(cstate->num_partitions *
- sizeof(ResultRelInfo));
- cstate->partition_tupconv_maps = (TupleConversionMap **)
- palloc0(cstate->num_partitions *
- sizeof(TupleConversionMap *));
-
- leaf_part_rri = cstate->partitions;
- i = 0;
- foreach(cell, leaf_parts)
- {
- Relation partrel;
-
- /*
- * We locked all the partitions above including the leaf
- * partitions. Note that each of the relations in
- * cstate->partitions will be closed by CopyFrom() after it's
- * finished with its processing.
- */
- partrel = heap_open(lfirst_oid(cell), NoLock);
-
- /*
- * Verify result relation is a valid target for the current
- * operation.
- */
- CheckValidResultRel(partrel, CMD_INSERT);
-
- InitResultRelInfo(leaf_part_rri,
- partrel,
- 1, /* dummy */
- false, /* no partition constraint
- * check */
- 0);
-
- /* Open partition indices */
- ExecOpenIndices(leaf_part_rri, false);
-
- if (!equalTupleDescs(tupDesc, RelationGetDescr(partrel)))
- cstate->partition_tupconv_maps[i] =
- convert_tuples_by_name(tupDesc,
- RelationGetDescr(partrel),
- gettext_noop("could not convert row type"));
- leaf_part_rri++;
- i++;
- }
+ cstate->partitions = partitions;
+ cstate->num_partitions = num_partitions;
+ cstate->partition_tupconv_maps = partition_tupconv_maps;
}
}
else
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d43a204808..bca34a509c 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -51,6 +51,7 @@
#include "miscadmin.h"
#include "optimizer/clauses.h"
#include "parser/parsetree.h"
+#include "rewrite/rewriteManip.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
#include "tcop/utility.h"
@@ -2999,6 +3000,97 @@ EvalPlanQualEnd(EPQState *epqstate)
}
/*
+ * ExecSetupPartitionTupleRouting - set up information needed during
+ * tuple routing for partitioned tables
+ *
+ * Output arguments:
+ * 'pd' receives an array of PartitionDispatch objects with one entry for
+ * every partitioned table in the partition tree
+ * 'partitions' receives an array of ResultRelInfo objects with one entry for
+ * every leaf partition in the partition tree
+ * 'tup_conv_maps' receives an array of TupleConversionMap objects with one
+ * entry for every leaf partition (required to convert input tuple based
+ * on the root table's rowtype to a leaf partition's rowtype after tuple
+ * routing is done
+ * 'num_parted' receives the number of partitioned tables in the partition
+ * tree (= the number of entries in the 'pd' output array)
+ * 'num_partitions' receives the number of leaf partitions in the partition
+ * tree (= the number of entries in the 'partitions' and 'tup_conv_maps'
+ * output arrays
+ *
+ * Note that all the relations in the partition tree are locked using the
+ * RowExclusiveLock mode upon return from this function.
+ */
+void
+ExecSetupPartitionTupleRouting(Relation rel,
+ PartitionDispatch **pd,
+ ResultRelInfo **partitions,
+ TupleConversionMap ***tup_conv_maps,
+ int *num_parted, int *num_partitions)
+{
+ TupleDesc tupDesc = RelationGetDescr(rel);
+ List *leaf_parts;
+ ListCell *cell;
+ int i;
+ ResultRelInfo *leaf_part_rri;
+
+ /* Get the tuple-routing information and lock partitions */
+ *pd = RelationGetPartitionDispatchInfo(rel, RowExclusiveLock, num_parted,
+ &leaf_parts);
+ *num_partitions = list_length(leaf_parts);
+ *partitions = (ResultRelInfo *) palloc(*num_partitions *
+ sizeof(ResultRelInfo));
+ *tup_conv_maps = (TupleConversionMap **) palloc0(*num_partitions *
+ sizeof(TupleConversionMap *));
+
+ leaf_part_rri = *partitions;
+ i = 0;
+ foreach(cell, leaf_parts)
+ {
+ Relation partrel;
+ TupleDesc part_tupdesc;
+
+ /*
+ * We locked all the partitions above including the leaf partitions.
+ * Note that each of the relations in *partitions are eventually
+ * closed by the caller.
+ */
+ partrel = heap_open(lfirst_oid(cell), NoLock);
+ part_tupdesc = RelationGetDescr(partrel);
+
+ /*
+ * Verify result relation is a valid target for the current operation.
+ */
+ CheckValidResultRel(partrel, CMD_INSERT);
+
+ /*
+ * Save a tuple conversion map to convert a tuple routed to this
+ * partition from the parent's type to the partition's.
+ */
+ (*tup_conv_maps)[i] = convert_tuples_by_name(tupDesc, part_tupdesc,
+ gettext_noop("could not convert row type"));
+
+ InitResultRelInfo(leaf_part_rri,
+ partrel,
+ 1, /* dummy */
+ false,
+ 0);
+
+ /*
+ * Open partition indices (remember we do not support ON CONFLICT in
+ * case of partitioned tables, so we do not need support information
+ * for speculative insertion)
+ */
+ if (leaf_part_rri->ri_RelationDesc->rd_rel->relhasindex &&
+ leaf_part_rri->ri_IndexRelationDescs == NULL)
+ ExecOpenIndices(leaf_part_rri, false);
+
+ leaf_part_rri++;
+ i++;
+ }
+}
+
+/*
* ExecFindPartition -- Find a leaf partition in the partition tree rooted
* at parent, for the heap tuple contained in *slot
*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ec440b353d..a9546106ce 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1718,68 +1718,22 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (operation == CMD_INSERT &&
rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
- int i,
- j,
- num_parted;
- List *leaf_parts;
- ListCell *cell;
- ResultRelInfo *leaf_part_rri;
-
- /* Get the tuple-routing information and lock partitions */
- mtstate->mt_partition_dispatch_info =
- RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
- &num_parted,
- &leaf_parts);
+ PartitionDispatch *partition_dispatch_info;
+ ResultRelInfo *partitions;
+ TupleConversionMap **partition_tupconv_maps;
+ int num_parted,
+ num_partitions;
+
+ ExecSetupPartitionTupleRouting(rel,
+ &partition_dispatch_info,
+ &partitions,
+ &partition_tupconv_maps,
+ &num_parted, &num_partitions);
+ mtstate->mt_partition_dispatch_info = partition_dispatch_info;
mtstate->mt_num_dispatch = num_parted;
- mtstate->mt_num_partitions = list_length(leaf_parts);
- mtstate->mt_partitions = (ResultRelInfo *)
- palloc0(mtstate->mt_num_partitions *
- sizeof(ResultRelInfo));
- mtstate->mt_partition_tupconv_maps = (TupleConversionMap **)
- palloc0(mtstate->mt_num_partitions *
- sizeof(TupleConversionMap *));
-
- leaf_part_rri = mtstate->mt_partitions;
- i = j = 0;
- foreach(cell, leaf_parts)
- {
- Oid partrelid = lfirst_oid(cell);
- Relation partrel;
-
- /*
- * We locked all the partitions above including the leaf
- * partitions. Note that each of the relations in
- * mtstate->mt_partitions will be closed by ExecEndModifyTable().
- */
- partrel = heap_open(partrelid, NoLock);
-
- /*
- * Verify result relation is a valid target for the current
- * operation
- */
- CheckValidResultRel(partrel, CMD_INSERT);
-
- InitResultRelInfo(leaf_part_rri,
- partrel,
- 1, /* dummy */
- false, /* no partition constraint checks */
- eflags);
-
- /* Open partition indices (note: ON CONFLICT unsupported)*/
- if (partrel->rd_rel->relhasindex && operation != CMD_DELETE &&
- leaf_part_rri->ri_IndexRelationDescs == NULL)
- ExecOpenIndices(leaf_part_rri, false);
-
- if (!equalTupleDescs(RelationGetDescr(rel),
- RelationGetDescr(partrel)))
- mtstate->mt_partition_tupconv_maps[i] =
- convert_tuples_by_name(RelationGetDescr(rel),
- RelationGetDescr(partrel),
- gettext_noop("could not convert row type"));
-
- leaf_part_rri++;
- i++;
- }
+ mtstate->mt_partitions = partitions;
+ mtstate->mt_num_partitions = num_partitions;
+ mtstate->mt_partition_tupconv_maps = partition_tupconv_maps;
}
/*
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 3f649faf2f..b74fa5eb5d 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -213,6 +213,11 @@ extern void EvalPlanQualSetPlan(EPQState *epqstate,
extern void EvalPlanQualSetTuple(EPQState *epqstate, Index rti,
HeapTuple tuple);
extern HeapTuple EvalPlanQualGetTuple(EPQState *epqstate, Index rti);
+extern void ExecSetupPartitionTupleRouting(Relation rel,
+ PartitionDispatch **pd,
+ ResultRelInfo **partitions,
+ TupleConversionMap ***tup_conv_maps,
+ int *num_parted, int *num_partitions);
extern int ExecFindPartition(ResultRelInfo *resultRelInfo,
PartitionDispatch *pd,
TupleTableSlot *slot,
--
2.11.0
>From 8c55a95e538e07d4b74d2985f01684749c791708 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Thu, 15 Dec 2016 16:27:04 +0900
Subject: [PATCH 2/3] Fix multiple issues with partition constraints
1. A bug in how they are generated
Firstly, since we always want to recurse when calling
RelationGetPartitionQual(), ie, consider the parent's partition
constraint (if any), get rid of the argument recurse; also in
the module-local generate_partition_qual() that it calls.
Move the code for doing parent attnos to child attnos mapping for
Vars in partition constraint expressions to a separate function
map_partition_varattnos() and call it from appropriate places. Doing
it in get_qual_from_partbound(), as is now, would produce wrong result
in certain multi-level partitioning cases, because it only considers
the current pair of parent-child relations. In certain multi-level
partitioning cases, attnums for the same key attribute(s) might differ
between multiple pairs of consecutive levels causing the same attribute
to be numbered differently in different Vars of the same expression
tree. Remember that we apply the whole partition constraint (list of
constraints of partitions at various levels) to a single (leaf
partition) relation.
With this commit, in generate_partition_qual(), we first generate the
the whole partition constraint (considering all levels of partitioning)
and then do the mapping from the root parent attnums to leaf partition
attnums.
2. A bug when attaching a partitioned table as partition
ATExecAttachPartition() failed to do the attnum mapping in the case where
attached partition is a partitioned table. It is possible in such case
that the leaf partitions of such a table that will be scanned for partition
constraint validation might have different attnums than their parent.
That might lead to incorrect results.
3. A bug when inserting 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.
4. A bug due to using the wrong TupleTableSlot after tuple routing
We must use the partition's tuple descriptor *after* a tuple is routed,
not the root table's. Partition's attributes, for example, may be
ordered diferently from the root table's, causing spurious not-null
violation errors due to asking for out-of-range attnum from a heap
tuple and getting null in return.
We must then switch back to the root table's for the next tuple, because
computing partition key of a tuple to be routed must be looking at the
root table's tuple descriptor. A dedicated TupleTableSlot is allocated
within EState called es_partition_tuple_slot whose descriptor is set to
a given leaf partition for every input tuple after it's routed.
5. A bug whereby ExecConstraints() shows wrong input row in error msgs
After a tuple is routed to a partition, it has been converted from the
root table's rowtype to the partition's. If such a tuple causes an
error in ExecConstraints(), the row shown in error messages might not
match the input row due to possible differences between the root
table's rowtype and the partition's.
To convert back to the correct row format, keep root table relation
descriptor and a reverse tuple conversion map in the ResultRelInfo's
of leaf partitions.
Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
src/backend/catalog/partition.c | 100 +++++++++++++------------
src/backend/commands/copy.c | 35 ++++++++-
src/backend/commands/tablecmds.c | 11 ++-
src/backend/executor/execMain.c | 120 ++++++++++++++++++++++++++----
src/backend/executor/nodeModifyTable.c | 25 +++++++
src/backend/optimizer/util/plancat.c | 2 +-
src/include/catalog/partition.h | 3 +-
src/include/executor/executor.h | 4 +-
src/include/nodes/execnodes.h | 5 ++
src/test/regress/expected/alter_table.out | 26 +++++++
src/test/regress/expected/insert.out | 36 +++++++++
src/test/regress/sql/alter_table.sql | 20 +++++
src/test/regress/sql/insert.sql | 22 ++++++
13 files changed, 340 insertions(+), 69 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 9980582b77..b6d0841c9b 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -122,7 +122,7 @@ static List *get_qual_for_list(PartitionKey key, PartitionBoundSpec *spec);
static List *get_qual_for_range(PartitionKey key, PartitionBoundSpec *spec);
static Oid get_partition_operator(PartitionKey key, int col,
StrategyNumber strategy, bool *need_relabel);
-static List *generate_partition_qual(Relation rel, bool recurse);
+static List *generate_partition_qual(Relation rel);
static PartitionRangeBound *make_one_range_bound(PartitionKey key, int index,
List *datums, bool lower);
@@ -850,10 +850,6 @@ get_qual_from_partbound(Relation rel, Relation parent, Node *bound)
PartitionBoundSpec *spec = (PartitionBoundSpec *) bound;
PartitionKey key = RelationGetPartitionKey(parent);
List *my_qual = NIL;
- TupleDesc parent_tupdesc = RelationGetDescr(parent);
- AttrNumber parent_attno;
- AttrNumber *partition_attnos;
- bool found_whole_row;
Assert(key != NULL);
@@ -874,38 +870,48 @@ get_qual_from_partbound(Relation rel, Relation parent, Node *bound)
(int) key->strategy);
}
- /*
- * Translate vars in the generated expression to have correct attnos. Note
- * that the vars in my_qual bear attnos dictated by key which carries
- * physical attnos of the parent. We must allow for a case where physical
- * attnos of a partition can be different from the parent.
- */
- partition_attnos = (AttrNumber *)
- palloc0(parent_tupdesc->natts * sizeof(AttrNumber));
- for (parent_attno = 1; parent_attno <= parent_tupdesc->natts;
- parent_attno++)
+ return my_qual;
+}
+
+/*
+ * map_partition_varattnos - maps varattno of any Vars in expr from the
+ * parent attno to partition attno.
+ *
+ * We must allow for a case where physical attnos of a partition can be
+ * different from the parent's.
+ */
+List *
+map_partition_varattnos(List *expr, Relation partrel, Relation parent)
+{
+ TupleDesc tupdesc = RelationGetDescr(parent);
+ AttrNumber attno;
+ AttrNumber *part_attnos;
+ bool found_whole_row;
+
+ part_attnos = (AttrNumber *) palloc0(tupdesc->natts * sizeof(AttrNumber));
+ for (attno = 1; attno <= tupdesc->natts; attno++)
{
- Form_pg_attribute attribute = parent_tupdesc->attrs[parent_attno - 1];
+ Form_pg_attribute attribute = tupdesc->attrs[attno - 1];
char *attname = NameStr(attribute->attname);
- AttrNumber partition_attno;
+ AttrNumber part_attno;
if (attribute->attisdropped)
continue;
- partition_attno = get_attnum(RelationGetRelid(rel), attname);
- partition_attnos[parent_attno - 1] = partition_attno;
+ part_attno = get_attnum(RelationGetRelid(partrel), attname);
+ part_attnos[attno - 1] = part_attno;
}
- my_qual = (List *) map_variable_attnos((Node *) my_qual,
- 1, 0,
- partition_attnos,
- parent_tupdesc->natts,
- &found_whole_row);
- /* there can never be a whole-row reference here */
+ expr = (List *) map_variable_attnos((Node *) expr,
+ 1, 0,
+ part_attnos,
+ tupdesc->natts,
+ &found_whole_row);
+ /* There can never be a whole-row reference here */
if (found_whole_row)
elog(ERROR, "unexpected whole-row reference found in partition key");
- return my_qual;
+ return expr;
}
/*
@@ -914,13 +920,13 @@ get_qual_from_partbound(Relation rel, Relation parent, Node *bound)
* Returns a list of partition quals
*/
List *
-RelationGetPartitionQual(Relation rel, bool recurse)
+RelationGetPartitionQual(Relation rel)
{
/* Quick exit */
if (!rel->rd_rel->relispartition)
return NIL;
- return generate_partition_qual(rel, recurse);
+ return generate_partition_qual(rel);
}
/* Turn an array of OIDs with N elements into a list */
@@ -1445,7 +1451,7 @@ get_partition_operator(PartitionKey key, int col, StrategyNumber strategy,
* into cache memory.
*/
static List *
-generate_partition_qual(Relation rel, bool recurse)
+generate_partition_qual(Relation rel)
{
HeapTuple tuple;
MemoryContext oldcxt;
@@ -1459,6 +1465,10 @@ generate_partition_qual(Relation rel, bool recurse)
/* Guard against stack overflow due to overly deep partition tree */
check_stack_depth();
+ /* Recursive callers may not have checked themselves */
+ if (!rel->rd_rel->relispartition)
+ return NIL;
+
/* Grab at least an AccessShareLock on the parent table */
parent = heap_open(get_partition_parent(RelationGetRelid(rel)),
AccessShareLock);
@@ -1466,20 +1476,18 @@ generate_partition_qual(Relation rel, bool recurse)
/* Quick copy */
if (rel->rd_partcheck)
{
- if (parent->rd_rel->relispartition && recurse)
- result = list_concat(generate_partition_qual(parent, true),
- copyObject(rel->rd_partcheck));
- else
- result = copyObject(rel->rd_partcheck);
+ result = list_concat(generate_partition_qual(parent),
+ copyObject(rel->rd_partcheck));
- heap_close(parent, AccessShareLock);
+ /* Mark Vars with correct attnos */
+ result = map_partition_varattnos(result, rel, parent);
+
+ /* Keep the parent locked until commit */
+ heap_close(parent, NoLock);
return result;
}
/* Get pg_class.relpartbound */
- if (!rel->rd_rel->relispartition) /* should not happen */
- elog(ERROR, "relation \"%s\" has relispartition = false",
- RelationGetRelationName(rel));
tuple = SearchSysCache1(RELOID, RelationGetRelid(rel));
boundDatum = SysCacheGetAttr(RELOID, tuple,
Anum_pg_class_relpartbound,
@@ -1492,18 +1500,16 @@ generate_partition_qual(Relation rel, bool recurse)
my_qual = get_qual_from_partbound(rel, parent, bound);
- /* If requested, add parent's quals to the list (if any) */
- if (parent->rd_rel->relispartition && recurse)
- {
- List *parent_check;
-
- parent_check = generate_partition_qual(parent, true);
- result = list_concat(parent_check, my_qual);
- }
+ /* Add the parent's quals to the list (if any) */
+ if (parent->rd_rel->relispartition)
+ result = list_concat(generate_partition_qual(parent), my_qual);
else
result = my_qual;
- /* Save a copy of my_qual in the relcache */
+ /* Mark Vars with correct attnos */
+ result = map_partition_varattnos(result, rel, parent);
+
+ /* Save a copy of *only* this rel's partition qual in the relcache */
oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
rel->rd_partcheck = copyObject(my_qual);
MemoryContextSwitchTo(oldcxt);
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index d5901651db..9cd84e80c7 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, NULL, NULL,
0);
ExecOpenIndices(resultRelInfo, false);
@@ -2436,6 +2441,15 @@ CopyFrom(CopyState cstate)
estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate);
/*
+ * Initialize a dedicated slot to manipulate tuples of any given
+ * partition's rowtype.
+ */
+ if (cstate->partition_dispatch_info)
+ estate->es_partition_tuple_slot = ExecInitExtraTupleSlot(estate);
+ else
+ estate->es_partition_tuple_slot = NULL;
+
+ /*
* 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()
* separately for every tuple. However, we can't do that if there are
@@ -2484,7 +2498,8 @@ CopyFrom(CopyState cstate)
for (;;)
{
- TupleTableSlot *slot;
+ TupleTableSlot *slot,
+ *oldslot = NULL;
bool skip_tuple;
Oid loaded_oid = InvalidOid;
@@ -2571,7 +2586,19 @@ CopyFrom(CopyState cstate)
map = cstate->partition_tupconv_maps[leaf_part_index];
if (map)
{
+ Relation partrel = resultRelInfo->ri_RelationDesc;
+
tuple = do_convert_tuple(tuple, map);
+
+ /*
+ * We must use the partition's tuple descriptor from this
+ * point on. Use a dedicated slot from this point on until
+ * we're finished dealing with the partition.
+ */
+ oldslot = slot;
+ slot = estate->es_partition_tuple_slot;
+ Assert(slot != NULL);
+ ExecSetSlotDescriptor(slot, RelationGetDescr(partrel));
ExecStoreTuple(tuple, slot, InvalidBuffer, true);
}
@@ -2667,6 +2694,10 @@ CopyFrom(CopyState cstate)
{
resultRelInfo = saved_resultRelInfo;
estate->es_result_relation_info = resultRelInfo;
+
+ /* Switch back to the slot corresponding to the root table */
+ Assert(oldslot != NULL);
+ slot = oldslot;
}
}
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1c219b03dd..67ff1715ea 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, NULL, NULL,
0);
resultRelInfo++;
}
@@ -13151,7 +13151,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
*/
partConstraint = list_concat(get_qual_from_partbound(attachRel, rel,
cmd->bound),
- RelationGetPartitionQual(rel, true));
+ RelationGetPartitionQual(rel));
partConstraint = (List *) eval_const_expressions(NULL,
(Node *) partConstraint);
partConstraint = (List *) canonicalize_qual((Expr *) partConstraint);
@@ -13323,6 +13323,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
Oid part_relid = lfirst_oid(lc);
Relation part_rel;
Expr *constr;
+ List *my_constr;
/* Lock already taken */
if (part_relid != RelationGetRelid(attachRel))
@@ -13345,8 +13346,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
tab = ATGetQueueEntry(wqueue, part_rel);
constr = linitial(partConstraint);
- tab->partition_constraint = make_ands_implicit((Expr *) constr);
-
+ my_constr = make_ands_implicit((Expr *) constr);
+ tab->partition_constraint = map_partition_varattnos(my_constr,
+ part_rel,
+ rel);
/* keep our lock until commit */
if (part_rel != attachRel)
heap_close(part_rel, NoLock);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index bca34a509c..055efdcf9c 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, NULL, NULL,
estate->es_instrument);
resultRelInfo++;
}
@@ -1217,7 +1223,9 @@ void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
+ Relation partition_root,
+ TupleConversionMap *partition_reverse_map,
int instrument_options)
{
MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
@@ -1255,10 +1263,13 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
- if (load_partition_check)
- resultRelInfo->ri_PartitionCheck =
- RelationGetPartitionQual(resultRelationDesc,
- true);
+ resultRelInfo->ri_PartitionCheck = partition_check;
+ /*
+ * Following fields are only looked at in some tuple-routing cases.
+ * In other case, they are set to NULL.
+ */
+ resultRelInfo->ri_PartitionRoot = partition_root;
+ resultRelInfo->ri_PartitionReverseMap = partition_reverse_map;
}
/*
@@ -1285,6 +1296,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;
@@ -1313,6 +1325,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.
*/
@@ -1321,7 +1337,7 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- true,
+ partcheck, NULL, NULL,
estate->es_instrument);
estate->es_trig_target_relations =
lappend(estate->es_trig_target_relations, rInfo);
@@ -1767,6 +1783,26 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
slot_attisnull(slot, attrChk))
{
char *val_desc;
+ Relation orig_rel = rel;
+ TupleDesc orig_tupdesc = tupdesc;
+
+ /*
+ * In case where the tuple is routed, it's been converted
+ * to the partition's rowtype, which might differ from the
+ * root table's. We must convert it back to the root table's
+ * type so that it's shown correctly in the error message.
+ */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ HeapTuple tuple = ExecFetchSlotTuple(slot);
+
+ rel = resultRelInfo->ri_PartitionRoot;
+ tupdesc = RelationGetDescr(rel);
+ Assert(resultRelInfo->ri_PartitionReverseMap != NULL);
+ tuple = do_convert_tuple(tuple,
+ resultRelInfo->ri_PartitionReverseMap);
+ ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ }
insertedCols = GetInsertedColumns(resultRelInfo, estate);
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
@@ -1780,9 +1816,9 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
ereport(ERROR,
(errcode(ERRCODE_NOT_NULL_VIOLATION),
errmsg("null value in column \"%s\" violates not-null constraint",
- NameStr(tupdesc->attrs[attrChk - 1]->attname)),
+ NameStr(orig_tupdesc->attrs[attrChk - 1]->attname)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtablecol(rel, attrChk)));
+ errtablecol(orig_rel, attrChk)));
}
}
}
@@ -1794,6 +1830,20 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL)
{
char *val_desc;
+ Relation orig_rel = rel;
+
+ /* See the comment above. */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ HeapTuple tuple = ExecFetchSlotTuple(slot);
+
+ rel = resultRelInfo->ri_PartitionRoot;
+ tupdesc = RelationGetDescr(rel);
+ Assert(resultRelInfo->ri_PartitionReverseMap != NULL);
+ tuple = do_convert_tuple(tuple,
+ resultRelInfo->ri_PartitionReverseMap);
+ ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ }
insertedCols = GetInsertedColumns(resultRelInfo, estate);
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
@@ -1806,9 +1856,9 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates check constraint \"%s\"",
- RelationGetRelationName(rel), failed),
+ RelationGetRelationName(orig_rel), failed),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0,
- errtableconstraint(rel, failed)));
+ errtableconstraint(orig_rel, failed)));
}
}
@@ -1816,6 +1866,20 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
!ExecPartitionCheck(resultRelInfo, slot, estate))
{
char *val_desc;
+ Relation orig_rel = rel;
+
+ /* See the comment above. */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ HeapTuple tuple = ExecFetchSlotTuple(slot);
+
+ rel = resultRelInfo->ri_PartitionRoot;
+ tupdesc = RelationGetDescr(rel);
+ Assert(resultRelInfo->ri_PartitionReverseMap != NULL);
+ tuple = do_convert_tuple(tuple,
+ resultRelInfo->ri_PartitionReverseMap);
+ ExecStoreTuple(tuple, slot, InvalidBuffer, false);
+ }
insertedCols = GetInsertedColumns(resultRelInfo, estate);
updatedCols = GetUpdatedColumns(resultRelInfo, estate);
@@ -1828,7 +1892,7 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
ereport(ERROR,
(errcode(ERRCODE_CHECK_VIOLATION),
errmsg("new row for relation \"%s\" violates partition constraint",
- RelationGetRelationName(rel)),
+ RelationGetRelationName(orig_rel)),
val_desc ? errdetail("Failing row contains %s.", val_desc) : 0));
}
}
@@ -3033,6 +3097,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,
@@ -3043,12 +3108,23 @@ 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;
+ TupleConversionMap *reverse_map;
/*
* We locked all the partitions above including the leaf partitions.
@@ -3070,10 +3146,28 @@ 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.
+ */
+ if (partcheck)
+ my_check = map_partition_varattnos(partcheck, partrel, rel);
+
+ /*
+ * We must save a reverse tuple conversion map as well, to show the
+ * correct input tuple in the error message shown by ExecConstraints()
+ * in case of routed tuples. Remember that at the point of failure,
+ * the tuple has been converted to the partition's type which might
+ * not match the input tuple.
+ */
+ reverse_map = convert_tuples_by_name(part_tupdesc, tupDesc,
+ gettext_noop("could not convert row type"));
+
InitResultRelInfo(leaf_part_rri,
partrel,
1, /* dummy */
- false,
+ my_check, rel, reverse_map,
0);
/*
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index a9546106ce..da4c96a863 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -262,6 +262,7 @@ ExecInsert(ModifyTableState *mtstate,
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
+ TupleTableSlot *oldslot = NULL;
/*
* get the heap tuple out of the tuple table slot, making sure we have a
@@ -318,7 +319,19 @@ ExecInsert(ModifyTableState *mtstate,
map = mtstate->mt_partition_tupconv_maps[leaf_part_index];
if (map)
{
+ Relation partrel = resultRelInfo->ri_RelationDesc;
+
tuple = do_convert_tuple(tuple, map);
+
+ /*
+ * We must use the partition's tuple descriptor from this
+ * point on, until we're finished dealing with the partition.
+ * Use the dedicated slot for that.
+ */
+ oldslot = slot;
+ slot = estate->es_partition_tuple_slot;
+ Assert(slot != NULL);
+ ExecSetSlotDescriptor(slot, RelationGetDescr(partrel));
ExecStoreTuple(tuple, slot, InvalidBuffer, true);
}
}
@@ -566,6 +579,10 @@ ExecInsert(ModifyTableState *mtstate,
{
resultRelInfo = saved_resultRelInfo;
estate->es_result_relation_info = resultRelInfo;
+
+ /* Switch back to the slot corresponding to the root table */
+ Assert(oldslot != NULL);
+ slot = oldslot;
}
/*
@@ -1734,7 +1751,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_partitions = partitions;
mtstate->mt_num_partitions = num_partitions;
mtstate->mt_partition_tupconv_maps = partition_tupconv_maps;
+
+ /*
+ * Initialize a dedicated slot to manipulate tuples of any given
+ * partition's rowtype.
+ */
+ estate->es_partition_tuple_slot = ExecInitExtraTupleSlot(estate);
}
+ else
+ estate->es_partition_tuple_slot = NULL;
/*
* Initialize any WITH CHECK OPTION constraints if needed.
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 72272d9bb7..150229ed6d 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -1228,7 +1228,7 @@ get_relation_constraints(PlannerInfo *root,
}
/* Append partition predicates, if any */
- pcqual = RelationGetPartitionQual(relation, true);
+ pcqual = RelationGetPartitionQual(relation);
if (pcqual)
{
/*
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index 21effbf87b..6ff821e6cf 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -70,7 +70,8 @@ extern bool partition_bounds_equal(PartitionKey key,
extern void check_new_partition_bound(char *relname, Relation parent, Node *bound);
extern Oid get_partition_parent(Oid relid);
extern List *get_qual_from_partbound(Relation rel, Relation parent, Node *bound);
-extern List *RelationGetPartitionQual(Relation rel, bool recurse);
+extern List *map_partition_varattnos(List *expr, Relation partrel, Relation parent);
+extern List *RelationGetPartitionQual(Relation rel);
/* For tuple routing */
extern PartitionDispatch *RelationGetPartitionDispatchInfo(Relation rel,
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index b74fa5eb5d..1cc5f2eb94 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,9 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- bool load_partition_check,
+ List *partition_check,
+ Relation partition_root,
+ TupleConversionMap *partition_reverse_map,
int instrument_options);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 5c3b8683f5..49db19cc75 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -349,6 +349,8 @@ typedef struct ResultRelInfo
List *ri_onConflictSetWhere;
List *ri_PartitionCheck;
List *ri_PartitionCheckExpr;
+ Relation ri_PartitionRoot;
+ TupleConversionMap *ri_PartitionReverseMap;
} ResultRelInfo;
/* ----------------
@@ -384,6 +386,9 @@ typedef struct EState
TupleTableSlot *es_trig_oldtup_slot; /* for TriggerEnabled */
TupleTableSlot *es_trig_newtup_slot; /* for TriggerEnabled */
+ /* Slot used to manipulate a tuple after it is routed to a partition */
+ TupleTableSlot *es_partition_tuple_slot;
+
/* Parameter info: */
ParamListInfo es_param_list_info; /* values of external params */
ParamExecData *es_param_exec_vals; /* values of internal params */
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 99e20eb922..0bf488884a 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3317,3 +3317,29 @@ drop cascades to table part_2
drop cascades to table part_5
drop cascades to table part_5_a
drop cascades to table part_1
+-- multi-level partitions with varying attnums for the same key attribute
+create table p (a int, b int) partition by range (a, b);
+create table p1 (b int, a int not null) partition by range (b);
+create table p11 (like p1);
+alter table p11 drop a;
+alter table p11 add a int;
+alter table p11 drop a;
+alter table p11 add a int not null;
+alter table p1 attach partition p11 for values from (1) to (5);
+-- check attnums for key attribute 'a'
+select attrelid::regclass, attname, attnum
+from pg_attribute
+where attname = 'a' and (attrelid = 'p'::regclass or attrelid = 'p1'::regclass or attrelid = 'p11'::regclass);
+ attrelid | attname | attnum
+----------+---------+--------
+ p | a | 1
+ p1 | a | 2
+ p11 | a | 4
+(3 rows)
+
+-- check that attaching partition fails with violating rows in a leaf partition
+insert into p11 (a, b) values (10, 4);
+alter table p attach partition p1 for values from (1, 1) to (1, 10);
+ERROR: partition constraint is violated by some row
+-- cleanup
+drop table p cascade;
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 561cefa3c4..e785885723 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -300,3 +300,39 @@ drop cascades to table part_null
drop cascades to table part_ee_ff
drop cascades to table part_ee_ff1
drop cascades to table part_ee_ff2
+-- multi-level partitions with varying attnums for the same key attribute
+create table p (a int, b int) partition by range (a, b);
+create table p1 (b int, a int not null) partition by range (b);
+create table p11 (like p1);
+alter table p11 drop a;
+alter table p11 add a int;
+alter table p11 drop a;
+alter table p11 add a int not null;
+alter table p1 attach partition p11 for values from (1) to (5);
+alter table p attach partition p1 for values from (1, 1) to (1, 10);
+-- check attnums for key attribute 'a'
+select attrelid::regclass, attname, attnum
+from pg_attribute
+where attname = 'a' and (attrelid = 'p'::regclass or attrelid = 'p1'::regclass or attrelid = 'p11'::regclass);
+ attrelid | attname | attnum
+----------+---------+--------
+ p | a | 1
+ p1 | a | 2
+ p11 | a | 4
+(3 rows)
+
+-- check that following inserts fail due to either partition constraint or tuple-routing failure
+insert into p11 (a, b) values (10, 4);
+ERROR: new row for relation "p11" violates partition constraint
+DETAIL: Failing row contains (4, 10).
+insert into p1 (a, b) values (10, 4);
+ERROR: new row for relation "p11" violates partition constraint
+DETAIL: Failing row contains (4, 10).
+insert into p (a, b) values (10, 4);
+ERROR: no partition of relation "p" found for row
+DETAIL: Failing row contains (10, 4).
+-- cleanup
+drop table p cascade;
+NOTICE: drop cascades to 2 other objects
+DETAIL: drop cascades to table p1
+drop cascades to table p11
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index b285a406d9..2cd9d16648 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2169,3 +2169,23 @@ ALTER TABLE list_parted2 ALTER COLUMN b TYPE text;
-- cleanup
DROP TABLE list_parted, list_parted2, range_parted CASCADE;
+
+-- multi-level partitions with varying attnums for the same key attribute
+create table p (a int, b int) partition by range (a, b);
+create table p1 (b int, a int not null) partition by range (b);
+create table p11 (like p1);
+alter table p11 drop a;
+alter table p11 add a int;
+alter table p11 drop a;
+alter table p11 add a int not null;
+alter table p1 attach partition p11 for values from (1) to (5);
+-- check attnums for key attribute 'a'
+select attrelid::regclass, attname, attnum
+from pg_attribute
+where attname = 'a' and (attrelid = 'p'::regclass or attrelid = 'p1'::regclass or attrelid = 'p11'::regclass);
+-- check that attaching partition fails with violating rows in a leaf partition
+insert into p11 (a, b) values (10, 4);
+alter table p attach partition p1 for values from (1, 1) to (1, 10);
+
+-- cleanup
+drop table p cascade;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 846bb5897a..656bae2d57 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -170,3 +170,25 @@ select tableoid::regclass, * from list_parted;
-- cleanup
drop table range_parted cascade;
drop table list_parted cascade;
+
+-- multi-level partitions with varying attnums for the same key attribute
+create table p (a int, b int) partition by range (a, b);
+create table p1 (b int, a int not null) partition by range (b);
+create table p11 (like p1);
+alter table p11 drop a;
+alter table p11 add a int;
+alter table p11 drop a;
+alter table p11 add a int not null;
+alter table p1 attach partition p11 for values from (1) to (5);
+alter table p attach partition p1 for values from (1, 1) to (1, 10);
+-- check attnums for key attribute 'a'
+select attrelid::regclass, attname, attnum
+from pg_attribute
+where attname = 'a' and (attrelid = 'p'::regclass or attrelid = 'p1'::regclass or attrelid = 'p11'::regclass);
+-- check that following inserts fail due to either partition constraint or tuple-routing failure
+insert into p11 (a, b) values (10, 4);
+insert into p1 (a, b) values (10, 4);
+insert into p (a, b) values (10, 4);
+
+-- cleanup
+drop table p cascade;
--
2.11.0
>From 33c6af64f9eb3a36c920ff220215dc9efb1f174c Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 21 Dec 2016 10:51:32 +0900
Subject: [PATCH 3/3] Add some more tests for tuple-routing
We fixed some issues with how PartitionDispatch related code handled
multi-level partitioned tables in commit a25665088d, but didn't add
any tests.
Reported by: Dmitry Ivanov, Robert Haas
Patch by: Amit Langote
Reports: https://www.postgresql.org/message-id/0d5b64c9-fa05-4dab-93e7-56576d1193ca%40postgrespro.ru
https://www.postgresql.org/message-id/CA%2BTgmoZ86v1G%2Bzx9etMiSQaBBvYMKfU-iitqZArSh5z0n8Q4cA%40mail.gmail.com
---
src/test/regress/expected/insert.out | 38 +++++++++++++++++++++++++++++++++++-
src/test/regress/sql/insert.sql | 18 +++++++++++++++++
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index e785885723..b5307fd79a 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -285,6 +285,34 @@ select tableoid::regclass, * from list_parted;
part_ee_ff2 | EE | 10
(8 rows)
+-- some more tests to exercise tuple-routing with multi-level partitioning
+create table part_gg partition of list_parted for values in ('gg') partition by range (b);
+create table part_gg1 partition of part_gg for values from (unbounded) to (1);
+create table part_gg2 partition of part_gg for values from (1) to (10) partition by range (b);
+create table part_gg2_1 partition of part_gg2 for values from (1) to (5);
+create table part_gg2_2 partition of part_gg2 for values from (5) to (10);
+create table part_ee_ff3 partition of part_ee_ff for values from (20) to (30) partition by range (b);
+create table part_ee_ff3_1 partition of part_ee_ff3 for values from (20) to (25);
+create table part_ee_ff3_2 partition of part_ee_ff3 for values from (25) to (30);
+truncate list_parted;
+insert into list_parted values ('aa'), ('cc');
+insert into list_parted select 'Ff', s.a from generate_series(1, 29) s(a);
+insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
+insert into list_parted (b) values (1);
+select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+ tableoid | a | min_b | max_b
+---------------+----+-------+-------
+ part_aa_bb | aa | |
+ part_cc_dd | cc | |
+ part_ee_ff1 | Ff | 1 | 9
+ part_ee_ff2 | Ff | 10 | 19
+ part_ee_ff3_1 | Ff | 20 | 24
+ part_ee_ff3_2 | Ff | 25 | 29
+ part_gg2_1 | gg | 1 | 4
+ part_gg2_2 | gg | 5 | 9
+ part_null | | 1 | 1
+(9 rows)
+
-- cleanup
drop table range_parted cascade;
NOTICE: drop cascades to 4 other objects
@@ -293,13 +321,21 @@ drop cascades to table part2
drop cascades to table part3
drop cascades to table part4
drop table list_parted cascade;
-NOTICE: drop cascades to 6 other objects
+NOTICE: drop cascades to 14 other objects
DETAIL: drop cascades to table part_aa_bb
drop cascades to table part_cc_dd
drop cascades to table part_null
drop cascades to table part_ee_ff
drop cascades to table part_ee_ff1
drop cascades to table part_ee_ff2
+drop cascades to table part_ee_ff3
+drop cascades to table part_ee_ff3_1
+drop cascades to table part_ee_ff3_2
+drop cascades to table part_gg
+drop cascades to table part_gg1
+drop cascades to table part_gg2
+drop cascades to table part_gg2_1
+drop cascades to table part_gg2_2
-- multi-level partitions with varying attnums for the same key attribute
create table p (a int, b int) partition by range (a, b);
create table p1 (b int, a int not null) partition by range (b);
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 656bae2d57..b3a8bda84c 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -167,6 +167,24 @@ insert into list_parted values ('EE', 1);
insert into part_ee_ff values ('EE', 10);
select tableoid::regclass, * from list_parted;
+-- some more tests to exercise tuple-routing with multi-level partitioning
+create table part_gg partition of list_parted for values in ('gg') partition by range (b);
+create table part_gg1 partition of part_gg for values from (unbounded) to (1);
+create table part_gg2 partition of part_gg for values from (1) to (10) partition by range (b);
+create table part_gg2_1 partition of part_gg2 for values from (1) to (5);
+create table part_gg2_2 partition of part_gg2 for values from (5) to (10);
+
+create table part_ee_ff3 partition of part_ee_ff for values from (20) to (30) partition by range (b);
+create table part_ee_ff3_1 partition of part_ee_ff3 for values from (20) to (25);
+create table part_ee_ff3_2 partition of part_ee_ff3 for values from (25) to (30);
+
+truncate list_parted;
+insert into list_parted values ('aa'), ('cc');
+insert into list_parted select 'Ff', s.a from generate_series(1, 29) s(a);
+insert into list_parted select 'gg', s.a from generate_series(1, 9) s(a);
+insert into list_parted (b) values (1);
+select tableoid::regclass::text, a, min(b) as min_b, max(b) as max_b from list_parted group by 1, 2 order by 1;
+
-- cleanup
drop table range_parted cascade;
drop table list_parted cascade;
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
