-
Notifications
You must be signed in to change notification settings - Fork 8
Implementation of cansplitpush, repeat_operator, replace_and_remove_nodes and find_lowest_nodes #245
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
base: main
Are you sure you want to change the base?
Conversation
|
@kylebd99 I have changed the codebase as requested. |
kylebd99
left a comment
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.
Hey, sorry for the slow review, I think everything looks good. I had a couple of very minor comments, but I'm going to go ahead and approve it. Please make the changes, then go ahead and merge.
| ["A"], | ||
| ), | ||
| # Nested case: | ||
| # root = MapJoin(mul, [A(i,j), B(i)]), reduce over i → [A] |
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.
Nit: I think you mean B(j) in this comment.
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.
Further, I'd appreciate 1 additional nested case where B has the index "i" which returns the mapjoin as the lowest root.
Hi! I have implemented cansplitpush, repeat_operator, replace_and_remove_nodes and find_lowest_nodes and have written test for every one of the functions. I have changed the backbone of annotated_query to make its semantic clearer. I have also file an issue for each function.
Fix #241, Fix #242, Fix #243 & Fix #244
Thank you for your time!!