-
Notifications
You must be signed in to change notification settings - Fork 74
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
Conversation
BREAKING CHANGE: RelCopyOnWriteVisitor now extends RelVisitor
core/src/main/java/io/substrait/relation/ExprCopyOnWriteVisitor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/substrait/relation/RelCopyOnWriteVisitor.java
Outdated
Show resolved
Hide resolved
db95285
to
ba112ff
Compare
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. |
There was a problem hiding this 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
core/src/main/java/io/substrait/relation/CopyOnWriteVisitor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/substrait/relation/CopyOnWriteVisitor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/substrait/relation/CopyOnWriteVisitor.java
Outdated
Show resolved
Hide resolved
core/src/main/java/io/substrait/relation/ExpressionCopyOnWriteVisitor.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks Victor!
BREAKING CHANGE: RelCopyOnWriteVisitor now extends RelVisitor and has generic type parameter
BREAKING CHANGE: RelCopyOnWriteVisitor now extends RelVisitor and has generic type parameter