Re: [PATCH] Provide rowcount for utility SELECTs

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.

Thoughts:

1. Looks like you've falsified the last comment block in PortalRunMulti().
2. I don't like the duplication of code in PortalRun() between the
first and second branches of the switch statement.
3. You've falsified the comment preceding that code, too.
4. Is there any reason to use pg_strcasecmp() instead of plain old strcmp()?

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