| From: | Tender Wang <tndrwang(at)gmail(dot)com> |
|---|---|
| To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
| Cc: | Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix an unnecessary cast calling elog in ExecHashJoinImpl |
| Date: | 2025-10-16 12:24:31 |
| Message-ID: | CAHewXNnEg0_MV2D=QZGMHgLwCnUZJPJAecf+E2_AR4JRqCOTsQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Richard Guo <guofenglinux(at)gmail(dot)com> 于2025年10月16日周四 17:53写道:
> On Thu, Oct 16, 2025 at 4:07 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> > On Mon, Sep 1, 2025 at 9:26 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> > > On Aug 30, 2025, at 14:09, Tender Wang <tndrwang(at)gmail(dot)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.)
>
Make sense.
Please check the v2 patch.
--
Thanks,
Tender Wang
| Attachment | Content-Type | Size |
|---|---|---|
| v2-Use-enum-to-define-the-state-machine-for-HashJoin-an.patch | text/plain | 4.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Philip Alger | 2025-10-16 12:36:05 | Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement |
| Previous Message | Chao Li | 2025-10-16 11:50:06 | Re: Add RESPECT/IGNORE NULLS and FROM FIRST/LAST options |