Re: Fix issuing of multiple command completions per command

Fernando Nasser <fnasser(at)redhat(dot)com> writes:
> Oops! I am attaching the corrected patch. Sorry.

Thanks, that'll save me some work.

> We should ask him to try the patch. Coax says he is not very
> concerned with the CommandInfo -- the application needs the
> right CommandTag.

No, he needs LASTOID to be right too --- and that's coming from
CommandInfo.

> And remember that there is still the 'Z'. This completion will close
> the
> flow of data packets that may have been sent. It is good for the
> async processing as well as the application may process the data
> while waiting for the 'Z' (otherwise it will not know that all the
> data has arrived).

But 'Z' only comes after we have processed all the queries in a query
string. The 'C' message should come out when, and only when, we have
finished processing a particular query within the string --- and that
includes everything that stems from rewriting that query, IMHO.

> The way I see it is that this is a palnnable query thing where
> Begin and End calls should open and close the flow of data.

> For plannable queries we have:

> BeginCommand()
> <send data>
> EndCommand()

Actually, the destination receiver setup and teardown calls are the
things that open and close the flow of data. BeginCommand/EndCommand
are vestigial things given the destination receiver mechanism --- which
was something that didn't exist originally; I added it a couple years
ago to clean up the interfaces to printtup.c. It looks like it's now
time to take the next step in cleaning up that stuff.

What I am thinking of doing right now is applying your patch together
with a modification to remove EndCommand and UpdateCommandInfo.
Instead of those two routines, ProcessQuery should construct a
command tag string and return same to its caller. Then
pg_exec_query_string can save the command tag and send it to the
client at the right time (namely, after it's processed the whole
querytree list).

As a second step I'd like to get rid of BeginCommand and fold its
functionality into the destination receiver setup routines. However,
that's not a bug fix but just code beautification.

> But consider also that the CommandInfo that corresponds to the data
> stream was already sent. And it is zero anyway if more than one row
> is affected, isn't it? Maybe it is not that broken after all
> (with tis patch applied, I mean).

Yes, it's pretty broken. Consider an INSERT command that itself inserts
only one row, but there are rules that add additional inserts or
updates. We must return the CommandInfo from the specified insert
itself, not the other actions.

regards, tom lane