Re: [PATCH] Covering SPGiST index - Mailing list pgsql-hackers

From Pavel Borisov
Subject Re: [PATCH] Covering SPGiST index
Date
Msg-id CALT9ZEHx3d1KbLXrx1gT-t2XXUie2-K6No+_a558B11KYZHJ=w@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Covering SPGiST index  (Anastasia Lubennikova <a.lubennikova@postgrespro.ru>)
Responses Re: [PATCH] Covering SPGiST index
List pgsql-hackers
3) I didn't quite get the meaning of the assertion, that is added in a few places:
     Assert(so->state.includeTupdesc->natts);
Should it be Assert(so->state.includeTupdesc->natts > 1) ?
It is rather Assert(so->state.includeTupdesc->natts > 0)  as INCLUDE tuple descriptor should not be initialized and filled in case of index without INCLUDE attributes and doesn't contain any info about key attribute which is processed by SpGist existing way separately for different SpGist tuple types i.e. leaf, prefix=inner and label tuples. So only INCLUDE attributes are counted there. This and similar Asserts are for the case includeTupdesc becomes mistakenly initialized by some future code change.
 
I completely agree with all the other suggestions and made corrections (see v8). Thank you very much for your review!
Also there is a separate patch 0002 to add VACUUM ANALYZE to index_including test which is not necessary for covering spgist.

One more point to note: in spgist_private.h I needed to shift down whole block between 
"typedef struct SpGistSearchItem"
and 
"} SpGistCache;"
to position it below tuples types declarations to insert pointer "SpGistLeafTuple leafTuple"; into struct SpGistSearchItem. This is the only change in this block and I apologize for possible inconvenience to review this change.

--
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com
Attachment

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Should we replace the checks for access method OID with handler OID?
Next
From: Alvaro Herrera
Date:
Subject: Re: [POC]Enable tuple change partition caused by BEFORE TRIGGER