Message-ID: <c898e301-ea62-e551-bfbc-2b7579150c14@gmail.com>
Date: Mon, 29 Apr 2024 17:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: BUG #17855: Uninitialised memory used when the name type value
 processed in binary mode of Memoize
Content-Language: en-US
To: David Rowley <dgrowleyml@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, pgsql-bugs@lists.postgresql.org,
 Tom Lane <tgl@sss.pgh.pa.us>
References: <17855-5f523e0f9769a566@postgresql.org>
 <CAApHDvqwwdO1EAo-6osu-LtTk1SD2iQjLQXhVd7Wamc-JfhfrQ@mail.gmail.com>
 <CAApHDvokmLEriy3+4vB0KjCQip5pMq0g_JJX-wHNDvBJKukr6w@mail.gmail.com>
 <CAApHDvqNo_7yipfLjVXatUUosEkKZrdhHxAObPuxSat8UQaUTw@mail.gmail.com>
 <b2a382d6-d261-fdb6-176c-d0cdbf63dce8@gmail.com>
 <CAApHDvqqvkorexCuVkSJ16LcqGKfUdLAn3Y+YsqKNT3Pe56oOA@mail.gmail.com>
 <3943232.1714060238@sss.pgh.pa.us>
 <CAApHDvpW3ZEyTyogjD6qJsnuTFujLk3yTyCPX-to7VhftkrakQ@mail.gmail.com>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: 
 <CAApHDvpW3ZEyTyogjD6qJsnuTFujLk3yTyCPX-to7VhftkrakQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/c898e301-ea62-e551-bfbc-2b7579150c14%40gmail.com>
Precedence: bulk

Hello David,

29.04.2024 03:54, David Rowley wrote:
> On Fri, 26 Apr 2024 at 03:50, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I do not like testing "atttypid == NAMEOID" one bit.  As noted,
>> that will fail on domains over name.  It will also result in
>> unnecessary work when reading non-btree indexes that contain
>> name, since I don't think anything else has the same hack
>> that btree name_ops does (grep for cstring in pg_opclass.dat).
>> I think the correct thing is to see whether the index opclass
>> for the column is btree name_ops.  We don't seem to have an
>> oid_symbol macro for that, but it shouldn't be hard to add,
>> at least in HEAD.
> On looking at the relcache code, I don't see anywhere that we store
> the opclass Oid in RelationData. IndexSupportInitialize() only records
> the opcfamily and opcintype. Assuming we can't add new fields to
> RelationData in the backbranches, is there a reason why we can't check
> for rd_opfamily of TEXT_BTREE_FAM_OID and rd_opcintype of NAMEOID?
>
> I've attached an updated patch that does it that way and also did a
> round of commenting the code.

With the v3 patch applied, `make check` fails for me under Valgrind:
ok 66        + create_index                            33585 ms
not ok 67    + create_index_spgist                     14446 ms
# (test process exited with exit code 2)

==00:00:04:57.033 4152513== Invalid read of size 4
==00:00:04:57.033 4152513==    at 0x456499: ExecInitIndexOnlyScan (nodeIndexonlyscan.c:657)
==00:00:04:57.033 4152513==    by 0x435EE6: ExecInitNode (execProcnode.c:225)
==00:00:04:57.033 4152513==    by 0x459780: ExecInitLimit (nodeLimit.c:477)
...
==00:00:04:57.033 4152513==  Address 0x73263b4 is 612 bytes inside a recently re-allocated block of size 1,024 alloc'd
==00:00:04:57.033 4152513==    at 0x4848899: malloc (vg_replace_malloc.c:381)
==00:00:04:57.033 4152513==    by 0x7A7DFC: AllocSetContextCreateInternal (aset.c:444)
==00:00:04:57.033 4152513==    by 0x775644: RelationInitIndexAccessInfo (relcache.c:1481)
==00:00:04:57.033 4152513==    by 0x775E4B: RelationBuildDesc (relcache.c:1207)
...
==00:00:04:57.033 4152513==
==00:00:04:57.033 4152513== Exit program on first error (--exit-on-first-error=yes)
2024-04-29 12:32:14.965 UTC postmaster[4109162] LOG:  server process (PID 4152513) exited with exit code 1
2024-04-29 12:32:14.965 UTC postmaster[4109162] DETAIL:  Failed process was running: SELECT p, dist FROM 
quad_point_tbl_ord_seq1 ORDER BY p <-> '0,0' LIMIT 10;

The quad_point_tbl_ord_seq1 index is defined as
CREATE INDEX ON quad_point_tbl_ord_seq1 USING spgist(p) INCLUDE(dist);

Best regards,
Alexander