Re: [HACKERS] Replacing lfirst() with lfirst_node() appropriately in planner.c - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: [HACKERS] Replacing lfirst() with lfirst_node() appropriately in planner.c
Date
Msg-id CAFjFpRc_zdcTPm40qYgNeAtv_LFrgw9R7s6Y3kVpdHrA3QZUPw@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Replacing lfirst() with lfirst_node() appropriately in planner.c  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] Replacing lfirst() with lfirst_node() appropriately inplanner.c
List pgsql-hackers
On Wed, Sep 6, 2017 at 1:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
>> On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke
>> <jeevan.chalke@enterprisedb.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.

+1. Thanks.

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

IMO, it won't be apparent as to why some code uses lfirst_node(List,
...) and some code uses (List *) lfirst().

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company



pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: [HACKERS] Making clausesel.c Smarter
Next
From: Catalin Iacob
Date:
Subject: Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection