Re: Gripes about walsender command processing

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Sep-16, Tom Lane wrote:
>> I don't see an easy way to improve on it though.  The only obvious
>> alternative would be to put another switch before the main one that
>> just fills a "const char *cmdtag" variable, but that seems ugly.

> The alternative of doing the assignment in each case of the same switch
> does not look too terrible:

>          case T_IdentifySystemCmd:
> +            cmdtag = "IDENTIFY_SYSTEM";
> +            set_ps_display(cmdtag);
>              IdentifySystem();
> +            EndReplicationCommand(cmdtag);
>              break;
 
>          case T_BaseBackupCmd:
> +            cmdtag = "BASE_BACKUP";
> +            set_ps_display(cmdtag);
>              PreventInTransactionBlock(true, cmdtag);
>              SendBaseBackup((BaseBackupCmd *) cmd_node);
> +            EndReplicationCommand(cmdtag);
>              break;

Yeah, that works for me.  It doesn't allow for having just one
set_ps_display() call ahead of the switch, but that isn't that
big a loss.  We cannot merge the EndReplicationCommand calls to
after the switch, because some of the cases don't want one here;
so that partial duplication is inescapable.

Note that your changes need to be backpatched into v13,
because AFAICS this code is violating the FE/BE protocol
right now --- it's just luck that libpq isn't moaning
about extra CommandComplete messages.  But I don't think
we want to change the ps title behavior in v13 at this
late date, so that part should be HEAD-only.

            regards, tom lane