Re: BRIN indexes - TRAP: BadArgument

On Tue, Sep 23, 2014 at 3:04 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Alvaro Herrera wrote:
>> Heikki Linnakangas wrote:
>> > If you add a new datatype, and define b-tree operators for it, what
>> > is required to create a minmax opclass for it? Would it be possible
>> > to generalize the functions in brin_minmax.c so that they can be
>> > reused for any datatype (with b-tree operators) without writing any
>> > new C code? I think we're almost there; the only thing that differs
>> > between each data type is the opcinfo function. Let's pass the type
>> > OID as argument to the opcinfo function. You could then have just a
>> > single minmax_opcinfo function, instead of the macro to generate a
>> > separate function for each built-in datatype.
>>
>> Yeah, that's how I had that initially.  I changed it to what it's now as
>> part of a plan to enable building cross-type opclasses, so you could
>> have "WHERE int8col=42" without requiring a cast of the constant to type
>> int8.  This might have been a thinko, because AFAICS it's possible to
>> build them with a constant opcinfo as well (I changed several other
>> things to support this, as described in a previous email.)  I will look
>> into this later.
>
> I found out that we don't really throw errors in such cases anymore; we
> insert casts instead.  Maybe there's a performance argument that it
> might be better to use existing cross-type operators than casting, but
> justifying this work just turned a lot harder.  Here's a patch that
> reverts opcinfo into a generic function that receives the type OID.
>
> I will look into adding some testing mechanism for the union support
> proc; with that I will just consider the patch ready for commit and will
> push.

With all respect, I think this is a bad idea.  I know you've put a lot
of energy into this patch and I'm confident it's made a lot of
progress.  But as with Stephen's patch, the final form deserves a
thorough round of looking over by someone else before it goes in.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company