Do you use PostGIS BRIN?

[Paul Ramsey]

So here’s a PR for review. The actual code change needed was just adding the merge support function (11) to all the opclasses. That seems to have fixed the crash and not changed any regression results. The add_value function is left in place, the logic there is fine and the need for a custom function is driven by the fact that our key type is not the same as our value type.

https://github.com/postgis/postgis/pull/787

Upgrades remain fraught. It’s certainly possible to add the necessary function entry to pg_amproc directly, but it’s a scary step to take.

P.

> On Jan 7, 2025, at 4:21 PM, Tomas Vondra <tomas at vondra.me> wrote:
> 
> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/postgis-devel/attachments/20250110/ff5b82c9/attachment-0001.htm>