Re: PATCH: Using BRIN indexes for sorted output

On 2/24/23 20:14, Tomas Vondra wrote:
>
>
> On 2/24/23 19:03, Matthias van de Meent wrote:
>> On Thu, 23 Feb 2023 at 19:48, Tomas Vondra
>> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>>>
>>> On 2/23/23 17:44, Matthias van de Meent wrote:
>>>> On Thu, 23 Feb 2023 at 16:22, Tomas Vondra
>>>> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>> On 2/23/23 15:19, Matthias van de Meent wrote:
>>>>>> Comments on 0001, mostly comments and patch design:
>>>>
>>>> One more comment:
>>>>
>>>>>>> + range->min_value = bval->bv_values[0];
>>>>>>> + range->max_value = bval->bv_values[1];
>>>>
>>>> This produces dangling pointers for minmax indexes on by-ref types
>>>> such as text and bytea, due to the memory context of the decoding
>>>> tuple being reset every iteration. :-(
>>>>
>>>
>>> Yeah, that sounds like a bug. Also a sign the tests should have some
>>> by-ref data types (presumably there are none, as that would certainly
>>> trip some asserts etc.).
>>
>> I'm not sure we currently trip asserts, as the data we store in the
>> memory context is very limited, making it unlikely we actually release
>> the memory region back to the OS.
>> I did get assertion failures by adding the attached patch, but I don't
>> think that's something we can do in the final release.
>>
>
> But we should randomize the memory if we ever do pfree(), and it's
> strange valgrind didn't complain when I ran tests with it. But yeah,
> I'll take a look and see if we can add some tests covering this.
>

There was no patch to trigger the assertions, but the attached patches
should fix that, I think. It pretty much just does datumCopy() after
reading the BRIN tuple from disk.

It's interesting I've been unable to hit the usual asserts checking
freed memory etc. even with text columns etc. I guess the BRIN tuple
memory happens to be aligned in a way that just happens to work.

It howver triggered an assert failure in minval_end, because it didn't
use the proper comparator.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company