Re: BRIN range operator class - Mailing list pgsql-hackers

> I thought we can do better than minmax for the inet data type,
> and ended up with a generalized opclass supporting both inet and range
> types.  Patch based on minmax-v20 attached.  It works well except
> a few small problems.  I will improve the patch and add into
> a commitfest after BRIN framework is committed.

I wanted to send a new version before the commitfest to get some
feedback, but it is still work in progress.  Patch attached rebased
to the current HEAD.  This version supports more operators and
box from geometric data types.  Opclasses are renamed to inclusion_ops
to be more generic.  The problems I mentioned remain beause I
couldn't solve them without touching the BRIN framework.

> To support more operators I needed to change amstrategies and
> amsupport on the catalog.  It would be nice if amsupport can be set
> to 0 like am strategies.

I think it would be nicer to get the functions from the operators
with using the strategy numbers instead of adding them directly as
support functions.  I looked around a bit but couldn't find
a sensible way to support it.  Is it possible without adding them
to the RelationData struct?

> Inet data types accept IP version 4 and version 6.  It isn't possible
> to represent union of addresses from different versions with a valid
> inet type.  So, I made the union function return NULL in this case.
> Then, I tried to store if returned value is NULL or not, in
> column->values[] as boolean, but it failed on the pfree() inside
> brin_dtuple_initilize().  It doesn't seem right to free the values
> based on attr->attbyval.

This problem remains.  There is also a similar problem with the
range types, namely empty ranges.  There should be special cases
for them on some of the strategies.  I tried to solve the problems
in several different ways, but got a segfault one line or another.
This makes me think that BRIN framework doesn't support to store
different types than the indexed column in the values array.
For example, brin_deform_tuple() iterates over the values array and
copies them using the length of the attr on the index, not the length
of the type defined by OpcInfo function.  If storing another types
aren't supported, why is it required to return oid's on the OpcInfo
function.  I am confused.

I didn't try to support other geometric types than box as I couldn't
managed to store a different type on the values array, but it would
be nice to get some feedback about the overall design.  I was
thinking to add a STORAGE parameter to the index to support other
geometric types.  I am not sure that adding the STORAGE parameter
to be used by the opclass implementation is the right way.  It
wouldn't be the actual thing that is stored by the index, it will be
an element in the values array.  Maybe, data type specific opclasses
is the way to go, not a generic one as I am trying.