[mapserver-dev] Re: postgis transaction patch (WAS: Voluminous Bug List)

[David Fuhry]

Paul,

Paul Ramsey wrote:
> If I have time to set up a proper testing environment I'll look at it,
> but you'll probably get to it first. The logic of
> msPOSTGISInitConnection seems pretty unassailable. I would suggest
> throwing in some tests for the "impossible" cases of PQTRANS_ACTIVE,
> and PQTRANS_UNKNOWN, and perhaps a logging message for
> PGTRANS_INTRANS, just to improve visibility of what's going on.

Done, and renamed the method msPOSTGISSanitizeConnection to better 
reflect its purpose.  Find the updated patch in trac.

> There is one piece of code you didn't retrofit, search for a *lower
> case* "begin" to find it. However, it's in a code path that should
> never get hit, so I don't think that's it.

It would be nice to see that whole section in the end of 
prepare_database removed, since AFAICT its only purpose is to support 
postgis <= 0.5, while it drags around baggage & obscures the code while 
doing so.  Should that be a separate patch, or included in this one?

> All this tap dancing re-inforces to me the need to do a
> clean-up/re-write of this stuff to *not* use binary cursors anymore.
> We should be doing base64 encoding of WKB for transit so we can avoid
> the whole binary cursor (and associated BEGIN/END/ROLLBACK) mess.
> Mildly slower, but way cleaner code.

Agreed.  Deferring to your judgment on how hex vs. binary performance 
would play out, since there seem to be a lot of variables.

Thanks,

Dave


> P.
> 
> On Tue, Apr 8, 2008 at 2:30 PM, David Fuhry <dfuhry at acm.org> wrote:
>> Paul,
>>
>>    The mappostgis.c patch is apparently still not completely correct yet;
>> see Carlos Molina's recent messages:
>>
>>  http://lists.osgeo.org/pipermail/mapserver-users/2008-March/055050.html
>>  http://lists.osgeo.org/pipermail/mapserver-users/2008-March/055072.html
>>
>>  and my response:
>>
>>  http://lists.osgeo.org/pipermail/mapserver-users/2008-April/055264.html
>>
>>    I won't be able to look at this in earnest until after next Monday, but
>> if you can give the patch a brief review, that would be appreciated.
>> Especially if you happen across the cause of the error that Carlos is
>> seeing.
>>
>>  Thanks,
>>
>>  Dave
>>
>>
>>  Steve Lime wrote:
>>
>>> I always focus on the milestone reports which are generally more
>> encouraging. ;-) I would welcome
>>> an additional set of eyes working through the backlog, so please feel
>> free. I took care of 1440 just now.
>>> While I've got you, one that could use your approval is 2497. There's a
>> patch attached that seems to
>>> help but I'd prefer to have someone familiar with PostGIS internals weight
>> in before applying.
>>> Steve
>>>
>>>
>>>>>> On 4/8/2008 at 3:01 PM, in message
>>>>>>
>>> <30fe546d0804081301tf7bece0k4757283b559da29f at mail.gmail.com>, "Paul
>> Ramsey"
>>> <pramsey at cleverelephant.ca> wrote:
>>>
>>>> Has anyone noticed that the list of Mapserver bugs is Really Long?
>>>>
>>>> http://trac.osgeo.org/mapserver/report/1
>>>> It also includes stuff like this:
>>>>
>>>> http://trac.osgeo.org/mapserver/ticket/1440
>>>> An issue from three years ago, that had a patch attached, and the
>>>> patch was applied and... the ticket is still open.
>>>>
>>>> Would a little gentle janitorial work through the issues be
>>>> appreciated or unwelcome? For the above example, turning it to fixed,
>>>> noting the patch was applied at 4.7 and that 4.6 is too old to think
>>>> about.
>>>>
>>>> I could do a few of these now and again, keep the volume low, and if I
>>>> make an error, hopefully since the relevant developers will be
>>>> notified, they will catch it and smack me.
>>>>
>>>> ?
>>>>
>>>> P.
>>>> _______________________________________________
>>>> mapserver-dev mailing list
>>>> mapserver-dev at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/mapserver-dev
>>> _______________________________________________
>>> mapserver-dev mailing list
>>> mapserver-dev at lists.osgeo.org
>>> http://lists.osgeo.org/mailman/listinfo/mapserver-dev
>>>