Re: Yet another small patch - reorderbuffer.c:1099 - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Yet another small patch - reorderbuffer.c:1099
Date
Msg-id CA+Tgmoa6OJWnezmyvVkrXO-B-OpYzT=GsoRKtXwjDQyoqDvEiw@mail.gmail.com
Whole thread Raw
In response to Re: Yet another small patch - reorderbuffer.c:1099  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: Yet another small patch - reorderbuffer.c:1099  (Aleksander Alekseev <a.alekseev@postgrespro.ru>)
List pgsql-hackers
On Tue, Apr 5, 2016 at 10:38 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> IMO the code is wrong.  There should be a single block comment saying
> something like "Remove the node from its containing list.  In the FOO
> case, the list corresponds to BAR and therefore we delete it because
> BAZ.  In the QUUX case the list is PLUGH and we delete in because THUD."
> Then a single line dlist_delete(...) follows.
>
> The current arrangement looks bizantine to me, for no reason.  If we
> think that one of the two branches might do something additional to the
> list deletion, surely that will be in a separate stanza with its own
> comment; and if we ever want to remove the list deletion from one of the
> two cases (something that strikes me as unlikely) then we will need to
> fix the comment, too.

+1 to everything here except for the way byzantine is spelled.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: WIP: Covering + unique indexes.
Next
From: David Steele
Date:
Subject: Re: [PATCH v12] GSSAPI encryption support