From: Michael Paquier <michael@paquier.xyz>
To: exclusion@gmail.com
To: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18135: Incorrect memory access occurs when attaching a partition with an index
Date: Thu, 28 Sep 2023 12:30:39 +0900
Message-ID: <ZRTzX0h68fxZ5Dth@paquier.xyz> (raw)
In-Reply-To: <18135-532f4a755e71e4d2@postgresql.org>
References: <18135-532f4a755e71e4d2@postgresql.org>
On Wed, Sep 27, 2023 at 10:00:01AM +0000, PG Bug reporting form wrote:
> executed under Valgrind, it leads to an incorrect memory access:
> ==00:00:00:03.947 396156== Invalid read of size 2
> ==00:00:00:03.947 396156== at 0x2E323D: CompareIndexInfo (index.c:2572)
> ==00:00:00:03.947 396156== by 0x3D009B: AttachPartitionEnsureIndexes
> (tablecmds.c:18797)
> ==00:00:00:03.947 396156== by 0x3D8B4F: ATExecAttachPartition
> (tablecmds.c:18578)
> ==00:00:00:03.947 396156== by 0x3D9A88: ATExecCmd (tablecmds.c:5379)
> ==00:00:00:03.947 396156== by 0x3D9BC7: ATRewriteCatalogs
> (tablecmds.c:5063)
I have just tested that on HEAD and REL_16_STABLE, but fail to see
this report, which is weird (3.19.0 here). Are you using any specific
option of valgrind I should be aware of? Here is what I used, for
reference:
valgrind \
--suppressions=$PG_SOURCE/src/tools/valgrind.supp \
--trace-children=yes --track-origins=yes --read-var-info=yes \
postgres -D REST_OF_ARGS
> 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.
Anyway, I can see your point here. info2's first attnum is 0 so we
look at an imaginary position in attmap->attnums. So, yes, that's
wrong.
> In addition, there is a check whether both indexes are (are not)
> expression indexes, but it's placed below...
Sure, but this makes the check a bit cheaper if the indexes to compare
use expr and non-expr attributes at the same attnums, no? Except if I
am missing something, the attached should be sufficient.
--
Michael
Attachments:compare-index-mem.patch (text/x-diff, 542B) downloaddiff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.cindex 716de171a4..bbc9b262a0 100644--- a/src/backend/catalog/index.c+++ b/src/backend/catalog/index.c@@ -2570,6 +2570,7 @@ CompareIndexInfo(const IndexInfo *info1, const IndexInfo *info2,
/* ignore expressions at this stage */
if ((info1->ii_IndexAttrNumbers[i] != InvalidAttrNumber) &&
+ (info2->ii_IndexAttrNumbers[i] != InvalidAttrNumber) &&
(attmap->attnums[info2->ii_IndexAttrNumbers[i] - 1] !=
info1->ii_IndexAttrNumbers[i]))
return false;
signature.asc (application/pgp-signature, 832B) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18135: Incorrect memory access occurs when attaching a partition with an index
In-Reply-To: <ZRTzX0h68fxZ5Dth@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox