Thread
Commits
-
Use IsA() macro in define.c, for sake of consistency.
- 24cb3a08a43b 19 (unreleased) landed
-
Use IsA macro, for sake of consistency
- 63d1b1cf7f1e 19 (unreleased) landed
-
Use IsA() macro instead of nodeTag comparison
Shinya Kato <shinya11.kato@gmail.com> — 2026-01-08T12:30:50Z
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
-
Re: Use IsA() macro instead of nodeTag comparison
Kirill Reshke <reshkekirill@gmail.com> — 2026-01-08T13:10:44Z
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
-
Re: Use IsA() macro instead of nodeTag comparison
Heikki Linnakangas <hlinnaka@iki.fi> — 2026-01-08T17:11:29Z
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
-
Re: Use IsA() macro instead of nodeTag comparison
Fujii Masao <masao.fujii@gmail.com> — 2026-01-09T00:32:07Z
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 -
Re: Use IsA() macro instead of nodeTag comparison
Chao Li <li.evan.chao@gmail.com> — 2026-01-09T00:56:00Z
> 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/ -
Re: Use IsA() macro instead of nodeTag comparison
Shinya Kato <shinya11.kato@gmail.com> — 2026-01-09T02:19:14Z
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 -
Re: Use IsA() macro instead of nodeTag comparison
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-01-09T05:47:33Z
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 -
Re: Use IsA() macro instead of nodeTag comparison
Fujii Masao <masao.fujii@gmail.com> — 2026-01-09T11:26:40Z
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