diff --git a/arangod/Aql/Ast.cpp b/arangod/Aql/Ast.cpp index 3c34bfd696..18edeba3e8 100644 --- a/arangod/Aql/Ast.cpp +++ b/arangod/Aql/Ast.cpp @@ -1768,9 +1768,9 @@ AstNode const* Ast::deduplicateArray (AstNode const* node) { for (auto& it : cache) { // TODO: check if the node needs cloning or if we can get away without - // copy->addMember(clone((*it).second)); copy->addMember(it.second); } + copy->sort(); return copy; } diff --git a/arangod/Aql/AstNode.cpp b/arangod/Aql/AstNode.cpp index 3fc31d03d6..395c93a580 100644 --- a/arangod/Aql/AstNode.cpp +++ b/arangod/Aql/AstNode.cpp @@ -324,11 +324,25 @@ int triagens::aql::CompareAstNodes (AstNode const* lhs, } if (lType == TRI_JSON_STRING) { + size_t maxLength = (std::max)(lhs->getStringLength(), rhs->getStringLength()); + if (compareUtf8) { - return TRI_compare_utf8(lhs->getStringValue(), rhs->getStringValue()); + return TRI_compare_utf8(lhs->getStringValue(), lhs->getStringLength(), rhs->getStringValue(), rhs->getStringLength()); } - return strcmp(lhs->getStringValue(), rhs->getStringValue()); - } + + int res = strncmp(lhs->getStringValue(), rhs->getStringValue(), maxLength); + + if (res != 0) { + return res; + } + + int diff = lhs->getStringLength() - rhs->getStringLength(); + + if (diff != 0) { + return diff < 0 ? -1 : 1; + } + return 0; + } if (lType == TRI_JSON_ARRAY) { size_t const numLhs = lhs->numMembers(); diff --git a/arangod/Aql/AstNode.h b/arangod/Aql/AstNode.h index eb6ff66618..7e78557049 100644 --- a/arangod/Aql/AstNode.h +++ b/arangod/Aql/AstNode.h @@ -683,9 +683,6 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// inline AstNode* getMemberUnchecked (size_t i) const noexcept { -#ifdef TRI_ENABLE_MAINTAINER_MODE - TRI_ASSERT(i < members.size()); -#endif return members[i]; } diff --git a/arangod/Aql/Condition.cpp b/arangod/Aql/Condition.cpp index 69f0b38fdf..0027f48265 100644 --- a/arangod/Aql/Condition.cpp +++ b/arangod/Aql/Condition.cpp @@ -27,8 +27,6 @@ /// @author Copyright 2012-2013, triAGENS GmbH, Cologne, Germany //////////////////////////////////////////////////////////////////////////////// -// TODO: sort IN values - #include "Condition.h" #include "Aql/Ast.h" #include "Aql/AstNode.h" @@ -232,7 +230,7 @@ bool ConditionPart::isCoveredBy (ConditionPart const& other) const { for (size_t j = 0; j < n2; ++j) { auto w = other.valueNode->getMemberUnchecked(j); - ConditionPartCompareResult res = ConditionPart::ResultsTable[CompareAstNodes(v, w, false) + 1][0][0]; + ConditionPartCompareResult res = ConditionPart::ResultsTable[CompareAstNodes(v, w, true) + 1][0][0]; if (res != CompareResult::OTHER_CONTAINED_IN_SELF && res != CompareResult::CONVERT_EQUAL && @@ -249,7 +247,7 @@ bool ConditionPart::isCoveredBy (ConditionPart const& other) const { // Results are -1, 0, 1, move to 0, 1, 2 for the lookup: ConditionPartCompareResult res = ConditionPart::ResultsTable - [CompareAstNodes(other.valueNode, valueNode, false) + 1] + [CompareAstNodes(other.valueNode, valueNode, true) + 1] [other.whichCompareOperation()] [whichCompareOperation()]; @@ -527,9 +525,7 @@ std::pair Condition::findIndexForAndNode (size_t position, } _root->changeMember(position, bestIndex->specializeCondition(node, reference)); -#if 0 _isSorted = sortOrs(); -#endif usedIndexes.emplace_back(bestIndex); @@ -777,8 +773,10 @@ bool Condition::sortOrs () { } } } - - TRI_ASSERT(parts.size() == _root->numMembers()); + + if (parts.size() != _root->numMembers()) { + return false; + } // now sort all conditions by variable name, attribute name, attribute value std::sort(parts.begin(), parts.end(), [] (ConditionPart const& lhs, ConditionPart const& rhs) -> bool { @@ -811,7 +809,7 @@ bool Condition::sortOrs () { if (ll != nullptr && lr != nullptr) { // both lower bounds are set - res = CompareAstNodes(ll, lr, false); + res = CompareAstNodes(ll, lr, true); if (res != 0) { return res < 0; @@ -882,21 +880,24 @@ void Condition::optimize (ExecutionPlan* plan) { TRI_ASSERT(andNode->type == NODE_TYPE_OPERATOR_NARY_AND); restartThisOrItem: - size_t const andNumMembers = andNode->numMembers(); + size_t andNumMembers = andNode->numMembers(); - // deduplicate all IN arrays + // deduplicate and sort all IN arrays size_t inComparisons = 0; for (size_t j = 0; j < andNumMembers; ++j) { auto op = andNode->getMemberUnchecked(j); + if (op->type == NODE_TYPE_OPERATOR_BINARY_IN) { ++inComparisons; } deduplicateInOperation(op); } + andNumMembers = andNode->numMembers(); if (andNumMembers <= 1) { // simple AND item with 0 or 1 members. nothing to do ++r; + n = _root->numMembers(); continue; } @@ -1017,11 +1018,12 @@ restartThisOrItem: for (size_t k = 0; k < values->numMembers(); ++k) { auto value = values->getMemberUnchecked(k); ConditionPartCompareResult res = ConditionPart::ResultsTable - [CompareAstNodes(value, other.valueNode, false) + 1] + [CompareAstNodes(value, other.valueNode, true) + 1] [0 /*NODE_TYPE_OPERATOR_BINARY_EQ*/] [other.whichCompareOperation()]; bool const keep = (res == CompareResult::OTHER_CONTAINED_IN_SELF || res == CompareResult::CONVERT_EQUAL); + if (keep) { inNode->addMember(value); } @@ -1045,7 +1047,7 @@ restartThisOrItem: // Results are -1, 0, 1, move to 0, 1, 2 for the lookup: ConditionPartCompareResult res = ConditionPart::ResultsTable - [CompareAstNodes(current.valueNode, other.valueNode, false) + 1] + [CompareAstNodes(current.valueNode, other.valueNode, true) + 1] [current.whichCompareOperation()] [other.whichCompareOperation()]; @@ -1059,23 +1061,23 @@ restartThisOrItem: goto fastForwardToNextOrItem; } case CompareResult::SELF_CONTAINED_IN_OTHER: { - andNode->removeMemberUnchecked(positions[0].first); + andNode->removeMemberUnchecked(positions.at(0).first); goto restartThisOrItem; } case CompareResult::OTHER_CONTAINED_IN_SELF: { - andNode->removeMemberUnchecked(positions[j].first); + andNode->removeMemberUnchecked(positions.at(j).first); goto restartThisOrItem; } case CompareResult::CONVERT_EQUAL: { // both ok, now transform to a == x (== y) - andNode->removeMemberUnchecked(positions[j].first); - auto origNode = andNode->getMemberUnchecked(positions[0].first); + andNode->removeMemberUnchecked(positions.at(j).first); + auto origNode = andNode->getMemberUnchecked(positions.at(0).first); auto newNode = plan->getAst()->createNode(NODE_TYPE_OPERATOR_BINARY_EQ); for (size_t iMemb = 0; iMemb < origNode->numMembers(); iMemb++) { newNode->addMember(origNode->getMemberUnchecked(iMemb)); } - andNode->changeMember(positions[0].first, newNode); - break; + andNode->changeMember(positions.at(0).first, newNode); + goto restartThisOrItem; } case CompareResult::DISJOINT: { break; @@ -1231,7 +1233,7 @@ bool Condition::canRemove (ConditionPart const& me, } //////////////////////////////////////////////////////////////////////////////// -/// @brief deduplicate IN condition values +/// @brief deduplicate IN condition values (and sort them) /// this may modify the node in place //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/Basics/Utf8Helper.cpp b/lib/Basics/Utf8Helper.cpp index 9ab8d40cf0..2d1c7d0950 100644 --- a/lib/Basics/Utf8Helper.cpp +++ b/lib/Basics/Utf8Helper.cpp @@ -742,7 +742,6 @@ int TRI_compare_utf8 (char const* left, return Utf8Helper::DefaultUtf8Helper.compareUtf8(left, leftLength, right, rightLength); } - //////////////////////////////////////////////////////////////////////////////// /// @brief Lowercase the characters in a UTF-8 string (implemented in Basic/Utf8Helper.cpp) ////////////////////////////////////////////////////////////////////////////////