| From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
|---|---|
| To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Kirill Reshke <reshkekirill(at)gmail(dot)com>, Shinya Kato <shinya11(dot)kato(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Use IsA() macro instead of nodeTag comparison |
| Date: | 2026-01-09 11:26:40 |
| Message-ID: | CAHGQGwHMBkKTf=6ZYV+cxyp5jJ3ACTAVbOHPH2ZyZ675QA06mg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, Jan 9, 2026 at 2:47 PM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)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(dot)fujii(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Jan 9, 2026 at 2:11 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> > >>
> > >> On 08/01/2026 15:10, Kirill Reshke wrote:
> > >>> On Thu, 8 Jan 2026 at 17:31, Shinya Kato <shinya11(dot)kato(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Gilles Darold | 2026-01-09 11:27:59 | Re: Pasword expiration warning |
| Previous Message | Rahila Syed | 2026-01-09 11:22:25 | Re: Enhancing Memory Context Statistics Reporting |