Re: [PATCH] Covering SPGiST index

From: "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru>
To: Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>
Cc: Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] Covering SPGiST index
Date: 2020-08-30 13:01:19
Message-ID: FBAB54E5-F666-4DB2-BDB7-22B20CF38BF3@yandex-team.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 27 авг. 2020 г., в 21:03, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> написал(а):
>
> see v8

For me is the only concerning point is putting nullmask and varatt bits into tuple->nextOffset.
But, probably, we can go with this.

But let's change macro a bit. When I see
SGLT_SET_OFFSET(leafTuple->nextOffset, InvalidOffsetNumber);
I expect that leafTuple->nextOffset is function argument by value and will not be changed.
For example see ItemPointerSetOffsetNumber() - it's not exposing ip_posid.

Also, I'd propose instead of
>*(leafChainDatums + i * natts) and leafChainIsnulls + i * natts
using something like
>int some_index = i * natts;
>leafChainDatumsp[some_index] and &leafChainIsnulls[some_index]
But, probably, it's a matter of taste...

Also I'm not sure would it be helpful to use instead of
>isnull[0] and leafDatum[0]
more complex
>#define SpgKeyIndex 0
>isnull[SpgKeyIndex] and leafDatum[SpgKeyIndex]
There is so many [0] in the patch...

Thanks!

Best regards, Andrey Borodin.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David CARLIER 2020-08-30 13:03:32 [PATCH v1] explicit_bzero.c: using explicit_memset on NetBSD
Previous Message Stephen Frost 2020-08-30 12:24:01 Re: [EXTERNAL] Re: WIP: WAL prefetch (another approach)