Robert Haas írta:
> On Tue, Feb 2, 2010 at 4:03 AM, Boszormenyi Zoltan <zb(at)cybertec(dot)at> wrote:
>> Thanks for testing it, with the attached patch your test case also
>> returns SELECT N.
> 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:
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)
... (everything else)
> 3. You've falsified the comment preceding that code, too.
* Set up global portal context pointers.
* We have to play a special game here to support utility
* VACUUM and CLUSTER, which internally start and commit
* When we are called to execute such a command,
* be pointing to the TopTransactionResourceOwner --- which will be
* destroyed and replaced in the course of the internal commit and
* restart. So we need to be prepared to restore it as pointing
* exit-time TopTransactionResourceOwner. (Ain't that ugly?
This idea of
* internally starting whole new transactions is not good.)
* CurrentMemoryContext has a similar problem, but the other
* 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.
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
Cybertec Schönig & Schönig GmbH
In response to
pgsql-hackers by date
|Next:||From: Robert Haas||Date: 2010-02-07 18:23:10|
|Subject: Re: [HACKERS] Re: Faster CREATE DATABASE by delaying fsync (was 8.4.1 ubuntu karmic slow createdb)|
|Previous:||From: Tom Lane||Date: 2010-02-07 17:15:49|
|Subject: Re: pg_class has no toast table? |