Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-27 Thread Alvaro Herrera
Andres Freund wrote: > If you want to do that, I'm unenthusiastically not objecting. But if so, > let's also do it in IdentifySystem(), SendTimeLineHistory(), > StartReplication(), SendBackupHeader(), SendXLogRecPtResult() - they're > modeled after each other. Okay, pushed with that, backpatched

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-22 Thread José Luis Tallón
On 10/22/2015 12:36 AM, Alvaro Herrera wrote: Andres Freund wrote: That seems fairly insignificant. For one this is a rather infrequent and expensive operation, for another every decent compiler can optimize those away. Note that those duplicate strlen() calls are there in a lot of places in wa

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Tom Lane
Alvaro Herrera writes: > Andres Freund wrote: >> for another every decent compiler can optimize those away. Note that >> those duplicate strlen() calls are there in a lot of places in >> walsender.c > It can? Tom has repeatedly argue the opposite, in the past. I'm prepared to believe that *some

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Andres Freund
On 2015-10-21 19:36:16 -0300, Alvaro Herrera wrote: > diff --git a/src/backend/replication/walsender.c > b/src/backend/replication/walsender.c > index c6043cd..5487cc0 100644 > --- a/src/backend/replication/walsender.c > +++ b/src/backend/replication/walsender.c > @@ -762,10 +762,10 @@ logical_rea

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Alvaro Herrera
Andres Freund wrote: > That seems fairly insignificant. For one this is a rather infrequent and > expensive operation, for another every decent compiler can optimize > those away. Note that those duplicate strlen() calls are there in a lot > of places in walsender.c diff --git a/src/backend/repli

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Alvaro Herrera
Andres Freund wrote: > On 2015-10-21 17:21:25 -0300, Alvaro Herrera wrote: > > I think the first assignment is also pointless -- I mean can't we just > > use MyReplicationSlot->data.name in both places where slot_name is used? > > Makes the lines a bit long. strlen(NameStr(MyReplicationSlot->data

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Andres Freund
On 2015-10-21 17:21:25 -0300, Alvaro Herrera wrote: > > It seems that the 2nd assignment was an oversight. Spotted by Bernd > > Helmle. Yea, that's obviously redudant. Will remove. > I think the first assignment is also pointless -- I mean can't we just > use MyReplicationSlot->data.name in both

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Alvaro Herrera
Euler Taveira wrote: > It seems that the 2nd assignment was an oversight. Spotted by Bernd > Helmle. I think the first assignment is also pointless -- I mean can't we just use MyReplicationSlot->data.name in both places where slot_name is used? In the same routine, it seems wasteful to be doing

Re: [HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-21 Thread Euler Taveira
On 20-10-2015 08:28, Bernd Helmle wrote: The 2nd assignment to slot_name looks unnecessary? Yes, it is. Seems to be an oversight. Patch attached. -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

[HACKERS] Duplicated assignment of slot_name in walsender.c

2015-10-20 Thread Bernd Helmle
walsender.c, CreateReplicationSlot() currently has this: slot_name = NameStr(MyReplicationSlot->data.name); if (cmd->kind == REPLICATION_KIND_LOGICAL) { [...] } else if (cmd->kind == REPLICATION_KIND_PHYSICAL && cmd->reserve_wal) { [