Re: Portal->commandTag as an enum

> On Mar 2, 2020, at 8:12 AM, Alvaro Herrera <alvherre@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
aboutthe embedding of the rowcount into the EndCommand output, but my preference is to treat these issues the same and
makea strong distinction between the commandtag and the embedded oid and/or rowcount.  It's hard to say how many future
featureswould 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
whywould 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