The changes are very well divided into logical units, so I think I could
understand the ideas. I'm not too familiar with the ecpg internals, so
consider this at least a coding review.
git apply: Clean, except for one finding below
Build: no errors/warnings
Documentation build: no errors/warnings. The changes do appear in ecpg.html.
Regression tests: all passed
Non-Linux platforms: I can't verify, but I haven't noticed any new
dependencies added.
Comments (in the source code): good.
(My) additional comments:
22.patch
--------
tuples_left > 0
instead of just
tuples_left
seems to me safer in for-loops. Not sure if there's a convention for
this though.
23.patch
--------
git apply --verbose ~/cybertec/patches/ecpq/23.patch
/home/anton/cybertec/patches/ecpq/23.patch:494: space before tab in indent.
/*------
/home/anton/cybertec/patches/ecpq/23.patch:495: space before tab in indent.
translator: this string will be truncated at
149 characters expanded. */
/home/anton/cybertec/patches/ecpq/23.patch:4019: trailing whitespace.
exec sql close :curname;
Tests - 23.patch
----------------
src/interfaces/ecpg/test/sql/cursorsubxact.pgc
/*
* Test the implicit RELEASE SAVEPOINT if a SAVEPOINT
* is used with an already existing name.
*/
Shouldn't it be "... if a CURSOR is used with an already existing
name?". Or just "... implicit RELEASE SAVEPOINT after an error"?
I'd also appreciate a comment where exactly the savepoint is
(implicitly) released.
23.patch and 24.patch
---------------------
SO_MAJOR_VERSION and also interfaces/ecpg/include/ecpglib.h is changed
Thus all client applications probably need to be preprocessed & compiled
against the PG 9.4. I don't know how this is usually enforced. I'm
mentioning it for the sake of completeness.
// Antonin Houska (Tony)
On 11/28/2013 03:21 PM, Boszormenyi Zoltan wrote:
> 2013-11-20 14:53 keltezéssel, Boszormenyi Zoltan írta:
>> 2013-11-20 14:41 keltezéssel, Boszormenyi Zoltan írta:
>>> 2013-11-12 07:15 keltezéssel, Boszormenyi Zoltan írta:
>>>> 2013-11-12 07:01 keltezéssel, Noah Misch írta:
>>>>> On Mon, Nov 11, 2013 at 10:17:54AM +0100, Boszormenyi Zoltan wrote:
>>>>>> The old contents of my GIT repository was removed so you need to
>>>>>> clone it fresh. https://github.com/zboszor/ecpg-readahead.git
>>>>>> I won't post the humongous patch again, since sending a 90KB
>>>>>> compressed file to everyone on the list is rude.
>>>>> Patches of that weight show up on a regular basis. I don't think it's
>>>>> rude.
>>>>
>>>> OK, here it is.
>>>
>>> ...
>>> Subsequent patches will come as reply to this email.
>>
>> Infrastructure changes in ecpglib/execute.c to split up
>> ECPGdo and ecpg_execute() and expose the parts as
>> functions internal to ecpglib.
>
> Rebased after killing the patch that changed the DECLARE CURSOR command tag.
> All infrastructure patches are attached, some of them compressed.
>
> Best regards,
> Zoltán Böszörményi
>
>
>
>
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
