Subject: Re: WIP: BRIN multi-range indexes
To: John Naylor <john.naylor@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <c1138ead-7668-f0e1-0638-c3be3237e812@2ndquadrant.com>
 <20201001222127.GA24173@alvherre.pgsql>
 <160434032777.4948.12542847889193566385.pgcf@coridan.postgresql.org>
 <20201102232545.zw4xir577gq55ogj@development>
 <88b3831d-8afa-4135-a420-27b12bf9fae4@enterprisedb.com>
 <CAFBsxsFwLTg3qfs6RcoQouGDh54GUZLr7+BEXfo86qYPuNaZjA@mail.gmail.com>
 <1fdcbd2a-ff0f-fb81-d227-22279d5cbcb7@enterprisedb.com>
 <CAFBsxsF0spTU=NMBNER4KwJ5NLmkoU7ModUg7g8HAqZ9k+7OgA@mail.gmail.com>
 <061b24c0-87eb-51a7-17d6-e1ba8c59fd23@enterprisedb.com>
 <CAFBsxsE05b=iYEFm9Aw--vodne80DSqbzYfUYLy0FYQWOJRmTw@mail.gmail.com>
 <79869c9d-95a2-0940-76a8-29c348707a66@enterprisedb.com>
 <CAFBsxsFbsFHNukFCb+_x+a0UmeivnTa5XHLzCrcAEWx_X_B26A@mail.gmail.com>
 <fbb3583b-35cb-8415-e719-1836865543f7@enterprisedb.com>
 <CAFBsxsF_nTeG8EX9gUkkpiAHOpdU_OEKbvsK3BzeMSdJbERbjg@mail.gmail.com>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
Message-ID: <0e2f18d5-cd6d-764b-3912-512e9c722b8a@enterprisedb.com>
Date: Wed, 10 Feb 2021 00:15:25 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: 
 <CAFBsxsF_nTeG8EX9gUkkpiAHOpdU_OEKbvsK3BzeMSdJbERbjg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 2/9/21 3:46 PM, John Naylor wrote:
> On Wed, Feb 3, 2021 at 7:54 PM Tomas Vondra 
> <tomas.vondra@enterprisedb.com <mailto:tomas.vondra@enterprisedb.com>> 
> wrote:
>  >
>  > [v-20210203]
> 
> Hi Tomas,
> 
> I have some random comments from reading the patch, but haven't gone 
> into detail in the newer aspects. I'll do so in the near future.
> 
> The cfbot seems to crash on this patch during make check, but it doesn't 
> crash for me. I'm not even sure what date that cfbot status is from.
> 

Yeah, I noticed that too, and I'm investigating.

I tried running the regression tests on a 32-bit machine (rpi4), which 
sometimes uncovers strange failures, and that indeed fails. There are 
two or three bugs.

Firstly, the allocation optimization patch does this:

     MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys * natts)

instead of

     MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * natts

and that sometimes produces the wrong result, triggering an assert.


Secondly, there seems to be an issue with cross-type bloom indexes. 
Imagine you have an int8 column, with a bloom index on it, and then you 
do this:

    WHERE column = '122'::int4;

Currently, we end up passing this to the consistent function, which 
tries to call hashint8 on the int4 datum - that succeeds on 64 bits 
(because both types are byval), but fails on 32-bits (where int8 is 
byref, so it fails on int4). Which causes a segfault.

I think I included those cross-type operators as a copy-paste from 
minmax indexes, but I see hash indexes don't allow this. And removing 
those cross-type rows from pg_amop.dat makes the crashes go away.

It's also possible I simplified the get_strategy_procinfo a bit too 
much. I see the minmax variant has subtype, so maybe we could do this 
instead (I recall the integer types should have "compatible" results).

There are a couple failues where the index does not produce the right 
number of results, though. I haven't investigated that yet. Once I fix 
this, I'll post an updated patch - hopefully that'll make cfbot happy.


regards

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