Re: Portal->commandTag as an enum

> On Mar 2, 2020, at 8:12 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)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