-
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
Optimize input/var property expression to get property from tuple by #4242
Optimize input/var property expression to get property from tuple by #4242
Conversation
index to avoid hash table lookup.
…g/nebula into optimize/get-property-by-index
Origin benchmark:
Current benchmark:
|
…timize/get-property-by-index
const Value& InputPropertyExpression::eval(ExpressionContext& ctx) { | ||
return ctx.getInputProp(prop_); | ||
if (!propIndex_.has_value()) { | ||
auto indexResult = ctx.getInputPropIndex(prop_); | ||
if (!indexResult.ok()) { | ||
return Value::kEmpty; | ||
} | ||
propIndex_ = indexResult.value(); | ||
} | ||
return ctx.getColumn(propIndex_.value()); | ||
} |
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 remember we have a ColumnIndexExpr
? It looks like that rewriteing InputPropExpr
to ColumnIndexExpr
is more easy
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.
Good job, we can add an optimize rule to do it. But current optimization is also useful now and no need to rewrite expression.
index to avoid hash table lookup.
What type of PR is this?
What problem(s) does this PR solve?
Issue(s) number:
Close #4174
Description:
How do you solve it?
Special notes for your reviewer, ex. impact of this fix, design document, etc:
Checklist:
Tests:
Affects:
Release notes:
Please confirm whether to be reflected in release notes and how to describe: