relispartition for index partitions

Started by Amit Langotealmost 8 years ago5 messages
#1Amit Langote
Langote_Amit_f8@lab.ntt.co.jp

Hi.

I noticed that relispartition isn't set for index's partitions.

create table p (a int) partition by list (a);
create table p12 partition of p for values in (1, 2);
create index on p (a);
select relname, relkind from pg_class where relnamespace =
'public'::regnamespace and relispartition is true;
relname | relkind
---------+---------
p12 | r
(1 row)

Is that intentional?

Thanks,
Amit

#2Andres Freund
andres@anarazel.de
In reply to: Amit Langote (#1)
Re: relispartition for index partitions

Hi,

On 2018-01-26 18:57:03 +0900, Amit Langote wrote:

I noticed that relispartition isn't set for index's partitions.

create table p (a int) partition by list (a);
create table p12 partition of p for values in (1, 2);
create index on p (a);
select relname, relkind from pg_class where relnamespace =
'public'::regnamespace and relispartition is true;
relname | relkind
---------+---------
p12 | r
(1 row)

Is that intentional?

This appears to be a question about
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8b08f7d4820fd7a8ef6152a9dd8c6e3cb01e5f99
et al. Could you look into it? It's been an open item for quite a
while.

Greetings,

Andres Freund

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Andres Freund (#2)
Re: relispartition for index partitions

Hello

Andres Freund wrote:

On 2018-01-26 18:57:03 +0900, Amit Langote wrote:

I noticed that relispartition isn't set for index's partitions.

Is that intentional?

This appears to be a question about
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=8b08f7d4820fd7a8ef6152a9dd8c6e3cb01e5f99
et al. Could you look into it? It's been an open item for quite a
while.

Sure. I'll get this done this week.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Amit Langote (#1)
1 attachment(s)
Re: relispartition for index partitions

Amit Langote wrote:

Hi.

I noticed that relispartition isn't set for index's partitions.

This patch should fix it.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

relispartition.patchtext/plain; charset=us-asciiDownload
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 5d73e92901..218c457fa4 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -933,6 +933,7 @@ index_create(Relation heapRelation,
 	indexRelation->rd_rel->relowner = heapRelation->rd_rel->relowner;
 	indexRelation->rd_rel->relam = accessMethodObjectId;
 	indexRelation->rd_rel->relhasoids = false;
+	indexRelation->rd_rel->relispartition = OidIsValid(parentIndexRelid);
 
 	/*
 	 * store index's pg_class entry
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f8108858ae..56e87d6251 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -490,6 +490,8 @@ static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation rel,
 static void validatePartitionedIndex(Relation partedIdx, Relation partedTbl);
 static void refuseDupeIndexAttach(Relation parentIdx, Relation partIdx,
 					  Relation partitionTbl);
+static void update_relispartition(Relation classRel, Oid partIdxId,
+					  bool newval);
 
 
 /* ----------------------------------------------------------------
@@ -14405,10 +14407,11 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
 		 */
 		for (i = 0; i < list_length(attachRelIdxs); i++)
 		{
+			Oid		cldIdxId = RelationGetRelid(attachrelIdxRels[i]);
 			Oid		cldConstrOid = InvalidOid;
 
 			/* does this index have a parent?  if so, can't use it */
-			if (has_superclass(RelationGetRelid(attachrelIdxRels[i])))
+			if (attachrelIdxRels[i]->rd_rel->relispartition)
 				continue;
 
 			if (CompareIndexInfo(attachInfos[i], info,
@@ -14429,7 +14432,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
 				{
 					cldConstrOid =
 						get_relation_idx_constraint_oid(RelationGetRelid(attachrel),
-														RelationGetRelid(attachrelIdxRels[i]));
+														cldIdxId);
 					/* no dice */
 					if (!OidIsValid(cldConstrOid))
 						continue;
@@ -14439,6 +14442,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
 				IndexSetParentIndex(attachrelIdxRels[i], idx);
 				if (OidIsValid(constraintOid))
 					ConstraintSetParentConstraint(cldConstrOid, constraintOid);
+				update_relispartition(NULL, cldIdxId, true);
 				found = true;
 				break;
 			}
@@ -14659,7 +14663,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	((Form_pg_class) GETSTRUCT(newtuple))->relispartition = false;
 	CatalogTupleUpdate(classRel, &newtuple->t_self, newtuple);
 	heap_freetuple(newtuple);
-	heap_close(classRel, RowExclusiveLock);
 
 	if (OidIsValid(defaultPartOid))
 	{
@@ -14692,8 +14695,10 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 
 		idx = index_open(idxid, AccessExclusiveLock);
 		IndexSetParentIndex(idx, InvalidOid);
+		update_relispartition(classRel, idxid, false);
 		relation_close(idx, AccessExclusiveLock);
 	}
+	heap_close(classRel, RowExclusiveLock);
 
 	/*
 	 * Invalidate the parent's relcache so that the partition is no longer
@@ -14772,6 +14777,39 @@ RangeVarCallbackForAttachIndex(const RangeVar *rv, Oid relOid, Oid oldRelOid,
 }
 
 /*
+ * Update the relispartition flag of the relation with the given OID, to the
+ * given value.
+ *
+ * classRel is the pg_class relation, already open and suitably locked; if
+ * passed as NULL, we open it internally and close before returning.
+ */
+static void
+update_relispartition(Relation classRel, Oid partIdxId, bool newval)
+{
+	HeapTuple	tup;
+	HeapTuple	newtup;
+	Form_pg_class classForm;
+	bool		opened = false;
+
+	if (classRel == NULL)
+	{
+		classRel = heap_open(RelationRelationId, RowExclusiveLock);
+		opened = true;
+	}
+
+	tup = SearchSysCache1(RELOID, ObjectIdGetDatum(partIdxId));
+	newtup = heap_copytuple(tup);
+	classForm = (Form_pg_class) GETSTRUCT(newtup);
+	classForm->relispartition = newval;
+	CatalogTupleUpdate(classRel, &tup->t_self, newtup);
+	heap_freetuple(newtup);
+	ReleaseSysCache(tup);
+
+	if (opened)
+		heap_close(classRel, RowExclusiveLock);
+}
+
+/*
  * ALTER INDEX i1 ATTACH PARTITION i2
  */
 static ObjectAddress
@@ -14815,8 +14853,8 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
 	ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partIdx));
 
 	/* Silently do nothing if already in the right state */
-	currParent = !has_superclass(partIdxId) ? InvalidOid :
-		get_partition_parent(partIdxId);
+	currParent = partIdx->rd_rel->relispartition ?
+		get_partition_parent(partIdxId) : InvalidOid;
 	if (currParent != RelationGetRelid(parentIdx))
 	{
 		IndexInfo  *childInfo;
@@ -14909,6 +14947,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
 		IndexSetParentIndex(partIdx, RelationGetRelid(parentIdx));
 		if (OidIsValid(constraintOid))
 			ConstraintSetParentConstraint(cldConstrId, constraintOid);
+		update_relispartition(NULL, partIdxId, true);
 
 		pfree(attmap);
 
@@ -15036,8 +15075,7 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl)
 	 * If this index is in turn a partition of a larger index, validating it
 	 * might cause the parent to become valid also.  Try that.
 	 */
-	if (updated &&
-		has_superclass(RelationGetRelid(partedIdx)))
+	if (updated && partedIdx->rd_rel->relispartition)
 	{
 		Oid			parentIdxId,
 					parentTblId;
#5Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Alvaro Herrera (#4)
Re: relispartition for index partitions

On 2018/04/12 5:33, Alvaro Herrera wrote:

Amit Langote wrote:

Hi.

I noticed that relispartition isn't set for index's partitions.

This patch should fix it.

Thanks. I saw your commit 9e9befac4a22 and changes seem fine.

Regards,
Amit