[postgis-devel] Issue 146 in postgis: 1.4 ST_Distance and ST_Dwithin doesn't handle collections correctly

[Paul Ramsey]

Niklas,

First of all, welcome to the PostGIS community. I appreciate your
energy and enthusiasm, and willingness to prepare patches.

I have modified your patch and applied it. I had to modify it, because
it contained an error in the logic, which you'll see when you review
the change history. This is going to sound funny coming from me
(commit first, ask questions later), but: I think you're working a bit
too fast.  You keep attaching patches, then re-patching them for
issues, and so on: it makes committers (well, it makes me) very leery
about ever applying them (without going through them with a
fine-toothed comb, which is equivalent to writing them ourselves,
timewise).

Fortunately, there is a solution for people like you and me (people in
a hurry): unit testing.  As an example, I have added a file to the
cunit test harness specifically for distance tests and then used it to
verify (somewhat (you'll see I only test variants of collections of
POINT)) that the patch works.  For this spatial database work, we have
lots of combinatoric possibilities that need to be checked (POINT vs
POLY, COLLECTION vs LINE, etc) and building out those possibilities in
the test suite ahead of time both lets us feel confident in our work,
and to make changes knowing that if we break something, we won't apply
that change and let it out into general circulation.

I'd suggest that for your new features or big re-workings you start
putting together a cunit test suite so you can test every facet of
them.

Thanks again for the patches and your energy and enthusiasm,

Paul

On Sun, Apr 5, 2009 at 5:10 AM,  <codesite-noreply at google.com> wrote:
>
> Comment #3 on issue 146 by post... at jordogskog.no: 1.4 ST_Distance and
> ST_Dwithin doesn't handle collections correctly
> http://code.google.com/p/postgis/issues/detail?id=146
>
> Hallo
>
> I found a bug in measures.c
> It could handle collections and mult as the first incomming geometry but not
> the
> second. I think this patch solves it :-)
> /Nicklas Avén
>
> Attachments:
>        st_distance_bugfix.patch  649 bytes