Re: Replacing lfirst() with lfirst_node() appropriately in planner.c

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Jeevan Chalke <jeevan(dot)chalke(at)enterprisedb(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Replacing lfirst() with lfirst_node() appropriately in planner.c
Date: 2017-09-05 20:02:14
Message-ID: 6963.1504641734@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> writes:
> On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke
> <jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
>> Attached patch for replacing linitial() with linital_node() appropriately in
>> planner.c

> Ok. The patch looks good to me. It compiles and make check passes.
> Here are both the patches rebased on the latest sources.

LGTM, pushed. I also changed a couple of places that left off any cast
of lfirst, eg:

- List *gset = lfirst(lc);
+ List *gset = (List *) lfirst(lc);

While this isn't really harmful, it's not per prevailing style.

BTW, I think we *could* use "lfirst_node(List, ...)" in cases where
we know the list is supposed to be a list of objects rather than ints
or Oids. I didn't do anything about that observation, though.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2017-09-05 21:20:51 Re: [Proposal] Allow users to specify multiple tables in VACUUM commands
Previous Message Jeff Janes 2017-09-05 19:47:41 Re: Fix performance of generic atomics