Re: Review: ECPG infrastructure changes part 1, was: Re: ECPG fixes
От | Boszormenyi Zoltan |
---|---|
Тема | Re: Review: ECPG infrastructure changes part 1, was: Re: ECPG fixes |
Дата | |
Msg-id | 529E0C8E.4070700@cybertec.at обсуждение исходный текст |
Ответ на | Review: ECPG infrastructure changes part 1, was: Re: ECPG fixes (Antonin Houska <antonin.houska@gmail.com>) |
Список | pgsql-hackers |
Thanks for the review. 2013-12-03 16:48 keltezéssel, Antonin Houska írta: > 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. I'll look at it, maybe the >0 had a reason. > > 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; I will fix the extra spaces. > 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. If you do this: SAVEPOINT A; <some operations> SAVEPOINT A; /* same savepoint name */ then the operations between the two are implicitly committed to the higher subtransaction, i.e. it works as if there was a RELEASE SAVEPOINT A; before the second SAVEPOINT A; It happens to be tested with a DECLARE CURSOR statement and the runtime has to refresh its knowledge about the cursor by propagating it into a subtransaction one level higher. > 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. The soversion has changed because of the changes in already existing exported functions. > > // 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 >> >> >> >> -- ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH Gröhrmühlgasse 26 A-2700 Wiener Neustadt, Austria Web: http://www.postgresql-support.de http://www.postgresql.at/
В списке pgsql-hackers по дате отправления: