From: Tom Lane <tgl@sss.pgh.pa.us>
To: exclusion@gmail.com
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18135: Incorrect memory access occurs when attaching a
 partition with an index
In-reply-to: <18135-532f4a755e71e4d2@postgresql.org>
References: <18135-532f4a755e71e4d2@postgresql.org>
Comments: In-reply-to PG Bug reporting form <noreply@postgresql.org>
	message dated "Wed, 27 Sep 2023 10:00:01 -0000"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1043676.1695922273.0@sss.pgh.pa.us>
Date: Thu, 28 Sep 2023 13:31:39 -0400
Message-ID: <1043711.1695922299@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1043711.1695922299%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1043676.1695922273.1@sss.pgh.pa.us>

PG Bug reporting form <noreply@postgresql.org> writes:
> The function CompareIndexInfo() contains the code:
> 		/* ignore expressions at this stage */
> 		if ((info1->ii_IndexAttrNumbers[i] != InvalidAttrNumber) &&
> 			(attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] !=
> 			 info1->ii_IndexAttrNumbers[i]))
> 			return false;

> where info1->ii_IndexAttrNumbers[i] is checked for InvalidAttrNumber
> (i. e. it's not an expression), but info2->ii_IndexAttrNumbers[i] is not.

Agreed, that's pretty broken, and it's not just that it risks an
invalid access.  I don't think this reliably rejects cases where
one index has an expression and the other doesn't.  Even if it does
work, it's way too complicated to convince oneself that that's
rejected.  I think for clarity we should code it as attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-CompareIndexInfo.patch";
	charset="us-ascii"
Content-ID: <1043676.1695922273.2@sss.pgh.pa.us>
Content-Description: fix-CompareIndexInfo.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 716de171a4..3d5adab2c5 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2559,7 +2559,7 @@ CompareIndexInfo(const IndexInfo *info1, const Index=
Info *info2,
 =

 	/*
 	 * and columns match through the attribute map (actual attribute numbers
-	 * might differ!)  Note that this implies that index columns that are
+	 * might differ!)  Note that this checks that index columns that are
 	 * expressions appear in the same positions.  We will next compare the
 	 * expressions themselves.
 	 */
@@ -2568,13 +2568,22 @@ CompareIndexInfo(const IndexInfo *info1, const Ind=
exInfo *info2,
 		if (attmap->maplen < info2->ii_IndexAttrNumbers[i])
 			elog(ERROR, "incorrect attribute map");
 =

-		/* ignore expressions at this stage */
-		if ((info1->ii_IndexAttrNumbers[i] !=3D InvalidAttrNumber) &&
-			(attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] !=3D
-			 info1->ii_IndexAttrNumbers[i]))
-			return false;
+		/* ignore expressions for now (but check their collation/opfamily) */
+		if (!(info1->ii_IndexAttrNumbers[i] =3D=3D InvalidAttrNumber &&
+			  info2->ii_IndexAttrNumbers[i] =3D=3D InvalidAttrNumber))
+		{
+			/* fail if just one index has an expression in this column */
+			if (info1->ii_IndexAttrNumbers[i] =3D=3D InvalidAttrNumber ||
+				info2->ii_IndexAttrNumbers[i] =3D=3D InvalidAttrNumber)
+				return false;
+
+			/* both are columns, so check for match after mapping */
+			if (attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] !=3D
+				info1->ii_IndexAttrNumbers[i])
+				return false;
+		}
 =

-		/* collation and opfamily is not valid for including columns */
+		/* collation and opfamily are not valid for included columns */
 		if (i >=3D info1->ii_NumIndexKeyAttrs)
 			continue;
 =


------- =_aaaaaaaaaa0--