Xuneng Zhou <[email protected]> 于2025年10月17日周五 10:31写道：
> 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); > I searched the entire codebase and found that the accumStats() function is in pgbench.c, we have pg_fatal("unexpected error status: %d", estatus); and the estatus is enum type. So we can no need to use cast, too. > The enum comment has inconsistent indentation: > > + /* > + * States of the ExecMergeJoin state machine > + */ > Yeah, copy-paste issue. Fixed. -- Thanks, Tender Wang
v3-Use-enum-to-define-the-state-machine-for-HashJoin-an.patch
Description: Binary data
