10000 Bug fix/internal issue #316 by gnusi · Pull Request #7911 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

Bug fix/internal issue #316 #7911

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.

Sign up for GitHub

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

Merged
merged 26 commits into from
Jan 10, 2019
Merged

Bug fix/internal issue #316 #7911

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
714c08f
allow using scorers outside ArangoSearch view context
gnusi Dec 21, 2018
9bd05ae
ensure query is properly optimized after replacement of scorer functions
gnusi Dec 23, 2018
104c9db
do not apply `handleViewsRule` to queries without views
gnusi Dec 24, 2018
30a9e29
simplify optimization rule for ArangoSearch views
gnusi Dec 24, 2018
b440b63
show ArangoSearch view scorers in query explanation
gnusi Dec 25, 2018
31d273c
fix tests
gnusi Dec 25, 2018
0e5da98
Merge branch 'devel' of https://github.com/arangodb/arangodb into bug…
gnusi Dec 25, 2018
a6d1805
fix tests
gnusi Dec 26, 2018
3f985c0
add stub for scorer related tests
gnusi Dec 26, 2018
cffbead
Merge branch 'devel' of https://github.com/arangodb/arangodb into bug…
gnusi Jan 8, 2019
aa3ca2e
reformat
gnusi Jan 8, 2019
e0591ca
check variable depth in `ViewExpressionContext::getVariableValue`
gnusi Jan 8, 2019
6c7ef9e
add some tests
gnusi Jan 8, 2019
08504d2
address js test failures
gnusi Jan 8, 2019
a8bda86
address jslint errors
gnusi Jan 9, 2019
d930573
ensure `IResearchViewNode` exposes variables used in scorers
gnusi Jan 9, 2019
5733b0a
ensure scorers with expressions are deduplicated
gnusi Jan 9, 2019
4030503
fix deduplication for indexed access
gnusi Jan 10, 2019
ece771e
more tests
gnusi Jan 10, 2019
6ebe204
partially address review comments
gnusi Jan 10, 2019
6772e87
address review comments
gnusi Jan 10, 2019
99779ce
simplify code
gnusi Jan 10, 2019
bb0d969
remove irrelevant, commented out code
gnusi Jan 10, 2019
b8ccf1b
merge
gnusi Jan 10, 2019
f3af469
ensure array comparisons are properly handled
gnusi Jan 10, 2019
886f19f
update changelog & loki
gnusi Jan 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ensure array comparisons are properly handled
  • Loading branch information
@gnusi
gnusi committed Jan 10, 2019
commit f3af469c5137be00290dc04937bd77ab7af8521c
29 changes: 27 additions & 2 deletions arangod/IResearch/AqlHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,15 @@ bool equalTo(aql::AstNode const* lhs, aql::AstNode const* rhs) {
case aql::NODE_TYPE_OBJECT:
case aql::NODE_TYPE_CALCULATED_OBJECT_ELEMENT:
case aql::NODE_TYPE_ARRAY:
case aql::NODE_TYPE_RANGE: {
case aql::NODE_TYPE_RANGE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_NE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_LT:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_LE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_GT:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_GE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_IN:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_NIN: {
return true;
}

Expand Down Expand Up @@ -143,6 +151,10 @@ bool equalTo(aql::AstNode const* lhs, aql::AstNode const* rhs) {
return lhsName == rhsName;
}

case aql::NODE_TYPE_QUANTIFIER: {
return lhs->value.value._int == rhs->value.value._int;
}

default: {
return false;
}
Expand Down Expand Up @@ -201,7 +213,15 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
case aql::NODE_TYPE_ARRAY:
case aql::NODE_TYPE_OBJECT:
case aql::NODE_TYPE_CALCULATED_OBJECT_ELEMENT:
case aql::NODE_TYPE_RANGE: {
case aql::NODE_TYPE_RANGE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_NE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_LT:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_LE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_GT:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_GE:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_IN:
case aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_NIN: {
return hash;
}

Expand Down Expand Up @@ -251,6 +271,11 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
node->getStringLength(), hash);
}

case aql::NODE_TYPE_QUANTIFIER: {
return fasthash64(static_cast<const void*>(&node->value.value._int),
sizeof(node->value.value._int), hash);
}

default: {
return fasthash64(static_cast<void const*>(&node), sizeof(&node), hash);
}
Expand Down
172 changes: 172 additions & 0 deletions tests/IResearch/IResearchQueryScorer-test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,178 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") {
}
}

// ensure scorers are deduplicated (array comparison operators)
{
std::string const queryString =
"LET obj = _NONDETERM_([ 2, 5 ]) "
"FOR d IN testView SEARCH d.name >= 'A' AND d.name <= 'C' "
"RETURN [ customscorer(d, obj any == 3), customscorer(d, obj any == 3) ]";

CHECK(arangodb::tests::assertRules(
vocbase, queryString, {
arangodb::aql::OptimizerRule::handleArangoSearchViewsRule,
}
));

arangodb::aql::Query query(
false,
vocbase,
arangodb::aql::QueryString(queryString),
std::shared_ptr<arangodb::velocypack::Builder>(),
arangodb::velocypack::Parser::fromJson("{}"),
arangodb::aql::PART_MAIN
);

query.prepare(arangodb::QueryRegistryFeature::registry());
auto* plan = query.plan();
REQUIRE(plan);

arangodb::SmallVector<arangodb::aql::ExecutionNode*>::allocator_type::arena_type a;
arangodb::SmallVector<arangodb::aql::ExecutionNode*> nodes{a};

// only one scorer
plan->findNodesOfType(nodes, arangodb::aql::ExecutionNode::ENUMERATE_IRESEARCH_VIEW, true);
REQUIRE(1 == nodes.size());
auto* viewNode = arangodb::aql::ExecutionNode::castTo<arangodb::iresearch::IResearchViewNode*>(nodes.front());
REQUIRE(viewNode);
auto& scorers = viewNode->scorers();
REQUIRE(1 == scorers.size());
auto* var = scorers.front().var;
REQUIRE(var);

// check scorer
{
auto* expr = scorers.front().node;
REQUIRE(expr);
REQUIRE(arangodb::aql::NODE_TYPE_FCALL == expr->type);
auto* fn = static_cast<arangodb::aql::Function*>(expr->getData());
REQUIRE(fn);
REQUIRE(arangodb::iresearch::isScorer(*fn));
CHECK("CUSTOMSCORER" == fn->name);

REQUIRE(1 == expr->numMembers());
auto* args = expr->getMember(0);
REQUIRE(args);
REQUIRE(arangodb::aql::NODE_TYPE_ARRAY == args->type);
REQUIRE(2 == args->numMembers());
auto* arg0 = args->getMember(0); // reference to d
REQUIRE(arg0);
REQUIRE(static_cast<void const*>(&viewNode->outVariable()) == arg0->getData());
auto* arg1 = args->getMember(1);
REQUIRE(arg1);
CHECK(arangodb::aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ == arg1->type);
}

// and 2 references
nodes.clear();
plan->findNodesOfType(nodes, arangodb::aql::ExecutionNode::CALCULATION, true);
REQUIRE(2 == nodes.size());
for (auto const* node : nodes) {
auto* calcNode = arangodb::aql::ExecutionNode::castTo<arangodb::aql::CalculationNode const*>(node);
REQUIRE(calcNode);
REQUIRE(calcNode->expression());

if (calcNode->outVariable()->name == "obj") {
continue;
}

auto* exprNode = calcNode->expression()->node();
REQUIRE(exprNode);
REQUIRE(arangodb::aql::NODE_TYPE_ARRAY == exprNode->type);
REQUIRE(2 == exprNode->numMembers());
for (size_t i = 0; i < exprNode->numMembers(); ++i) {
auto* sub = exprNode->getMember(i);
REQUIRE(arangodb::aql::NODE_TYPE_REFERENCE == sub->type);
CHECK(static_cast<const void*>(var) == sub->getData());
}
}
}

// ensure scorers aren't deduplicated (array comparison operator)
{
std::string const queryString =
"LET obj = _NONDETERM_([ 2, 5 ]) "
"FOR d IN testView SEARCH d.name >= 'A' AND d.name <= 'C' "
"RETURN [ customscorer(d, obj any == 3), customscorer(d, obj all == 3) ]";

CHECK(arangodb::tests::assertRules(
vocbase, queryString, {
arangodb::aql::OptimizerRule::handleArangoSearchViewsRule,
}
));

arangodb::aql::Query query(
false,
vocbase,
arangodb::aql::QueryString(queryString),
std::shared_ptr<arangodb::velocypack::Builder>(),
arangodb::velocypack::Parser::fromJson("{}"),
arangodb::aql::PART_MAIN
);

query.prepare(arangodb::QueryRegistryFeature::registry());
auto* plan = query.plan();
REQUIRE(plan);

arangodb::SmallVector<arangodb::aql::ExecutionNode*>::allocator_type::arena_type a;
arangodb::SmallVector<arangodb::aql::ExecutionNode*> nodes{a};

// only one scorer
plan->findNodesOfType(nodes, arangodb::aql::ExecutionNode::ENUMERATE_IRESEARCH_VIEW, true);
REQUIRE(1 == nodes.size());
auto* viewNode = arangodb::aql::ExecutionNode::castTo<arangodb::iresearch::IResearchViewNode*>(nodes.front());
REQUIRE(viewNode);
auto& scorers = viewNode->scorers();
REQUIRE(2 == scorers.size());
auto* var = scorers.front().var;
REQUIRE(var);

// check scorers
{
auto* expr = scorers[0].node;
REQUIRE(expr);
REQUIRE(arangodb::aql::NODE_TYPE_FCALL == expr->type);
auto* fn = static_cast<arangodb::aql::Function*>(expr->getData());
REQUIRE(fn);
REQUIRE(arangodb::iresearch::isScorer(*fn));
CHECK("CUSTOMSCORER" == fn->name);

REQUIRE(1 == expr->numMembers());
auto* args = expr->getMember(0);
REQUIRE(args);
REQUIRE(arangodb::aql::NODE_TYPE_ARRAY == args->type);
REQUIRE(2 == args->numMembers());
auto* arg0 = args->getMember(0); // reference to d
REQUIRE(arg0);
REQUIRE(static_cast<void const*>(&viewNode->outVariable()) == arg0->getData());
auto* arg1 = args->getMember(1);
REQUIRE(arg1);
CHECK(arangodb::aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ == arg1->type);
}

{
auto* expr = scorers[1].node;
REQUIRE(expr);
REQUIRE(arangodb::aql::NODE_TYPE_FCALL == expr->type);
auto* fn = static_cast<arangodb::aql::Function*>(expr->getData());
REQUIRE(fn);
REQUIRE(arangodb::iresearch::isScorer(*fn));
CHECK("CUSTOMSCORER" == fn->name);

REQUIRE(1 == expr->numMembers());
auto* args = expr->getMember(0);
REQUIRE(args);
REQUIRE(arangodb::aql::NODE_TYPE_ARRAY == args->type);
REQUIRE(2 == args->numMembers());
auto* arg0 = args->getMember(0); // reference to d
REQUIRE(arg0);
REQUIRE(static_cast<void const*>(&viewNode->outVariable()) == arg0->getData());
auto* arg1 = args->getMember(1);
REQUIRE(arg1);
CHECK(arangodb::aql::NODE_TYPE_OPERATOR_BINARY_ARRAY_EQ == arg1->type);
}
}

// con't deduplicate scorers with default values
{
std::string const queryString =
Expand Down
0