[postgis-devel] Support Functions

[Paul Ramsey]

For those not following along on IRC, I managed to track down the
issue, which was a fun combination of argument swapping and supportfn
needs. The PR is now in shape to be merged (IMO)

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

Speak now, etc.

P

On Sun, Mar 3, 2019 at 7:24 AM Paul Ramsey <pramsey at cleverelephant.ca> wrote:
>
>
>
> > On Mar 3, 2019, at 12:47 AM, Sandro Santilli <strk at kbt.io> wrote:
> >
> > On Fri, Mar 01, 2019 at 01:29:56PM -0800, Paul Ramsey wrote:
> >> This PR is getting closer, but there’s still some outstanding stuff…
> >>
> >> https://github.com/postgis/postgis/pull/378 <https://github.com/postgis/postgis/pull/378>
> >>
> >> * In order to pass regression I had to change a number of references to _ST_Function to the new ST_Function, in places in raster and topology where there were direct calls to those signatures. Shouldn’t make any difference as the “index aware” calls only use indexes when they exist and make sense
> >
> > I was thinking about this tonight...
> > By enforcing use of an index are we reducing flexibility ?
> > IIRC in the topology code we call _ST_Function specifically to _avoid_
> > the bounding box check. Just because we already know the inputs
> > overlap, so we want to save some cycles.
>
> As currently in the PR, I have restored the existence of the _ST_Functions, in all their non-indexed glory. With v12 you would get _ST_Functions with old behaviour and ST_Functions that have a support function clause, and add the index operator information via the support function API.
>
> > I guess your new code won't do any bbox check but rather hint the
> > optimizer so when the optimizer is not involved (direct call on two
> > inputs?) there won't be any slowdown ?
>
> No slowdown, the effect is precisely the same as our inlining.
>
> > But still, what happens when there are multiple indices defined on
> > tables ? Say GiST and BRIN... Is the new code going to enforce use
> > of one over the other ?
>
> Same as our current situation, which is, when you talk to Tom about it… interesting and arbitrary. But the new approach is the same as the old.
>
> >
> >> * Most problematically, even when the _ST_Function calls are removed, topology fails regression, with what appear to be actual differences in behaviour. It’s not clear to me why my changes would have caused behaviour changes. Unfortunately it’s really hard to figure out what precise geometry combinations are being exercised in the topology tests.
> >
> > My wild guess would be about change in default ordering, which
> > is likely not always enforced by topology code. Which test is
> > failing ?
>
> So, I have new data points.
> I have been able to induce everything to pass, but only be setting the index strategy in the support function operator code [1] to “overlaps” in all cases. That includes cases for contains/covers and within/coveredby, which in the old implementation are actually handled by the “contains” operator. This is definitely an unexpected behaviour change, and it would be wise for us to figure out precisely what inputs are giving arise to new behaviour, because it makes no logical sense as it stands. I can return the old behaviour to the PR if you’re willing to try and explain what the state is when things fail? If you are willing to help I can provide a lot more information about the state of things when odd results happen.
>
> P.
>
> [1] https://github.com/pramsey/postgis/blob/b4fe950e43382ba41e3b24b3880b2567cb981f97/postgis/gserialized_supportfn.c#L74-L91
>
>
> >
> > --strk;
> > _______________________________________________
> > postgis-devel mailing list
> > postgis-devel at lists.osgeo.org
> > https://lists.osgeo.org/mailman/listinfo/postgis-devel
>