Skip to content
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.

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

Incorrect filter interpretation #3039

Closed
6 of 28 tasks
gurisko opened this issue Aug 15, 2017 · 3 comments
Closed
6 of 28 tasks

Incorrect filter interpretation #3039

gurisko opened this issue Aug 15, 2017 · 3 comments
Assignees
Labels
1 Bug 2 Fixed Resolution 3 AQL Query language related
Milestone

Comments

@gurisko
Copy link

gurisko commented Aug 15, 2017

my environment running ArangoDB

I'm using the latest ArangoDB of the respective release series:

  • 2.8
  • 3.0
  • 3.1
  • 3.2
  • self-compiled devel branch

Mode:

  • Cluster
  • Single-Server

Storage-Engine:

  • mmfiles
  • rocksdb

On this operating system:

  • DCOS on
    • AWS
    • Azure
    • own infrastructure
  • Linux
    • Debian .deb
    • Ubuntu .deb
    • SUSE .rpm
    • RedHat .rpm
    • Fedora .rpm
    • Gentoo
    • docker - official docker library
    • other:
  • Windows, version:
  • MacOS, version:

this is an AQL-related issue:

  • I'm using graph features

I'm issuing AQL via:

  • web interface with this browser: running on this OS:
  • arangosh
  • this Driver:

I've run db._explain("<my aql query>") and it didn't shed more light on this.
The AQL query in question is:

Query string:
     FOR dev IN vDevice
       FOR site IN vSite
         LET $site = site._id 
         FILTER site._id == dev.ad
           && dev.configReg != null
           && LENGTH(dev.configReg) > 0
           && !(dev.configReg == "0x2102" || dev.configReg == "0x2101" || dev.configReg == "0xf")
         RETURN dev
 

Execution plan:
 Id   NodeType                  Est.   Comment
  1   SingletonNode                1   * ROOT
  3   EnumerateCollectionNode     30     - FOR site IN vSite   /* full collection scan */
  8   IndexNode                  420       - FOR dev IN vDevice   /* hash index scan */
  9   CalculationNode            420         - LET #3 = ((dev.`configReg` == "0x2102") && (LENGTH(dev.`configReg`) > 0))   /* simple expression */   /* collections used: dev : vDevice */
  6   FilterNode                 420         - FILTER #3
  7   ReturnNode                 420         - RETURN dev

Indexes used:
 By   Type   Collection   Unique   Sparse   Selectivity   Fields     Ranges
  8   hash   vDevice      false    false         6.79 %   [ `ad` ]   (site.`_id` == dev.`ad`)

Optimization rules applied:
 Id   RuleName
  1   move-calculations-up
  2   remove-unnecessary-calculations
  3   interchange-adjacent-enumerations
  4   use-indexes
  5   remove-filter-covered-by-index

I can forward you dataset via email.

I am aware of how to rewrite the query to make it work.
One odd solution is provided below (5th line of the query):

Query string:
     FOR dev IN vDevice
       FOR site IN vSite
         LET $site = site._id 
         FILTER site._id == dev.ad
         FILTER dev.configReg != null
           && LENGTH(dev.configReg) > 0
           && !(dev.configReg == "0x2102" || dev.configReg == "0x2101" || dev.configReg == "0xf")
         RETURN dev
 

Execution plan:
 Id   NodeType                  Est.   Comment
  1   SingletonNode                1   * ROOT
  2   EnumerateCollectionNode    383     - FOR dev IN vDevice   /* full collection scan */
  7   CalculationNode            383       - LET #5 = (((dev.`configReg` != null) && (LENGTH(dev.`configReg`) > 0)) && ! (((dev.`configReg` == "0x2102") || (dev.`configReg` == "0x2101")) || (dev.`configReg` == "0xf")))   /* simple expression */   /* collections used: dev : vDevice */
  8   FilterNode                 383       - FILTER #5
 10   IndexNode                  383       - FOR site IN vSite   /* primary index scan */
  9   ReturnNode                 383         - RETURN dev

Indexes used:
 By   Type      Collection   Unique   Sparse   Selectivity   Fields       Ranges
 10   primary   vSite        true     false       100.00 %   [ `_key` ]   (site.`_id` == dev.`ad`)

Optimization rules applied:
 Id   RuleName
  1   move-calculations-up
  2   move-filters-up
  3   remove-unnecessary-calculations
  4   use-indexes
  5   remove-filter-covered-by-index
  6   remove-unnecessary-calculations-2
@jsteemann
Copy link
Contributor

Potential fix:

diff --git a/arangod/Aql/Ast.cpp b/arangod/Aql/Ast.cpp
index 726bceca4e..d63a89177c 100644
--- a/arangod/Aql/Ast.cpp
+++ b/arangod/Aql/Ast.cpp
@@ -2509,6 +2509,10 @@ AstNode* Ast::optimizeUnaryOperatorArithmetic(AstNode* node) {
 /// the unary NOT operation will be replaced with the result of the operation
 AstNode* Ast::optimizeNotExpression(AstNode* node) {
   TRI_ASSERT(node != nullptr);
+  if (node->type != NODE_TYPE_OPERATOR_UNARY_NOT) {
+    return node;
+  }
+
   TRI_ASSERT(node->type == NODE_TYPE_OPERATOR_UNARY_NOT);
   TRI_ASSERT(node->numMembers() == 1);
 
diff --git a/arangod/Aql/Condition.cpp b/arangod/Aql/Condition.cpp
index a2400265d5..63443abb1b 100644
--- a/arangod/Aql/Condition.cpp
+++ b/arangod/Aql/Condition.cpp
@@ -1386,12 +1386,11 @@ AstNode* Condition::transformNode(AstNode* node) {
         auto negated = transformNode(_ast->createNodeUnaryOperator(
             NODE_TYPE_OPERATOR_UNARY_NOT, sub->getMemberUnchecked(i)));
         auto optimized = _ast->optimizeNotExpression(negated);
-
         newOperator->addMember(optimized);
       }
 
-      return newOperator;
-    }
+      return transformNode(newOperator);
+    } 
 
     node->changeMember(0, transformNode(sub));
   }

I still need to confirm the fix with some test cases.

@jsteemann jsteemann self-assigned this Aug 15, 2017
@jsteemann jsteemann added 1 Bug 3 AQL Query language related labels Aug 15, 2017
jsteemann added a commit that referenced this issue Aug 16, 2017
@jsteemann
Copy link
Contributor

Fixed in 3.2.2

@jsteemann jsteemann added this to the 3.2.2 milestone Aug 16, 2017
@jsteemann jsteemann added the 2 Fixed Resolution label Aug 16, 2017
fceller pushed a commit that referenced this issue Aug 17, 2017
ObiWahn added a commit that referenced this issue Aug 18, 2017
…ture/planning-499-different-error-codes-for-version-check

* 'devel' of https://github.com/arangodb/arangodb:
  Feature/remove manual zippery (#3036)
  Fixed issue with autoincrement data not being persisted properly in RocksDB (#3059). (#3067)
  Bug fix/issues 1708 (#3060)
  Fix issue #3037: Foxx, internal server error when I try to add a new service (#3056)
  Bug fix/v8 syslog (#3055)
  Bug fix/small issues 1608 (#3049)
  fixed issue #3044 (#3048)
  fixed issue #3039 (#3045)
  Fix foxx github url (#3042)
  we need to substitute the package name here too - else enterprise pac… (#3025)
  Fixing engine stats in arangosh (#3038)
  MSVC is pendantic (but right) (#3047)
  reduce log spam (#3051)
  Feature/build docker to build using stretch container (#3062)
  build docker to build using stretch container (#3061)
  fix and extend journal-related tests (#3043)
@dothebart
Copy link
Contributor

ArangoDB 3.2.2 containing this bugfix is available for download - thanks for reporting.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 Bug 2 Fixed Resolution 3 AQL Query language related
Projects
None yet
Development

No branches or pull requests

3 participants