Re: [PATCH] Provide rowcount for utility SELECTs
От | Boszormenyi Zoltan |
---|---|
Тема | Re: [PATCH] Provide rowcount for utility SELECTs |
Дата | |
Msg-id | 4B6EFC6A.50507@cybertec.at обсуждение исходный текст |
Ответ на | Re: [PATCH] Provide rowcount for utility SELECTs (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: [PATCH] Provide rowcount for utility SELECTs
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
Robert Haas írta: > On Tue, Feb 2, 2010 at 4:03 AM, Boszormenyi Zoltan <zb@cybertec.at> wrote: > >> Thanks for testing it, with the attached patch your test case also >> returns SELECT N. >> > > Thoughts: > > 1. Looks like you've falsified the last comment block in PortalRunMulti(). > You mean the "fake something up" part? Will fix the comment. > 2. I don't like the duplication of code in PortalRun() between the > first and second branches of the switch statement. > The PORTAL_ONE_SELECT and PORTAL_ONE_RETURNING/PORTAL_UTIL_SELECT cases differ only in that the latter case runs this below everything else: if (!portal->holdStore) FillPortalStore(portal,isTopLevel); Would it be desired to unify these cases? This way there would be no code duplication. Something like: if (portal->strategy != PORTAL_ONE_SELECT && !portal->holdStore) FillPortalStore(portal,isTopLevel); ... (everything else) > 3. You've falsified the comment preceding that code, too. > This one? /* * Set up global portal context pointers. * * We have to play a special game here to supportutility commands like * VACUUM and CLUSTER, which internally start and commit transactions. * When we are called to execute such a command, CurrentResourceOwner will * be pointing to the TopTransactionResourceOwner --- which will be * destroyed andreplaced in the course of the internal commit and * restart. So we need to be prepared to restore it as pointing to the * exit-time TopTransactionResourceOwner. (Ain't that ugly? This idea of * internally starting whole new transactions is not good.) * CurrentMemoryContext has a similarproblem, but the other pointers we * save here will be NULL or pointing to longer-lived objects. */ I can't see why. Can you clarify? Or this one? /* we know the query is supposed to set the tag */ if (completionTag && portal->commandTag) ... The condition and the comment still seems to be true. Which comment are you talking about? > 4. Is there any reason to use pg_strcasecmp() instead of plain old strcmp()? > I don't have any particular reason, strcmp() would do. > Someone who knows the overall structure of the code better than I do > will have to comment on whether there are any code paths to worry > about that do not go through PortalRun(). > > A general concern I have is that this what we're basically doing here > is handling the most common case in ProcessQuery() and then installing > fallback mechanisms to pick up any stragglers: but the fallback > mechanisms only guarantee that we'll add a number to the command tag, > not that it will be meaningful. Unfortunately, my limited imagination > can't quite figure out in what cases we'll get a SELECT command tag > back other than (1) plain old SELECT, (2) SELECT INTO, and (3) CTAS, > so I'm not sure what to go test. > > ...Robert > > Best regards, Zoltán Böszörményi -- Bible has answers for everything. Proof: "But let your communication be, Yea, yea; Nay, nay: for whatsoever is more than these cometh of evil." (Matthew 5:37) - basics of digital technology. "May your kingdom come" - superficial description of plate tectonics ---------------------------------- Zoltán Böszörményi Cybertec Schönig & Schönig GmbH http://www.postgresql.at/
В списке pgsql-hackers по дате отправления:
Следующее
От: Robert HaasДата:
Сообщение: Re: Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)