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

feat: exhaustive copy on write visitors #199

Merged
merged 11 commits into from
Nov 7, 2023
Merged

Conversation

vbarua
Copy link
Member

@vbarua vbarua commented Nov 7, 2023

BREAKING CHANGE: RelCopyOnWriteVisitor now extends RelVisitor and has generic type parameter

BREAKING CHANGE: RelCopyOnWriteVisitor now extends RelVisitor
@vbarua vbarua force-pushed the vbarua/copy-on-write-updates branch from db95285 to ba112ff Compare November 7, 2023 00:56
@vbarua vbarua marked this pull request as ready for review November 7, 2023 16:13
@vbarua
Copy link
Member Author

vbarua commented Nov 7, 2023

These changes introduce a new public ExpressionCopyOnWriteVisitor class and switch the RelCopyOnWriteVisitor over to use the RelVisitor (instead of AbstractRelVisitor) to force the implementation of all visit methods.

The purpose of this is to ensure a complete visitation over relations and expressions in order to enable relation rewrites THROUGH subqueries in expressions.

Copy link
Contributor

@patientstreetlight patientstreetlight left a comment

Choose a reason for hiding this comment

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

Mostly minor comments and a question

Copy link
Contributor

@patientstreetlight patientstreetlight left a comment

Choose a reason for hiding this comment

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

:shipit: thanks Victor!

@vbarua vbarua merged commit 39c56ab into main Nov 7, 2023
7 checks passed
@vbarua vbarua deleted the vbarua/copy-on-write-updates branch November 7, 2023 21:08
ajegou pushed a commit to ajegou/substrait-java that referenced this pull request Mar 29, 2024
BREAKING CHANGE: RelCopyOnWriteVisitor now extends RelVisitor and has generic type parameter
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