Skip to content

feat: allow the creation VirtualTableScans with no rows #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
vbarua merged 5 commits into from
Jan 22, 2026
Merged

feat: allow the creation VirtualTableScans with no rows #682

vbarua merged 5 commits into from
Jan 22, 2026

Conversation

@gord02
Copy link
Contributor

@gord02 gord02 commented Jan 20, 2026 •
edited by benbellick
Loading

This PR removes the io.substrait.relation.EmptyScan class. It instead allows io.substrait.relation.VirtualTableScan to be empty.

Closes #673

BREAKING CHANGE: The io.substrait.relation.EmptyScan class has been removed.

@github-actions
Copy link

github-actions bot commented Jan 20, 2026

ACTION NEEDED

Substrait follows the Conventional Commits
specification for
release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@gord02 gord02 force-pushed the gordon.hamilton/removeEmptyScan branch from a66fe19 to 0c267da Compare January 20, 2026 15:54
@gord02 gord02 changed the title removing emptyScan in favor of emptyVirtualTableScan feat: removing emptyScan in favor of emptyVirtualTableScan Jan 20, 2026
benbellick
benbellick reviewed Jan 20, 2026
View reviewed changes
Copy link
Member

@benbellick benbellick left a comment

Choose a reason for hiding this comment

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

all looks reasonable to me! Only thing really is just putting that roundtrip test back in.

Also, maybe we will want to mark this as a breaking change since we are technically modifying the API here by dropping a public class. @vbarua

benbellick
benbellick reviewed Jan 21, 2026
View reviewed changes
benbellick
benbellick reviewed Jan 21, 2026
View reviewed changes
Copy link
Member

@benbellick benbellick left a comment

Choose a reason for hiding this comment

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

Just the one comment here and then I think it's great!
https://github.com/substrait-io/substrait-java/pull/682/files#r2712955699

@gord02 gord02 changed the title feat: removing emptyScan in favor of emptyVirtualTableScan breaking: removing emptyScan in favor of emptyVirtualTableScan Jan 21, 2026
asolimando
asolimando approved these changes Jan 21, 2026
View reviewed changes
Copy link
Contributor

@asolimando asolimando left a comment

Choose a reason for hiding this comment

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

I have checked only the isthum/Calcite part and LGTM, asking only for one extra test

benbellick
benbellick reviewed Jan 22, 2026
View reviewed changes
Copy link
Member

@benbellick benbellick left a comment

Choose a reason for hiding this comment

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

Just one tiny thing left!

@benbellick benbellick changed the title breaking: removing emptyScan in favor of emptyVirtualTableScan refactor: removing emptyScan in favor of emptyVirtualTableScan Jan 22, 2026
@benbellick benbellick changed the title refactor: removing emptyScan in favor of emptyVirtualTableScan refactor: removing emptyScan in favor of emptyVirtualTableScan Jan 22, 2026
@benbellick benbellick changed the title refactor: removing emptyScan in favor of emptyVirtualTableScan refactor(core): removing emptyScan in favor of emptyVirtualTableScan Jan 22, 2026
@benbellick benbellick self-requested a review January 22, 2026 15:05
benbellick
benbellick approved these changes Jan 22, 2026
View reviewed changes
Copy link
Member

@benbellick benbellick left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@vbarua vbarua changed the title refactor(core): removing emptyScan in favor of emptyVirtualTableScan refactor(core): replace EmptyScan usage with empty VirtualTable Jan 22, 2026
@vbarua vbarua changed the title refactor(core): replace EmptyScan usage with empty VirtualTable refactor: replace EmptyScan usage with empty VirtualTableScan Jan 22, 2026
@vbarua vbarua changed the title refactor: replace EmptyScan usage with empty VirtualTableScan feat: allow the creation VirtualTableScans with no rows Jan 22, 2026
@vbarua vbarua merged commit 32add4d into substrait-io:main Jan 22, 2026
32 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vbarua vbarua vbarua approved these changes

@benbellick benbellick benbellick approved these changes

+1 more reviewer

@asolimando asolimando asolimando approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Drop EmptyScan in favor of allowing empty VirtualTableScan

4 participants

@gord02 @vbarua @asolimando @benbellick