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 <[email protected]> wrote: > On Thu, Oct 16, 2025 at 4:07 PM Xuneng Zhou <[email protected]> wrote: > > On Mon, Sep 1, 2025 at 9:26 AM Chao Li <[email protected]> wrote: > > > On Aug 30, 2025, at 14:09, Tender Wang <[email protected]> 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 > > >
