8000 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
fix tests
  • Loading branch information
@gnusi
gnusi committed Dec 26, 2018
commit a6d18057c6bb504e969c98765ffcdb021e459732
96 changes: 28 additions & 68 deletions arangod/IResearch/AqlHelper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,53 +152,39 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
return hash;
}

auto hashMembers = [](aql::AstNode const& node, size_t& hash) {
for (size_t i = 0, n = node.numMembers(); i < n; ++i) {
auto sub = node.getMemberUnchecked(i);

if (sub) {
hash = iresearch::hash(sub, hash);
}
}

return hash;
};

switch (node->type) {
case aql::NODE_TYPE_VARIABLE: {
hash = fasthash64(static_cast<const void*>("variable"), 8, hash);
return fasthash64(node->getData(), sizeof(void*), hash);
}

case aql::NODE_TYPE_ATTRIBUTE_ACCESS: {
hash = fasthash64(static_cast<const void*>("access"), 6, hash);

// FIXME

struct Visitor {
explicit Visitor(size_t& hash) noexcept
: hash(hash) {
}

bool attributeAccess(arangodb::aql::AstNode const& node) noexcept {
irs::string_ref value;
parseValue(value, node);

hash = fasthash64(static_cast<const void*>("attribute"), 9, hash);
hash = fasthash64(value.c_str(), value.size(), hash);
return true;
}

bool expansion(arangodb::aql::AstNode const&) noexcept {
hash = fasthash64(static_cast<const void*>("*"), 1, hash);
return true;
}

bool indexAccess(arangodb::aql::AstNode const& node) noexcept {
hash = fasthash64(static_cast<const void*>("index"), 5, hash);
hash = iresearch::hash(&node, hash);
return true;
}

size_t& hash;
} hasher(hash);

aql::AstNode const* head = nullptr;
visitAttributeAccess(head, node, hasher);
hash = fasthash64(static_cast<const void*>("attribute access"), 16, hash);
hash = aql::AstNode(node->value).hashValue(hash);
return hashMembers(*node, hash);
}

if (head) {
hash = fasthash64(head, sizeof(head), hash);
}
case aql::NODE_TYPE_INDEXED_ACCESS: {
hash = fasthash64(static_cast<const void*>("indexed access"), 14, hash);
hash = aql::AstNode(node->value).hashValue(hash);
return hashMembers(*node, hash);
}

return hash;
case aql::NODE_TYPE_EXPANSION: {
hash = fasthash64(static_cast<const void*>("*"), 1, hash);
return hashMembers(*node, hash);
}

case aql::NODE_TYPE_VALUE: {
Expand All @@ -225,14 +211,7 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {

case aql::NODE_TYPE_ARRAY: {
hash = fasthash64(static_cast<const void*>("array"), 5, hash);
for (size_t i = 0, n = node->numMembers(); i < n; ++i) {
auto sub = node->getMemberUnchecked(i);

if (sub) {
hash = iresearch::hash(sub, hash);
}
}
return hash;
return hashMembers(*node, hash);
}

case aql::NODE_TYPE_OBJECT: {
Expand Down Expand Up @@ -265,38 +244,19 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
hash = fasthash64(static_cast<const void*>("fcall"), 5, hash);
hash = fasthash64(node->getData(), sizeof(void*), hash);
hash = fasthash64(fn->name.c_str(), fn->name.size(), hash);
for (size_t i = 0, n = node->numMembers(); i < n; ++i) {
auto sub = node->getMemberUnchecked(i);
TRI_ASSERT(sub);
hash = iresearch::hash(sub, hash);
}
return hash;
return hashMembers(*node, hash);
}

case aql::NODE_TYPE_FCALL_USER: {
hash = fasthash64(static_cast<const void*>("fcalluser"), 9, hash);
hash = fasthash64(static_cast<const void*>(node->getStringValue()),
node->getStringLength(), hash);
for (size_t i = 0, n = node->numMembers(); i < n; ++i) {
auto sub = node->getMemberUnchecked(i);

if (sub) {
hash = iresearch::hash(sub, hash);
}
}
return hash;
return hashMembers(*node, hash);
}

case aql::NODE_TYPE_RANGE: {
hash = fasthash64(static_cast<const void*>("range"), 5, hash);
for (size_t i = 0, n = node->numMembers(); i < n; ++i) {
auto sub = node->getMemberUnchecked(i);

if (sub) {
hash = iresearch::hash(sub, hash);
}
}
return hash;
return hashMembers(*node, hash);
}

default: {
Expand Down
169 changes: 118 additions & 51 deletions tests/IResearch/IResearchQueryJoin-test.cpp
View file
Open in desktop
< 10000 template class="js-line-alert-template">
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,46 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
CHECK(expectedDoc == expectedDocs.end());
}

// {
// std::string const query = "LET attr = [ _NONDETERM_('seq'), "
// "FOR i IN 1..5 "
// " FOR x IN collection_1 FILTER x.seq == i "
// " FOR d IN testView SEARCH d.seq == x.seq AND d.name == x.name "
// " SORT customscorer(d, x[attr]) DESC "
// "RETURN d";
//
// CHECK(arangodb::tests::assertRules(
// vocbase, query,
// {
// arangodb::aql::OptimizerRule::handleArangoSearchViewsRule,
// }
// ));
//
// std::vector<arangodb::velocypack::Slice> expectedDocs {
// arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
// arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
// };
//
// auto queryResult = arangodb::tests::executeQuery(vocbase, query);
// REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
//
// auto result = queryResult.result->slice();
// CHECK(result.isArray());
//
// arangodb::velocypack::ArrayIterator resultIt(result);
// REQUIRE(expectedDocs.size() == resultIt.size());
//
// // Check documents
// auto expectedDoc = expectedDocs.begin();
// for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
// auto const actualDoc = resultIt.value();
// auto const resolved = actualDoc.resolveExternals();
//
// CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
// }
// CHECK(expectedDoc == expectedDocs.end());
// }

// FOR i IN 1..5
// FOR x IN collection_0 SEARCH x.seq == i
// FOR d IN SEARCH d.seq == x.seq && d.name == x.name
Expand Down Expand Up @@ -1647,16 +1687,14 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
CHECK(expectedDoc == expectedDocs.end());
}

// we don't support scorers as a part of any expression (in sort or filter)
//
// FOR i IN 1..5
// FOR d IN testView SEARCH d.seq == i
// FOR x IN collection_1 FILTER x.seq == d.seq && x.seq == TFIDF(d)
{
std::string const query =
"FOR i IN 1..5 "
" FOR d IN testView SEARCH d.seq == i "
" FOR x IN collection_1 FILTER x.seq == d.seq && x.seq == TFIDF(d)"
" FOR x IN collection_1 FILTER x.seq == d.seq && x.seq == customscorer(d, i)"
"RETURN x";

CHECK(arangodb::tests::assertRules(
Expand All @@ -1665,21 +1703,37 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
}
));

std::vector<arangodb::velocypack::Slice> expectedDocs {
arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
};

auto queryResult = arangodb::tests::executeQuery(vocbase, query);
REQUIRE(TRI_ERROR_NOT_IMPLEMENTED == queryResult.code);
REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);

auto result = queryResult.result->slice();
CHECK(result.isArray());

arangodb::velocypack::ArrayIterator resultIt(result);
REQUIRE(expectedDocs.size() == resultIt.size());

// Check documents
auto expectedDoc = expectedDocs.begin();
for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
auto const actualDoc = resultIt.value();
auto const resolved = actualDoc.resolveExternals();

CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
}
CHECK(expectedDoc == expectedDocs.end());
}

// we don't support scorers as a part of any expression (in sort or filter)
//
// FOR i IN 1..5
// FOR d IN testView SEARCH d.seq == i
// FOR x IN collection_1 FILTER x.seq == d.seq && x.seq == TFIDF(d)
{
std::string const query =
"FOR i IN 1..5 "
" FOR d IN testView SEARCH d.seq == i "
" FOR x IN collection_1 FILTER x.seq == d.seq "
"SORT 1 + TFIDF(d)"
"SORT 1 + customscorer(d, i) DESC "
"RETURN d";

CHECK(arangodb::tests::assertRules(
Expand All @@ -1688,73 +1742,86 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
}
));

std::vector<arangodb::velocypack::Slice> expectedDocs {
arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
};

auto queryResult = arangodb::tests::executeQuery(vocbase, query);
REQUIRE(TRI_ERROR_NOT_IMPLEMENTED == queryResult.code);
REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);

auto result = queryResult.result->slice();
CHECK(result.isArray());

arangodb::velocypack::ArrayIterator resultIt(result);
REQUIRE(expectedDocs.size() == resultIt.size());

// Check documents
auto expectedDoc = expectedDocs.begin();
for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
auto const actualDoc = resultIt.value();
auto const resolved = actualDoc.resolveExternals();

CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
}
CHECK(expectedDoc == expectedDocs.end());
}

// we don't support scorers outside the view node
//
// FOR d IN (FOR c IN testView SEARCH c.name >= 'E' && c.seq < 10 SORT customscorer(c) DESC LIMIT 3 RETURN c)
// FOR x IN collection_1 FILTER x.seq == d.seq
// SORT customscorer(d, x.seq)
// multiple sorts
{
std::string const query =
"FOR d IN (FOR c IN testView SEARCH c.name >= 'E' && c.seq < 10 SORT customscorer(c) DESC LIMIT 3 RETURN c) "
" FOR x IN collection_1 FILTER x.seq == d.seq "
" SORT customscorer(d, x.seq) "
"RETURN x";
"FOR i IN 1..5 "
" FOR d IN testView SEARCH d.seq == i SORT tfidf(d, i > 0) ASC "
" FOR x IN collection_1 FILTER x.seq == d.seq && x.name == d.name "
"SORT customscorer(d, i) DESC RETURN d";

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

std::vector<arangodb::velocypack::Slice> expectedDocs {
arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
};

auto queryResult = arangodb::tests::executeQuery(vocbase, query);
REQUIRE(TRI_ERROR_NOT_IMPLEMENTED == queryResult.code);
REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);

auto result = queryResult.result->slice();
CHECK(result.isArray());

arangodb::velocypack::ArrayIterator resultIt(result);
REQUIRE(expectedDocs.size() == resultIt.size());

// Check documents
auto expectedDoc = expectedDocs.begin();
for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
auto const actualDoc = resultIt.value();
auto const resolved = actualDoc.resolveExternals();

CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
}
CHECK(expectedDoc == expectedDocs.end());
}

// multiple sorts (not supported now)
// FOR i IN 1..5
// FOR d IN SEARCH d.seq == i SORT customscorer(d, i) ASC
// FOR x IN collection_0 FILTER x.seq == d.seq && x.name == d.name
// SORT customscorer(d, i) DESC
// FIXME
{
std::string const query =
"FOR i IN 1..5 "
" FOR d IN testView SEARCH d.seq == i SORT tfidf(d, i) ASC "
" FOR x IN collection_1 FILTER x.seq == d.seq && x.name == d.name "
"SORT customscorer(d, i) DESC RETURN d";
"FOR d IN (FOR c IN testView SEARCH c.name >= 'E' && c.seq < 10 SORT customscorer(c) DESC LIMIT 3 RETURN c) "
" FOR x IN collection_1 FILTER x.seq == d.seq "
" SORT customscorer(d, x.seq) "
"RETURN x";

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

std::vector<arangodb::velocypack::Slice> expectedDocs {
arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
};

auto queryResult = arangodb::tests::executeQuery(vocbase, query);
REQUIRE(TRI_ERROR_NOT_IMPLEMENTED == queryResult.code);

// auto result = queryResult.result->slice();
// CHECK(result.isArray());
//
// arangodb::velocypack::ArrayIterator resultIt(result);
// REQUIRE(expectedDocs.size() == resultIt.size());
//
// // Check documents
// auto expectedDoc = expectedDocs.begin();
// for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
// auto const actualDoc = resultIt.value();
// auto const resolved = actualDoc.resolveExternals();
//
// CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
// }
// CHECK(expectedDoc == expectedDocs.end());
}
}

Expand Down
0