Do you use PostGIS BRIN?

[Paul Ramsey]

Tomas,
Thanks for taking a run at the BRIN improvement!

>> 
> Understood. Interesting use of BRIN to build indexes quickly, until the
> regular indexes get built. It probably does not make sense to keep the
> BRIN indexes after that - the other indexes are likely to be faster, so
> it'd just allow the planner to make mistakes.

I think in the context of OSM, the dumps are somewhat spatially correlated, so a BRIN index ends up being a quick build with a reasonable query performance, particularly for bulk operations.

> 
> Yeah. I spent some time today looking at this, and it's indeed a bit
> more complicated than I assumed. There's a couple reasons ...
> 
> 1) The opclass (I only looked at brin_geometry_inclusion_ops_2d) has
> opcintype != opckeytype, but the BRIN code does not actually expect (or
> allow) that. I guess this might be why the opclass overrides the
> ADD_VALUE function, because the built-in function simply uses the first
> value as the union, without converting it to box2df.
> 
> This means we can't get rid of the custom add_value, but AFAICS the
> _merge is just a simplified variant of it, as it only deals with the
> box2df summaries.

I’m wondering if a clean-sheet BRIN might not end up as a nicer implementation in the end, especially if we have to define a new opclass anyways. Perhaps a 2-d only implementation, which would still be useful for people but not require us to cart around all the extra higher-dimensional opclasses we got in our initial implementation.

> Yes. I think the main challenge is the case when people installed the
> new library, but are running with the old definitions. And you're right
> the best solution would be to "neuter" the "old" functions by adding
> something like elog(ERROR) into them.
> 
> But as I wrote above, I'm not aware of a way to modify an opclass
> (there's ALTER OPERATOR CLASS, but it only allows changing name, owner
> and schema).
> 
> So the existing opclass would need to be neutered, and a completely new
> opclass would need to be added.

We have hacked the system tables in the past, to sneak opclass fixes in, so it’s possible. If we do a new opclass I wonder if we will end up carrying the old ones around forever because old installations might have existing BRIN indexes that would fail upgrade if we removed the opclasses they depend on.

> 
>> Then later on we could decide to fix it, only fix it for PostGIS
>> 3.6, or just remove the entrails of BRIN for 3.6 and above.
> 
> 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.

I was wondering if the “lowest impact” neutering of BRIN as it exists in our stable branches would be to add a check on the values of the parallelism GUCs and then elog(ERROR) out if parallelism is turned on. Or I guess, since parallel BRIN index build is the actual crashing case, neuter things only in the presence of Pg17. Or some combination of the two.

> 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.
> 
> 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.

Thanks!
P

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