Do you use PostGIS BRIN?

[Tomas Vondra]

On 1/8/25 00:56, Paul Ramsey wrote:
> 
>> I think adding new functions is not a breaking change - there's the risk
>> people might not have updated the definitions, but in the worst case the
>> those functions would not be called. It's the removal of functions that
>> breaks things.
> 
> Yes, unfortunately there’s no way to “clean up” the BRIN code, the
> function stubs end up having to lie around forever and ever, it seems.
> 
>> Attached is a patch fixing the brin_geometry_inclusion_ops_2d opclass
>> (at least I believe so). The 3d/4d opclasses would need a similar fix,
>> derived from gidx_brin_inclusion_add_value.
> 
> Reading through the existing code and the default implementations in
> pgsql, I think that we need to keep geom2d_brin_inclusion_add_value
> around, since the “value” handed in by the API will be a geometry, and
> the default implementation will just copy that whole value into
> the BrinValues, while we want instead to have just the box2df copied in.
> The GIST API has a “compress” method to go from the column type to the
> key type, but it seems like we have to handle this at the point of
> consumption which is the add_value support function. Am I right in this,
> or is there some special machinery that would ensure that the "Datum
> newval” coming into the brin_inclusion_add_value() was a box2df even
> though the column type is geometry? 

Yes, I believe this is correct - I believe it matches what I wrote in my
earlier response (about having to keep the custom add_value). It's
mostly down to not really supporting (opcintype!=opckeytype) in BRIN,
which is why the _merge() can get different types when called from
different places. If there was a way to ensure add_value only gets
box2df, that wouldn't be a problem.

I was thinking maybe it would be possible to define the opclass for
box2df only, and then add an implicit cast geometry->box2df, but I
haven't tried if that would allow indexing the geometry column. It's a
bit weird and I believe not every geometry can be cast to box2df anyway.

> The BRIN API has a lot of good stuff now, which would be good to add to
> simplify some of the other code (check for empty, check for contained,
> check for mergeable) I am not sure when in the history of BRIN it was
> added, do you have a quick feel for that?
> 

I believe most of this originates all the way back in 9.5. There were
some changes (e.g. moving the NULL handling into the common BRIN code,
but other than that I think the changes were minimal). So it's old.

>> This doesn't make amvalidate 100% happy though, it still complains about
>> missing functions, but I believe it just gets confused by the cross-type
>> operators.
> 
> To what extent do you think we need those cross-type operators? box2df
> is a stub type, and it is not creatable in SQL, to my knowledge, what
> does having those op(geometry, box2df), op(box2df, box2df), etc, etc,
> operators buy us?
> 

Not sure, I'm not familiar with how these types are used, but I guess
you need them if you support querying the geometry column using these
other types.


regards

-- 
Tomas Vondra