Re: Portal->commandTag as an enum

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. 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.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services