Обсуждение: Use IsA() macro instead of nodeTag comparison
Hi hackers, In copy.c, nodeTag was being compared directly, so I replaced it with the IsA() predicate macro for consistency. I verified that there are no other direct comparisons left by running the following command: $ git grep "nodeTag(.*) ==" -- Best regards, Shinya Kato NTT OSS Center
Вложения
On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11.kato@gmail.com> wrote: > > Hi hackers, Hi > In copy.c, nodeTag was being compared directly, so I replaced it with > the IsA() predicate macro for consistency. Oops. Looks like oversight in 6c8f670. This is indeed case where we should use IsA() > I verified that there are no other direct comparisons left by running > the following command: > $ git grep "nodeTag(.*) ==" Yep, look like this is the only case in copy.c > -- > Best regards, > Shinya Kato > NTT OSS Center -- Best regards, Kirill Reshke
On 08/01/2026 15:10, Kirill Reshke wrote: > On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11.kato@gmail.com> wrote: >> In copy.c, nodeTag was being compared directly, so I replaced it with >> the IsA() predicate macro for consistency. > > Oops. Looks like oversight in 6c8f670. This is indeed case where we > should use IsA() > >> I verified that there are no other direct comparisons left by running >> the following command: >> $ git grep "nodeTag(.*) ==" > > Yep, look like this is the only case in copy.c Pushed, thanks - Heikki
On Fri, Jan 9, 2026 at 2:11 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> On 08/01/2026 15:10, Kirill Reshke wrote:
> > On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11.kato@gmail.com> wrote:
> >> In copy.c, nodeTag was being compared directly, so I replaced it with
> >> the IsA() predicate macro for consistency.
> >
> > Oops. Looks like oversight in 6c8f670. This is indeed case where we
> > should use IsA()
> >
> >> I verified that there are no other direct comparisons left by running
> >> the following command:
> >> $ git grep "nodeTag(.*) =="
> >
> > Yep, look like this is the only case in copy.c
I found a similar issue in define.c. Should we fix it there as well?
diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
index 63601a2c0b4..8313431397f 100644
--- a/src/backend/commands/define.c
+++ b/src/backend/commands/define.c
@@ -349,7 +349,7 @@ defGetStringList(DefElem *def)
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("%s requires a parameter",
def->defname)));
- if (nodeTag(def->arg) != T_List)
+ if (!IsA(def->arg, List))
elog(ERROR, "unrecognized node type: %d", (int)
nodeTag(def->arg));
foreach(cell, (List *) def->arg)
Regards,
--
Fujii Masao
> On Jan 9, 2026, at 08:32, Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Fri, Jan 9, 2026 at 2:11 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 08/01/2026 15:10, Kirill Reshke wrote:
>>> On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11.kato@gmail.com> wrote:
>>>> In copy.c, nodeTag was being compared directly, so I replaced it with
>>>> the IsA() predicate macro for consistency.
>>>
>>> Oops. Looks like oversight in 6c8f670. This is indeed case where we
>>> should use IsA()
>>>
>>>> I verified that there are no other direct comparisons left by running
>>>> the following command:
>>>> $ git grep "nodeTag(.*) =="
>>>
>>> Yep, look like this is the only case in copy.c
>
> I found a similar issue in define.c. Should we fix it there as well?
>
> diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
> index 63601a2c0b4..8313431397f 100644
> --- a/src/backend/commands/define.c
> +++ b/src/backend/commands/define.c
> @@ -349,7 +349,7 @@ defGetStringList(DefElem *def)
> (errcode(ERRCODE_SYNTAX_ERROR),
> errmsg("%s requires a parameter",
> def->defname)));
> - if (nodeTag(def->arg) != T_List)
> + if (!IsA(def->arg, List))
> elog(ERROR, "unrecognized node type: %d", (int)
> nodeTag(def->arg));
>
> foreach(cell, (List *) def->arg)
>
> Regards,
>
> --
> Fujii Masao
Yep, I did a search with `nodeTag\(.*\)\s+(?:!=|==).*`, and this is the only finding.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Fri, Jan 9, 2026 at 9:56 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> > I found a similar issue in define.c. Should we fix it there as well?
> >
> > diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
> > index 63601a2c0b4..8313431397f 100644
> > --- a/src/backend/commands/define.c
> > +++ b/src/backend/commands/define.c
> > @@ -349,7 +349,7 @@ defGetStringList(DefElem *def)
> > (errcode(ERRCODE_SYNTAX_ERROR),
> > errmsg("%s requires a parameter",
> > def->defname)));
> > - if (nodeTag(def->arg) != T_List)
> > + if (!IsA(def->arg, List))
> > elog(ERROR, "unrecognized node type: %d", (int)
> > nodeTag(def->arg));
> >
> > foreach(cell, (List *) def->arg)
> >
>
> Yep, I did a search with `nodeTag\(.*\)\s+(?:!=|==).*`, and this is the only finding.
Oh, I missed that, sorry. Thanks, LGTM.
--
Best regards,
Shinya Kato
NTT OSS Center
Hi,
On Fri, Jan 09, 2026 at 08:56:00AM +0800, Chao Li wrote:
>
>
> > On Jan 9, 2026, at 08:32, Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Fri, Jan 9, 2026 at 2:11 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> >>
> >> On 08/01/2026 15:10, Kirill Reshke wrote:
> >>> On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11.kato@gmail.com> wrote:
> >>>> In copy.c, nodeTag was being compared directly, so I replaced it with
> >>>> the IsA() predicate macro for consistency.
> >>>
> >>> Oops. Looks like oversight in 6c8f670. This is indeed case where we
> >>> should use IsA()
> >>>
> >>>> I verified that there are no other direct comparisons left by running
> >>>> the following command:
> >>>> $ git grep "nodeTag(.*) =="
> >>>
> >>> Yep, look like this is the only case in copy.c
> >
> > I found a similar issue in define.c. Should we fix it there as well?
> >
> > diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
> > index 63601a2c0b4..8313431397f 100644
> > --- a/src/backend/commands/define.c
> > +++ b/src/backend/commands/define.c
> > @@ -349,7 +349,7 @@ defGetStringList(DefElem *def)
> > (errcode(ERRCODE_SYNTAX_ERROR),
> > errmsg("%s requires a parameter",
> > def->defname)));
> > - if (nodeTag(def->arg) != T_List)
> > + if (!IsA(def->arg, List))
> > elog(ERROR, "unrecognized node type: %d", (int)
> > nodeTag(def->arg));
> >
> > foreach(cell, (List *) def->arg)
> >
> > Regards,
> >
> > --
> > Fujii Masao
>
> Yep, I did a search with `nodeTag\(.*\)\s+(?:!=|==).*`, and this is the only finding.
Yeah, I was going to submit the exact same patch detected with the help of
[1]. That's the only remaining case.
[1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/nodetag.cocci
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Fri, Jan 9, 2026 at 2:47 PM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On Fri, Jan 09, 2026 at 08:56:00AM +0800, Chao Li wrote:
> >
> >
> > > On Jan 9, 2026, at 08:32, Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Fri, Jan 9, 2026 at 2:11 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> > >>
> > >> On 08/01/2026 15:10, Kirill Reshke wrote:
> > >>> On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11.kato@gmail.com> wrote:
> > >>>> In copy.c, nodeTag was being compared directly, so I replaced it with
> > >>>> the IsA() predicate macro for consistency.
> > >>>
> > >>> Oops. Looks like oversight in 6c8f670. This is indeed case where we
> > >>> should use IsA()
> > >>>
> > >>>> I verified that there are no other direct comparisons left by running
> > >>>> the following command:
> > >>>> $ git grep "nodeTag(.*) =="
> > >>>
> > >>> Yep, look like this is the only case in copy.c
> > >
> > > I found a similar issue in define.c. Should we fix it there as well?
> > >
> > > diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c
> > > index 63601a2c0b4..8313431397f 100644
> > > --- a/src/backend/commands/define.c
> > > +++ b/src/backend/commands/define.c
> > > @@ -349,7 +349,7 @@ defGetStringList(DefElem *def)
> > > (errcode(ERRCODE_SYNTAX_ERROR),
> > > errmsg("%s requires a parameter",
> > > def->defname)));
> > > - if (nodeTag(def->arg) != T_List)
> > > + if (!IsA(def->arg, List))
> > > elog(ERROR, "unrecognized node type: %d", (int)
> > > nodeTag(def->arg));
> > >
> > > foreach(cell, (List *) def->arg)
> > >
> > > Regards,
> > >
> > > --
> > > Fujii Masao
> >
> > Yep, I did a search with `nodeTag\(.*\)\s+(?:!=|==).*`, and this is the only finding.
Pushed. Thanks!
> Yeah, I was going to submit the exact same patch detected with the help of
> [1]. That's the only remaining case.
>
> [1]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/nodetag.cocci
Interesting!
Regards,
--
Fujii Masao