10000 Bug fix/aql auto keep on collect (#4795) · mnemosdev/arangodb@b325d05 · GitHub
[go: up one dir, main page]
Skip to content

Commit b325d05

Browse files
jsteemannjsteemann
authored
Bug fix/aql auto keep on collect (arangodb#4795)
* automatically detect which variables to keep in AQL COLLECT If a COLLECT INTO is used, then it is detected which sub-attributes of the into variables are used later in the query, and automatic KEEP instructions are added to the COLLECT if possible Example query: FOR doc1 IN collection1 FOR doc2 IN collection2 COLLECT x = doc1.x INTO g RETURN { x, all: g[*].doc1.y } would be turned into FOR doc1 IN collection1 FOR doc2 IN collection2 COLLECT x = doc1.x INTO g KEEP doc1 RETURN { x, all: g[*].doc1.y } and prevent `doc2` from being temporarily stored in the variable `g` * fix runtime warnings * increase default flush timeout * added tests
1 parent 4486f89 commit b325d05

16 files changed

+332
-211
lines changed

‎arangod/Aql/Ast.cpp

Lines changed: 161 additions & 84 deletions
Large diffs are not rendered by default.

‎arangod/Aql/Ast.h

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ class Ast {
404404
/// @brief determines the top-level attributes in an expression, grouped by
405405
/// variable
406406
static TopLevelAttributes getReferencedAttributes(AstNode const*, bool&);
407+
static std::unordered_set<std::string> getReferencedAttributesForKeep(AstNode const*, Variable const* searchVariable, bool&);
407408

408409
static bool populateSingleAttributeAccess(AstNode const* node,
409410
Variable const* variable,
@@ -446,8 +447,7 @@ class Ast {
446447

447448
/// @brief traverse the AST using a depth-first visitor
448449
static AstNode* traverseAndModify(AstNode*,
449-
std::function<AstNode*(AstNode*, void*)>,
450-
void*);
450+
std::function<AstNode*(AstNode*)> const&);
451451

452452
private:
453453
/// @brief make condition from example
@@ -512,22 +512,19 @@ class Ast {
512512
public:
513513
/// @brief traverse the AST, using pre- and post-order visitors
514514
static AstNode* traverseAndModify(AstNode*,
515-
std::function<bool(AstNode const*, void*)>,
516-
std::function<AstNode*(AstNode*, void*)>,
517-
std::function<void(AstNode const*, void*)>,
518-
void*);
515+
std::function<bool(AstNode const*)> const&,
516+
std::function<AstNode*(AstNode*)> const&,
517+
std::function<void(AstNode const*)> const&);
519518

520519
/// @brief traverse the AST, using pre- and post-order visitors
521520
static void traverseReadOnly(AstNode const*,
522-
std::function<void(AstNode const*, void*)>,
523-
std::function<void(AstNode const*, void*)>,
524-
std::function<void(AstNode const*, void*)>,
525-
void*);
521+
std::function<bool(AstNode const*)> const&,
522+
std::function<void(AstNode const*)> const&);
526523

527524
/// @brief traverse the AST using a depth-first visitor, with const nodes
528525
static void traverseReadOnly(AstNode const*,
529-
std::function<void(AstNode const*, void*)>,
530-
void*);
526+
std::function<void(AstNode const*)> const&);
527+
531528
private:
532529
/// @brief normalize a function name
533530
std::pair<std::string, bool> normalizeFunctionName(char const* functionName, size_t length);

‎arangod/Aql/AstNode.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,11 +1109,11 @@ void AstNode::toVelocyPack(VPackBuilder& builder, bool verbose) const {
11091109

11101110
TRI_ASSERT(variable != nullptr);
11111111
builder.add("name", VPackValue(variable->name));
1112-
builder.add("id", VPackValue(static_cast<double>(variable->id)));
1112+
builder.add("id", VPackValue(variable->id));
11131113
}
11141114

11151115
if (type == NODE_TYPE_EXPANSION) {
1116-
builder.add("levels", VPackValue(static_cast<double>(getIntValue(true))));
1116+
builder.add("levels", VPackValue(getIntValue(true)));
11171117
}
11181118

11191119
// dump sub-nodes

‎arangod/Aql/CollectNode.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,21 @@ class CollectNode : public ExecutionNode {
179179
TRI_ASSERT(!hasExpressionVariable());
180180
_expressionVariable = variable;
181181
}
182+
183+
/// @brief return whether or not the collect has keep variables
184+
bool hasKeepVariables() const {
185+
return !_keepVariables.empty();
186+
}
187+
188+
/// @brief return the keep variables
189+
std::vector<Variable const*> const& keepVariables() const {
190+
return _keepVariables;
191+
}
192+
193+
/// @brief set list of variables to keep if INTO is used
194+
void setKeepVariables(std::vector<Variable const*>&& variables) {
195+
_keepVariables = std::move(variables);
196+
}
182197

183198
/// @brief return the variable map
184199
std::unordered_map<VariableId, std::string const> const& variableMap() const {

‎arangod/Aql/ConditionFinder.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,13 +180,12 @@ bool ConditionFinder::handleFilterCondition(
180180
// expressions represented by the variables
181181
var = it.second->clone(_plan->getAst());
182182

183-
auto func = [&](AstNode* node, void* data) -> AstNode* {
183+
auto func = [&](AstNode* node) -> AstNode* {
184184
if (node->type == NODE_TYPE_REFERENCE) {
185-
auto plan = static_cast<ExecutionPlan*>(data);
186185
auto variable = static_cast<Variable*>(node->getData());
187186

188187
if (variable != nullptr) {
189-
auto setter = plan->getVarSetBy(variable->id);
188+
auto setter = _plan->getVarSetBy(variable->id);
190189

191190
if (setter != nullptr && setter->getType() == EN::CALCULATION) {
192191
auto s = static_cast<CalculationNode*>(setter);
@@ -202,7 +201,7 @@ bool ConditionFinder::handleFilterCondition(
202201
return node;
203202
};
204203

205-
var = Ast::traverseAndModify(var, func, _plan);
204+
var = Ast::traverseAndModify(var, func);
206205
}
207206
condition->andCombine(var);
208207
foundCondition = true;

‎arangod/Aql/ExecutionNode.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,16 @@ class ExecutionNode {
452452
}
453453
return ids;
454454
}
455+
456+
/// @brief tests whether the node sets one of the passed variables
457+
bool setsVariable(std::unordered_set<Variable const*> const& which) const {
458+
for (auto const& v : getVariablesSetHere()) {
459+
if (which.find(v) != which.end()) {
460+
return true;
461+
}
462+
}
463+
return false;
464+
}
455465

456466
/// @brief setVarsUsedLater
457467
void setVarsUsedLater(std::unordered_set<Variable const*>& v) {

‎arangod/Aql/ExecutionPlan.cpp

Lines changed: 4 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ ExecutionNode* ExecutionPlan::createCalculation(
387387
// (that are of type NODE_TYPE_COLLECTION) with their string equivalents
388388
// for example, this will turn `WITHIN(collection, ...)` into
389389
// `WITHIN("collection", ...)`
390-
auto visitor = [this, &containsCollection](AstNode* node, void*) {
390+
auto visitor = [this, &containsCollection](AstNode* node) {
391391
if (node->type == NODE_TYPE_FCALL) {
392392
auto func = static_cast<Function*>(node->getData());
393393

@@ -414,13 +414,13 @@ ExecutionNode* ExecutionPlan::createCalculation(
414414
};
415415

416416
// replace NODE_TYPE_COLLECTION function call arguments in the expression
417-
auto node = Ast::traverseAndModify(const_cast<AstNode*>(expression), visitor, nullptr);
417+
auto node = Ast::traverseAndModify(const_cast<AstNode*>(expression), visitor);
418418

419419
if (containsCollection) {
420420
// we found at least one occurence of NODE_TYPE_COLLECTION
421421
// now replace them with proper (FOR doc IN collection RETURN doc)
422422
// subqueries
423-
auto visitor = [this, &previous](AstNode* node, void*) {
423+
auto visitor = [this, &previous](AstNode* node) {
424424
if (node->type == NODE_TYPE_COLLECTION) {
425425
// create an on-the-fly subquery for a full collection access
426426
AstNode* rootNode = _ast->createNodeSubquery();
@@ -454,7 +454,7 @@ ExecutionNode* ExecutionPlan::createCalculation(
454454
};
455455

456456
// replace remaining NODE_TYPE_COLLECTION occurrences in the expression
457-
node = Ast::traverseAndModify(node, visitor, nullptr);
457+
node = Ast::traverseAndModify(node, visitor);
458458
}
459459

460460
if (!isDistinct && node->type == NODE_TYPE_REFERENCE) {
@@ -1771,59 +1771,6 @@ void ExecutionPlan::findEndNodes(SmallVector<ExecutionNode*>& result,
17711771
root()->walk(&finder);
17721772
}
17731773

1774-
/// @brief check linkage of execution plan
1775-
#if 0
1776-
class LinkChecker : public WalkerWorker<ExecutionNode> {
1777-
1778-
public:
1779-
LinkChecker () {
1780-
}
1781-
1782-
bool before (ExecutionNode* en) {
1783-
auto deps = en->getDependencies();
1784-
for (auto x : deps) {
1785-
auto parents = x->getParents();
1786-
bool ok = false;
1787-
for (auto it = parents.begin(); it != parents.end(); ++it) {
1788-
if (*it == en) {
1789-
ok = true;
1790-
break;
1791-
}
1792-
}
1793-
if (! ok) {
1794-
std::cout << "Found dependency which does not have us as a parent!"
1795-
<< std::endl;
1796-
}
1797-
}
1798-
auto parents = en->getParents();
1799-
if (parents.size() > 1) {
1800-
std::cout << "Found a node with more than one parent!" << std::endl;
1801-
}
1802-
for (auto x : parents) {
1803-
auto deps = x->getDependencies();
1804-
bool ok = false;
1805-
for (auto it = deps.begin(); it != deps.end(); ++it) {
1806-
if (*it == en) {
1807-
ok = true;
1808-
break;
1809-
}
1810-
}
1811-
if (! ok) {
1812-
std::cout << "Found parent which does not have us as a dependency!"
1813-
<< std::endl;
1814-
}
1815-
}
1816-
return false;
1817-
}
1818-
};
1819-
1820-
void ExecutionPlan::checkLinkage () {
1821-
LinkChecker checker;
1822-
root()->walk(&checker);
1823-
}
1824-
1825-
#endif
1826-
18271774
/// @brief helper struct for findVarUsage
18281775
struct VarUsageFinder final : public WalkerWorker<ExecutionNode> {
18291776
std::unordered_set<Variable const*> _usedLater;

‎arangod/Aql/ExecutionPlan.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,6 @@ class ExecutionPlan {
166166
void findEndNodes(SmallVector<ExecutionNode*>& result,
167167
bool enterSubqueries) const;
168168

169-
/// @brief check linkage
170-
#if 0
171-
void checkLinkage();
172-
#endif
173-
174169
/// @brief determine and set _varsUsedLater and _valid and _varSetBy
175170
void findVarUsage();
176171

‎arangod/Aql/Expression.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ AqlValue Expression::execute(transaction::Methods* trx, ExpressionContext* ctx,
123123

124124
TRI_ASSERT(_type != UNPROCESSED);
125125
_expressionContext = ctx;
126-
126+
127127
// and execute
128128
switch (_type) {
129129
case JSON: {

‎arangod/Aql/Expression.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class Expression {
6969
~Expression();
7070

7171
/// @brief replace the root node
72-
void replaceNode (AstNode* node) {
72+
void replaceNode(AstNode* node) {
7373
_node = node;
7474
invalidate();
7575
}

0 commit comments

Comments
 (0)
0