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