Skip to content

Conversation

@kumarUjjawal
Copy link
Contributor

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the physical-plan Changes to the physical-plan crate label Nov 15, 2025
@kumarUjjawal kumarUjjawal changed the title refactor with assert_or_internal_err!() in datafusion/physical-plan minor: refactor with assert_or_internal_err!() in datafusion/physical-plan Nov 15, 2025
Comment on lines 915 to 917
if self.mode == PartitionMode::Partitioned && left_partitions != right_partitions
{
return internal_err!(
if self.mode == PartitionMode::Partitioned {
assert_eq_or_internal_err!(
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're gonna do this refactor we should refactor this entire if statement away, instead of only moving half the condition inside it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"Column {:?} is not found in the original schema of the TestMemoryExec",
ambiguous_column
.as_ref()
.expect("checked by assert_or_internal_err!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Just unwrap it, expect here adds unnecessary noise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@2010YOUY01 2010YOUY01 enabled auto-merge November 16, 2025 02:02
@2010YOUY01 2010YOUY01 added this pull request to the merge queue Nov 16, 2025
Merged via the queue into apache:main with commit 85777c4 Nov 16, 2025
32 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants