Re: Fix an unnecessary cast calling elog in ExecHashJoinImpl - Mailing list pgsql-hackers

From wenhui qiu
Subject Re: Fix an unnecessary cast calling elog in ExecHashJoinImpl
Date
Msg-id CAGjGUAL-iMb4XbWr8RUV_5dzP=P6aNZDBMUvFyZAq4b3Qe1X+Q@mail.gmail.com
Whole thread Raw
In response to Re: Fix an unnecessary cast calling elog in ExecHashJoinImpl  (Richard Guo <guofenglinux@gmail.com>)
List 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


pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Logical Replication of sequences
Next
From: Amit Kapila
Date:
Subject: Re: [PATCH TEST] Fix logical replication setup in subscription test `t/009_matviews.pl`