-
Notifications
You must be signed in to change notification settings - Fork 5
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
Prototype: Directly call parent node's function #161
base: main
Are you sure you want to change the base?
Conversation
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.
Few things i saw during meeting
// Operation 1: Reader [] => This is a generator for input data. | ||
#[derive(Getters, Setters)] | ||
pub struct Reader { | ||
node: Option<Weak<RwLock<ExecutionNode>>>, |
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.
node: Weak<RwLock<ExecutionNode>>,
?
Then we can check during upgrade like match node.upgrade() {Some(...) => ..., None => ...}
// Operation 2: Appender [] => Can have custom map implementations. | ||
#[derive(Getters, Setters)] | ||
pub struct Mapper { | ||
node: Option<Weak<RwLock<ExecutionNode>>>, |
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.
node: Weak<RwLock<ExecutionNode>>,
?
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.
I think we can get rid of the Option.
I initially added it to first create operation without a node. Then create the execution node with this operation while updating the node in the operation.
But Weak
itself should be good enough to support that.
let value = self.input_data.get(partition_num); | ||
if let Some(x) = value { | ||
Some(*x) | ||
} else { | ||
None | ||
} |
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.
Just return self.input_data.get(partition_num)
?
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.
So, turns out .get()
on a Vec<Item>
returns Option<&Item>
.
Directly returning the value mismatches the signature (Option<i64>
vs Option<&i64>
). That's why did the reconstruction.
But there should be some better way.
Initial prototype of directly calling parent node's function instead of passing data through channels.
Things currently working:
Things to add: