Thread: on patch for close_ps() func. in geo_ops.c

Being a new comer to postgres hacking I am afraid
I did not test enough with regards to the previous patch I had sent
that attempted to fixthe "##" operator for point to a line segment.
My changes fixed the problems I had seen, but the
regression for geometry now fails with the error:

regression=> SELECT '' AS thirty, p.f1, l.s, p.f1 ## l.s AS closest
regression->    FROM LSEG_TBL l, POINT_TBL p;
PQexec() -- Request was sent to backend, but backend closed the channel
before responding.
        This probably means the backend terminated abnormally before or
while processing the request.
regression=>

Clearly I have to do more testing and fixing. I will do so, do the
regression tests etc. before claiming to fix anything. Sorry about this
folks, I am new to this and apologize for my mistakes.

Gautam

> Being a new comer to postgres hacking I am afraid
> I did not test enough with regards to the previous patch I had sent
> that attempted to fix the "##" operator for point to a line segment.
> My changes fixed the problems I had seen, but the
> regression for geometry now fails with the error:
>
> regression=> SELECT '' AS thirty, p.f1, l.s, p.f1 ## l.s AS closest
> regression->    FROM LSEG_TBL l, POINT_TBL p;
> PQexec() -- Request was sent to backend, but backend closed the
> channel before responding.
>
> Clearly I have to do more testing and fixing. I will do so, do the
> regression tests etc. before claiming to fix anything. Sorry about
> this folks, I am new to this and apologize for my mistakes.

Everyone is or was new to Postgres development at one time or another.
We have several months until the next release to work through the fixes
and features you want to add, so there is no problem with this.

As you gain experience with developing patches and fixes for Postgres,
you will hopefully find ways to test and submit the patches in the most
reliable manner, but _we all_ have had to go through that learning
period (well, OK, we are all still learning about that :)

Anyway, don't get discouraged. Let me know when you have some more
patches/fixes to apply, and I can help test them and package them for
the source tree.

The only unsuccessful code developers are the ones who submit a patch
and then walk away; stick with the problem and you'll be making a very
valuable contribution. I'll help where you need it.

Talk to you soon, and we're looking forward to more patches...

                      - Tom

> I have gone back and done more coding and more testing. The regression
> test for geometry no longer causes the back end to core dump. The
> abort was not happening the "close_ps()" function that I had hacked,
> but is in interpt_sl() routine. This routine dumps core if asked to
> find an intersection between a line segment and a line which in fact
> do not intersect. What I did was to fix close_ps() to not call
> interpt_sl() with parameters that do not intersect. I handle such
> special cases separately (and hopefully cleanly) in close_ps().
> Please let me know what you think. If you think of I will clean up
> and send proper patches (in right order this time, hopefully!)

Things look good. Would it be possible to fix interpt_sl() while you are
looking at this? Otherwise it will lurk in the code waiting to bite
someone else later. At the moment it is not directly callable as an SQL
function, but could/should be now that the "line" type is visible to
users.

Anyway, no need really to "send proper patches"; from here on how about
sending me patches based on the last file (or patch) you sent? Use "diff
-c" to generate the patch...

We should settle on an external representation for the "line" type;
although a point/slope representation is nice and intuitive, I'd suggest
trying the Ax+By+C=0 representation (used internally too) since we can
then avoid having representation problems with vertical lines (which
have infinite slope). Another possibility is to use a line segment
representation (two points) but we might have to be careful about
precision and rounding issues.

Once things settle down a bit I'll integrate the whole thing into the
source tree; there are several other files which have been touched in
the catalog and elsewhere and we'll need to move those patches to the
current source tree and test them before committing.

                    - Tom