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
reformat
  • Loading branch information
@gnusi
gnusi committed Jan 8, 2019
commit aa3ca2e0e38ba53b987dd4c70a7eb2ed2ca61e8f
169 changes: 67 additions & 102 deletions arangod/IResearch/AqlHelper.cpp
View file
Open in desktop
< F438 /tr>
Original file line number Diff line number Diff line change
Expand Up @@ -21,29 +21,29 @@
////////////////////////////////////////////////////////////////////////////////

#include "AqlHelper.h"
#include "IResearchCommon.h"
#include "IResearchDocument.h"
#include "Misc.h"
#include "Aql/ExecutionPlan.h"
#include "Aql/Expression.h"
#include "Aql/ExpressionContext.h"
#include "Aql/Function.h"
#include "Aql/Variable.h"
#include "Logger/LogMacros.h"
#include "Basics/fasthash.h"
#include "IResearchCommon.h"
#include "IResearchDocument.h"
#include "Logger/LogMacros.h"
#include "Misc.h"

#define __STDC_FORMAT_MACROS
#include <inttypes.h>

namespace {

arangodb::aql::AstNodeType const CmpMap[] {
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ, // NODE_TYPE_OPERATOR_BINARY_EQ: 3 == a <==> a == 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_NE, // NODE_TYPE_OPERATOR_BINARY_NE: 3 != a <==> a != 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GT, // NODE_TYPE_OPERATOR_BINARY_LT: 3 < a <==> a > 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE, // NODE_TYPE_OPERATOR_BINARY_LE: 3 <= a <==> a >= 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LT, // NODE_TYPE_OPERATOR_BINARY_GT: 3 > a <==> a < 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LE // NODE_TYPE_OPERATOR_BINARY_GE: 3 >= a <==> a <= 3
arangodb::aql::AstNodeType const CmpMap[]{
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ, // NODE_TYPE_OPERATOR_BINARY_EQ: 3 == 8000 a <==> a == 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_NE, // NODE_TYPE_OPERATOR_BINARY_NE: 3 != a <==> a != 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GT, // NODE_TYPE_OPERATOR_BINARY_LT: 3 < a <==> a > 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE, // NODE_TYPE_OPERATOR_BINARY_LE: 3 <= a <==> a >= 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LT, // NODE_TYPE_OPERATOR_BINARY_GT: 3 > a <==> a < 3
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LE // NODE_TYPE_OPERATOR_BINARY_GE: 3 >= a <==> a <= 3
};

}
Expand Down Expand Up @@ -121,7 +121,6 @@ bool equalTo(aql::AstNode const* lhs, aql::AstNode const* rhs) {
return false;
}


for (size_t i = 0; i < n; ++i) {
if (!equalTo(lhs->getMemberUnchecked(i), rhs->getMemberUnchecked(i))) {
return false;
Expand All @@ -141,9 +140,7 @@ bool equalTo(aql::AstNode const* lhs, aql::AstNode const* rhs) {
return true;
}

default: {
return lhs == rhs;
}
default: { return lhs == rhs; }
}
}

Expand Down Expand Up @@ -220,11 +217,8 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
auto sub = node->getMemberUnchecked(i);

if (sub) {
hash = fasthash64(
static_cast<const void*>(sub->getStringValue()),
sub->getStringLength(),
hash
);
hash = fasthash64(static_cast<const void*>(sub->getStringValue()),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this assumes each object attribute member is of type NODE_TYPE_OBJECT_ELEMENT.
At least in theory object attribute members can also have the type NODE_TYPE_CALCULATED_OBJECT_ELEMENT, but I am not sure if that is allowed here.
Example:

{ [ CONCAT("test" + 1) ] : "foo" }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to check it, thanks

sub->getStringLength(), hash);
TRI_ASSERT(sub->numMembers() > 0);
hash = iresearch::hash(sub->getMemberUnchecked(0), hash);
}
Expand Down Expand Up @@ -288,38 +282,33 @@ irs::string_ref getFuncName(aql::AstNode const& node) {
// --SECTION-- AqlValueTraits implementation
// ----------------------------------------------------------------------------

arangodb::aql::AstNode const ScopedAqlValue::INVALID_NODE(
arangodb::aql::NODE_TYPE_ROOT
);
arangodb::aql::AstNode const ScopedAqlValue::INVALID_NODE(arangodb::aql::NODE_TYPE_ROOT);

// ----------------------------------------------------------------------------
// --SECTION-- ScopedAqlValue implementation
// ----------------------------------------------------------------------------

bool ScopedAqlValue::execute(
arangodb::iresearch::QueryContext const& ctx
) {
bool ScopedAqlValue::execute(arangodb::iresearch::QueryContext const& ctx) {
if (_executed && _node->isDeterministic()) {
// constant expression, nothing to do
return true;
}

if (!ctx.plan) { // || !ctx.ctx) {
if (!ctx.plan) { // || !ctx.ctx) {
// can't execute expression without `ExecutionPlan`
return false;
}

TRI_ASSERT(ctx.ctx); //FIXME remove, uncomment condition
TRI_ASSERT(ctx.ctx); // FIXME remove, uncomment condition

if (!ctx.ast) {
// can't execute expression without `AST` and `ExpressionContext`
return false;
}

// don't really understand why we need `ExecutionPlan` and `Ast` here
arangodb::aql::Expression expr(
ctx.plan, ctx.ast, const_cast<arangodb::aql::AstNode*>(_node)
);
arangodb::aql::Expression expr(ctx.plan, ctx.ast,
const_cast<arangodb::aql::AstNode*>(_node));

destroy();

Expand All @@ -339,16 +328,13 @@ bool ScopedAqlValue::execute(
return true;
}

bool normalizeCmpNode(
arangodb::aql::AstNode const& in,
arangodb::aql::Variable const& ref,
NormalizedCmpNode& out) {
bool normalizeCmpNode(arangodb::aql::AstNode const& in,
arangodb::aql::Variable const& ref, NormalizedCmpNode& out) {
static_assert(adjacencyChecker<arangodb::aql::AstNodeType>::checkAdjacency<
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE, arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GT,
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LE, arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LT,
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_NE, arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ>(),
"Values are not adjacent"
);
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE, arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GT,
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LE, arangodb::aql::NODE_TYPE_OPERATOR_BINARY_LT,
arangodb::aql::NODE_TYPE_OPERATOR_BINARY_NE, arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ>(),
"Values are not adjacent");

if (!in.isDeterministic()) {
// unable normalize nondeterministic node
Expand All @@ -357,9 +343,8 @@ bool normalizeCmpNode(

auto cmp = in.type;

if (cmp < arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ
|| cmp > arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE
|| in.numMembers() != 2) {
if (cmp < arangodb::aql::NODE_TYPE_OPERATOR_BINARY_EQ ||
cmp > arangodb::aql::NODE_TYPE_OPERATOR_BINARY_GE || in.numMembers() != 2) {
// wrong `in` type
return false;
}
Expand All @@ -372,7 +357,7 @@ bool normalizeCmpNode(
if (!arangodb::iresearch::checkAttributeAccess(attribute, ref)) {
if (!arangodb::iresearch::checkAttributeAccess(value, ref)) {
// no suitable attribute access node found
return false;
return false;
}

std::swap(attribute, value);
Expand All @@ -391,22 +376,19 @@ bool normalizeCmpNode(
return true;
}

bool attributeAccessEqual(
arangodb::aql::AstNode const* lhs,
arangodb::aql::AstNode const* rhs,
QueryContext const* ctx
) {
bool attributeAccessEqual(arangodb::aql::AstNode const* lhs,
arangodb::aql::AstNode const* rhs, QueryContext const* ctx) {
struct NodeValue {
enum class Type {
INVALID = 0,
EXPANSION, // [*]
ACCESS, // [<offset>] | [<string>] | .
VALUE // REFERENCE | VALUE
EXPANSION, // [*]
ACCESS, // [<offset>] | [<string>] | .
VALUE // REFERENCE | VALUE
};

bool read(arangodb::aql::AstNode const* node, QueryContext const* ctx) noexcept {
this->strVal = irs::string_ref::NIL;
this->iVal= 0;
this->iVal = 0;
this->type = Type::INVALID;
this->root = nullptr;

Expand All @@ -417,31 +399,28 @@ bool attributeAccessEqual(
auto const n = node->numMembers();
auto const type = node->type;

if (n >= 2 && arangodb::aql::NODE_TYPE_EXPANSION == type) { // [*]
if (n >= 2 && arangodb::aql::NODE_TYPE_EXPANSION == type) { // [*]
auto* itr = node->getMemberUnchecked(0);
auto* ref = node->getMemberUnchecked(1);

if (itr && itr->numMembers() == 2) {
auto* var = itr->getMemberUnchecked(0);
auto* root = itr->getMemberUnchecked(1);

if (ref
&& arangodb::aql::NODE_TYPE_ITERATOR == itr->type
&& arangodb::aql::NODE_TYPE_REFERENCE == ref->type
&& root && var
&& arangodb::aql::NODE_TYPE_VARIABLE == var->type) {
if (ref && arangodb::aql::NODE_TYPE_ITERATOR == itr->type &&
arangodb::aql::NODE_TYPE_REFERENCE == ref->type && root && var &&
arangodb::aql::NODE_TYPE_VARIABLE == var->type) {
this->type = Type::EXPANSION;
this->root = root;
return true;
}
}

} else if (n == 2 && arangodb::aql::NODE_TYPE_INDEXED_ACCESS == type) { // [<something>]
} else if (n == 2 && arangodb::aql::NODE_TYPE_INDEXED_ACCESS == type) { // [<something>]
auto* root = node->getMemberUnchecked(0);
auto* offset = node->getMemberUnchecked(1);

if (root && offset) {

aqlValue.reset(*offset);

if (!ctx) {
Expand Down Expand Up @@ -483,29 +462,26 @@ bool attributeAccessEqual(
return true;
}

} else if (!n) { // end of attribute path (base case)
} else if (!n) { // end of attribute path (base case)

if (arangodb::aql::NODE_TYPE_REFERENCE == type) {
this->iVal = reinterpret_cast<int64_t>(node->value.value._data);
this->type = Type::VALUE;
this->root = node;
return false; // end of path
return false; // end of path
} else if (arangodb::aql::VALUE_TYPE_STRING == node->value.type) {
this->strVal = getStringRef(*node);
this->type = Type::VALUE;
this->root = node;
return false; // end of path
return false; // end of path
}

}

return false; // invalid input
return false; // invalid input
}

bool operator==(const NodeValue& rhs) const noexcept {
return type == rhs.type
&& strVal == rhs.strVal
&& iVal == rhs.iVal;
return type == rhs.type && strVal == rhs.strVal && iVal == rhs.iVal;
}

bool operator!=(const NodeValue& rhs) const noexcept {
Expand All @@ -515,7 +491,7 @@ bool attributeAccessEqual(
arangodb::iresearch::ScopedAqlValue aqlValue;
irs::string_ref strVal;
int64_t iVal;
Type type { Type::INVALID };
Type type{Type::INVALID};
arangodb::aql::AstNode const* root;
} lhsValue, rhsValue;

Expand All @@ -528,16 +504,12 @@ bool attributeAccessEqual(
rhs = rhsValue.root;
}
return lhsValue.type != NodeValue::Type::INVALID
&& rhsValue.type != NodeValue::Type::INVALID
&& rhsValue == lhsValue;
return lhsValue.type != NodeValue::Type::INVALID &&
rhsValue.type != NodeValue::Type::INVALID && rhsValue == lhsValue;
}

bool nameFromAttributeAccess(
std::string& name,
arangodb::aql::AstNode const& node,
QueryContext const& ctx
) {
bool nameFromAttributeAccess(std::string& name, arangodb::aql::AstNode const& node,
QueryContext const& ctx) {
struct {
bool attributeAccess(arangodb::aql::AstNode const& node) {
irs::string_ref strValue;
Expand All @@ -552,7 +524,7 @@ bool nameFromAttributeAccess(
}

bool expansion(arangodb::aql::AstNode const&) const {
return false; // do not support [*]
return false; // do not support [*]
}

bool indexAccess(arangodb::aql::AstNode const& node) {
Expand Down Expand Up @@ -600,49 +572,42 @@ bool nameFromAttributeAccess(
ScopedAqlValue value_;
std::string* str_;
QueryContext const* ctx_;
char buf_[21]; // enough to hold all numbers up to 64-bits
char buf_[21]; // enough to hold all numbers up to 64-bits
} builder;

name.clear();
builder.str_ = &name;
builder.ctx_ = &ctx;

arangodb::aql::AstNode const* head = nullptr;
return visitAttributeAccess(head, &node, builder)
&& head
&& arangodb::aql::NODE_TYPE_REFERENCE == head->type;
return visitAttributeAccess(head, &node, builder) && head &&
arangodb::aql::NODE_TYPE_REFERENCE == head->type;
}

arangodb::aql::AstNode const* checkAttributeAccess(
arangodb::aql::AstNode const* node,
arangodb::aql::Variable const& ref
) noexcept {
arangodb::aql::AstNode const* checkAttributeAccess(arangodb::aql::AstNode const* node,
arangodb::aql::Variable const& ref) noexcept {
struct {
bool attributeAccess(arangodb::aql::AstNode const&) const {
return true;
}
bool attributeAccess(arangodb::aql::AstNode const&) const { return true; }

bool indexAccess(arangodb::aql::AstNode const&) const {
return true;
}
bool indexAccess(arangodb::aql::AstNode const&) const { return true; }

bool expansion(arangodb::aql::AstNode const&) const {
return false; // do not support [*]
return false; // do not support [*]
}
} checker;

arangodb::aql::AstNode const* head = nullptr;

return node
&& arangodb::aql::NODE_TYPE_REFERENCE != node->type // do not allow root node to be REFERENCE
&& visitAttributeAccess(head, node, checker)
&& head && arangodb::aql::NODE_TYPE_REFERENCE == head->type
&& reinterpret_cast<void const*>(&ref) == head->getData() // same variable
? node : nullptr;
return node && arangodb::aql::NODE_TYPE_REFERENCE != node->type // do not allow root node to be REFERENCE
&& visitAttributeAccess(head, node, checker) && head &&
arangodb::aql::NODE_TYPE_REFERENCE == head->type &&
reinterpret_cast<void const*>(&ref) == head->getData() // same variable
? node
: nullptr;
}

} // iresearch
} // arangodb
} // namespace iresearch
} // namespace arangodb

// -----------------------------------------------------------------------------
// --SECTION-- END-OF-FILE
Expand Down
Loading
0