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. -- Thanks, Tender Wang
From eb0237190ffc84d7fe3ff38e6d4499761193f505 Mon Sep 17 00:00:00 2001 From: Tender Wang <[email protected]> Date: Thu, 16 Oct 2025 20:10:18 +0800 Subject: [PATCH] Use enum to define the state machine for HashJoin and MergeJoin. --- src/backend/executor/nodeHashjoin.c | 11 +-------- src/backend/executor/nodeMergejoin.c | 16 +------------ src/include/nodes/execnodes.h | 34 ++++++++++++++++++++++++++-- src/tools/pgindent/typedefs.list | 2 ++ 4 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c index 5661ad76830..a5908892381 100644 --- a/src/backend/executor/nodeHashjoin.c +++ b/src/backend/executor/nodeHashjoin.c @@ -174,15 +174,6 @@ #include "utils/wait_event.h" -/* - * States of the ExecHashJoin state machine - */ -#define HJ_BUILD_HASHTABLE 1 -#define HJ_NEED_NEW_OUTER 2 -#define HJ_SCAN_BUCKET 3 -#define HJ_FILL_OUTER_TUPLE 4 -#define HJ_FILL_INNER_TUPLES 5 -#define HJ_NEED_NEW_BATCH 6 /* Returns true if doing null-fill on outer relation */ #define HJ_FILL_OUTER(hjstate) ((hjstate)->hj_NullInnerTupleSlot != NULL) @@ -669,7 +660,7 @@ ExecHashJoinImpl(PlanState *pstate, bool parallel) default: elog(ERROR, "unrecognized hashjoin state: %d", - (int) node->hj_JoinState); + node->hj_JoinState); } } } diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c index a233313128a..2aee0502758 100644 --- a/src/backend/executor/nodeMergejoin.c +++ b/src/backend/executor/nodeMergejoin.c @@ -99,20 +99,6 @@ #include "utils/lsyscache.h" -/* - * States of the ExecMergeJoin state machine - */ -#define EXEC_MJ_INITIALIZE_OUTER 1 -#define EXEC_MJ_INITIALIZE_INNER 2 -#define EXEC_MJ_JOINTUPLES 3 -#define EXEC_MJ_NEXTOUTER 4 -#define EXEC_MJ_TESTOUTER 5 -#define EXEC_MJ_NEXTINNER 6 -#define EXEC_MJ_SKIP_TEST 7 -#define EXEC_MJ_SKIPOUTER_ADVANCE 8 -#define EXEC_MJ_SKIPINNER_ADVANCE 9 -#define EXEC_MJ_ENDOUTER 10 -#define EXEC_MJ_ENDINNER 11 /* * Runtime data for each mergejoin clause @@ -1426,7 +1412,7 @@ ExecMergeJoin(PlanState *pstate) */ default: elog(ERROR, "unrecognized mergejoin state: %d", - (int) node->mj_JoinState); + node->mj_JoinState); } } } diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index a36653c37f9..3e1365dd466 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2195,6 +2195,24 @@ typedef struct NestLoopState * InnerEContext workspace for computing inner tuple's join values * ---------------- */ + + /* + * States of the ExecMergeJoin state machine + */ +typedef enum MJ_ExecState { + EXEC_MJ_INITIALIZE_OUTER = 1, + EXEC_MJ_INITIALIZE_INNER, + EXEC_MJ_JOINTUPLES, + EXEC_MJ_NEXTOUTER, + EXEC_MJ_TESTOUTER, + EXEC_MJ_NEXTINNER, + EXEC_MJ_SKIP_TEST, + EXEC_MJ_SKIPOUTER_ADVANCE, + EXEC_MJ_SKIPINNER_ADVANCE, + EXEC_MJ_ENDOUTER, + EXEC_MJ_ENDINNER +} MJ_ExecState; + /* private in nodeMergejoin.c: */ typedef struct MergeJoinClauseData *MergeJoinClause; @@ -2203,7 +2221,7 @@ typedef struct MergeJoinState JoinState js; /* its first field is NodeTag */ int mj_NumClauses; MergeJoinClause mj_Clauses; /* array of length mj_NumClauses */ - int mj_JoinState; + MJ_ExecState mj_JoinState; bool mj_SkipMarkRestore; bool mj_ExtraMarks; bool mj_ConstFalseJoin; @@ -2246,6 +2264,18 @@ typedef struct MergeJoinState * ---------------- */ + /* + * States of the ExecHashJoin state machine + */ +typedef enum HJ_ExecState { + HJ_BUILD_HASHTABLE = 1, + HJ_NEED_NEW_OUTER, + HJ_SCAN_BUCKET, + HJ_FILL_OUTER_TUPLE, + HJ_FILL_INNER_TUPLES, + HJ_NEED_NEW_BATCH +} HJ_ExecState; + /* these structs are defined in executor/hashjoin.h: */ typedef struct HashJoinTupleData *HashJoinTuple; typedef struct HashJoinTableData *HashJoinTable; @@ -2265,7 +2295,7 @@ typedef struct HashJoinState TupleTableSlot *hj_NullOuterTupleSlot; TupleTableSlot *hj_NullInnerTupleSlot; TupleTableSlot *hj_FirstOuterTupleSlot; - int hj_JoinState; + HJ_ExecState hj_JoinState; bool hj_MatchedOuter; bool hj_OuterNotEmpty; } HashJoinState; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index ee1cab6190f..8fe926cbffb 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1145,6 +1145,7 @@ HASH_SEQ_STATUS HE HEntry HIST_ENTRY +HJ_ExecState HKEY HLOCAL HMAC_CTX @@ -1654,6 +1655,7 @@ MGVTBL MINIDUMPWRITEDUMP MINIDUMP_TYPE MJEvalResult +MJ_ExecState MTTargetRelLookup MVDependencies MVDependency -- 2.34.1
