Skip to content
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.

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

fix: virtualTable expression should represent a row of expression #727

Merged

Conversation

anshuldata
Copy link
Contributor

@anshuldata anshuldata commented Oct 28, 2024

BREAKING CHANGE: changes the message type for Expressions field in VirtualTable

Fixed issue mentioned in this
I have fixed it by fixing recently introduced field. Though this is a breaking change but since this field is introduced very recently so I assume this won't have many consumers. So, instead of having another field I decided to fix existing field

@anshuldata anshuldata changed the title feat: fix VirtualTable to represent to accomodate multiple columns feat: fix VirtualTable to represent to accomodate multiple columns of expression type Oct 28, 2024
@anshuldata anshuldata changed the title feat: fix VirtualTable to represent to accomodate multiple columns of expression type feat(type)!: fix VirtualTable to represent to accommodate multiple columns of expression type Oct 28, 2024
@anshuldata anshuldata changed the title feat(type)!: fix VirtualTable to represent to accommodate multiple columns of expression type fix: fix VirtualTable to represent to accommodate multiple columns of expression type Oct 28, 2024
@anshuldata anshuldata changed the title fix: fix VirtualTable to represent to accommodate multiple columns of expression type fix: VirtualTable expression should represent a row of expression Oct 28, 2024
Copy link

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.

Copy link
Contributor

@jacques-n jacques-n left a comment

Choose a reason for hiding this comment

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

Oops!

Makes sense.

@jacques-n
Copy link
Contributor

@vbarua @EpsilonPrime @westonpace would be good to get your eyes on this so we can fix quickly

@anshuldata anshuldata changed the title fix: VirtualTable expression should represent a row of expression fix: virtualTable expression should represent a row of expression Oct 29, 2024
@anshuldata anshuldata merged commit a2df42c into substrait-io:main Oct 29, 2024
21 of 25 checks passed
@anshuldata anshuldata deleted the anshul/FixValueTableExpression branch October 29, 2024 09:45
@jacques-n
Copy link
Contributor

@anshuldata , this shouldn't have been merged until we had at least 2 SMC approvals since it is a spec change. Please refer to https://substrait.io/governance/#substrait-voting-process in future to make sure we're following process. Thanks!

@anshuldata
Copy link
Contributor Author

his shouldn't have been merged until we had at least 2 SMC approvals since it is a spec change

got it. I will be careful in future. Thanks for pointing it out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants