-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
profiling storage detail #2754
profiling storage detail #2754
Conversation
cangfengzhs
commented
Aug 30, 2021
•
edited
Loading
edited
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.
Generally LGTM
Add the detail to IndexScanExecutor as well.
Use camel style naming.
679db39
to
59a3a70
Compare
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.
LGTM
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.
LGTM, good job
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.
Please create a new PR to improve this
|
||
AggregateNode(RuntimeContext* context, IterateNode<T>* upstream, EdgeContext* edgeContext) | ||
: IterateNode<T>(upstream), context_(context), edgeContext_(edgeContext) {} | ||
: IterateNode<T>(upstream), context_(context), edgeContext_(edgeContext) { | ||
IterateNode<T>::name_ = "AggregateNode"; |
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 really don't recommend this way to initialize the fields of the base class. Please provide the constructor in the base class to do this, such as:
IterateNode<T>::IterateNode<T>(const std::string& name, ...) : name_(name) {}
AggregateNode(RuntimeContext* context, IterateNode<T>* upstream, EdgeContext* edgeContext)
: IterateNode("AggregateNode", upstream), context_(context)...{}
|
||
FetchEdgeNode(RuntimeContext* context, | ||
EdgeContext* edgeContext, | ||
EdgeType edgeType, | ||
const std::vector<PropContext>* props, | ||
StorageExpressionContext* expCtx = nullptr, | ||
Expression* exp = nullptr) | ||
: EdgeNode(context, edgeContext, edgeType, props, expCtx, exp) {} | ||
: EdgeNode(context, edgeContext, edgeType, props, expCtx, exp) { | ||
name_ = "FetchEdgeNode"; |
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.
ditto
@@ -44,18 +44,21 @@ class IndexOutputNode final : public RelNode<T> { | |||
fields_(fields) { | |||
type_ = context_->isEdge() ? IndexResultType::kEdgeFromIndexScan | |||
: IndexResultType::kVertexFromIndexScan; | |||
RelNode<T>::name_ = "IndexOpuputNode"; |
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.
ditto