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
[DO NOT REVIEW] Try issue 3166 #3404
base: master
Are you sure you want to change the base?
Conversation
2a406cc
to
e1b2108
Compare
d7a5d4f
to
f395a9d
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.
I don't think it's a good design to fill the flip vector inside rel table scan. the reason is simply because that info is passed down from operator, while not relying on anything inside storage. Better move the filling of flip vector into operators.
common::RelDataDirection direction; | ||
|
||
// This direction is not |
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.
Complete the sentence here.
void StructPackFunctions::undirectedRelPackExecFunc( | ||
const std::vector<std::shared_ptr<ValueVector>>& parameters, ValueVector& result, void*) { | ||
KU_ASSERT(parameters.size() > 1); | ||
// Force copy of |
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.
Complete the sentence.
@@ -33,6 +33,17 @@ void StructPackFunctions::compileFunc(FunctionBindData* /*bindData*/, | |||
} | |||
} | |||
|
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.
Better clearly state the implicit assumption of src
being 0 and dst
being 1 here.
namespace evaluator { | ||
|
||
class NodeRelExpressionEvaluator final : public ExpressionEvaluator { | ||
class PatternExpressionEvaluator : public ExpressionEvaluator { |
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.
Can u keep final
?
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.
Let's also rename the file to pattern_evaluator.h
.
I need to check this against CI first.