Thread: [HACKERS] [sqlsmith] Planner crash on foreign table join

[HACKERS] [sqlsmith] Planner crash on foreign table join

From
Andreas Seltenreich
Date:
Hi,

testing master at f0e44021df with a loopback postgres_fdw installed, I
see lots of crashes on queries joining foreign tables with various
expressions.  Below is a reduced recipe for the regression database and
a backtrace.

regards,
Andreas

--8<---------------cut here---------------start------------->8---
create extension postgres_fdw;
create server myself foreign data wrapper postgres_fdw;
create schema fdw_postgres;
create user fdw login;
grant all on schema public to fdw;
grant all on all tables in schema public to fdw;
create user mapping for public server myself options (user 'fdw');
import foreign schema public from server myself into fdw_postgres;

explain select from fdw_postgres.hslot   left join fdw_postgres.num_exp_div   on ((exists (values (1))) and (values
(1))is null);
 
--8<---------------cut here---------------end--------------->8---

Program terminated with signal SIGSEGV, Segmentation fault.
#0  bms_get_singleton_member (a=0x10, member=member@entry=0x7fffb577cafc) at bitmapset.c:577
#1  0x000056425107b531 in find_relation_from_clauses (clauses=0x564251a68570, root=0x564251a273d8) at clausesel.c:445
#2  clauselist_selectivity (root=root@entry=0x564251a273d8, clauses=0x564251a68570, varRelid=varRelid@entry=0,
jointype=JOIN_LEFT,sjinfo=0x564251a661c0) at clausesel.c:128
 
#3  0x00007f61d3d9f22f in postgresGetForeignJoinPaths (root=<optimized out>, joinrel=0x564251a66ba8,
outerrel=<optimizedout>, innerrel=<optimized out>, jointype=<optimized out>, extra=0x7fffb577cc50) at
postgres_fdw.c:4466
#4  0x000056425108a238 in add_paths_to_joinrel (root=root@entry=0x564251a273d8, joinrel=joinrel@entry=0x564251a66ba8,
outerrel=outerrel@entry=0x564251a65378,innerrel=innerrel@entry=0x564251a65f30, jointype=jointype@entry=JOIN_LEFT,
sjinfo=sjinfo@entry=0x564251a661c0,restrictlist=0x564251a681c8) at joinpath.c:278
 
#5  0x000056425108bff2 in populate_joinrel_with_paths (restrictlist=<optimized out>, sjinfo=0x564251a661c0,
joinrel=0x564251a66ba8,rel2=0x564251a65f30, rel1=0x564251a65378, root=0x564251a273d8) at joinrels.c:795
 
#6  make_join_rel (root=root@entry=0x564251a273d8, rel1=rel1@entry=0x564251a65378, rel2=rel2@entry=0x564251a65f30) at
joinrels.c:731
#7  0x000056425108c7ef in make_rels_by_clause_joins (other_rels=<optimized out>, old_rel=<optimized out>,
root=<optimizedout>) at joinrels.c:277
 
#8  join_search_one_level (root=root@entry=0x564251a273d8, level=level@entry=2) at joinrels.c:99
#9  0x0000564251079bdb in standard_join_search (root=0x564251a273d8, levels_needed=2, initial_rels=<optimized out>) at
allpaths.c:2385
#10 0x000056425107ac7b in make_one_rel (root=root@entry=0x564251a273d8, joinlist=joinlist@entry=0x564251a65998) at
allpaths.c:184
#11 0x0000564251099ef4 in query_planner (root=root@entry=0x564251a273d8, tlist=tlist@entry=0x0,
qp_callback=qp_callback@entry=0x56425109aeb0<standard_qp_callback>, qp_extra=qp_extra@entry=0x7fffb577cff0) at
planmain.c:253
#12 0x000056425109dbc2 in grouping_planner (root=root@entry=0x564251a273d8,
inheritance_update=inheritance_update@entry=0'\000', tuple_fraction=<optimized out>, tuple_fraction@entry=0) at
planner.c:1684
#13 0x00005642510a0133 in subquery_planner (glob=glob@entry=0x564251a2a6d0, parse=parse@entry=0x5642519aac60,
parent_root=parent_root@entry=0x0,hasRecursion=hasRecursion@entry=0 '\000', tuple_fraction=tuple_fraction@entry=0) at
planner.c:833
#14 0x00005642510a0f71 in standard_planner (parse=0x5642519aac60, cursorOptions=256, boundParams=0x0) at planner.c:333
#15 0x00005642511458cd in pg_plan_query (querytree=querytree@entry=0x5642519aac60, cursorOptions=256,
boundParams=boundParams@entry=0x0)at postgres.c:802
 
#16 0x0000564250fa9a40 in ExplainOneQuery (query=0x5642519aac60, cursorOptions=<optimized out>, into=0x0,
es=0x564251a513a0,queryString=0x564251a09590 "explain select from\n  fdw_postgres.hslot\n    left join
fdw_postgres.num_exp_div\n   on ((exists (values (1))) and (values (1)) is null);", params=0x0, queryEnv=0x0) at
explain.c:367
#17 0x0000564250faa005 in ExplainQuery (pstate=pstate@entry=0x564251a511f0, stmt=stmt@entry=0x564251a0aa58,
queryString=queryString@entry=0x564251a09590"explain select from\n  fdw_postgres.hslot\n    left join
fdw_postgres.num_exp_div\n   on ((exists (values (1))) and (values (1)) is null);", params=params@entry=0x0,
queryEnv=queryEnv@entry=0x0,dest=dest@entry=0x564251a51308) at explain.c:256
 
#18 0x000056425114b9cb in standard_ProcessUtility (pstmt=0x564251a0b2e0, queryString=0x564251a09590 "explain select
from\n fdw_postgres.hslot\n    left join fdw_postgres.num_exp_div\n    on ((exists (values (1))) and (values (1)) is
null);",context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x564251a51308, completionTag=0x7fffb577d390
"")at utility.c:680
 
#19 0x00005642511487b4 in PortalRunUtility (portal=0x564251a07580, pstmt=0x564251a0b2e0, isTopLevel=<optimized out>,
setHoldSnapshot=<optimizedout>, dest=<optimized out>, completionTag=0x7fffb577d390 "") at pquery.c:1179
 
#20 0x0000564251149633 in FillPortalStore (portal=portal@entry=0x564251a07580, isTopLevel=isTopLevel@entry=1 '\001') at
pquery.c:1039
#21 0x000056425114a21d in PortalRun (portal=portal@entry=0x564251a07580, count=count@entry=9223372036854775807,
isTopLevel=isTopLevel@entry=1'\001', run_once=run_once@entry=1 '\001', dest=dest@entry=0x564251a0b378,
altdest=altdest@entry=0x564251a0b378,completionTag=0x7fffb577d5b0 "") at pquery.c:769
 
#22 0x0000564251145d8a in exec_simple_query (query_string=0x564251a09590 "explain select from\n  fdw_postgres.hslot\n
left join fdw_postgres.num_exp_div\n    on ((exists (values (1))) and (values (1)) is null);") at postgres.c:1105
 
#23 0x0000564251147ab1 in PostgresMain (argc=<optimized out>, argv=argv@entry=0x5642519b2e00, dbname=<optimized out>,
username=<optimizedout>) at postgres.c:4075
 
#24 0x0000564250e5c4cc in BackendRun (port=0x5642519a7d70) at postmaster.c:4317
#25 BackendStartup (port=0x5642519a7d70) at postmaster.c:3989
#26 ServerLoop () at postmaster.c:1729
#27 0x00005642510d07c3 in PostmasterMain (argc=3, argv=0x5642519844d0) at postmaster.c:1337
#28 0x0000564250e5db2d in main (argc=3, argv=0x5642519844d0) at main.c:228




Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Andrew Gierth
Date:
>>>>> "Andreas" == Andreas Seltenreich <seltenreich@gmx.de> writes:
Andreas> Hi,Andreas> testing master at f0e44021df with a loopback postgres_fdwAndreas> installed, I see lots of crashes
onqueries joining foreignAndreas> tables with various expressions.  Below is a reduced recipeAndreas> for the
regressiondatabase and a backtrace.
 

Commit ac2b095088 assumes that clauselist_selectivity is being passed a
list of RelOptInfo, but postgres_fdw is passing it a list of bare
clauses.  One of them is wrong :-)

-- 
Andrew (irc:RhodiumToad)



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Andreas" == Andreas Seltenreich <seltenreich@gmx.de> writes:
>  Andreas> testing master at f0e44021df with a loopback postgres_fdw
>  Andreas> installed, I see lots of crashes on queries joining foreign
>  Andreas> tables with various expressions.  Below is a reduced recipe
>  Andreas> for the regression database and a backtrace.

> Commit ac2b095088 assumes that clauselist_selectivity is being passed a
> list of RelOptInfo, but postgres_fdw is passing it a list of bare
> clauses.  One of them is wrong :-)

It's a bit scary that apparently none of the committed regression tests
caught that.

More generally, I think the convention up to now has been that
clauselist_selectivity would work on either RestrictInfos or bare boolean
clauses, caching its results in the former case but succeeding anyway.
If we're to standardize on only one of those behaviors it should certainly
be the former, but I think postgres_fdw is probably not the only code that
will be broken if we remove the option for the latter.
        regards, tom lane



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
I wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>> Commit ac2b095088 assumes that clauselist_selectivity is being passed a
>> list of RelOptInfo, but postgres_fdw is passing it a list of bare
>> clauses.  One of them is wrong :-)

> It's a bit scary that apparently none of the committed regression tests
> caught that.

Experimentation shows that actually, the standard regression tests
provide dozens of opportunities for find_relation_from_clauses to fail on
non-RestrictInfo input.  However, it lacks any IsA check, and the only
thing that it does with the alleged rinfo is if (bms_get_singleton_member(rinfo->clause_relids, &relid))
As long as there's some kind of object pointer where the clause_relids
field would be, it's highly likely that bms_get_singleton_member will just
return false without crashing, thereby obscuring the fault.  Andreas'
example kills it by causing the argument to be a Param node, whose field
layout doesn't put a pointer there.

This makes me wonder whether we were being penny-wise and pound-foolish
by not making Bitmapsets be a kind of Node, so that there could be IsA
assertions in the bitmapset.c routines, as there are for Lists.  Most
Bitmapsets in a typical backend probably have only one payload word
(ie highest member < 32), so right now they occupy 8 bytes.  Adding
a nodetag would kick them up to the next palloc category, 16 bytes,
which is probably why I didn't do it like that to begin with.
Still, that decision is looking unduly byte-miserly in 2017.
        regards, tom lane



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Andrew Gierth
Date:
>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> Experimentation shows that actually, the standard regression testsTom> provide dozens of opportunities for
find_relation_from_clausestoTom> fail on non-RestrictInfo input.  However, it lacks any IsA check,
 

In a discussion with Andres on the hash grouping sets review thread, I
proposed that we should have something of the form

#define lfirst_node(_type_, l) (castNode(_type_,lfirst(l)))

to replace the current idiom of
   foreach(l, blah)   {       SomeType *x = (SomeType *) lfirst(l);

(in my code I tend to omit the (SomeType *), which I dislike because it
adds no real protection)

by
   foreach(l, blah)   {       SomeType *x = lfirst_node(SomeType, l);

in order to get that IsA check in there in a convenient way.

-- 
Andrew (irc:RhodiumToad)



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Robert Haas
Date:
On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> This makes me wonder whether we were being penny-wise and pound-foolish
> by not making Bitmapsets be a kind of Node, so that there could be IsA
> assertions in the bitmapset.c routines, as there are for Lists.  Most
> Bitmapsets in a typical backend probably have only one payload word
> (ie highest member < 32), so right now they occupy 8 bytes.  Adding
> a nodetag would kick them up to the next palloc category, 16 bytes,
> which is probably why I didn't do it like that to begin with.
> Still, that decision is looking unduly byte-miserly in 2017.

I think it's pretty dubious to change this, honestly.  Just because it
would have caught this one bug doesn't make it an especially valuable
thing in general.  Bytes are still not free.

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



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>  Tom> Experimentation shows that actually, the standard regression tests
>  Tom> provide dozens of opportunities for find_relation_from_clauses to
>  Tom> fail on non-RestrictInfo input.  However, it lacks any IsA check,

> In a discussion with Andres on the hash grouping sets review thread, I
> proposed that we should have something of the form

> #define lfirst_node(_type_, l) (castNode(_type_,lfirst(l)))

That seems like a fairly good idea.  A significant fraction of the
existing castNode() calls are being applied to lfirst(something),
and this would shorten that idiom a bit.

There's another noticeable fraction that are being applied to
linitial(something), but I'm not sure if defining linitial_node()
is worth the trouble.
        regards, tom lane



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> This makes me wonder whether we were being penny-wise and pound-foolish
>> by not making Bitmapsets be a kind of Node, so that there could be IsA
>> assertions in the bitmapset.c routines, as there are for Lists.

> I think it's pretty dubious to change this, honestly.  Just because it
> would have caught this one bug doesn't make it an especially valuable
> thing in general.  Bytes are still not free.

Yeah, true.  OTOH I recall Andres lobbying to change the bitmap word
size to 64 bits on 64-bit hardware, and it *would* be free in that case
due to alignment padding.

What I think I might do is write a trial patch that turns Bitmapsets
into Nodes, and see if it catches any other existing bugs.  If it does
not, that would be good evidence for your position.

We could also consider installing the nodetag only in Assert-enabled
builds, although that approach would prevent us from applying followon
simplifications such as not having to treat bitmapset fields specially
in copyfuncs.c and like places.
        regards, tom lane



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Andres Freund
Date:
On 2017-04-08 17:20:28 -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> This makes me wonder whether we were being penny-wise and pound-foolish
> >> by not making Bitmapsets be a kind of Node, so that there could be IsA
> >> assertions in the bitmapset.c routines, as there are for Lists.
> 
> > I think it's pretty dubious to change this, honestly.  Just because it
> > would have caught this one bug doesn't make it an especially valuable
> > thing in general.  Bytes are still not free.
> 
> Yeah, true.  OTOH I recall Andres lobbying to change the bitmap word
> size to 64 bits on 64-bit hardware, and it *would* be free in that case
> due to alignment padding.

Hah, yes, I did. A loong time ago ;) I still think it's a good idea, and
probably has become more useful with just about anyone using 64bits
these days.  Also interesting for tidbitmap, which reuses bitmapset's
bitmapword.

> We could also consider installing the nodetag only in Assert-enabled
> builds, although that approach would prevent us from applying followon
> simplifications such as not having to treat bitmapset fields specially
> in copyfuncs.c and like places.

Yea, don't like this much.


- Andres



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think it's pretty dubious to change this, honestly.  Just because it
>> would have caught this one bug doesn't make it an especially valuable
>> thing in general.  Bytes are still not free.

> What I think I might do is write a trial patch that turns Bitmapsets
> into Nodes, and see if it catches any other existing bugs.  If it does
> not, that would be good evidence for your position.

I made the attached quick-hack patch, and found that check-world
passes just fine with it.  That's not complete proof that we have
no other bugs of this ilk, but it definitely supports the idea
that we don't really need to add the overhead.  I'll just put this
in the archives for possible future reference.

(Or perhaps Andreas would like to try bashing on a copy with this
installed.)

            regards, tom lane

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index bf8545d..39d7b41 100644
*** a/src/backend/nodes/bitmapset.c
--- b/src/backend/nodes/bitmapset.c
*************** bms_copy(const Bitmapset *a)
*** 115,120 ****
--- 115,121 ----

      if (a == NULL)
          return NULL;
+     Assert(IsA(a, Bitmapset));
      size = BITMAPSET_SIZE(a->nwords);
      result = (Bitmapset *) palloc(size);
      memcpy(result, a, size);
*************** bms_equal(const Bitmapset *a, const Bitm
*** 145,150 ****
--- 146,153 ----
      }
      else if (b == NULL)
          return bms_is_empty(a);
+     Assert(IsA(a, Bitmapset));
+     Assert(IsA(b, Bitmapset));
      /* Identify shorter and longer input */
      if (a->nwords <= b->nwords)
      {
*************** bms_make_singleton(int x)
*** 187,192 ****
--- 190,196 ----
      wordnum = WORDNUM(x);
      bitnum = BITNUM(x);
      result = (Bitmapset *) palloc0(BITMAPSET_SIZE(wordnum + 1));
+     result->type = T_Bitmapset;
      result->nwords = wordnum + 1;
      result->words[wordnum] = ((bitmapword) 1 << bitnum);
      return result;
*************** void
*** 201,207 ****
--- 205,214 ----
  bms_free(Bitmapset *a)
  {
      if (a)
+     {
+         Assert(IsA(a, Bitmapset));
          pfree(a);
+     }
  }


*************** bms_union(const Bitmapset *a, const Bitm
*** 222,227 ****
--- 229,236 ----
      int            otherlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return bms_copy(b);
*************** bms_intersect(const Bitmapset *a, const
*** 256,261 ****
--- 265,272 ----
      int            resultlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL || b == NULL)
          return NULL;
*************** bms_difference(const Bitmapset *a, const
*** 287,292 ****
--- 298,305 ----
      int            shortlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return NULL;
*************** bms_is_subset(const Bitmapset *a, const
*** 311,316 ****
--- 324,331 ----
      int            longlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return true;            /* empty set is a subset of anything */
*************** bms_subset_compare(const Bitmapset *a, c
*** 349,354 ****
--- 364,371 ----
      int            longlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
      {
*************** bms_is_member(int x, const Bitmapset *a)
*** 427,432 ****
--- 444,450 ----
          elog(ERROR, "negative bitmapset member not allowed");
      if (a == NULL)
          return false;
+     Assert(IsA(a, Bitmapset));
      wordnum = WORDNUM(x);
      bitnum = BITNUM(x);
      if (wordnum >= a->nwords)
*************** bms_overlap(const Bitmapset *a, const Bi
*** 445,450 ****
--- 463,470 ----
      int            shortlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL || b == NULL)
          return false;
*************** bms_overlap_list(const Bitmapset *a, con
*** 468,473 ****
--- 488,494 ----
      int            wordnum,
                  bitnum;

+     Assert(a == NULL || IsA(a, Bitmapset));
      if (a == NULL || b == NIL)
          return false;

*************** bms_nonempty_difference(const Bitmapset
*** 496,501 ****
--- 517,524 ----
      int            shortlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return false;
*************** bms_singleton_member(const Bitmapset *a)
*** 531,536 ****
--- 554,560 ----

      if (a == NULL)
          elog(ERROR, "bitmapset is empty");
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      for (wordnum = 0; wordnum < nwords; wordnum++)
      {
*************** bms_get_singleton_member(const Bitmapset
*** 574,579 ****
--- 598,604 ----

      if (a == NULL)
          return false;
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      for (wordnum = 0; wordnum < nwords; wordnum++)
      {
*************** bms_num_members(const Bitmapset *a)
*** 610,615 ****
--- 635,641 ----

      if (a == NULL)
          return 0;
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      for (wordnum = 0; wordnum < nwords; wordnum++)
      {
*************** bms_membership(const Bitmapset *a)
*** 639,644 ****
--- 665,671 ----

      if (a == NULL)
          return BMS_EMPTY_SET;
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      for (wordnum = 0; wordnum < nwords; wordnum++)
      {
*************** bms_is_empty(const Bitmapset *a)
*** 667,672 ****
--- 694,700 ----

      if (a == NULL)
          return true;
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      for (wordnum = 0; wordnum < nwords; wordnum++)
      {
*************** bms_add_member(Bitmapset *a, int x)
*** 704,709 ****
--- 732,738 ----
          elog(ERROR, "negative bitmapset member not allowed");
      if (a == NULL)
          return bms_make_singleton(x);
+     Assert(IsA(a, Bitmapset));
      wordnum = WORDNUM(x);
      bitnum = BITNUM(x);

*************** bms_del_member(Bitmapset *a, int x)
*** 741,746 ****
--- 770,776 ----
          elog(ERROR, "negative bitmapset member not allowed");
      if (a == NULL)
          return NULL;
+     Assert(IsA(a, Bitmapset));
      wordnum = WORDNUM(x);
      bitnum = BITNUM(x);
      if (wordnum < a->nwords)
*************** bms_add_members(Bitmapset *a, const Bitm
*** 759,764 ****
--- 789,796 ----
      int            otherlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return bms_copy(b);
*************** bms_int_members(Bitmapset *a, const Bitm
*** 793,798 ****
--- 825,832 ----
      int            shortlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return NULL;
*************** bms_del_members(Bitmapset *a, const Bitm
*** 819,824 ****
--- 853,860 ----
      int            shortlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return NULL;
*************** bms_join(Bitmapset *a, Bitmapset *b)
*** 842,847 ****
--- 878,885 ----
      int            otherlen;
      int            i;

+     Assert(a == NULL || IsA(a, Bitmapset));
+     Assert(b == NULL || IsA(b, Bitmapset));
      /* Handle cases where either input is NULL */
      if (a == NULL)
          return b;
*************** bms_first_member(Bitmapset *a)
*** 889,894 ****
--- 927,933 ----

      if (a == NULL)
          return -1;
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      for (wordnum = 0; wordnum < nwords; wordnum++)
      {
*************** bms_next_member(const Bitmapset *a, int
*** 942,947 ****
--- 981,987 ----

      if (a == NULL)
          return -2;
+     Assert(IsA(a, Bitmapset));
      nwords = a->nwords;
      prevbit++;
      mask = (~(bitmapword) 0) << BITNUM(prevbit);
*************** bms_hash_value(const Bitmapset *a)
*** 987,992 ****
--- 1027,1033 ----

      if (a == NULL)
          return 0;                /* All empty sets hash to 0 */
+     Assert(IsA(a, Bitmapset));
      for (lastword = a->nwords; --lastword >= 0;)
      {
          if (a->words[lastword] != 0)
diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h
index 109f7b0..d672ee5 100644
*** a/src/include/nodes/bitmapset.h
--- b/src/include/nodes/bitmapset.h
***************
*** 20,25 ****
--- 20,27 ----
  #ifndef BITMAPSET_H
  #define BITMAPSET_H

+ #include "nodes/nodes.h"
+
  /*
   * Forward decl to save including pg_list.h
   */
*************** typedef int32 signedbitmapword; /* must
*** 36,41 ****
--- 38,44 ----

  typedef struct Bitmapset
  {
+     NodeTag        type;
      int            nwords;            /* number of words in array */
      bitmapword    words[FLEXIBLE_ARRAY_MEMBER];    /* really [nwords] */
  } Bitmapset;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index f59d719..46408c2 100644
*** a/src/include/nodes/nodes.h
--- b/src/include/nodes/nodes.h
*************** typedef enum NodeTag
*** 291,296 ****
--- 291,297 ----
      T_List,
      T_IntList,
      T_OidList,
+     T_Bitmapset,

      /*
       * TAGS FOR EXTENSIBLE NODES (extensible.h)

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Mark Dilger
Date:
> On Apr 8, 2017, at 5:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I think it's pretty dubious to change this, honestly.  Just because it
>>> would have caught this one bug doesn't make it an especially valuable
>>> thing in general.  Bytes are still not free.
>
>> What I think I might do is write a trial patch that turns Bitmapsets
>> into Nodes, and see if it catches any other existing bugs.  If it does
>> not, that would be good evidence for your position.
>
> I made the attached quick-hack patch, and found that check-world
> passes just fine with it.

Not so for me.  I get a failure almost immediately:

Running in no-clean mode.  Mistakes will not be cleaned up.
The files belonging to this database system will be owned by user "mark".
This user must also own the server process.

The database cluster will be initialized with locales COLLATE:  en_US.UTF-8 CTYPE:    en_US.UTF-8 MESSAGES: C MONETARY:
en_US.UTF-8NUMERIC:  en_US.UTF-8 TIME:     en_US.UTF-8 
The default database encoding has accordingly been set to "UTF8".
The default text search configuration will be set to "english".
Data page checksums are disabled.
creating directory /Users/mark/hydra/postgresql/src/test/regress/./tmp_check/data ... ok
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 128MB
selecting dynamic shared memory implementation ... posix
creating configuration files ... ok
running bootstrap script ... TRAP: FailedAssertion("!(((((const Node*)(a))->type) == T_Bitmapset))", File:
"bitmapset.c",Line: 731) 
child process was terminated by signal 6: Abort trap
initdb: data directory "/Users/mark/hydra/postgresql/src/test/regress/./tmp_check/data" not removed at user's request




Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Mark Dilger
Date:
> On Apr 8, 2017, at 6:38 PM, Mark Dilger <hornschnorter@gmail.com> wrote:
>
>
>> On Apr 8, 2017, at 5:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> I wrote:
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> I think it's pretty dubious to change this, honestly.  Just because it
>>>> would have caught this one bug doesn't make it an especially valuable
>>>> thing in general.  Bytes are still not free.
>>
>>> What I think I might do is write a trial patch that turns Bitmapsets
>>> into Nodes, and see if it catches any other existing bugs.  If it does
>>> not, that would be good evidence for your position.
>>
>> I made the attached quick-hack patch, and found that check-world
>> passes just fine with it.
>
> Not so for me.  I get a failure almost immediately:

I recant.  Looks like I didn't get the patch applied quite right.  So sorry for the noise.


Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Mark Dilger
Date:
> On Apr 8, 2017, at 6:48 PM, Mark Dilger <hornschnorter@gmail.com> wrote:
>
>
>> On Apr 8, 2017, at 6:38 PM, Mark Dilger <hornschnorter@gmail.com> wrote:
>>
>>
>>> On Apr 8, 2017, at 5:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>
>>> I wrote:
>>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>>> On Sat, Apr 8, 2017 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>> I think it's pretty dubious to change this, honestly.  Just because it
>>>>> would have caught this one bug doesn't make it an especially valuable
>>>>> thing in general.  Bytes are still not free.
>>>
>>>> What I think I might do is write a trial patch that turns Bitmapsets
>>>> into Nodes, and see if it catches any other existing bugs.  If it does
>>>> not, that would be good evidence for your position.
>>>
>>> I made the attached quick-hack patch, and found that check-world
>>> passes just fine with it.
>>
>> Not so for me.  I get a failure almost immediately:
>
> I recant.  Looks like I didn't get the patch applied quite right.  So sorry for the noise.

The regression tests now fail on a number of tests due to a server crash:

2017-04-08 18:55:19.826 PDT [90779] pg_regress/errors STATEMENT:  select infinite_recurse();
TRAP: FailedAssertion("!(((((const Node*)(a))->type) == T_Bitmapset))", File: "bitmapset.c", Line: 601)
2017-04-08 18:55:22.487 PDT [90242] LOG:  server process (PID 90785) was terminated by signal 6: Abort trap
2017-04-08 18:55:22.487 PDT [90242] DETAIL:  Failed process was running: explain (costs off)   select * from onek2
whereunique2 = 11 and stringu1 = 'ATAAAA'; 


This is very near where the original crash reported in this thread was crashing, probably only
different due to the extra lines of Assert that were added.  Am I missing some portion of the
fix that you are testing?  I have only applied the patch that Tom included in the previous email.

mark


Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
Mark Dilger <hornschnorter@gmail.com> writes:
> This is very near where the original crash reported in this thread was crashing, probably only
> different due to the extra lines of Assert that were added.  Am I missing some portion of the
> fix that you are testing?  I have only applied the patch that Tom included in the previous email.

No, that was the whole patch --- but did you do a full "make clean" and
rebuild?  The patch changed NodeTag numbers which would affect the whole
tree.
        regards, tom lane



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Mark Dilger
Date:
> On Apr 8, 2017, at 7:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Mark Dilger <hornschnorter@gmail.com> writes:
>> This is very near where the original crash reported in this thread was crashing, probably only
>> different due to the extra lines of Assert that were added.  Am I missing some portion of the
>> fix that you are testing?  I have only applied the patch that Tom included in the previous email.
>
> No, that was the whole patch --- but did you do a full "make clean" and
> rebuild?  The patch changed NodeTag numbers which would affect the whole
> tree.

I had trouble applying your patch using

 mark$ patch -p 1 < patch
patching file src/backend/nodes/bitmapset.c
Hunk #1 FAILED at 115.
Hunk #2 FAILED at 146.
Hunk #3 FAILED at 190.
Hunk #4 FAILED at 205.
Hunk #5 FAILED at 229.
Hunk #6 FAILED at 265.
Hunk #7 FAILED at 298.
Hunk #8 FAILED at 324.
Hunk #9 FAILED at 364.
Hunk #10 FAILED at 444.
Hunk #11 FAILED at 463.
Hunk #12 FAILED at 488.
Hunk #13 FAILED at 517.
Hunk #14 FAILED at 554.
Hunk #15 FAILED at 598.
Hunk #16 FAILED at 635.
Hunk #17 FAILED at 665.
Hunk #18 FAILED at 694.
Hunk #19 FAILED at 732.
Hunk #20 FAILED at 770.
Hunk #21 FAILED at 789.
Hunk #22 FAILED at 825.
Hunk #23 FAILED at 853.
Hunk #24 FAILED at 878.
Hunk #25 FAILED at 927.
Hunk #26 FAILED at 981.
Hunk #27 FAILED at 1027.
27 out of 27 hunks FAILED -- saving rejects to file src/backend/nodes/bitmapset.c.rej
patching file src/include/nodes/bitmapset.h
Hunk #1 FAILED at 20.
Hunk #2 FAILED at 38.
2 out of 2 hunks FAILED -- saving rejects to file src/include/nodes/bitmapset.h.rej
patching file src/include/nodes/nodes.h
Hunk #1 FAILED at 291.
1 out of 1 hunk FAILED -- saving rejects to file src/include/nodes/nodes.h.rej


So I did the patching manually against my copy of the current master,
aba696d1af9a267eee85d69845c3cdeccf788525, and then ran:

make distclean; ./configure --enable-cassert --enable-tap-tests --enable-depend && make -j4 && make check-world

I am attaching my patch, which I admit on closer inspection differs from yours, but not
in any way I can tell is relevant:



I'm going to pull completely fresh sources and reapply and retest, though you are welcome
to review my patch while I do that.

mark
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Mark Dilger
Date:
> On Apr 8, 2017, at 7:41 PM, Mark Dilger <hornschnorter@gmail.com> wrote:
>
>
>> On Apr 8, 2017, at 7:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Mark Dilger <hornschnorter@gmail.com> writes:
>>> This is very near where the original crash reported in this thread was crashing, probably only
>>> different due to the extra lines of Assert that were added.  Am I missing some portion of the
>>> fix that you are testing?  I have only applied the patch that Tom included in the previous email.
>>
>> No, that was the whole patch --- but did you do a full "make clean" and
>> rebuild?  The patch changed NodeTag numbers which would affect the whole
>> tree.
>
> <snip>

> I'm going to pull completely fresh sources and reapply and retest, though you are welcome
> to review my patch while I do that.

That fixed it.  Thanks.




Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Andreas Seltenreich
Date:
Tom Lane writes:

> I made the attached quick-hack patch, and found that check-world
> passes just fine with it.  That's not complete proof that we have
> no other bugs of this ilk, but it definitely supports the idea
> that we don't really need to add the overhead.  I'll just put this
> in the archives for possible future reference.
>
> (Or perhaps Andreas would like to try bashing on a copy with this
> installed.)

I certainly do :-).  SQLsmith has been fuzzing for couple hours with the
patch applied, and so far none of the assertions fired.  I'll leave the
patch on my fuzzing branch until merging becomes burdensome.

regards,
Andreas



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Thomas Munro
Date:
On Sun, Apr 9, 2017 at 8:27 AM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
>     foreach(l, blah)
>     {
>         SomeType *x = (SomeType *) lfirst(l);
>
> (in my code I tend to omit the (SomeType *), which I dislike because it
> adds no real protection)

Just BTW, without that cast it's not compilable as C++, so I'm
guessing that Peter E will finish up putting it back in wherever you
leave it out...

-- 
Thomas Munro
http://www.enterprisedb.com



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Andrew Gierth
Date:
>>>>> "Thomas" == Thomas Munro <thomas.munro@enterprisedb.com> writes:
>> SomeType *x = (SomeType *) lfirst(l);>> >> (in my code I tend to omit the (SomeType *), which I dislike because>> it
addsno real protection)
 
Thomas> Just BTW, without that cast it's not compilable as C++, so I'mThomas> guessing that Peter E will finish up
puttingit back inThomas> wherever you leave it out...
 

There's north of 150 other examples (just grep for '= lfirst' in the
source). Some were even committed by Peter E :-)

In the discussion with Andres the same point came up for palloc, for
which I suggested we add something along the lines of:

#define palloc_object(_type_) (_type_ *) palloc(sizeof(_type_))
#define palloc_array(_type_, n) (_type_ *) palloc((n) * sizeof(_type_))

palloc() without a cast is even more common than lfirst() without one,
and something like half of those (and 80%+ of the pallocs that do have a
cast) are palloc(sizeof(...)) or palloc(something * sizeof(...)).

-- 
Andrew (irc:RhodiumToad)



Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

From
Tom Lane
Date:
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> In the discussion with Andres the same point came up for palloc, for
> which I suggested we add something along the lines of:

> #define palloc_object(_type_) (_type_ *) palloc(sizeof(_type_))
> #define palloc_array(_type_, n) (_type_ *) palloc((n) * sizeof(_type_))

I'm far less excited about that, mainly because you'd have to also cover
palloc0, repalloc, MemoryContextAlloc, etc etc.  Also I've not seen
very many actual bugs that this would've helped with.
        regards, tom lane



I wrote:
> Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
>> In a discussion with Andres on the hash grouping sets review thread, I
>> proposed that we should have something of the form

>> #define lfirst_node(_type_, l) (castNode(_type_,lfirst(l)))

> That seems like a fairly good idea.  A significant fraction of the
> existing castNode() calls are being applied to lfirst(something),
> and this would shorten that idiom a bit.

PFA a patch to do this.  It turns out that just under half of the
castNode() calls in the current tree have List-cell-extraction
functions as arguments, and so can be replaced like this.  So I think
this is a great idea and we should do it; it's a definite notational
improvement.

As with the original addition of castNode, it seems like a good idea
to back-patch the additions to pg_list.h, so that we won't have
back-patching problems for new code using this feature.

> There's another noticeable fraction that are being applied to
> linitial(something), but I'm not sure if defining linitial_node()
> is worth the trouble.

It is, and in fact I ended up providing equivalents for all the
List-cell-extraction functions.  All except lfourth_node() are
actually in use in this patch.

Barring objections, I'll push this shortly.

            regards, tom lane

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 6b7503d..bf03e67 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -2393,7 +2393,7 @@ JumbleRangeTable(pgssJumbleState *jstate, List *rtable)

     foreach(lc, rtable)
     {
-        RangeTblEntry *rte = castNode(RangeTblEntry, lfirst(lc));
+        RangeTblEntry *rte = lfirst_node(RangeTblEntry, lc);

         APP_JUMB(rte->rtekind);
         switch (rte->rtekind)
@@ -2656,7 +2656,7 @@ JumbleExpr(pgssJumbleState *jstate, Node *node)
                 JumbleExpr(jstate, (Node *) caseexpr->arg);
                 foreach(temp, caseexpr->args)
                 {
-                    CaseWhen   *when = castNode(CaseWhen, lfirst(temp));
+                    CaseWhen   *when = lfirst_node(CaseWhen, temp);

                     JumbleExpr(jstate, (Node *) when->expr);
                     JumbleExpr(jstate, (Node *) when->result);
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 129bdb5..c5149a0 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1350,7 +1350,7 @@ deparseExplicitTargetList(List *tlist, List **retrieved_attrs,

     foreach(lc, tlist)
     {
-        TargetEntry *tle = castNode(TargetEntry, lfirst(lc));
+        TargetEntry *tle = lfirst_node(TargetEntry, lc);

         if (i > 0)
             appendStringInfoString(buf, ", ");
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 2851869..6670df5 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -1169,7 +1169,7 @@ postgresGetForeignPlan(PlannerInfo *root,
      */
     foreach(lc, scan_clauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);

         /* Ignore any pseudoconstants, they're dealt with elsewhere */
         if (rinfo->pseudoconstant)
@@ -5022,8 +5022,8 @@ conversion_error_callback(void *arg)
         EState       *estate = fsstate->ss.ps.state;
         TargetEntry *tle;

-        tle = castNode(TargetEntry, list_nth(fsplan->fdw_scan_tlist,
-                                             errpos->cur_attno - 1));
+        tle = list_nth_node(TargetEntry, fsplan->fdw_scan_tlist,
+                            errpos->cur_attno - 1);

         /*
          * Target list can have Vars and expressions.  For Vars, we can get
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 1eb7930..1492722 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -854,7 +854,7 @@ get_object_address(ObjectType objtype, Node *object,

                     objlist = castNode(List, object);
                     domaddr = get_object_address_type(OBJECT_DOMAIN,
-                                                      castNode(TypeName, linitial(objlist)),
+                                                      linitial_node(TypeName, objlist),
                                                       missing_ok);
                     constrname = strVal(lsecond(objlist));

@@ -932,8 +932,8 @@ get_object_address(ObjectType objtype, Node *object,
                 break;
             case OBJECT_CAST:
                 {
-                    TypeName   *sourcetype = castNode(TypeName, linitial(castNode(List, object)));
-                    TypeName   *targettype = castNode(TypeName, lsecond(castNode(List, object)));
+                    TypeName   *sourcetype = linitial_node(TypeName, castNode(List, object));
+                    TypeName   *targettype = lsecond_node(TypeName, castNode(List, object));
                     Oid            sourcetypeid;
                     Oid            targettypeid;

@@ -947,7 +947,7 @@ get_object_address(ObjectType objtype, Node *object,
                 break;
             case OBJECT_TRANSFORM:
                 {
-                    TypeName   *typename = castNode(TypeName, linitial(castNode(List, object)));
+                    TypeName   *typename = linitial_node(TypeName, castNode(List, object));
                     char       *langname = strVal(lsecond(castNode(List, object)));
                     Oid            type_id = LookupTypeNameOid(NULL, typename, missing_ok);
                     Oid            lang_id = get_language_oid(langname, missing_ok);
@@ -1597,7 +1597,7 @@ get_object_address_opf_member(ObjectType objtype,
     {
         ObjectAddress typaddr;

-        typenames[i] = castNode(TypeName, lfirst(cell));
+        typenames[i] = lfirst_node(TypeName, cell);
         typaddr = get_object_address_type(OBJECT_TYPE, typenames[i], missing_ok);
         typeoids[i] = typaddr.objectId;
         if (++i >= 2)
@@ -2319,8 +2319,8 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
         case OBJECT_CAST:
             {
                 /* We can only check permissions on the source/target types */
-                TypeName   *sourcetype = castNode(TypeName, linitial(castNode(List, object)));
-                TypeName   *targettype = castNode(TypeName, lsecond(castNode(List, object)));
+                TypeName   *sourcetype = linitial_node(TypeName, castNode(List, object));
+                TypeName   *targettype = lsecond_node(TypeName, castNode(List, object));
                 Oid            sourcetypeid = typenameTypeId(NULL, sourcetype);
                 Oid            targettypeid = typenameTypeId(NULL, targettype);

@@ -2345,7 +2345,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address,
             break;
         case OBJECT_TRANSFORM:
             {
-                TypeName   *typename = castNode(TypeName, linitial(castNode(List, object)));
+                TypeName   *typename = linitial_node(TypeName, castNode(List, object));
                 Oid            typeid = typenameTypeId(NULL, typename);

                 if (!pg_type_ownercheck(typeid, roleid))
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index f058d12..eaeabf1 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -928,7 +928,7 @@ fmgr_sql_validator(PG_FUNCTION_ARGS)
             querytree_list = NIL;
             foreach(lc, raw_parsetree_list)
             {
-                RawStmt    *parsetree = castNode(RawStmt, lfirst(lc));
+                RawStmt    *parsetree = lfirst_node(RawStmt, lc);
                 List       *querytree_sublist;

                 querytree_sublist = pg_analyze_and_rewrite_params(parsetree,
diff --git a/src/backend/commands/aggregatecmds.c b/src/backend/commands/aggregatecmds.c
index 2341129..a84c614 100644
--- a/src/backend/commands/aggregatecmds.c
+++ b/src/backend/commands/aggregatecmds.c
@@ -109,13 +109,13 @@ DefineAggregate(ParseState *pstate, List *name, List *args, bool oldstyle, List
             aggKind = AGGKIND_ORDERED_SET;
         else
             numDirectArgs = 0;
-        args = castNode(List, linitial(args));
+        args = linitial_node(List, args);
     }

     /* Examine aggregate's definition clauses */
     foreach(pl, parameters)
     {
-        DefElem    *defel = castNode(DefElem, lfirst(pl));
+        DefElem    *defel = lfirst_node(DefElem, pl);

         /*
          * sfunc1, stype1, and initcond1 are accepted as obsolete spellings
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index e32d7a1..87b215d 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -1636,7 +1636,7 @@ AtSubCommit_Notify(void)
     List       *parentPendingActions;
     List       *parentPendingNotifies;

-    parentPendingActions = castNode(List, linitial(upperPendingActions));
+    parentPendingActions = linitial_node(List, upperPendingActions);
     upperPendingActions = list_delete_first(upperPendingActions);

     Assert(list_length(upperPendingActions) ==
@@ -1647,7 +1647,7 @@ AtSubCommit_Notify(void)
      */
     pendingActions = list_concat(parentPendingActions, pendingActions);

-    parentPendingNotifies = castNode(List, linitial(upperPendingNotifies));
+    parentPendingNotifies = linitial_node(List, upperPendingNotifies);
     upperPendingNotifies = list_delete_first(upperPendingNotifies);

     Assert(list_length(upperPendingNotifies) ==
@@ -1679,13 +1679,13 @@ AtSubAbort_Notify(void)
      */
     while (list_length(upperPendingActions) > my_level - 2)
     {
-        pendingActions = castNode(List, linitial(upperPendingActions));
+        pendingActions = linitial_node(List, upperPendingActions);
         upperPendingActions = list_delete_first(upperPendingActions);
     }

     while (list_length(upperPendingNotifies) > my_level - 2)
     {
-        pendingNotifies = castNode(List, linitial(upperPendingNotifies));
+        pendingNotifies = linitial_node(List, upperPendingNotifies);
         upperPendingNotifies = list_delete_first(upperPendingNotifies);
     }
 }
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index 835cb26..9264d7f 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -71,7 +71,7 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e

     foreach(pl, parameters)
     {
-        DefElem    *defel = castNode(DefElem, lfirst(pl));
+        DefElem    *defel = lfirst_node(DefElem, pl);
         DefElem   **defelp;

         if (pg_strcasecmp(defel->defname, "from") == 0)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 8c58808..561a2fc 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -1034,7 +1034,7 @@ ProcessCopyOptions(ParseState *pstate,
     /* Extract options from the statement node tree */
     foreach(option, options)
     {
-        DefElem    *defel = castNode(DefElem, lfirst(option));
+        DefElem    *defel = lfirst_node(DefElem, option);

         if (strcmp(defel->defname, "format") == 0)
         {
@@ -1488,7 +1488,7 @@ BeginCopy(ParseState *pstate,
             /* examine queries to determine which error message to issue */
             foreach(lc, rewritten)
             {
-                Query       *q = castNode(Query, lfirst(lc));
+                Query       *q = lfirst_node(Query, lc);

                 if (q->querySource == QSRC_QUAL_INSTEAD_RULE)
                     ereport(ERROR,
@@ -1505,7 +1505,7 @@ BeginCopy(ParseState *pstate,
                      errmsg("multi-statement DO INSTEAD rules are not supported for COPY")));
         }

-        query = castNode(Query, linitial(rewritten));
+        query = linitial_node(Query, rewritten);

         /* The grammar allows SELECT INTO, but we don't support that */
         if (query->utilityStmt != NULL &&
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index f49b391..06425cc 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -323,7 +323,7 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
             elog(ERROR, "unexpected rewrite result for %s",
                  is_matview ? "CREATE MATERIALIZED VIEW" :
                  "CREATE TABLE AS SELECT");
-        query = castNode(Query, linitial(rewritten));
+        query = linitial_node(Query, rewritten);
         Assert(query->commandType == CMD_SELECT);

         /* plan the query --- note we disallow parallelism */
diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c
index cb948f0..c18bc31 100644
--- a/src/backend/commands/dropcmds.c
+++ b/src/backend/commands/dropcmds.c
@@ -214,7 +214,7 @@ type_in_list_does_not_exist_skipping(List *typenames, const char **msg,

     foreach(l, typenames)
     {
-        TypeName   *typeName = castNode(TypeName, lfirst(l));
+        TypeName   *typeName = lfirst_node(TypeName, l);

         if (typeName != NULL)
         {
@@ -371,8 +371,8 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
                 {
                     /* XXX quote or no quote? */
                     msg = gettext_noop("cast from type %s to type %s does not exist, skipping");
-                    name = TypeNameToString(castNode(TypeName, linitial(castNode(List, object))));
-                    args = TypeNameToString(castNode(TypeName, lsecond(castNode(List, object))));
+                    name = TypeNameToString(linitial_node(TypeName, castNode(List, object)));
+                    args = TypeNameToString(lsecond_node(TypeName, castNode(List, object)));
                 }
             }
             break;
@@ -380,7 +380,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object)
             if (!type_in_list_does_not_exist_skipping(list_make1(linitial(castNode(List, object))), &msg, &name))
             {
                 msg = gettext_noop("transform for type %s language \"%s\" does not exist, skipping");
-                name = TypeNameToString(castNode(TypeName, linitial(castNode(List, object))));
+                name = TypeNameToString(linitial_node(TypeName, castNode(List, object)));
                 args = strVal(lsecond(castNode(List, object)));
             }
             break;
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e549f9d..9359d0a 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -253,7 +253,7 @@ ExplainQuery(ParseState *pstate, ExplainStmt *stmt, const char *queryString,
         /* Explain every plan */
         foreach(l, rewritten)
         {
-            ExplainOneQuery(castNode(Query, lfirst(l)),
+            ExplainOneQuery(lfirst_node(Query, l),
                             CURSOR_OPT_PARALLEL_OK, NULL, es,
                             queryString, params, queryEnv);

@@ -408,7 +408,7 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,

         rewritten = QueryRewrite(castNode(Query, copyObject(ctas->query)));
         Assert(list_length(rewritten) == 1);
-        ExplainOneQuery(castNode(Query, linitial(rewritten)),
+        ExplainOneQuery(linitial_node(Query, rewritten),
                         0, ctas->into, es,
                         queryString, params, queryEnv);
     }
@@ -427,7 +427,7 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es,

         rewritten = QueryRewrite(castNode(Query, copyObject(dcs->query)));
         Assert(list_length(rewritten) == 1);
-        ExplainOneQuery(castNode(Query, linitial(rewritten)),
+        ExplainOneQuery(linitial_node(Query, rewritten),
                         dcs->options, NULL, es,
                         queryString, params, queryEnv);
     }
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 6be9bc4..d371a2a 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -714,7 +714,7 @@ execute_sql_string(const char *sql, const char *filename)
      */
     foreach(lc1, raw_parsetree_list)
     {
-        RawStmt    *parsetree = castNode(RawStmt, lfirst(lc1));
+        RawStmt    *parsetree = lfirst_node(RawStmt, lc1);
         List       *stmt_list;
         ListCell   *lc2;

@@ -727,7 +727,7 @@ execute_sql_string(const char *sql, const char *filename)

         foreach(lc2, stmt_list)
         {
-            PlannedStmt *stmt = castNode(PlannedStmt, lfirst(lc2));
+            PlannedStmt *stmt = lfirst_node(PlannedStmt, lc2);

             CommandCounterIncrement();

diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index 4ffe1bc..96cf296 100644
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -1589,7 +1589,7 @@ ImportForeignSchema(ImportForeignSchemaStmt *stmt)
          */
         foreach(lc2, raw_parsetree_list)
         {
-            RawStmt    *rs = castNode(RawStmt, lfirst(lc2));
+            RawStmt    *rs = lfirst_node(RawStmt, lc2);
             CreateForeignTableStmt *cstmt = (CreateForeignTableStmt *) rs->stmt;
             PlannedStmt *pstmt;

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index b9aedb2..ffcae34 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -578,7 +578,7 @@ update_proconfig_value(ArrayType *a, List *set_items)

     foreach(l, set_items)
     {
-        VariableSetStmt *sstmt = castNode(VariableSetStmt, lfirst(l));
+        VariableSetStmt *sstmt = lfirst_node(VariableSetStmt, l);

         if (sstmt->kind == VAR_RESET_ALL)
             a = NULL;
@@ -972,7 +972,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)

         foreach(lc, castNode(List, transformDefElem))
         {
-            Oid            typeid = typenameTypeId(NULL, lfirst(lc));
+            Oid            typeid = typenameTypeId(NULL,
+                                                lfirst_node(TypeName, lc));
             Oid            elt = get_base_element_type(typeid);

             typeid = elt ? elt : typeid;
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 2f93328..9ffd91e 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -264,7 +264,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
      * The stored query was rewritten at the time of the MV definition, but
      * has not been scribbled on by the planner.
      */
-    dataQuery = castNode(Query, linitial(actions));
+    dataQuery = linitial_node(Query, actions);

     /*
      * Check for active uses of the relation in the current transaction, such
diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index 822b331..ab51d1a 100644
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -460,7 +460,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
      */
     foreach(l, stmt->items)
     {
-        CreateOpClassItem *item = castNode(CreateOpClassItem, lfirst(l));
+        CreateOpClassItem *item = lfirst_node(CreateOpClassItem, l);
         Oid            operOid;
         Oid            funcOid;
         Oid            sortfamilyOid;
@@ -834,7 +834,7 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
      */
     foreach(l, items)
     {
-        CreateOpClassItem *item = castNode(CreateOpClassItem, lfirst(l));
+        CreateOpClassItem *item = lfirst_node(CreateOpClassItem, l);
         Oid            operOid;
         Oid            funcOid;
         Oid            sortfamilyOid;
@@ -959,7 +959,7 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
      */
     foreach(l, items)
     {
-        CreateOpClassItem *item = castNode(CreateOpClassItem, lfirst(l));
+        CreateOpClassItem *item = lfirst_node(CreateOpClassItem, l);
         Oid            lefttype,
                     righttype;
         OpFamilyMember *member;
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index f57cf87..167910f 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -83,7 +83,7 @@ PerformCursorOpen(DeclareCursorStmt *cstmt, ParamListInfo params,
     if (list_length(rewritten) != 1)
         elog(ERROR, "non-SELECT statement in DECLARE CURSOR");

-    query = castNode(Query, linitial(rewritten));
+    query = linitial_node(Query, rewritten);

     if (query->commandType != CMD_SELECT)
         elog(ERROR, "non-SELECT statement in DECLARE CURSOR");
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 00cc513..d265c77 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -267,7 +267,7 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
             ereport(ERROR,
                     (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                      errmsg("prepared statement is not a SELECT")));
-        pstmt = castNode(PlannedStmt, linitial(plan_list));
+        pstmt = linitial_node(PlannedStmt, plan_list);
         if (pstmt->commandType != CMD_SELECT)
             ereport(ERROR,
                     (errcode(ERRCODE_WRONG_OBJECT_TYPE),
@@ -679,7 +679,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
     /* Explain each query */
     foreach(p, plan_list)
     {
-        PlannedStmt *pstmt = castNode(PlannedStmt, lfirst(p));
+        PlannedStmt *pstmt = lfirst_node(PlannedStmt, p);

         if (pstmt->commandType != CMD_UTILITY)
             ExplainOnePlan(pstmt, into, es, query_string, paramLI, queryEnv,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 60f8b7f..abb262b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -5942,7 +5942,7 @@ ATExecSetIdentity(Relation rel, const char *colName, Node *def, LOCKMODE lockmod

     foreach(option, castNode(List, def))
     {
-        DefElem       *defel = castNode(DefElem, lfirst(option));
+        DefElem       *defel = lfirst_node(DefElem, option);

         if (strcmp(defel->defname, "generated") == 0)
         {
@@ -9547,7 +9547,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid refRelId, char *cmd,
     querytree_list = NIL;
     foreach(list_item, raw_parsetree_list)
     {
-        RawStmt    *rs = castNode(RawStmt, lfirst(list_item));
+        RawStmt    *rs = lfirst_node(RawStmt, list_item);
         Node       *stmt = rs->stmt;

         if (IsA(stmt, IndexStmt))
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index ebf23a0..d05e51c 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -340,7 +340,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,

         foreach(lc, varList)
         {
-            TriggerTransition   *tt = castNode(TriggerTransition, lfirst(lc));
+            TriggerTransition   *tt = lfirst_node(TriggerTransition, lc);

             if (!(tt->isTable))
                 ereport(ERROR,
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 14b9779..de26497 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -1388,7 +1388,7 @@ roleSpecsToIds(List *memberNames)

     foreach(l, memberNames)
     {
-        RoleSpec   *rolespec = castNode(RoleSpec, lfirst(l));
+        RoleSpec   *rolespec = lfirst_node(RoleSpec, l);
         Oid            roleid;

         roleid = get_rolespec_oid(rolespec, false);
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 6909a67..996acae 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -516,7 +516,7 @@ DefineView(ViewStmt *stmt, const char *queryString,

         foreach(targetList, viewParse->targetList)
         {
-            TargetEntry *te = castNode(TargetEntry, lfirst(targetList));
+            TargetEntry *te = lfirst_node(TargetEntry, targetList);

             /* junk columns don't get aliases */
             if (te->resjunk)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 97ec8fb..15d693f 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -322,7 +322,7 @@ ExecBuildProjectionInfo(List *targetList,
     /* Now compile each tlist column */
     foreach(lc, targetList)
     {
-        TargetEntry *tle = castNode(TargetEntry, lfirst(lc));
+        TargetEntry *tle = lfirst_node(TargetEntry, lc);
         Var           *variable = NULL;
         AttrNumber    attnum = 0;
         bool        isSafeVar = false;
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index f002ee2..c4a9553 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -160,7 +160,7 @@ ExecResetTupleTable(List *tupleTable,    /* tuple table */

     foreach(lc, tupleTable)
     {
-        TupleTableSlot *slot = castNode(TupleTableSlot, lfirst(lc));
+        TupleTableSlot *slot = lfirst_node(TupleTableSlot, lc);

         /* Always release resources and reset the slot to empty */
         ExecClearTuple(slot);
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index ce7b064..df3d650 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -978,7 +978,7 @@ ExecCleanTargetListLength(List *targetlist)

     foreach(tl, targetlist)
     {
-        TargetEntry *curTle = castNode(TargetEntry, lfirst(tl));
+        TargetEntry *curTle = lfirst_node(TargetEntry, tl);

         if (!curTle->resjunk)
             len++;
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 3cadf95..a35ba32 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -479,14 +479,14 @@ init_execution_state(List *queryTree_list,

     foreach(lc1, queryTree_list)
     {
-        List       *qtlist = castNode(List, lfirst(lc1));
+        List       *qtlist = lfirst_node(List, lc1);
         execution_state *firstes = NULL;
         execution_state *preves = NULL;
         ListCell   *lc2;

         foreach(lc2, qtlist)
         {
-            Query       *queryTree = castNode(Query, lfirst(lc2));
+            Query       *queryTree = lfirst_node(Query, lc2);
             PlannedStmt *stmt;
             execution_state *newes;

@@ -707,7 +707,7 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
     flat_query_list = NIL;
     foreach(lc, raw_parsetree_list)
     {
-        RawStmt    *parsetree = castNode(RawStmt, lfirst(lc));
+        RawStmt    *parsetree = lfirst_node(RawStmt, lc);
         List       *queryTree_sublist;

         queryTree_sublist = pg_analyze_and_rewrite_params(parsetree,
@@ -1555,7 +1555,7 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
     parse = NULL;
     foreach(lc, queryTreeList)
     {
-        Query       *q = castNode(Query, lfirst(lc));
+        Query       *q = lfirst_node(Query, lc);

         if (q->canSetTag)
             parse = q;
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 0109aee..c2b8618 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -2866,7 +2866,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)

         if (phaseidx > 0)
         {
-            aggnode = castNode(Agg, list_nth(node->chain, phaseidx - 1));
+            aggnode = list_nth_node(Agg, node->chain, phaseidx - 1);
             sortnode = castNode(Sort, aggnode->plan.lefttree);
         }
         else
@@ -3360,7 +3360,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
          */
         foreach(arg, pertrans->aggref->args)
         {
-            TargetEntry *source_tle = castNode(TargetEntry, lfirst(arg));
+            TargetEntry *source_tle = lfirst_node(TargetEntry, arg);
             TargetEntry *tle;

             tle = flatCopyTargetEntry(source_tle);
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 40419c8..f9ab0d6 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -517,7 +517,7 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags)
     hoperators = NIL;
     foreach(l, node->hashclauses)
     {
-        OpExpr       *hclause = castNode(OpExpr, lfirst(l));
+        OpExpr       *hclause = lfirst_node(OpExpr, l);

         lclauses = lappend(lclauses, ExecInitExpr(linitial(hclause->args),
                                                   (PlanState *) hjstate));
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index b098034..5630eae 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -401,7 +401,7 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
     epq_arowmarks = NIL;
     foreach(lc, node->rowMarks)
     {
-        PlanRowMark *rc = castNode(PlanRowMark, lfirst(lc));
+        PlanRowMark *rc = lfirst_node(PlanRowMark, lc);
         ExecRowMark *erm;
         ExecAuxRowMark *aerm;

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 0b524e0..1f4ee9e 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1969,7 +1969,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
      */
     foreach(l, node->rowMarks)
     {
-        PlanRowMark *rc = castNode(PlanRowMark, lfirst(l));
+        PlanRowMark *rc = lfirst_node(PlanRowMark, l);
         ExecRowMark *erm;

         /* ignore "parent" rowmarks; they are irrelevant at runtime */
diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c
index b3a0258..e8fa4c8 100644
--- a/src/backend/executor/nodeSubplan.c
+++ b/src/backend/executor/nodeSubplan.c
@@ -817,7 +817,7 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
         i = 1;
         foreach(l, oplist)
         {
-            OpExpr       *opexpr = castNode(OpExpr, lfirst(l));
+            OpExpr       *opexpr = lfirst_node(OpExpr, l);
             Expr       *expr;
             TargetEntry *tle;
             Oid            rhs_eq_oper;
@@ -1148,7 +1148,7 @@ ExecInitAlternativeSubPlan(AlternativeSubPlan *asplan, PlanState *parent)
      */
     foreach(lc, asplan->subplans)
     {
-        SubPlan    *sp = castNode(SubPlan, lfirst(lc));
+        SubPlan    *sp = lfirst_node(SubPlan, lc);
         SubPlanState *sps = ExecInitSubPlan(sp, parent);

         asstate->subplans = lappend(asstate->subplans, sps);
@@ -1197,8 +1197,8 @@ ExecAlternativeSubPlan(AlternativeSubPlanState *node,
                        bool *isNull)
 {
     /* Just pass control to the active subplan */
-    SubPlanState *activesp = castNode(SubPlanState,
-                                      list_nth(node->subplans, node->active));
+    SubPlanState *activesp = list_nth_node(SubPlanState,
+                                           node->subplans, node->active);

     return ExecSubPlan(activesp, econtext, isNull);
 }
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index ca547dc..a4a6e5b 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1233,9 +1233,9 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
     if (!(portal->cursorOptions & (CURSOR_OPT_SCROLL | CURSOR_OPT_NO_SCROLL)))
     {
         if (list_length(stmt_list) == 1 &&
-            castNode(PlannedStmt, linitial(stmt_list))->commandType != CMD_UTILITY &&
-            castNode(PlannedStmt, linitial(stmt_list))->rowMarks == NIL &&
-            ExecSupportsBackwardScan(castNode(PlannedStmt, linitial(stmt_list))->planTree))
+            linitial_node(PlannedStmt, stmt_list)->commandType != CMD_UTILITY &&
+            linitial_node(PlannedStmt, stmt_list)->rowMarks == NIL &&
+            ExecSupportsBackwardScan(linitial_node(PlannedStmt, stmt_list)->planTree))
             portal->cursorOptions |= CURSOR_OPT_SCROLL;
         else
             portal->cursorOptions |= CURSOR_OPT_NO_SCROLL;
@@ -1249,8 +1249,8 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
     if (portal->cursorOptions & CURSOR_OPT_SCROLL)
     {
         if (list_length(stmt_list) == 1 &&
-            castNode(PlannedStmt, linitial(stmt_list))->commandType != CMD_UTILITY &&
-            castNode(PlannedStmt, linitial(stmt_list))->rowMarks != NIL)
+            linitial_node(PlannedStmt, stmt_list)->commandType != CMD_UTILITY &&
+            linitial_node(PlannedStmt, stmt_list)->rowMarks != NIL)
             ereport(ERROR,
                     (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                      errmsg("DECLARE SCROLL CURSOR ... FOR UPDATE/SHARE is not supported"),
@@ -1274,7 +1274,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,

         foreach(lc, stmt_list)
         {
-            PlannedStmt *pstmt = castNode(PlannedStmt, lfirst(lc));
+            PlannedStmt *pstmt = lfirst_node(PlannedStmt, lc);

             if (!CommandIsReadOnly(pstmt))
             {
@@ -1770,7 +1770,7 @@ _SPI_prepare_plan(const char *src, SPIPlanPtr plan)

     foreach(list_item, raw_parsetree_list)
     {
-        RawStmt    *parsetree = castNode(RawStmt, lfirst(list_item));
+        RawStmt    *parsetree = lfirst_node(RawStmt, list_item);
         List       *stmt_list;
         CachedPlanSource *plansource;

@@ -1874,7 +1874,7 @@ _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan)

     foreach(list_item, raw_parsetree_list)
     {
-        RawStmt    *parsetree = castNode(RawStmt, lfirst(list_item));
+        RawStmt    *parsetree = lfirst_node(RawStmt, list_item);
         CachedPlanSource *plansource;

         plansource = CreateOneShotCachedPlan(parsetree,
@@ -2035,7 +2035,7 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,

         foreach(lc2, stmt_list)
         {
-            PlannedStmt *stmt = castNode(PlannedStmt, lfirst(lc2));
+            PlannedStmt *stmt = lfirst_node(PlannedStmt, lc2);
             bool        canSetTag = stmt->canSetTag;
             DestReceiver *dest;

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 4149e9f..3e8189c 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -111,7 +111,7 @@ exprType(const Node *expr)

                     if (!qtree || !IsA(qtree, Query))
                         elog(ERROR, "cannot get type for untransformed sublink");
-                    tent = castNode(TargetEntry, linitial(qtree->targetList));
+                    tent = linitial_node(TargetEntry, qtree->targetList);
                     Assert(!tent->resjunk);
                     type = exprType((Node *) tent->expr);
                     if (sublink->subLinkType == ARRAY_SUBLINK)
@@ -324,7 +324,7 @@ exprTypmod(const Node *expr)

                     if (!qtree || !IsA(qtree, Query))
                         elog(ERROR, "cannot get type for untransformed sublink");
-                    tent = castNode(TargetEntry, linitial(qtree->targetList));
+                    tent = linitial_node(TargetEntry, qtree->targetList);
                     Assert(!tent->resjunk);
                     return exprTypmod((Node *) tent->expr);
                     /* note we don't need to care if it's an array */
@@ -382,7 +382,7 @@ exprTypmod(const Node *expr)
                     return -1;    /* no point in trying harder */
                 foreach(arg, cexpr->args)
                 {
-                    CaseWhen   *w = castNode(CaseWhen, lfirst(arg));
+                    CaseWhen   *w = lfirst_node(CaseWhen, arg);

                     if (exprType((Node *) w->result) != casetype)
                         return -1;
@@ -809,7 +809,7 @@ exprCollation(const Node *expr)

                     if (!qtree || !IsA(qtree, Query))
                         elog(ERROR, "cannot get collation for untransformed sublink");
-                    tent = castNode(TargetEntry, linitial(qtree->targetList));
+                    tent = linitial_node(TargetEntry, qtree->targetList);
                     Assert(!tent->resjunk);
                     coll = exprCollation((Node *) tent->expr);
                     /* collation doesn't change if it's converted to array */
@@ -1054,7 +1054,7 @@ exprSetCollation(Node *expr, Oid collation)

                     if (!qtree || !IsA(qtree, Query))
                         elog(ERROR, "cannot set collation for untransformed sublink");
-                    tent = castNode(TargetEntry, linitial(qtree->targetList));
+                    tent = linitial_node(TargetEntry, qtree->targetList);
                     Assert(!tent->resjunk);
                     Assert(collation == exprCollation((Node *) tent->expr));
                 }
@@ -2058,7 +2058,7 @@ expression_tree_walker(Node *node,
                 /* we assume walker doesn't care about CaseWhens, either */
                 foreach(temp, caseexpr->args)
                 {
-                    CaseWhen   *when = castNode(CaseWhen, lfirst(temp));
+                    CaseWhen   *when = lfirst_node(CaseWhen, temp);

                     if (walker(when->expr, context))
                         return true;
@@ -3308,7 +3308,7 @@ raw_expression_tree_walker(Node *node,
                 /* we assume walker doesn't care about CaseWhens, either */
                 foreach(temp, caseexpr->args)
                 {
-                    CaseWhen   *when = castNode(CaseWhen, lfirst(temp));
+                    CaseWhen   *when = lfirst_node(CaseWhen, temp);

                     if (walker(when->expr, context))
                         return true;
@@ -3807,7 +3807,7 @@ planstate_walk_subplans(List *plans,

     foreach(lc, plans)
     {
-        SubPlanState *sps = castNode(SubPlanState, lfirst(lc));
+        SubPlanState *sps = lfirst_node(SubPlanState, lc);

         if (walker(sps->planstate, context))
             return true;
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 1f510c2..52643d0 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -757,7 +757,7 @@ extract_nonindex_conditions(List *qual_clauses, List *indexquals)

     foreach(lc, qual_clauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);

         if (rinfo->pseudoconstant)
             continue;            /* we may drop pseudoconstants here */
@@ -1990,7 +1990,7 @@ cost_windowagg(Path *path, PlannerInfo *root,
      */
     foreach(lc, windowFuncs)
     {
-        WindowFunc *wfunc = castNode(WindowFunc, lfirst(lc));
+        WindowFunc *wfunc = lfirst_node(WindowFunc, lc);
         Cost        wfunccost;
         QualCost    argcosts;

@@ -3066,7 +3066,7 @@ final_cost_hashjoin(PlannerInfo *root, HashPath *path,
         innerbucketsize = 1.0;
         foreach(hcl, hashclauses)
         {
-            RestrictInfo *restrictinfo = castNode(RestrictInfo, lfirst(hcl));
+            RestrictInfo *restrictinfo = lfirst_node(RestrictInfo, hcl);
             Selectivity thisbucketsize;

             /*
@@ -3760,7 +3760,7 @@ compute_semi_anti_join_factors(PlannerInfo *root,
         joinquals = NIL;
         foreach(l, restrictlist)
         {
-            RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+            RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);

             if (!rinfo->is_pushed_down)
                 joinquals = lappend(joinquals, rinfo);
@@ -4192,7 +4192,7 @@ calc_joinrel_size_estimate(PlannerInfo *root,
         /* Grovel through the clauses to separate into two lists */
         foreach(l, restrictlist)
         {
-            RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+            RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);

             if (rinfo->is_pushed_down)
                 pushedquals = lappend(pushedquals, rinfo);
@@ -4568,7 +4568,7 @@ set_subquery_size_estimates(PlannerInfo *root, RelOptInfo *rel)
      */
     foreach(lc, subroot->parse->targetList)
     {
-        TargetEntry *te = castNode(TargetEntry, lfirst(lc));
+        TargetEntry *te = lfirst_node(TargetEntry, lc);
         Node       *texpr = (Node *) te->expr;
         int32        item_width = 0;

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index cec9822..6e4bae8 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1277,7 +1277,7 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,

     foreach(lc, clauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
         List       *pathlist;
         Path       *bitmapqual;
         ListCell   *j;
@@ -2188,7 +2188,7 @@ match_clauses_to_index(IndexOptInfo *index,

     foreach(lc, clauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);

         match_clause_to_index(index, rinfo, clauseset);
     }
diff --git a/src/backend/optimizer/path/joinrels.c b/src/backend/optimizer/path/joinrels.c
index 6a0c67b..5a68de3 100644
--- a/src/backend/optimizer/path/joinrels.c
+++ b/src/backend/optimizer/path/joinrels.c
@@ -1250,7 +1250,7 @@ restriction_is_constant_false(List *restrictlist, bool only_pushed_down)
      */
     foreach(lc, restrictlist)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);

         if (only_pushed_down && !rinfo->is_pushed_down)
             continue;
diff --git a/src/backend/optimizer/plan/analyzejoins.c b/src/backend/optimizer/plan/analyzejoins.c
index cb283f6..69b9be4 100644
--- a/src/backend/optimizer/plan/analyzejoins.c
+++ b/src/backend/optimizer/plan/analyzejoins.c
@@ -601,7 +601,7 @@ rel_is_distinct_for(PlannerInfo *root, RelOptInfo *rel, List *clause_list)
          */
         foreach(l, clause_list)
         {
-            RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+            RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
             Oid            op;
             Var           *var;

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 124fef7..717b78c 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -2550,7 +2550,7 @@ create_indexscan_plan(PlannerInfo *root,
     qpqual = NIL;
     foreach(l, scan_clauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);

         if (rinfo->pseudoconstant)
             continue;            /* we may drop pseudoconstants here */
@@ -2710,7 +2710,7 @@ create_bitmap_scan_plan(PlannerInfo *root,
     qpqual = NIL;
     foreach(l, scan_clauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);
         Node       *clause = (Node *) rinfo->clause;

         if (rinfo->pseudoconstant)
@@ -3867,7 +3867,7 @@ create_mergejoin_plan(PlannerInfo *root,
     i = 0;
     foreach(lc, best_path->path_mergeclauses)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc);
         EquivalenceClass *oeclass;
         EquivalenceClass *ieclass;
         PathKey    *opathkey;
@@ -4414,7 +4414,7 @@ fix_indexqual_references(PlannerInfo *root, IndexPath *index_path)

     forboth(lcc, index_path->indexquals, lci, index_path->indexqualcols)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lcc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lcc);
         int            indexcol = lfirst_int(lci);
         Node       *clause;

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 17cd683..4d5ee01 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4631,7 +4631,7 @@ create_one_window_path(PlannerInfo *root,
             window_target = copy_pathtarget(window_target);
             foreach(lc2, wflists->windowFuncs[wc->winref])
             {
-                WindowFunc *wfunc = castNode(WindowFunc, lfirst(lc2));
+                WindowFunc *wfunc = lfirst_node(WindowFunc, lc2);

                 add_column_to_pathtarget(window_target, (Expr *) wfunc, 0);
                 window_target->width += get_typavgwidth(wfunc->wintype, -1);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index cdb8e95..1278371 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -224,7 +224,7 @@ set_plan_references(PlannerInfo *root, Plan *plan)
      */
     foreach(lc, root->rowMarks)
     {
-        PlanRowMark *rc = castNode(PlanRowMark, lfirst(lc));
+        PlanRowMark *rc = lfirst_node(PlanRowMark, lc);
         PlanRowMark *newrc;

         /* flat copy is enough since all fields are scalars */
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 87cc44d..8d0d8ae 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -433,7 +433,7 @@ get_first_col_type(Plan *plan, Oid *coltype, int32 *coltypmod,
     /* In cases such as EXISTS, tlist might be empty; arbitrarily use VOID */
     if (plan->targetlist)
     {
-        TargetEntry *tent = castNode(TargetEntry, linitial(plan->targetlist));
+        TargetEntry *tent = linitial_node(TargetEntry, plan->targetlist);

         if (!tent->resjunk)
         {
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index 881d85e..a1be858 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -1750,7 +1750,7 @@ translate_col_privs(const Bitmapset *parent_privs,
     attno = InvalidAttrNumber;
     foreach(lc, translated_vars)
     {
-        Var           *var = castNode(Var, lfirst(lc));
+        Var           *var = lfirst_node(Var, lc);

         attno++;
         if (var == NULL)        /* ignore dropped columns */
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 59d71c1..e196c5e 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -3090,7 +3090,7 @@ eval_const_expressions_mutator(Node *node,
                 const_true_cond = false;
                 foreach(arg, caseexpr->args)
                 {
-                    CaseWhen   *oldcasewhen = castNode(CaseWhen, lfirst(arg));
+                    CaseWhen   *oldcasewhen = lfirst_node(CaseWhen, arg);
                     Node       *casecond;
                     Node       *caseresult;

diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c
index 9cbcaed..b6867e3 100644
--- a/src/backend/optimizer/util/orclauses.c
+++ b/src/backend/optimizer/util/orclauses.c
@@ -188,7 +188,7 @@ extract_or_clause(RestrictInfo *or_rinfo, RelOptInfo *rel)

             foreach(lc2, andargs)
             {
-                RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc2));
+                RestrictInfo *rinfo = lfirst_node(RestrictInfo, lc2);

                 if (restriction_is_or_clause(rinfo))
                 {
diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index 6f79f96..e946290 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -335,7 +335,7 @@ get_actual_clauses(List *restrictinfo_list)

     foreach(l, restrictinfo_list)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);

         Assert(!rinfo->pseudoconstant);

@@ -359,7 +359,7 @@ extract_actual_clauses(List *restrictinfo_list,

     foreach(l, restrictinfo_list)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);

         if (rinfo->pseudoconstant == pseudoconstant)
             result = lappend(result, rinfo->clause);
@@ -389,7 +389,7 @@ extract_actual_join_clauses(List *restrictinfo_list,

     foreach(l, restrictinfo_list)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(l));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, l);

         if (rinfo->is_pushed_down)
         {
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index c4140a6..567dd54 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -829,7 +829,7 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
         AttrNumber    attr_num;
         TargetEntry *tle;

-        col = castNode(ResTarget, lfirst(icols));
+        col = lfirst_node(ResTarget, icols);
         attr_num = (AttrNumber) lfirst_int(attnos);

         tle = makeTargetEntry(expr,
@@ -954,7 +954,7 @@ transformInsertRow(ParseState *pstate, List *exprlist,
         Expr       *expr = (Expr *) lfirst(lc);
         ResTarget  *col;

-        col = castNode(ResTarget, lfirst(icols));
+        col = lfirst_node(ResTarget, icols);

         expr = transformAssignedExpr(pstate, expr,
                                      EXPR_KIND_INSERT_TARGET,
@@ -2300,7 +2300,7 @@ transformUpdateTargetList(ParseState *pstate, List *origTlist)
         }
         if (orig_tl == NULL)
             elog(ERROR, "UPDATE target count mismatch --- internal error");
-        origTarget = castNode(ResTarget, lfirst(orig_tl));
+        origTarget = lfirst_node(ResTarget, orig_tl);

         attrno = attnameAttNum(pstate->p_target_relation,
                                origTarget->name, true);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 29ca5f1..89d2836 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -798,7 +798,7 @@ stmtmulti:    stmtmulti ';' stmt
                     if ($1 != NIL)
                     {
                         /* update length of previous stmt */
-                        updateRawStmtEnd(castNode(RawStmt, llast($1)), @2);
+                        updateRawStmtEnd(llast_node(RawStmt, $1), @2);
                     }
                     if ($3 != NULL)
                         $$ = lappend($1, makeRawStmt($3, @2 + 1));
diff --git a/src/backend/parser/parse_collate.c b/src/backend/parser/parse_collate.c
index cc235d4..aa443f2 100644
--- a/src/backend/parser/parse_collate.c
+++ b/src/backend/parser/parse_collate.c
@@ -514,7 +514,7 @@ assign_collations_walker(Node *node, assign_collations_context *context)

                 if (qtree->targetList == NIL)
                     return false;
-                tent = castNode(TargetEntry, linitial(qtree->targetList));
+                tent = linitial_node(TargetEntry, qtree->targetList);
                 if (tent->resjunk)
                     return false;

@@ -649,7 +649,7 @@ assign_collations_walker(Node *node, assign_collations_context *context)

                             foreach(lc, expr->args)
                             {
-                                CaseWhen   *when = castNode(CaseWhen, lfirst(lc));
+                                CaseWhen   *when = lfirst_node(CaseWhen, lc);

                                 /*
                                  * The condition expressions mustn't affect
@@ -865,7 +865,7 @@ assign_aggregate_collations(Aggref *aggref,
     /* Process aggregated args, holding resjunk ones at arm's length */
     foreach(lc, aggref->args)
     {
-        TargetEntry *tle = castNode(TargetEntry, lfirst(lc));
+        TargetEntry *tle = lfirst_node(TargetEntry, lc);

         if (tle->resjunk)
             assign_expr_collations(loccontext->pstate, (Node *) tle);
@@ -909,7 +909,7 @@ assign_ordered_set_collations(Aggref *aggref,
     /* Process aggregated args appropriately */
     foreach(lc, aggref->args)
     {
-        TargetEntry *tle = castNode(TargetEntry, lfirst(lc));
+        TargetEntry *tle = lfirst_node(TargetEntry, lc);

         if (merge_sort_collations)
             (void) assign_collations_walker((Node *) tle, loccontext);
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 323be23..4f9b1a7 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -1669,7 +1669,7 @@ transformCaseExpr(ParseState *pstate, CaseExpr *c)
     resultexprs = NIL;
     foreach(l, c->args)
     {
-        CaseWhen   *w = castNode(CaseWhen, lfirst(l));
+        CaseWhen   *w = lfirst_node(CaseWhen, l);
         CaseWhen   *neww = makeNode(CaseWhen);
         Node       *warg;

@@ -2334,7 +2334,7 @@ transformXmlExpr(ParseState *pstate, XmlExpr *x)

     foreach(lc, x->named_args)
     {
-        ResTarget  *r = castNode(ResTarget, lfirst(lc));
+        ResTarget  *r = lfirst_node(ResTarget, lc);
         Node       *expr;
         char       *argname;

diff --git a/src/backend/parser/parse_node.c b/src/backend/parser/parse_node.c
index 34006c7..fb3d117 100644
--- a/src/backend/parser/parse_node.c
+++ b/src/backend/parser/parse_node.c
@@ -337,7 +337,7 @@ transformArraySubscripts(ParseState *pstate,
      */
     foreach(idx, indirection)
     {
-        A_Indices  *ai = castNode(A_Indices, lfirst(idx));
+        A_Indices  *ai = lfirst_node(A_Indices, idx);
         Node       *subexpr;

         if (isSlice)
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 062b8a1..f960f38 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -931,7 +931,7 @@ markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte,
             JoinExpr   *j;

             if (rtindex > 0 && rtindex <= list_length(pstate->p_joinexprs))
-                j = castNode(JoinExpr, list_nth(pstate->p_joinexprs, rtindex - 1));
+                j = list_nth_node(JoinExpr, pstate->p_joinexprs, rtindex - 1);
             else
                 j = NULL;
             if (j == NULL)
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index 0d7a2b1..b71b17b 100644
--- a/src/backend/parser/parse_type.c
+++ b/src/backend/parser/parse_type.c
@@ -478,7 +478,7 @@ TypeNameListToString(List *typenames)
     initStringInfo(&string);
     foreach(l, typenames)
     {
-        TypeName   *typeName = castNode(TypeName, lfirst(l));
+        TypeName   *typeName = lfirst_node(TypeName, l);

         if (l != list_head(typenames))
             appendStringInfoChar(&string, ',');
@@ -719,7 +719,7 @@ typeStringToTypeName(const char *str)
      */
     if (list_length(raw_parsetree_list) != 1)
         goto fail;
-    stmt = (SelectStmt *) castNode(RawStmt, linitial(raw_parsetree_list))->stmt;
+    stmt = (SelectStmt *) linitial_node(RawStmt, raw_parsetree_list)->stmt;
     if (stmt == NULL ||
         !IsA(stmt, SelectStmt) ||
         stmt->distinctClause != NIL ||
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 9266996..e546194 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -388,7 +388,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column,

     foreach(option, seqoptions)
     {
-        DefElem    *defel = castNode(DefElem, lfirst(option));
+        DefElem    *defel = lfirst_node(DefElem, option);

         if (strcmp(defel->defname, "sequence_name") == 0)
         {
@@ -605,7 +605,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)

     foreach(clist, column->constraints)
     {
-        Constraint *constraint = castNode(Constraint, lfirst(clist));
+        Constraint *constraint = lfirst_node(Constraint, clist);

         switch (constraint->contype)
         {
@@ -1635,7 +1635,7 @@ transformIndexConstraints(CreateStmtContext *cxt)
      */
     foreach(lc, cxt->ixconstraints)
     {
-        Constraint *constraint = castNode(Constraint, lfirst(lc));
+        Constraint *constraint = lfirst_node(Constraint, lc);

         Assert(constraint->contype == CONSTR_PRIMARY ||
                constraint->contype == CONSTR_UNIQUE ||
@@ -1956,8 +1956,8 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
             List       *opname;

             Assert(list_length(pair) == 2);
-            elem = castNode(IndexElem, linitial(pair));
-            opname = castNode(List, lsecond(pair));
+            elem = linitial_node(IndexElem, pair);
+            opname = lsecond_node(List, pair);

             index->indexParams = lappend(index->indexParams, elem);
             index->excludeOpNames = lappend(index->excludeOpNames, opname);
@@ -1984,7 +1984,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)

         foreach(columns, cxt->columns)
         {
-            column = castNode(ColumnDef, lfirst(columns));
+            column = lfirst_node(ColumnDef, columns);
             if (strcmp(column->colname, key) == 0)
             {
                 found = true;
@@ -2013,7 +2013,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)

             foreach(inher, cxt->inhRelations)
             {
-                RangeVar   *inh = castNode(RangeVar, lfirst(inher));
+                RangeVar   *inh = lfirst_node(RangeVar, inher);
                 Relation    rel;
                 int            count;

@@ -2823,7 +2823,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
                      */
                     foreach(lc, castNode(List, cmd->def))
                     {
-                        DefElem       *def = castNode(DefElem, lfirst(lc));
+                        DefElem       *def = lfirst_node(DefElem, lc);

                         if (strcmp(def->defname, "generated") == 0)
                             newdef = lappend(newdef, def);
@@ -2900,7 +2900,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
      */
     foreach(l, cxt.alist)
     {
-        IndexStmt  *idxstmt = castNode(IndexStmt, lfirst(l));
+        IndexStmt  *idxstmt = lfirst_node(IndexStmt, l);

         idxstmt = transformIndexStmt(relid, idxstmt, queryString);
         newcmd = makeNode(AlterTableCmd);
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index 396c36f..86d588b 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -171,7 +171,7 @@ InsertRule(char *rulname,
     if (event_qual != NULL)
     {
         /* Find query containing OLD/NEW rtable entries */
-        Query       *qry = castNode(Query, linitial(action));
+        Query       *qry = linitial_node(Query, action);

         qry = getInsertSelectQuery(qry, NULL);
         recordDependencyOnExpr(&myself, event_qual, qry->rtable,
@@ -284,7 +284,7 @@ DefineQueryRewrite(char *rulename,
      */
     foreach(l, action)
     {
-        query = castNode(Query, lfirst(l));
+        query = lfirst_node(Query, l);
         if (query->resultRelation == 0)
             continue;
         /* Don't be fooled by INSERT/SELECT */
@@ -326,7 +326,7 @@ DefineQueryRewrite(char *rulename,
         /*
          * ... the one action must be a SELECT, ...
          */
-        query = castNode(Query, linitial(action));
+        query = linitial_node(Query, action);
         if (!is_instead ||
             query->commandType != CMD_SELECT)
             ereport(ERROR,
@@ -480,7 +480,7 @@ DefineQueryRewrite(char *rulename,

         foreach(l, action)
         {
-            query = castNode(Query, lfirst(l));
+            query = lfirst_node(Query, l);

             if (!query->returningList)
                 continue;
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index cb860ec..4dcb713 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -2367,7 +2367,7 @@ view_cols_are_auto_updatable(Query *viewquery,
      * there should be a single base relation.
      */
     Assert(list_length(viewquery->jointree->fromlist) == 1);
-    rtr = castNode(RangeTblRef, linitial(viewquery->jointree->fromlist));
+    rtr = linitial_node(RangeTblRef, viewquery->jointree->fromlist);

     /* Initialize the optional return values */
     if (updatable_cols != NULL)
@@ -2619,7 +2619,7 @@ adjust_view_column_set(Bitmapset *cols, List *targetlist)

             foreach(lc, targetlist)
             {
-                TargetEntry *tle = (TargetEntry *) lfirst(lc);
+                TargetEntry *tle = lfirst_node(TargetEntry, lc);
                 Var           *var;

                 if (tle->resjunk)
@@ -2806,7 +2806,7 @@ rewriteTargetView(Query *parsetree, Relation view)
      * view contains a single base relation.
      */
     Assert(list_length(viewquery->jointree->fromlist) == 1);
-    rtr = castNode(RangeTblRef, linitial(viewquery->jointree->fromlist));
+    rtr = linitial_node(RangeTblRef, viewquery->jointree->fromlist);

     base_rt_index = rtr->rtindex;
     base_rte = rt_fetch(base_rt_index, viewquery->rtable);
@@ -3162,7 +3162,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
      */
     foreach(lc1, parsetree->cteList)
     {
-        CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc1);
+        CommonTableExpr *cte = lfirst_node(CommonTableExpr, lc1);
         Query       *ctequery = castNode(Query, cte->ctequery);
         List       *newstuff;

@@ -3179,7 +3179,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
         if (list_length(newstuff) == 1)
         {
             /* Push the single Query back into the CTE node */
-            ctequery = castNode(Query, linitial(newstuff));
+            ctequery = linitial_node(Query, newstuff);
             /* WITH queries should never be canSetTag */
             Assert(!ctequery->canSetTag);
             cte->ctequery = (Node *) ctequery;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 41801f1..75c2d9a 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -850,7 +850,7 @@ pg_plan_queries(List *querytrees, int cursorOptions, ParamListInfo boundParams)

     foreach(query_list, querytrees)
     {
-        Query       *query = castNode(Query, lfirst(query_list));
+        Query       *query = lfirst_node(Query, query_list);
         PlannedStmt *stmt;

         if (query->commandType == CMD_UTILITY)
@@ -966,7 +966,7 @@ exec_simple_query(const char *query_string)
      */
     foreach(parsetree_item, parsetree_list)
     {
-        RawStmt    *parsetree = castNode(RawStmt, lfirst(parsetree_item));
+        RawStmt    *parsetree = lfirst_node(RawStmt, parsetree_item);
         bool        snapshot_set = false;
         const char *commandTag;
         char        completionTag[COMPLETION_TAG_BUFSIZE];
@@ -1291,7 +1291,7 @@ exec_parse_message(const char *query_string,    /* string to execute */
         bool        snapshot_set = false;
         int            i;

-        raw_parse_tree = castNode(RawStmt, linitial(parsetree_list));
+        raw_parse_tree = linitial_node(RawStmt, parsetree_list);

         /*
          * Get the command name for possible use in status display.
@@ -2154,7 +2154,7 @@ errdetail_execute(List *raw_parsetree_list)

     foreach(parsetree_item, raw_parsetree_list)
     {
-        RawStmt    *parsetree = castNode(RawStmt, lfirst(parsetree_item));
+        RawStmt    *parsetree = lfirst_node(RawStmt, parsetree_item);

         if (IsA(parsetree->stmt, ExecuteStmt))
         {
@@ -2508,7 +2508,7 @@ IsTransactionExitStmtList(List *pstmts)
 {
     if (list_length(pstmts) == 1)
     {
-        PlannedStmt *pstmt = castNode(PlannedStmt, linitial(pstmts));
+        PlannedStmt *pstmt = linitial_node(PlannedStmt, pstmts);

         if (pstmt->commandType == CMD_UTILITY &&
             IsTransactionExitStmt(pstmt->utilityStmt))
@@ -2523,7 +2523,7 @@ IsTransactionStmtList(List *pstmts)
 {
     if (list_length(pstmts) == 1)
     {
-        PlannedStmt *pstmt = castNode(PlannedStmt, linitial(pstmts));
+        PlannedStmt *pstmt = linitial_node(PlannedStmt, pstmts);

         if (pstmt->commandType == CMD_UTILITY &&
             IsA(pstmt->utilityStmt, TransactionStmt))
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 988c9ff..e30aeb1 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -496,7 +496,7 @@ PortalStart(Portal portal, ParamListInfo params,
                  * Create QueryDesc in portal's context; for the moment, set
                  * the destination to DestNone.
                  */
-                queryDesc = CreateQueryDesc(castNode(PlannedStmt, linitial(portal->stmts)),
+                queryDesc = CreateQueryDesc(linitial_node(PlannedStmt, portal->stmts),
                                             portal->sourceText,
                                             GetActiveSnapshot(),
                                             InvalidSnapshot,
@@ -1036,7 +1036,7 @@ FillPortalStore(Portal portal, bool isTopLevel)
             break;

         case PORTAL_UTIL_SELECT:
-            PortalRunUtility(portal, castNode(PlannedStmt, linitial(portal->stmts)),
+            PortalRunUtility(portal, linitial_node(PlannedStmt, portal->stmts),
                              isTopLevel, true, treceiver, completionTag);
             break;

@@ -1232,7 +1232,7 @@ PortalRunMulti(Portal portal,
      */
     foreach(stmtlist_item, portal->stmts)
     {
-        PlannedStmt *pstmt = castNode(PlannedStmt, lfirst(stmtlist_item));
+        PlannedStmt *pstmt = lfirst_node(PlannedStmt, stmtlist_item);

         /*
          * If we got a cancel signal in prior command, quit
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 241b81a..5f11af2 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -7809,7 +7809,7 @@ get_rule_expr(Node *node, deparse_context *context,
                 appendStringInfoString(buf, "(alternatives: ");
                 foreach(lc, asplan->subplans)
                 {
-                    SubPlan    *splan = castNode(SubPlan, lfirst(lc));
+                    SubPlan    *splan = lfirst_node(SubPlan, lc);

                     if (splan->useHashTable)
                         appendStringInfo(buf, "hashed %s", splan->plan_name);
@@ -8364,7 +8364,7 @@ get_rule_expr(Node *node, deparse_context *context,
                             get_rule_expr((Node *) linitial(xexpr->args),
                                           context, true);

-                            con = castNode(Const, lsecond(xexpr->args));
+                            con = lsecond_node(Const, xexpr->args);
                             Assert(!con->constisnull);
                             if (DatumGetBool(con->constvalue))
                                 appendStringInfoString(buf,
@@ -8387,7 +8387,7 @@ get_rule_expr(Node *node, deparse_context *context,
                             else
                                 get_rule_expr((Node *) con, context, false);

-                            con = castNode(Const, lthird(xexpr->args));
+                            con = lthird_node(Const, xexpr->args);
                             if (con->constisnull)
                                  /* suppress STANDALONE NO VALUE */ ;
                             else
@@ -8899,7 +8899,7 @@ get_agg_expr(Aggref *aggref, deparse_context *context,
      */
     if (DO_AGGSPLIT_COMBINE(aggref->aggsplit))
     {
-        TargetEntry *tle = castNode(TargetEntry, linitial(aggref->args));
+        TargetEntry *tle = linitial_node(TargetEntry, aggref->args);

         Assert(list_length(aggref->args) == 1);
         resolve_special_varno((Node *) tle->expr, context, original_aggref,
@@ -9360,7 +9360,7 @@ get_sublink_expr(SubLink *sublink, deparse_context *context)
             sep = "";
             foreach(l, ((BoolExpr *) sublink->testexpr)->args)
             {
-                OpExpr       *opexpr = castNode(OpExpr, lfirst(l));
+                OpExpr       *opexpr = lfirst_node(OpExpr, l);

                 appendStringInfoString(buf, sep);
                 get_rule_expr(linitial(opexpr->args), context, true);
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 51c50ef..a35b93b 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -6243,7 +6243,7 @@ deconstruct_indexquals(IndexPath *path)

     forboth(lcc, path->indexquals, lci, path->indexqualcols)
     {
-        RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lcc));
+        RestrictInfo *rinfo = lfirst_node(RestrictInfo, lcc);
         int            indexcol = lfirst_int(lci);
         Expr       *clause;
         Node       *leftop,
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 50da116..abff747 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -979,7 +979,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
     is_transient = false;
     foreach(lc, plist)
     {
-        PlannedStmt *plannedstmt = castNode(PlannedStmt, lfirst(lc));
+        PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc);

         if (plannedstmt->commandType == CMD_UTILITY)
             continue;            /* Ignore utility statements */
@@ -1074,7 +1074,7 @@ cached_plan_cost(CachedPlan *plan, bool include_planner)

     foreach(lc, plan->stmt_list)
     {
-        PlannedStmt *plannedstmt = castNode(PlannedStmt, lfirst(lc));
+        PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc);

         if (plannedstmt->commandType == CMD_UTILITY)
             continue;            /* Ignore utility statements */
@@ -1462,7 +1462,7 @@ QueryListGetPrimaryStmt(List *stmts)

     foreach(lc, stmts)
     {
-        Query       *stmt = castNode(Query, lfirst(lc));
+        Query       *stmt = lfirst_node(Query, lc);

         if (stmt->canSetTag)
             return stmt;
@@ -1481,7 +1481,7 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)

     foreach(lc1, stmt_list)
     {
-        PlannedStmt *plannedstmt = castNode(PlannedStmt, lfirst(lc1));
+        PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc1);
         int            rt_index;
         ListCell   *lc2;

@@ -1551,7 +1551,7 @@ AcquirePlannerLocks(List *stmt_list, bool acquire)

     foreach(lc, stmt_list)
     {
-        Query       *query = castNode(Query, lfirst(lc));
+        Query       *query = lfirst_node(Query, lc);

         if (query->commandType == CMD_UTILITY)
         {
@@ -1618,7 +1618,7 @@ ScanQueryForLocks(Query *parsetree, bool acquire)
     /* Recurse into subquery-in-WITH */
     foreach(lc, parsetree->cteList)
     {
-        CommonTableExpr *cte = castNode(CommonTableExpr, lfirst(lc));
+        CommonTableExpr *cte = lfirst_node(CommonTableExpr, lc);

         ScanQueryForLocks(castNode(Query, cte->ctequery), acquire);
     }
@@ -1676,7 +1676,7 @@ PlanCacheComputeResultDesc(List *stmt_list)
     {
         case PORTAL_ONE_SELECT:
         case PORTAL_ONE_MOD_WITH:
-            query = castNode(Query, linitial(stmt_list));
+            query = linitial_node(Query, stmt_list);
             return ExecCleanTypeFromTL(query->targetList, false);

         case PORTAL_ONE_RETURNING:
@@ -1685,7 +1685,7 @@ PlanCacheComputeResultDesc(List *stmt_list)
             return ExecCleanTypeFromTL(query->returningList, false);

         case PORTAL_UTIL_SELECT:
-            query = castNode(Query, linitial(stmt_list));
+            query = linitial_node(Query, stmt_list);
             Assert(query->utilityStmt);
             return UtilityTupleDescriptor(query->utilityStmt);

@@ -1742,7 +1742,7 @@ PlanCacheRelCallback(Datum arg, Oid relid)

             foreach(lc, plansource->gplan->stmt_list)
             {
-                PlannedStmt *plannedstmt = castNode(PlannedStmt, lfirst(lc));
+                PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc);

                 if (plannedstmt->commandType == CMD_UTILITY)
                     continue;    /* Ignore utility statements */
@@ -1815,7 +1815,7 @@ PlanCacheFuncCallback(Datum arg, int cacheid, uint32 hashvalue)
         {
             foreach(lc, plansource->gplan->stmt_list)
             {
-                PlannedStmt *plannedstmt = castNode(PlannedStmt, lfirst(lc));
+                PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc);
                 ListCell   *lc3;

                 if (plannedstmt->commandType == CMD_UTILITY)
@@ -1888,7 +1888,7 @@ ResetPlanCache(void)
          */
         foreach(lc, plansource->query_list)
         {
-            Query       *query = castNode(Query, lfirst(lc));
+            Query       *query = lfirst_node(Query, lc);

             if (query->commandType != CMD_UTILITY ||
                 UtilityContainsQuery(query->utilityStmt))
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 19d258d..cdab76f 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7366,7 +7366,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
             }
             else if (strcmp(stmt->name, "TRANSACTION SNAPSHOT") == 0)
             {
-                A_Const    *con = castNode(A_Const, linitial(stmt->args));
+                A_Const    *con = linitial_node(A_Const, stmt->args);

                 if (stmt->is_local)
                     ereport(ERROR,
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 3a3259b..5983aed 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -153,7 +153,7 @@ PortalGetPrimaryStmt(Portal portal)

     foreach(lc, portal->stmts)
     {
-        PlannedStmt *stmt = castNode(PlannedStmt, lfirst(lc));
+        PlannedStmt *stmt = lfirst_node(PlannedStmt, lc);

         if (stmt->canSetTag)
             return stmt;
diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 90e84bc..9df7fb3 100644
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -106,26 +106,32 @@ list_length(const List *l)
 #define lfirst(lc)                ((lc)->data.ptr_value)
 #define lfirst_int(lc)            ((lc)->data.int_value)
 #define lfirst_oid(lc)            ((lc)->data.oid_value)
+#define lfirst_node(type,lc)    castNode(type, lfirst(lc))

 #define linitial(l)                lfirst(list_head(l))
 #define linitial_int(l)            lfirst_int(list_head(l))
 #define linitial_oid(l)            lfirst_oid(list_head(l))
+#define linitial_node(type,l)    castNode(type, linitial(l))

 #define lsecond(l)                lfirst(lnext(list_head(l)))
 #define lsecond_int(l)            lfirst_int(lnext(list_head(l)))
 #define lsecond_oid(l)            lfirst_oid(lnext(list_head(l)))
+#define lsecond_node(type,l)    castNode(type, lsecond(l))

 #define lthird(l)                lfirst(lnext(lnext(list_head(l))))
 #define lthird_int(l)            lfirst_int(lnext(lnext(list_head(l))))
 #define lthird_oid(l)            lfirst_oid(lnext(lnext(list_head(l))))
+#define lthird_node(type,l)        castNode(type, lthird(l))

 #define lfourth(l)                lfirst(lnext(lnext(lnext(list_head(l)))))
 #define lfourth_int(l)            lfirst_int(lnext(lnext(lnext(list_head(l)))))
 #define lfourth_oid(l)            lfirst_oid(lnext(lnext(lnext(list_head(l)))))
+#define lfourth_node(type,l)    castNode(type, lfourth(l))

 #define llast(l)                lfirst(list_tail(l))
 #define llast_int(l)            lfirst_int(list_tail(l))
 #define llast_oid(l)            lfirst_oid(list_tail(l))
+#define llast_node(type,l)        castNode(type, llast(l))

 /*
  * Convenience macros for building fixed-length lists
@@ -204,6 +210,7 @@ extern ListCell *list_nth_cell(const List *list, int n);
 extern void *list_nth(const List *list, int n);
 extern int    list_nth_int(const List *list, int n);
 extern Oid    list_nth_oid(const List *list, int n);
+#define list_nth_node(type,list,n)    castNode(type, list_nth(list, n))

 extern bool list_member(const List *list, const void *datum);
 extern bool list_member_ptr(const List *list, const void *datum);
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 8d7c7ca..7a40c99 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3639,7 +3639,7 @@ exec_stmt_execsql(PLpgSQL_execstate *estate,

             foreach(l2, plansource->query_list)
             {
-                Query       *q = castNode(Query, lfirst(l2));
+                Query       *q = lfirst_node(Query, l2);

                 if (q->canSetTag)
                 {
@@ -6835,7 +6835,7 @@ exec_simple_recheck_plan(PLpgSQL_expr *expr, CachedPlan *cplan)
      */
     if (list_length(cplan->stmt_list) != 1)
         return;
-    stmt = castNode(PlannedStmt, linitial(cplan->stmt_list));
+    stmt = linitial_node(PlannedStmt, cplan->stmt_list);

     /*
      * 2. It must be a RESULT plan --> no scan's required

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-04-10 12:20:16 -0400, Tom Lane wrote:
> Barring objections, I'll push this shortly.

+1, to just about all of it