added test, fixed it

arangodb · KVS85 · Mar 19, 2020 · Mar 18, 2020 · Mar 18, 2020 · Mar 18, 2020
commit 88d30d0f84f37dce5e440b5802a4b46726e0f322
diff --git a/arangod/RocksDBEngine/RocksDBVPackIndex.cpp b/arangod/RocksDBEngine/RocksDBVPackIndex.cpp
@@ -750,10 +750,18 @@ namespace {
   bool attributesEqual(VPackSlice first, VPackSlice second,
                        std::vector<arangodb::basics::AttributeName>::const_iterator begin,
                        std::vector<arangodb::basics::AttributeName>::const_iterator end) {
-    TRI_ASSERT(first.isObject());
-    TRI_ASSERT(second.isObject());
-
     for (; begin != end; ++begin) {
+      // check if, after fetching the subattribute, we are point to a non-object.
+      // e.g. if the index is on field ["a.b"], the first iteration of this loop
+      // will look for subattribute "a" in the original document. this will always
+      // work. however, when looking for "b", we have to make sure that "a" was
+      // an object. otherwise we must not call Slice::get() on it. In case one of
+      // the subattributes we found so far is not an object, we fall back to the
+      // regular comparison
+      if (!first.isObject() || !second.isObject()) {
+        break;
+      }
+
       // fetch subattribute
       first = first.get(begin->name);
       if (first.isExternal()) {
@@ -787,17 +795,6 @@ namespace {
       if (notF1 || notF2) { // one of the paths was not found
         break;
       }
-
-      // check if, after fetching the subattribute, we are point to a non-object.
-      // e.g. if the index is on field ["a.b"], the first iteration of this loop
-      // will look for subattribute "a" in the original document. this will always
-      // work. however, when looking for "b", we have to make sure that "a" was
-      // an object. otherwise we must not call Slice::get() on it. In case one of
-      // the subattributes we found so far is not an object, we fall back to the
-      // regular comparison
-      if (!first.isObject() || !second.isObject()) {
-        break;
-      }
     }
 
     return basics::VelocyPackHelper::equal(first, second, true);

diff --git a/tests/js/common/shell/shell-hash-index.js b/tests/js/common/shell/shell-hash-index.js
@@ -1,5 +1,5 @@
 /*jshint globalstrict:false, strict:false */
-/*global fail, assertEqual, assertTrue, assertFalse, assertNotEqual */
+/*global fail, assertEqual, assertTrue, assertFalse, assertNull, assertNotEqual */
 
 /////////////////////////////////////////////////////////////////////////////
8C13
///
 /// @brief test the hash index
@@ -604,7 +604,25 @@ function HashIndexSuite() {
         }
         i *= 2;
       }
-    }
+    },
+
+    testUniqueIndexNullSubattribute : function () {
+      let idx = collection.ensureIndex({ type: "hash", unique: true, fields: ["a.b"] });
+
+      assertEqual("hash", idx.type);
+      assertEqual(true, idx.unique);
+      assertEqual(["a.b"], idx.fields);
+      assertEqual(true, idx.isNewlyCreated);
+
+      // as "a" is null here, "a.b" should also be null, at least it should not fail when accessing it via the index
+      collection.insert({ _key: "test", a : null });
+      collection.update("test", { something: "test2" });
+
+      let doc = collection.document("test");
+      assertNull(doc.a);
+      assertEqual("test2", doc.something);
+    },
+
   };
 }