Hi Tender, On Thu, Oct 16, 2025 at 8:24 PM Tender Wang <[email protected]> wrote: > > > > Richard Guo <[email protected]> 于2025年10月16日周四 17:53写道： >> >> 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.) > > > Make sense. > > Please check the v2 patch.
If we decide to converts HashJoin and MergeJoin state machine definitions from #define macros to enum types, we might need to keep the (int) casts from the elog() error messages: elog(ERROR, "unrecognized hashjoin state: %d", (int) node->hj_JoinState); elog(ERROR, "unrecognized mergejoin state: %d", (int) node->mj_JoinState); The enum comment has inconsistent indentation: + /* + * States of the ExecMergeJoin state machine + */ Best, Xuneng
