Re: WIP: BRIN multi-range indexes

> On Dec 19, 2017, at 5:16 PM, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
>
>
>
> On 12/19/2017 08:38 PM, Mark Dilger wrote:
>>
>>> On Nov 18, 2017, at 12:45 PM, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com> wrote:
>>>
>>> Hi,
>>>
>>> Apparently there was some minor breakage due to duplicate OIDs, so here
>>> is the patch series updated to current master.
>>>
>>> regards
>>>
>>> --
>>> Tomas Vondra http://www.2ndQuadrant.com
>>> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>> <0001-Pass-all-keys-to-BRIN-consistent-function-at-once.patch.gz><0002-BRIN-bloom-indexes.patch.gz><0003-BRIN-multi-range-minmax-indexes.patch.gz><0004-Move-IS-NOT-NULL-checks-to-bringetbitmap.patch.gz>
>>
>>
>> After applying these four patches to my copy of master, the regression
>> tests fail for F_SATISFIES_HASH_PARTITION 5028 as attached.
>>
>
> D'oh! There was an incorrect OID referenced in pg_opclass, which was
> also used by the satisfies_hash_partition() function. Fixed patches
> attached.

Your use of type ScanKey in src/backend/access/brin/brin.c is a bit confusing. A
ScanKey is defined elsewhere as a pointer to ScanKeyData. When you define
an array of ScanKeys, you use pointer-to-pointer style:

+ ScanKey **keys,
+ **nullkeys;

But when you allocate space for the array, you don't treat it that way:

+ keys = palloc0(sizeof(ScanKey) * bdesc->bd_tupdesc->natts);
+ nullkeys = palloc0(sizeof(ScanKey) * bdesc->bd_tupdesc->natts);

But then again when you use nullkeys, you treat it as a two-dimensional array:

+ nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key;

and likewise when you allocate space within keys:

+ keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);

Could you please clarify? I have been awake a bit too long; hopefully, I am
not merely missing the obvious.

mark