Skip to content

Conversation

@timsaucer
Copy link
Member

Which issue does this PR close?

#18158 introduced a regression in the table type created with DataFrame::into_view()

Rationale for this change

Correct regression

What changes are included in this PR?

One line fix

Are these changes tested?

Unit test added

Are there any user-facing changes?

None

@github-actions github-actions bot added the core Core DataFusion crate label Nov 11, 2025
@timsaucer timsaucer changed the title Bugfix/into view bugfix: correct regression on TableType for into_view Nov 11, 2025
@timsaucer
Copy link
Member Author

@r1b mind a quick review? One line change to the code, plus unit test

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Makes sense to me -- thanks @timsaucer

Comment on lines +1630 to +1632
let table_provider = df_impl.clone().into_view();
assert_eq!(table_provider.table_type(), TableType::View);
ctx.register_table("test_table", table_provider)?;
Copy link
Member

Choose a reason for hiding this comment

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

This TableType::View assert is the only one test check for into_view and it is in a test that looks like not added particularly for it. That's said it is very possible it will be forgotten easily.

Maybe just add a simple test specific for into_view?

alamb pushed a commit that referenced this pull request Nov 11, 2025
…F51 (#18618)

This is the same as #18617 but
targeted at the DF-51 release branch
@alamb
Copy link
Contributor

alamb commented Nov 11, 2025

Thanks @timsaucer @viirya and @Jefffrey -- great call to add a test

@alamb alamb added this pull request to the merge queue Nov 11, 2025
Merged via the queue into apache:main with commit a8f0d59 Nov 11, 2025
29 checks passed
@r1b
Copy link
Contributor

r1b commented Nov 11, 2025

Thank you @timsaucer

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

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants