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

Hi Michael,

28.09.2023 06:30, Michael Paquier wrote:
> 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)

Thank you for paying attention to it!

> 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

Please try the following procedure (I've simplified my own):
With the attached patch (for HEAD) applied
CPPFLAGS="-DUSE_VALGRIND -Og" ./configure -q --enable-debug --enable-cassert && make -s -j8
sed 's/create index on idxpart1 ((a + b));/create index on idxpart1 ((a));/' -i src/test/regress/sql/indexing.sql
TESTS=indexing make check-tests

I get:
not ok 1     - indexing                                 9844 ms
# (test process exited with exit code 2)

(I use valgrind-3.18.1.)

If you still see no error, please share details of your method for running
valgrind.

>
>> 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.

I thought about placing that check before the loop, but your fix looks
more clear (and my testing confirms that it works).

Best regards,
Alexander