Skip to content

fix: ProjectionPushdown internal error on NestedLoopJoin mark joins#22902

Merged
kosiew merged 6 commits into
apache:mainfrom
lyne7-sc:fix/mark_join_pushdown
Jun 18, 2026
Merged

fix: ProjectionPushdown internal error on NestedLoopJoin mark joins#22902
kosiew merged 6 commits into
apache:mainfrom
lyne7-sc:fix/mark_join_pushdown

Conversation

@lyne7-sc

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

A query whose subquery becomes a mark join (LeftMark/RightMark) can panic during the ProjectionPushdown physical optimization with an internal assertion error.

The pushdown helper try_pushdown_through_join assumes the join output schema is the plain concatenation of its two children (left ++ right) and uses join_table_borders to split the projected columns into a left group and a right group by column index. Mark joins break this assumption: they append an extra mark boolean column that does not originate from either child, so the column-index split misroutes columns to the wrong side and the subsequent child-projection rewrite fails its name-match assertion.

What changes are included in this PR?

In HashJoinExec::try_swapping_with_projection and NestedLoopJoinExec::try_swapping_with_projection, skip the try_pushdown_through_join path for mark joins (LeftMark/RightMark) and fall through to embedding the projection into the join instead.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes, only bug fixes.

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) physical-plan Changes to the physical-plan crate labels Jun 11, 2026

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lyne7-sc
Thanks for the fix. I do not see any blocking issues, just one small cleanup suggestion.

}

// TODO: split by `col`/`JoinSide` instead so mark joins can also push down to children.
let is_mark_join =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit: is_mark_join is only used once here, so I think this could be a little more direct if the matches! guard were inlined where the child pushdown is skipped.

if !matches!(self.join_type(), JoinType::LeftMark | JoinType::RightMark)
    && let Some(JoinData { ... }) = try_pushdown_through_join(...)?
{
    ...
} else {
    try_embed_projection(projection, self)
}

The same cleanup looks like it would apply to NestedLoopJoinExec too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I’ve inlined the guard here.

@kosiew kosiew added this pull request to the merge queue Jun 18, 2026
Merged via the queue into apache:main with commit 2836480 Jun 18, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProjectionPushdown internal error on NestedLoopJoin mark joins

2 participants