Re: Fix an unnecessary cast calling elog in ExecHashJoinImpl

wenhui qiu <qiuwenhuifx@gmail.com>

From: wenhui qiu <qiuwenhuifx@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: Xuneng Zhou <xunengzhou@gmail.com>, Chao Li <li.evan.chao@gmail.com>, tndrwang@gmail.com, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-10-16T10:04:03Z
Lists: pgsql-hackers
Hi Richard
> (These values represent the state of the Join state machine for
> HashJoin and MergeJoin respectively, so I kind of wonder if it might
> be better to define them as enum rather than using int.)
Agree +1 ,enum is safer because it has a fixed set of predefined values,
whereas int has a much larger range of possible values.

On Thu, Oct 16, 2025 at 5:53 PM Richard Guo <guofenglinux@gmail.com> wrote:

> On Thu, Oct 16, 2025 at 4:07 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > On Mon, Sep 1, 2025 at 9:26 AM Chao Li <li.evan.chao@gmail.com> wrote:
> > > On Aug 30, 2025, at 14:09, Tender Wang <tndrwang@gmail.com> wrote:
> > > While debugging the HashJoin codes, I noticed below codes in
> ExecHashJoinImpl():
> > >
> > > elog(ERROR, "unrecognized hashjoin state: %d",
> > > (int) node->hj_JoinState);
> > >
> > > The type of hj_JoinState is already int, so the cast seems unnecessary.
> > > So I remove it in the attached patch.
>
> > > Yes, hj_JoinState is of type int, so the type cast to int is not
> needed. The change looks good to me.
>
> > LGTM.
>
> I think we can remove this cast, although it's so trivial that it
> doesn't seem to have any real impact.  A similar cast also exists for
> mj_JoinState in ExecMergeJoin().
>
> (These values represent the state of the Join state machine for
> HashJoin and MergeJoin respectively, so I kind of wonder if it might
> be better to define them as enum rather than using int.)
>
> - Richard
>
>
>