Re: [PATCH] Add sortsupport for range types and btree_gist - Mailing list pgsql-hackers

From Andrey M. Borodin
Subject Re: [PATCH] Add sortsupport for range types and btree_gist
Date
Msg-id 24557304-11ED-4937-94F5-60C49DFAE4D9@yandex-team.ru
Whole thread Raw
In response to Re: [PATCH] Add sortsupport for range types and btree_gist  (jian he <jian.universality@gmail.com>)
Responses Re: [PATCH] Add sortsupport for range types and btree_gist  (Bernd Helmle <mailings@oopsware.de>)
List pgsql-hackers

> On 10 Jan 2024, at 19:18, jian he <jian.universality@gmail.com> wrote:
>
> what I am confused:
> In fmgr.h
>
> /*
> * Support for cleaning up detoasted copies of inputs.  This must only
> * be used for pass-by-ref datatypes, and normally would only be used
> * for toastable types.  If the given pointer is different from the
> * original argument, assume it's a palloc'd detoasted copy, and pfree it.
> * NOTE: most functions on toastable types do not have to worry about this,
> * but we currently require that support functions for indexes not leak
> * memory.
> */
> #define PG_FREE_IF_COPY(ptr,n) \
> do { \
> if ((Pointer) (ptr) != PG_GETARG_POINTER(n)) \
> pfree(ptr); \
> } while (0)
>
> but the doc (https://www.postgresql.org/docs/current/gist-extensibility.html)
> says:
> All the GiST support methods are normally called in short-lived memory
> contexts; that is, CurrentMemoryContext will get reset after each
> tuple is processed. It is therefore not very important to worry about
> pfree'ing everything you palloc.
> ENDOF_QUOTE
>
> so I am not sure in range_gist_cmp, we need the following:
> `
> if ((Datum) range_a != a)
> pfree(range_a);
> if ((Datum) range_b != b)
> pfree(range_b);
> `

I think GiST sortsupport comments are more relevant, so there's no need for this pfree()s.

Also, please check other thread, maybe you will find some useful code there [0,1]. It was committed[2] once, but
reverted.Please make sure that corrections made there are taken into account in your patch. 

Thanks for working on this!


Best regards, Andrey Borodin.

[0] https://commitfest.postgresql.org/31/2824/
[1]
https://www.postgresql.org/message-id/flat/285041639646332%40sas1-bf93f9015d57.qloud-c.yandex.net#0e5b4ed57d861d38a3d836c9ec09c0c5
[2] https://github.com/postgres/postgres/commit/9f984ba6d23dc6eecebf479ab1d3f2e550a4e9be




pgsql-hackers by date:

Previous
From: Jelte Fennema-Nio
Date:
Subject: Re: POC: Extension for adding distributed tracing - pg_tracing
Next
From: Andrei Lepikhov
Date:
Subject: Re: Multidimensional Histograms