Re: Gripes about walsender command processing

On 2020-Sep-16, Tom Lane wrote:

> This looks moderately reasonable to me. However, with the process
> title reporting I want to add, we're going to end up with a switch
> that looks like
>
> case T_IdentifySystemCmd:
> + set_ps_display("IDENTIFY_SYSTEM");
> IdentifySystem();
> + EndReplicationCommand("IDENTIFY_SYSTEM");
> break;
>
> case T_BaseBackupCmd:
> + set_ps_display("BASE_BACKUP");
> PreventInTransactionBlock(true, "BASE_BACKUP");
> SendBaseBackup((BaseBackupCmd *) cmd_node);
> + EndReplicationCommand("BASE_BACKUP");
> break;
>
> which is starting to look a bit repetitive and copy-pasteo-prone.

Agreed.

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

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