public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 4/7] Fix a bug of insertion into an internal partition.
8+ messages / 3 participants
[nested] [flat]

* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07  amit <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: amit @ 2016-12-13 06:07 UTC (permalink / raw)

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.

Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
 src/backend/commands/copy.c          |  1 -
 src/backend/commands/tablecmds.c     |  1 -
 src/backend/executor/execMain.c      | 41 ++++++++++++++++++++++++++++--------
 src/include/executor/executor.h      |  1 -
 src/test/regress/expected/insert.out |  6 ++++++
 src/test/regress/sql/insert.sql      |  5 +++++
 6 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index afbfb9f6e8..5aa4449e3e 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2429,7 +2429,6 @@ CopyFrom(CopyState cstate)
 	InitResultRelInfo(resultRelInfo,
 					  cstate->rel,
 					  1,		/* dummy rangetable index */
-					  true,		/* do load partition check expression */
 					  NULL,
 					  0);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3c08551d38..2e51124eb3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,6 @@ ExecuteTruncate(TruncateStmt *stmt)
 		InitResultRelInfo(resultRelInfo,
 						  rel,
 						  0,	/* dummy rangetable index */
-						  false,
 						  NULL,
 						  0);
 		resultRelInfo++;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6a82c18571..c991a18ce8 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -824,10 +824,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 
 			resultRelationOid = getrelid(resultRelationIndex, rangeTable);
 			resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
 			InitResultRelInfo(resultRelInfo,
 							  resultRelation,
 							  resultRelationIndex,
-							  true,
 							  NULL,
 							  estate->es_instrument);
 			resultRelInfo++;
@@ -1218,10 +1218,11 @@ void
 InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
 				  Relation partition_root,
 				  int instrument_options)
 {
+	List   *partition_check = NIL;
+
 	MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
 	resultRelInfo->type = T_ResultRelInfo;
 	resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -1257,14 +1258,38 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
 	resultRelInfo->ri_ConstraintExprs = NULL;
 	resultRelInfo->ri_junkFilter = NULL;
 	resultRelInfo->ri_projectReturning = NULL;
-	if (load_partition_check)
-		resultRelInfo->ri_PartitionCheck =
-							RelationGetPartitionQual(resultRelationDesc);
 
 	/*
-	 * The following gets set to NULL unless we are initializing leaf
-	 * partitions for tuple-routing.
+	 * If partition_root has been specified, that means we are builiding the
+	 * ResultRelationInfo for one of its leaf partitions.  In that case, we
+	 * need *not* initialize the leaf partition's constraint, but rather the
+	 * the partition_root's (if any).  We must do that explicitly like this,
+	 * because implicit partition constraints are not inherited like user-
+	 * defined constraints and would fail to be enforced by ExecConstraints()
+	 * after a tuple is routed to a leaf partition.
 	 */
+	if (partition_root)
+	{
+		/*
+		 * Root table itself may or may not be a partition; partition_check
+		 * would be NIL in the latter case.
+		 */
+		partition_check = RelationGetPartitionQual(partition_root);
+
+		/*
+		 * This is not our own partition constraint, but rather an ancestor's.
+		 * So any Vars in it bear the ancestor's attribute numbers.  We must
+		 * switch them to our own.
+		 */
+		if (partition_check != NIL)
+			partition_check = map_partition_varattnos(partition_check,
+													  resultRelationDesc,
+													  partition_root);
+	}
+	else
+		partition_check = RelationGetPartitionQual(resultRelationDesc);
+
+	resultRelInfo->ri_PartitionCheck = partition_check;
 	resultRelInfo->ri_PartitionRoot = partition_root;
 }
 
@@ -1328,7 +1353,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
 	InitResultRelInfo(rInfo,
 					  rel,
 					  0,		/* dummy rangetable index */
-					  true,
 					  NULL,
 					  estate->es_instrument);
 	estate->es_trig_target_relations =
@@ -3133,7 +3157,6 @@ ExecSetupPartitionTupleRouting(Relation rel,
 		InitResultRelInfo(leaf_part_rri,
 						  partrel,
 						  1,	 /* dummy */
-						  false,
 						  rel,
 						  0);
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 70ecf108a3..4fed4e101f 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,6 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
 extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
 				  Relation partition_root,
 				  int instrument_options);
 extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b120954997..6a6aac5a88 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -339,6 +339,12 @@ alter table p add constraint check_b check (b = 3);
 insert into p values (1, 2);
 ERROR:  new row for relation "p11" violates check constraint "check_b"
 DETAIL:  Failing row contains (1, 2).
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+ERROR:  new row for relation "p11" violates partition constraint
+DETAIL:  Failing row contains (3, 2).
 -- cleanup
 drop table p cascade;
 NOTICE:  drop cascades to 2 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 3d2fdb92c5..171196df6d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -200,5 +200,10 @@ alter table p add constraint check_b check (b = 3);
 -- after "(1, 2)" is routed to it
 insert into p values (1, 2);
 
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+
 -- cleanup
 drop table p cascade;
-- 
2.11.0


--------------AEE611E30AEE235CC4F15C3B
Content-Type: text/x-diff;
 name="0005-Add-some-more-tests-for-tuple-routing.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0005-Add-some-more-tests-for-tuple-routing.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07  amit <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: amit @ 2016-12-13 06:07 UTC (permalink / raw)

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.

Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
 src/backend/commands/copy.c          |  7 +++++-
 src/backend/commands/tablecmds.c     |  2 +-
 src/backend/executor/execMain.c      | 42 ++++++++++++++++++++++++++++++------
 src/include/executor/executor.h      |  3 +--
 src/test/regress/expected/insert.out |  4 ++++
 src/test/regress/sql/insert.sql      |  3 +++
 6 files changed, 50 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..1b19bc38ef 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,18 @@ 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);
+
 		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 b74fa5eb5d..c8e42ae2eb 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


--------------6BBEDC11B506E66A7D75776C
Content-Type: text/x-diff;
 name="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots-2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots-2.pat";
 filename*1="ch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH 4/5] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07  amit <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: amit @ 2016-12-13 06:07 UTC (permalink / raw)

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.

Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
 src/backend/commands/copy.c          |  1 -
 src/backend/commands/tablecmds.c     |  1 -
 src/backend/executor/execMain.c      | 41 ++++++++++++++++++++++++++++--------
 src/include/executor/executor.h      |  1 -
 src/test/regress/expected/insert.out |  6 ++++++
 src/test/regress/sql/insert.sql      |  5 +++++
 6 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index a20e22daaf..2cb89190e7 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2419,7 +2419,6 @@ CopyFrom(CopyState cstate)
 	InitResultRelInfo(resultRelInfo,
 					  cstate->rel,
 					  1,		/* dummy rangetable index */
-					  true,		/* do load partition check expression */
 					  NULL,
 					  0);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3db830f0c7..096ca181be 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,6 @@ ExecuteTruncate(TruncateStmt *stmt)
 		InitResultRelInfo(resultRelInfo,
 						  rel,
 						  0,	/* dummy rangetable index */
-						  false,
 						  NULL,
 						  0);
 		resultRelInfo++;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6954ba8d32..3e2c1cb790 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -824,10 +824,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 
 			resultRelationOid = getrelid(resultRelationIndex, rangeTable);
 			resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
 			InitResultRelInfo(resultRelInfo,
 							  resultRelation,
 							  resultRelationIndex,
-							  true,
 							  NULL,
 							  estate->es_instrument);
 			resultRelInfo++;
@@ -1218,10 +1218,11 @@ void
 InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
 				  Relation partition_root,
 				  int instrument_options)
 {
+	List   *partition_check = NIL;
+
 	MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
 	resultRelInfo->type = T_ResultRelInfo;
 	resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -1257,14 +1258,38 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
 	resultRelInfo->ri_ConstraintExprs = NULL;
 	resultRelInfo->ri_junkFilter = NULL;
 	resultRelInfo->ri_projectReturning = NULL;
-	if (load_partition_check)
-		resultRelInfo->ri_PartitionCheck =
-							RelationGetPartitionQual(resultRelationDesc);
 
 	/*
-	 * The following gets set to NULL unless we are initializing leaf
-	 * partitions for tuple-routing.
+	 * If partition_root has been specified, that means we are builiding the
+	 * ResultRelationInfo for one of its leaf partitions.  In that case, we
+	 * need *not* initialize the leaf partition's constraint, but rather the
+	 * the partition_root's (if any).  We must do that explicitly like this,
+	 * because implicit partition constraints are not inherited like user-
+	 * defined constraints and would fail to be enforced by ExecConstraints()
+	 * after a tuple is routed to a leaf partition.
 	 */
+	if (partition_root)
+	{
+		/*
+		 * Root table itself may or may not be a partition; partition_check
+		 * would be NIL in the latter case.
+		 */
+		partition_check = RelationGetPartitionQual(partition_root);
+
+		/*
+		 * This is not our own partition constraint, but rather an ancestor's.
+		 * So any Vars in it bear the ancestor's attribute numbers.  We must
+		 * switch them to our own.
+		 */
+		if (partition_check != NIL)
+			partition_check = map_partition_varattnos(partition_check,
+													  resultRelationDesc,
+													  partition_root);
+	}
+	else
+		partition_check = RelationGetPartitionQual(resultRelationDesc);
+
+	resultRelInfo->ri_PartitionCheck = partition_check;
 	resultRelInfo->ri_PartitionRoot = partition_root;
 }
 
@@ -1328,7 +1353,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
 	InitResultRelInfo(rInfo,
 					  rel,
 					  0,		/* dummy rangetable index */
-					  true,
 					  NULL,
 					  estate->es_instrument);
 	estate->es_trig_target_relations =
@@ -3137,7 +3161,6 @@ ExecSetupPartitionTupleRouting(Relation rel,
 		InitResultRelInfo(leaf_part_rri,
 						  partrel,
 						  1,	 /* dummy */
-						  false,
 						  rel,
 						  0);
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index f85b26b00a..9c7878f847 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,6 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
 extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
 				  Relation partition_root,
 				  int instrument_options);
 extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b120954997..6a6aac5a88 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -339,6 +339,12 @@ alter table p add constraint check_b check (b = 3);
 insert into p values (1, 2);
 ERROR:  new row for relation "p11" violates check constraint "check_b"
 DETAIL:  Failing row contains (1, 2).
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+ERROR:  new row for relation "p11" violates partition constraint
+DETAIL:  Failing row contains (3, 2).
 -- cleanup
 drop table p cascade;
 NOTICE:  drop cascades to 2 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 3d2fdb92c5..171196df6d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -200,5 +200,10 @@ alter table p add constraint check_b check (b = 3);
 -- after "(1, 2)" is routed to it
 insert into p values (1, 2);
 
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+
 -- cleanup
 drop table p cascade;
-- 
2.11.0


--------------57363B45666F3B09901F4C0C
Content-Type: text/x-diff;
 name="0005-Add-some-more-tests-for-tuple-routing.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0005-Add-some-more-tests-for-tuple-routing.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07  amit <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: amit @ 2016-12-13 06:07 UTC (permalink / raw)

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


--------------487C465F02616E3296F6B86F
Content-Type: text/x-diff;
 name="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0005-Fix-oddities-of-tuple-routing-and-TupleTableSlots.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH 4/5] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07  amit <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: amit @ 2016-12-13 06:07 UTC (permalink / raw)

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.
---
 src/backend/commands/copy.c            | 19 +++++++++++++++++--
 src/backend/commands/tablecmds.c       |  2 +-
 src/backend/executor/execMain.c        | 22 +++++++++++++++-------
 src/backend/executor/nodeModifyTable.c | 12 +++++++++++-
 src/include/executor/executor.h        |  3 +--
 5 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index e9177491e2..b05055808f 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1411,6 +1411,7 @@ BeginCopy(ParseState *pstate,
 			int				i,
 							num_parted;
 			ResultRelInfo  *leaf_part_rri;
+			List		   *partcheck = NIL;
 
 			/* Get the tuple-routing information and lock partitions */
 			cstate->partition_dispatch_info =
@@ -1426,6 +1427,15 @@ BeginCopy(ParseState *pstate,
 									palloc0(cstate->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, true);
+
 			leaf_part_rri = cstate->partitions;
 			i = 0;
 			foreach(cell, leaf_parts)
@@ -1449,7 +1459,7 @@ BeginCopy(ParseState *pstate,
 				InitResultRelInfo(leaf_part_rri,
 								  partrel,
 								  1,	 /* dummy */
-								  false, /* no partition constraint check */
+								  partcheck,
 								  0);
 
 				/* Open partition indices */
@@ -2335,6 +2345,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 */
@@ -2451,6 +2462,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, true);
+
 	/*
 	 * 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
@@ -2460,7 +2475,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 f94cab60a3..48adeedbe0 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1329,7 +1329,7 @@ ExecuteTruncate(TruncateStmt *stmt)
 		InitResultRelInfo(resultRelInfo,
 						  rel,
 						  0,	/* dummy rangetable index */
-						  false,
+						  NIL,	/* No need for partition constraints */
 						  0);
 		resultRelInfo++;
 	}
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index d43a204808..963cd2ae43 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -820,13 +820,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, true);
+
 			InitResultRelInfo(resultRelInfo,
 							  resultRelation,
 							  resultRelationIndex,
-							  true,
+							  partcheck,
 							  estate->es_instrument);
 			resultRelInfo++;
 		}
@@ -1216,7 +1222,7 @@ void
 InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
+				  List *partition_check,
 				  int instrument_options)
 {
 	MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
@@ -1254,10 +1260,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,
-													 true);
+	resultRelInfo->ri_PartitionCheck = partition_check;
 }
 
 /*
@@ -1284,6 +1287,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 +1316,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, true);
+
 	/*
 	 * Make the new entry in the right context.
 	 */
@@ -1320,7 +1328,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);
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index ec440b353d..e236c0e0a7 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1724,6 +1724,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 		List			   *leaf_parts;
 		ListCell		   *cell;
 		ResultRelInfo	   *leaf_part_rri;
+		List			   *partcheck = NIL;
 
 		/* Get the tuple-routing information and lock partitions */
 		mtstate->mt_partition_dispatch_info =
@@ -1739,6 +1740,15 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 						palloc0(mtstate->mt_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, true);
+
 		leaf_part_rri = mtstate->mt_partitions;
 		i = j = 0;
 		foreach(cell, leaf_parts)
@@ -1762,7 +1772,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 			InitResultRelInfo(leaf_part_rri,
 							  partrel,
 							  1,		/* dummy */
-							  false,	/* no partition constraint checks */
+							  partcheck,
 							  eflags);
 
 			/* Open partition indices (note: ON CONFLICT unsupported)*/
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index b4d09f9564..74213da078 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);
-- 
2.11.0


--------------87B7BEABA41A98E25D2BEF30
Content-Type: text/x-diff;
 name="0005-Fix-a-tuple-routing-bug-in-multi-level-partitioned-t.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0005-Fix-a-tuple-routing-bug-in-multi-level-partitioned-t.pa";
 filename*1="tch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH 4/7] Fix a bug of insertion into an internal partition.
@ 2016-12-13 06:07  amit <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: amit @ 2016-12-13 06:07 UTC (permalink / raw)

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.

Reported by: n/a
Patch by: Amit Langote
Reports: n/a
---
 src/backend/commands/copy.c          |  1 -
 src/backend/commands/tablecmds.c     |  1 -
 src/backend/executor/execMain.c      | 41 ++++++++++++++++++++++++++++--------
 src/include/executor/executor.h      |  1 -
 src/test/regress/expected/insert.out |  6 ++++++
 src/test/regress/sql/insert.sql      |  5 +++++
 6 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index afbfb9f6e8..5aa4449e3e 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2429,7 +2429,6 @@ CopyFrom(CopyState cstate)
 	InitResultRelInfo(resultRelInfo,
 					  cstate->rel,
 					  1,		/* dummy rangetable index */
-					  true,		/* do load partition check expression */
 					  NULL,
 					  0);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3c08551d38..2e51124eb3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1323,7 +1323,6 @@ ExecuteTruncate(TruncateStmt *stmt)
 		InitResultRelInfo(resultRelInfo,
 						  rel,
 						  0,	/* dummy rangetable index */
-						  false,
 						  NULL,
 						  0);
 		resultRelInfo++;
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 6a82c18571..c991a18ce8 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -824,10 +824,10 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 
 			resultRelationOid = getrelid(resultRelationIndex, rangeTable);
 			resultRelation = heap_open(resultRelationOid, RowExclusiveLock);
+
 			InitResultRelInfo(resultRelInfo,
 							  resultRelation,
 							  resultRelationIndex,
-							  true,
 							  NULL,
 							  estate->es_instrument);
 			resultRelInfo++;
@@ -1218,10 +1218,11 @@ void
 InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
 				  Relation partition_root,
 				  int instrument_options)
 {
+	List   *partition_check = NIL;
+
 	MemSet(resultRelInfo, 0, sizeof(ResultRelInfo));
 	resultRelInfo->type = T_ResultRelInfo;
 	resultRelInfo->ri_RangeTableIndex = resultRelationIndex;
@@ -1257,14 +1258,38 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
 	resultRelInfo->ri_ConstraintExprs = NULL;
 	resultRelInfo->ri_junkFilter = NULL;
 	resultRelInfo->ri_projectReturning = NULL;
-	if (load_partition_check)
-		resultRelInfo->ri_PartitionCheck =
-							RelationGetPartitionQual(resultRelationDesc);
 
 	/*
-	 * The following gets set to NULL unless we are initializing leaf
-	 * partitions for tuple-routing.
+	 * If partition_root has been specified, that means we are builiding the
+	 * ResultRelationInfo for one of its leaf partitions.  In that case, we
+	 * need *not* initialize the leaf partition's constraint, but rather the
+	 * the partition_root's (if any).  We must do that explicitly like this,
+	 * because implicit partition constraints are not inherited like user-
+	 * defined constraints and would fail to be enforced by ExecConstraints()
+	 * after a tuple is routed to a leaf partition.
 	 */
+	if (partition_root)
+	{
+		/*
+		 * Root table itself may or may not be a partition; partition_check
+		 * would be NIL in the latter case.
+		 */
+		partition_check = RelationGetPartitionQual(partition_root);
+
+		/*
+		 * This is not our own partition constraint, but rather an ancestor's.
+		 * So any Vars in it bear the ancestor's attribute numbers.  We must
+		 * switch them to our own.
+		 */
+		if (partition_check != NIL)
+			partition_check = map_partition_varattnos(partition_check,
+													  resultRelationDesc,
+													  partition_root);
+	}
+	else
+		partition_check = RelationGetPartitionQual(resultRelationDesc);
+
+	resultRelInfo->ri_PartitionCheck = partition_check;
 	resultRelInfo->ri_PartitionRoot = partition_root;
 }
 
@@ -1328,7 +1353,6 @@ ExecGetTriggerResultRel(EState *estate, Oid relid)
 	InitResultRelInfo(rInfo,
 					  rel,
 					  0,		/* dummy rangetable index */
-					  true,
 					  NULL,
 					  estate->es_instrument);
 	estate->es_trig_target_relations =
@@ -3133,7 +3157,6 @@ ExecSetupPartitionTupleRouting(Relation rel,
 		InitResultRelInfo(leaf_part_rri,
 						  partrel,
 						  1,	 /* dummy */
-						  false,
 						  rel,
 						  0);
 
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 70ecf108a3..4fed4e101f 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -189,7 +189,6 @@ extern void CheckValidResultRel(Relation resultRel, CmdType operation);
 extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
 				  Relation resultRelationDesc,
 				  Index resultRelationIndex,
-				  bool load_partition_check,
 				  Relation partition_root,
 				  int instrument_options);
 extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index b120954997..6a6aac5a88 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -339,6 +339,12 @@ alter table p add constraint check_b check (b = 3);
 insert into p values (1, 2);
 ERROR:  new row for relation "p11" violates check constraint "check_b"
 DETAIL:  Failing row contains (1, 2).
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+ERROR:  new row for relation "p11" violates partition constraint
+DETAIL:  Failing row contains (3, 2).
 -- cleanup
 drop table p cascade;
 NOTICE:  drop cascades to 2 other objects
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 3d2fdb92c5..171196df6d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -200,5 +200,10 @@ alter table p add constraint check_b check (b = 3);
 -- after "(1, 2)" is routed to it
 insert into p values (1, 2);
 
+-- check that inserting into an internal partition successfully results in
+-- checking its partition constraint before inserting into the leaf partition
+-- selected by tuple-routing
+insert into p1 (a, b) values (2, 3);
+
 -- cleanup
 drop table p cascade;
-- 
2.11.0


--------------C187C3B1305137A7AECB8485
Content-Type: text/x-diff;
 name="0005-Add-some-more-tests-for-tuple-routing.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0005-Add-some-more-tests-for-tuple-routing.patch"



^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing.
@ 2021-02-04 19:36  Heikki Linnakangas <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Heikki Linnakangas @ 2021-02-04 19:36 UTC (permalink / raw)

If a multi-byte character is escaped with a backslash in TEXT mode input,
we didn't always treat the escape correctly. If:

- a multi-byte character is escaped with a backslash, and
- the second byte of the character is 0x5C, i.e. the ASCII code of a
  backslash (\), and
- the next character is a dot (.), then

CopyReadLineText function would incorrectly interpret the sequence as an
end-of-copy marker (\.). This can only happen in encodings that can
"embed" ascii characters as the second byte. One example of such sequence
is '\x5ca45c2e666f6f' in Big5 encoding. If you put that in a file, and
load it with COPY FROM, you'd incorrectly get an "end-of-copy marker
corrupt" error.

Backpatch to all supported versions.

Reviewed-by: John Naylor, Kyotaro Horiguchi
Discussion: https://www.postgresql.org/message-id/a897f84f-8dca-8798-3139-07da5bb38728%40iki.fi
---
 src/backend/commands/copyfromparse.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c
index b843d315b1..315b16fd7a 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -1084,7 +1084,7 @@ CopyReadLineText(CopyFromState cstate)
 				break;
 			}
 			else if (!cstate->opts.csv_mode)
-
+			{
 				/*
 				 * If we are here, it means we found a backslash followed by
 				 * something other than a period.  In non-CSV mode, anything
@@ -1095,8 +1095,16 @@ CopyReadLineText(CopyFromState cstate)
 				 * backslashes are not special, so we want to process the
 				 * character after the backslash just like a normal character,
 				 * so we don't increment in those cases.
+				 *
+				 * Set 'c' to skip whole character correctly in multi-byte
+				 * encodings.  If we don't have the whole character in the
+				 * buffer yet, we might loop back to process it, after all,
+				 * but that's OK because multi-byte characters cannot have any
+				 * special meaning.
 				 */
 				raw_buf_ptr++;
+				c = c2;
+			}
 		}
 
 		/*
-- 
2.30.0


--------------CFC80B40DFDD66DF067F288D--





^ permalink  raw  reply  [nested|flat] 8+ messages in thread

* Re: Changing "Hot Standby" to "hot standby"
@ 2022-03-11 06:24  Daniel Westermann (DWE) <[email protected]>
  0 siblings, 0 replies; 8+ messages in thread

From: Daniel Westermann (DWE) @ 2022-03-11 06:24 UTC (permalink / raw)
  To: Michael Paquier <[email protected]>; Robert Treat <[email protected]>; +Cc: Kyotaro Horiguchi <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>

>Looks the same as v5 for me, that applies the same consistency rules
>everywhere in the docs.  So applied this one.

Thank you, Michael






^ permalink  raw  reply  [nested|flat] 8+ messages in thread


end of thread, other threads:[~2022-03-11 06:24 UTC | newest]

Thread overview: 8+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-12-13 06:07 [PATCH 4/5] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/5] Fix a bug of insertion into an internal partition. amit <[email protected]>
2016-12-13 06:07 [PATCH 4/7] Fix a bug of insertion into an internal partition. amit <[email protected]>
2021-02-04 19:36 [PATCH v2 1/1] Fix a corner-case in COPY FROM backslash processing. Heikki Linnakangas <[email protected]>
2022-03-11 06:24 Re: Changing "Hot Standby" to "hot standby" Daniel Westermann (DWE) <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox