Re: BUG #18135: Incorrect memory access occurs when attaching a partition with an index

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