Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: account for struct fields in VirtualTableScan check #255

Merged

Conversation

Blizzara
Copy link
Contributor

@Blizzara Blizzara commented May 3, 2024

Fixes #254 and adds a test

@CLAassistant
Copy link

CLAassistant commented May 3, 2024

CLA assistant check
All committers have signed the CLA.

@Blizzara Blizzara force-pushed the avo/allow-structs-in-virtual-tables branch from 6089c13 to 0097759 Compare May 3, 2024 15:58
@Blizzara Blizzara changed the title fix VirtualTableScan check to correctly count struct fields fix: account for struct fields in VirtualTableScan check May 3, 2024
@Blizzara
Copy link
Contributor Author

Blizzara commented May 7, 2024

Pending confirmation from my employer on the CLA - will get back to this in a week hopefully!

@Blizzara Blizzara force-pushed the avo/allow-structs-in-virtual-tables branch from 0097759 to d49a330 Compare May 18, 2024 06:31
@Blizzara
Copy link
Contributor Author

@vbarua @jacques-n I've signed the CLA and think this would be ready for review/to be merged!

Copy link
Member

@vbarua vbarua left a comment

Choose a reason for hiding this comment

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

One very small request around imports, but otherwise your changes look good.

@vbarua vbarua merged commit 3bbcf82 into substrait-io:main May 30, 2024
8 checks passed
@Blizzara Blizzara deleted the avo/allow-structs-in-virtual-tables branch May 30, 2024 19:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StructTypes in a VirtualTableScan
4 participants