Re: [PATCH] Covering SPGiST index

>
> I have a wild idea of renaming nextOffset in SpGistLeafTupleData.
> Or at least documenting in comments that this field is more than just an
> offset.
>
Seems reasonable as previous changes localized mentions of nextOffset only
to leaf tuple definition and access macros. So I've done this renaming.

> This looks like assert rather than real runtime check in spgLeafTupleSize()
>
> + if (state->includeTupdesc->natts + 1 >= INDEX_MAX_KEYS)
> + ereport(ERROR,
> + (errcode(ERRCODE_TOO_MANY_COLUMNS),
> + errmsg("number of index columns
> (%d) exceeds limit (%d)",
> +
> state->includeTupdesc->natts, INDEX_MAX_KEYS)));
> Even if you go with check, number of columns is
> state->includeTupdesc->natts + 1.
>
I placed this check only once on SpGist state creation and replaced the
other checks to asserts.

> Also I'd refactor this
> + /* Form descriptor for INCLUDE columns if any */
>
Also done. Thanks a lot!
See v10.

--
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>