Horton

Thread

Commits

  1. Remove unimplemented/undocumented geometric functions & operators.

  2. Doc: de-document unimplemented geometric operators.

  3. Implement poly_distance().

  1. Triage for unimplemented geometric operators

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-12-06T23:18:38Z

    Over in [1] it was pointed out that I overenthusiastically
documented several geometric operators that, in fact, are
only stubs that throw errors when called.  Specifically
these are

dist_lb:	<->(line,box)
dist_bl:	<->(box,line)
close_sl:	lseg ## line
close_lb:	line ## box
poly_distance:	polygon <-> polygon
path_center:	@@ path
(this also underlies point(path), which is not documented anyway)

There are three reasonable responses:

1. Remove the documentation, leave the stubs in place;
2. Rip out the stubs and catalog entries too (only possible in HEAD);
3. Supply implementations.

I took a brief look at these, and none of them seem exactly hard
to implement, with the exception of path_center which seems not to
have a non-arbitrary definition.  (We could model it on poly_center
but that one seems rather arbitrary; also, should open paths behave
any differently than closed ones?)  close_lb would also be rather
arbitrary for the case of a line that intersects the box, though
we could follow close_sb's lead and return the line's closest point
to the box center.

On the other hand, they've been unimplemented for more than twenty years
and no one has stepped forward to fill the gap, which sure suggests that
nobody cares and we shouldn't expend effort and code space on them.

The only one I feel a bit bad about dropping is poly_distance, mainly
on symmetry grounds: we have distance operators for all the geometric
types, so dropping this one would leave a rather obvious hole.  The
appropriate implementation seems like a trivial copy and paste job:
distance is zero if the polygons overlap per poly_overlap, otherwise
it's the same as the closed-path case of path_distance.

So my inclination for HEAD is to implement poly_distance and nuke
the others.  I'm a bit less sure about the back branches, but maybe
just de-document all of them there.

Thoughts?

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/5405b243-4523-266e-6139-ad9f80a9d9fc%40postgrespro.ru

  2. Re: Triage for unimplemented geometric operators

    Laurenz Albe <laurenz.albe@cybertec.at> — 2021-12-07T03:24:30Z

    On Mon, 2021-12-06 at 18:18 -0500, Tom Lane wrote:
> Over in [1] it was pointed out that I overenthusiastically
> documented several geometric operators that, in fact, are
> only stubs that throw errors when called.  Specifically
> these are
> 
> dist_lb:        <->(line,box)
> dist_bl:        <->(box,line)
> close_sl:       lseg ## line
> close_lb:       line ## box
> poly_distance:  polygon <-> polygon
> path_center:    @@ path
> (this also underlies point(path), which is not documented anyway)
> 
> There are three reasonable responses:
> 
> 1. Remove the documentation, leave the stubs in place;
> 2. Rip out the stubs and catalog entries too (only possible in HEAD);
> 3. Supply implementations.
> 
> I took a brief look at these, and none of them seem exactly hard
> to implement, with the exception of path_center which seems not to
> have a non-arbitrary definition.  (We could model it on poly_center
> but that one seems rather arbitrary; also, should open paths behave
> any differently than closed ones?)  close_lb would also be rather
> arbitrary for the case of a line that intersects the box, though
> we could follow close_sb's lead and return the line's closest point
> to the box center.
> 
> On the other hand, they've been unimplemented for more than twenty years
> and no one has stepped forward to fill the gap, which sure suggests that
> nobody cares and we shouldn't expend effort and code space on them.
> 
> The only one I feel a bit bad about dropping is poly_distance, mainly
> on symmetry grounds: we have distance operators for all the geometric
> types, so dropping this one would leave a rather obvious hole.  The
> appropriate implementation seems like a trivial copy and paste job:
> distance is zero if the polygons overlap per poly_overlap, otherwise
> it's the same as the closed-path case of path_distance.
> 
> So my inclination for HEAD is to implement poly_distance and nuke
> the others.  I'm a bit less sure about the back branches, but maybe
> just de-document all of them there.
> 
> Thoughts?
>
> [1] https://www.postgresql.org/message-id/flat/5405b243-4523-266e-6139-ad9f80a9d9fc%40postgrespro.ru

I agree with option #2 for HEAD; if you feel motivated to implement
"poly_distance", fine.

About the back branches, removing the documentation is a good choice.

I think the lack of complaints is because everybody who needs serious
geometry processing uses PostGIS.

Yours,
Laurenz Albe

  3. Re: Triage for unimplemented geometric operators

    Anton Voloshin <a.voloshin@postgrespro.ru> — 2021-12-07T15:54:57Z

    On 07/12/2021 06:18, Tom Lane wrote:
> So my inclination for HEAD is to implement poly_distance and nuke
> the others.  I'm a bit less sure about the back branches, but maybe
> just de-document all of them there.

I agree, seems to be a reasonable compromise. Removing 20+-years old 
unused and slightly misleading code probably should overweight the 
natural inclination to implement all of the functions promised in the 
catalog. Enhancing software by deleting the code is not an everyday 
chance and IMHO should be taken, even when it requires an extra 
catversion bump.

I am kind of split on whether it is worth it to implement poly_distance 
in back branches. Maybe there is a benefit in implementing: it should 
not cause any reasonable incompatibilities and will introduce somewhat 
better compatibility with v15+. We could even get away with not updating 
v10..12' docs on poly_distance because it's not mentioned anyway.

I agree on de-documenting all of the unimplemented functions in v13 and 
v14. Branches before v13 should not require any changes (including 
documentation) because detailed table on which operators support which 
geometry primitives only came in 13, and I could not find in e.g. 12's 
documentation any references to the cases you listed previously:
 > dist_lb:	<->(line,box)
 > dist_bl:	<->(box,line)
 > close_sl:	lseg ## line
 > close_lb:	line ## box
 > poly_distance:	polygon <-> polygon
 > path_center:	@@ path

-- 
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru