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
