> On Mar 2, 2020, at 8:12 AM, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
>
> On 2020-Feb-29, Mark Dilger wrote:
>
>> You may want to think about the embedding of InvalidOid into the EndCommand
>> output differently from how you think about the embedding of the rowcount
>> into the EndCommand output, but my preference is to treat these issues the
>> same and make a strong distinction between the commandtag and the embedded
>> oid and/or rowcount. It's hard to say how many future features would be
>> crippled by having the embedded InvalidOid in the commandtag, but as an
>> example *right now* in the works, we have a feature to count how many
>> commands of a given type have been executed. It stands to reason that
>> feature, whether accepted in its current form or refactored, would not want
>> to show users a pg_stats table like this:
>>
>> cnt command
>> ---- -------------
>> 5 INSERT 0
>> 37 SELECT
>>
>> What the heck is the zero doing after the INSERT? That's the hardcoded
>> InvalidOid that you are apparently arguing for. You could get around that
>> by having the pg_sql_stats patch have its own separate set of command tag
>> strings, but why would we intentionally design that sort of duplication into
>> the solution?
>
> This is what I think Tom means to use in EndCommand:
>
> if (command_tag_display_rowcount(tag) && !force_undecorated_output)
> snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
> tag == CMDTAG_INSERT ?
> "%s 0 " UINT64_FORMAT : "%s " UINT64_FORMAT,
> tagname, qc->nprocessed);
> else
> ... no rowcount ...
>
> The point is not to change the returned tag in any way -- just to make
> the method to arrive at it not involve the additional data column in the
> data file, instead hardcode the behavior in EndCommand.
Thanks, Álvaro, I think I get it now. I thought Tom was arguing to have
"INSERT 0" rather than "INSERT" be the commandtag.
> I don't
> understand your point of pg_stats_sql having to deal with this in a
> particular way. How is that patch obtaining the command tags? I would
> hope it calls GetCommandTagName() rather than call CommandEnd, but maybe
> I misunderstand where it hooks.
My objection was based on my misunderstanding of what Tom was requesting.
I can rework the patch the way Tom suggests.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
