Date: Thu, 5 Nov 2020 14:36:06 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@postgresql.org
Subject: list_free() in index_get_partition()
Message-ID: <20201105203606.GF22691@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="6K2R/cS9K4qvcBNq"
Content-Disposition: inline
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--6K2R/cS9K4qvcBNq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Seems to be missing.

The 2nd patch does some more cleanup - Before, a failed syscache lookup would
ERROR, but I don't think that's supposed to happen.  get_rel_relispartition()
would instead return false, and we won't call get_partition_parent().

commit a8ad949f22b8dd7b23049b0b0704e5be9233e319
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Date:   Thu Nov 5 12:06:49 2020 -0600

    list_free() in index_get_partition()
    
    which was added at: a6da0047158b8a227f883aeed19eb7fcfbef11fb

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 239ac017fa..4dfac39adf 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -153,44 +153,45 @@ Oid
 index_get_partition(Relation partition, Oid indexId)
 {
 	List	   *idxlist = RelationGetIndexList(partition);
 	ListCell   *l;
 
 	foreach(l, idxlist)
 	{
 		Oid			partIdx = lfirst_oid(l);
 		HeapTuple	tup;
 		Form_pg_class classForm;
 		bool		ispartition;
 
 		tup = SearchSysCache1(RELOID, ObjectIdGetDatum(partIdx));
 		if (!HeapTupleIsValid(tup))
 			elog(ERROR, "cache lookup failed for relation %u", partIdx);
 		classForm = (Form_pg_class) GETSTRUCT(tup);
 		ispartition = classForm->relispartition;
 		ReleaseSysCache(tup);
 		if (!ispartition)
 			continue;
-		if (get_partition_parent(lfirst_oid(l)) == indexId)
+		if (get_partition_parent(partIdx) == indexId)
 		{
 			list_free(idxlist);
 			return partIdx;
 		}
 	}
 
+	list_free(idxlist);
 	return InvalidOid;
 }

commit 0a01cb7561d6ec74aa5829040bd1478e7b113d89
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Date:   Thu Nov 5 12:14:24 2020 -0600

    index_get_partition(): use lsyscache ??
    
    The old code failed when !HeapTupleIsValid(), the new code will return false -
    is that ok ??  Two of the existing callers error anyway.

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 4dfac39adf..3d78bc0872 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -28,6 +28,7 @@
 #include "partitioning/partbounds.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/fmgroids.h"
+#include "utils/lsyscache.h"
 #include "utils/partcache.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
@@ -158,17 +159,8 @@ index_get_partition(Relation partition, Oid indexId)
 	foreach(l, idxlist)
 	{
 		Oid			partIdx = lfirst_oid(l);
-		HeapTuple	tup;
-		Form_pg_class classForm;
-		bool		ispartition;
-
-		tup = SearchSysCache1(RELOID, ObjectIdGetDatum(partIdx));
-		if (!HeapTupleIsValid(tup))
-			elog(ERROR, "cache lookup failed for relation %u", partIdx);
-		classForm = (Form_pg_class) GETSTRUCT(tup);
-		ispartition = classForm->relispartition;
-		ReleaseSysCache(tup);
-		if (!ispartition)
+
+		if (!get_rel_relispartition(partIdx))
 			continue;
 		if (get_partition_parent(partIdx) == indexId)
 		{

--6K2R/cS9K4qvcBNq
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-Fixes-to-index_get_partition.patch"

From 90e155f660314ec70de14177241dd1ec029d9311 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 5 Nov 2020 12:06:49 -0600
Subject: [PATCH 1/2] Fixes to index_get_partition()

which was added at: a6da0047158b8a227f883aeed19eb7fcfbef11fb

free list in cases where there's no parent
use lsyscache: get_rel_relispartition()
---
 src/backend/catalog/partition.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 239ac017fa..4dfac39adf 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -170,13 +170,14 @@ index_get_partition(Relation partition, Oid indexId)
 		ReleaseSysCache(tup);
 		if (!ispartition)
 			continue;
-		if (get_partition_parent(lfirst_oid(l)) == indexId)
+		if (get_partition_parent(partIdx) == indexId)
 		{
 			list_free(idxlist);
 			return partIdx;
 		}
 	}
 
+	list_free(idxlist);
 	return InvalidOid;
 }
 
-- 
2.17.0


--6K2R/cS9K4qvcBNq
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0002-index_get_partition-use-lsyscache.patch"

From 768f91199b5e197759b2ead6d838a72974dca6b6 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 5 Nov 2020 12:14:24 -0600
Subject: [PATCH 2/2] index_get_partition(): use lsyscache ??

The old code failed when !HeapTupleIsValid(), the new code will return false -
is that ok ??
---
 src/backend/catalog/partition.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 4dfac39adf..3d78bc0872 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -28,6 +28,7 @@
 #include "partitioning/partbounds.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/fmgroids.h"
+#include "utils/lsyscache.h"
 #include "utils/partcache.h"
 #include "utils/rel.h"
 #include "utils/syscache.h"
@@ -158,17 +159,8 @@ index_get_partition(Relation partition, Oid indexId)
 	foreach(l, idxlist)
 	{
 		Oid			partIdx = lfirst_oid(l);
-		HeapTuple	tup;
-		Form_pg_class classForm;
-		bool		ispartition;
-
-		tup = SearchSysCache1(RELOID, ObjectIdGetDatum(partIdx));
-		if (!HeapTupleIsValid(tup))
-			elog(ERROR, "cache lookup failed for relation %u", partIdx);
-		classForm = (Form_pg_class) GETSTRUCT(tup);
-		ispartition = classForm->relispartition;
-		ReleaseSysCache(tup);
-		if (!ispartition)
+
+		if (!get_rel_relispartition(partIdx))
 			continue;
 		if (get_partition_parent(partIdx) == indexId)
 		{
-- 
2.17.0


--6K2R/cS9K4qvcBNq--