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

2017-09-06 Thread Alvaro Herrera
Ashutosh Bapat wrote: > On Wed, Sep 6, 2017 at 1:32 AM, Tom Lane wrote: > > 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

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

2017-09-05 Thread Ashutosh Bapat
On Wed, Sep 6, 2017 at 1:32 AM, Tom Lane wrote: > Ashutosh Bapat writes: >> On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke >> 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 passe

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

2017-09-05 Thread Tom Lane
Ashutosh Bapat writes: > On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke > 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 sour

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

2017-09-05 Thread Ashutosh Bapat
On Tue, Sep 5, 2017 at 4:02 PM, Jeevan Chalke wrote: > Hi, > > lfirst_node() changes are missing for List node type and I was thinking > about adding those. But it looks like we cannot do so as List can be a > T_List, T_IntList, or T_OidList. So I am OK with that. Thanks for investigating the cas

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

2017-09-05 Thread Jeevan Chalke
Hi, lfirst_node() changes are missing for List node type and I was thinking about adding those. But it looks like we cannot do so as List can be a T_List, T_IntList, or T_OidList. So I am OK with that. Apart from this, changes look good to me. Everything works fine. As we are now doing it for lf

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

2017-07-14 Thread Ashutosh Bapat
On Thu, Jul 13, 2017 at 9:15 PM, Alvaro Herrera wrote: > Ashutosh Bapat wrote: > >> Happened to stumble across some instances of lfirst() which could use >> lfirst_node() in planner.c. Here's patch which replaces calls to >> lfirst() extracting node pointers by lfirst_node() in planner.c. > > Soun

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

2017-07-13 Thread Alvaro Herrera
Ashutosh Bapat wrote: > Happened to stumble across some instances of lfirst() which could use > lfirst_node() in planner.c. Here's patch which replaces calls to > lfirst() extracting node pointers by lfirst_node() in planner.c. Sounds good. > Are we carrying out such replacements in master or th

[HACKERS] Replacing lfirst() with lfirst_node() appropriately in planner.c

2017-07-12 Thread Ashutosh Bapat
Hi, Happened to stumble across some instances of lfirst() which could use lfirst_node() in planner.c. Here's patch which replaces calls to lfirst() extracting node pointers by lfirst_node() in planner.c. I have covered all the occurences of lfirst() except 1. those extract generic pointers like Pa