Skip to content

minor: Remove null_equals_null todo comment - #16045

Closed
jonathanc-n wants to merge 2 commits into
apache:mainfrom
jonathanc-n:null-aware-join
Closed

jonathanc-n wants to merge 2 commits into
apache:mainfrom
jonathanc-n:null-aware-join

Conversation

@jonathanc-n

Copy link
Copy Markdown
Contributor

Rationale for this change

While looking through the join implementations, I noticed that null_equals_null was marked for todo. Physical planner will not call on NestedLoopJoin if there is an equi condition.

@berkaysynnada

Copy link
Copy Markdown
Contributor

Doesn't it make a difference for <= and >= ?

@comphead

comphead commented May 14, 2025 •

Copy link
Copy Markdown
Contributor

I would still keep it.
In future the user can pick the NLJ by hint or planner itself can take NLJ if it considered the NLJ cost is smaller than HJ for some conditions for equijoin. What we probably should do is to create a ticket to support null_equals_null for NLJ and update todo with exact ticket link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants