[BP] MB-18712 Fix to a bug on comparison between NULL and zero-filled…

… chunk

- In the current HB+trie, it cannot distinguish NULL and zero-filled
chunks because they are same (0x0000...) in the disk node.

- It causes an issue when
 1) an iterator is initialized with a sub-string of the smallest key, and
 2) the initial key is exactly aligned to chunk size, and
 3) the rest of string excluding the prefix are all zero.

- To address this issue, we need to handle that case separately during
iterator initialization.

Change-Id: Ib85e7fa232860af058924095f7e5fee3cddeff33
Reviewed-on: http://review.couchbase.org/61554
Tested-by: buildbot <build@couchbase.com>
Reviewed-by: Chiyoung Seo <chiyoung@couchbase.com>

src/hbtrie.cc

@@ -917,7 +917,31 @@ static hbtrie_result _hbtrie_next(struct hbtrie_iterator *it,
                     btree_iterator_init(&btree, &item_new->btree_it, NULL);
                 }
             } else {
-                btree_iterator_init(&btree, &item_new->btree_it, chunk);
+                bool null_btree_init_key = false;
+                if (!HBTRIE_ITR_IS_MOVED(it) &&
+                    ((uint64_t)item_new->chunkno+1) * trie->chunksize == it->keylen) {
+                    // Next chunk is the last chunk of the current iterator key
+                    // (happens only on iterator_init(), it internally calls next()).
+                    uint8_t *k_temp = alca(uint8_t, trie->chunksize);
+                    memset(k_temp, 0x0, trie->chunksize);
+                    k_temp[trie->chunksize - 1] = trie->chunksize;
+                    if (!memcmp(k_temp, chunk, trie->chunksize)) {
+                        // Extra chunk is same to the specific pattern
+                        // ([0x0] [0x0] ... [trie->chunksize])
+                        // which means that given iterator key is exactly aligned
+                        // to chunk size and shorter than the position of the
+                        // next chunk.
+                        // To guarantee lexicographical order between
+                        // NULL and zero-filled key (NULL < 0x0000...),
+                        // we should init btree iterator with NULL key.
+                        null_btree_init_key = true;
+                    }
+                }
+                if (null_btree_init_key) {
+                    btree_iterator_init(&btree, &item_new->btree_it, NULL);
+                } else {
+                    btree_iterator_init(&btree, &item_new->btree_it, chunk);
+                }
             }
             list_push_back(&it->btreeit_list, &item_new->le);
 

tests/functional/iterator_functional_test.cc

@@ -3937,6 +3937,79 @@ void iterator_uncommited_seeks()
     TEST_RESULT("iterator single doc range");
 }
 
+void iterator_init_using_substring_test()
+{
+    // MB-18712
+    TEST_INIT();
+    memleak_start();
+
+    int i, r;
+    fdb_file_handle *dbfile;
+    fdb_kvs_handle *db;
+    fdb_iterator *fit;
+    fdb_doc *rdoc;
+    fdb_config config;
+    fdb_kvs_config kvs_config;
+    fdb_status s; (void)s;
+    char valuebuf[256], cmd[256];
+
+    config = fdb_get_default_config();
+    config.buffercache_size = 0;
+    kvs_config = fdb_get_default_kvs_config();
+
+    sprintf(cmd, SHELL_DEL " %s*", "./iterator_test");
+    r = system(cmd); (void)r;
+
+    s = fdb_open(&dbfile, "./iterator_test1", &config);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    s = fdb_kvs_open(dbfile, &db, NULL, &kvs_config);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    uint8_t key1[] = {0x73, 0x37, 0x35, 0x34, 0x36, 0x32, 0x34, 0xff, 0x00, 0x00, 0x00};
+    uint8_t key2[] = {0x73, 0x37, 0x35, 0x34, 0x36, 0x32, 0x34, 0xff, 0x00, 0x00, 0x01};
+    // skey is a substring of key1
+    uint8_t skey[] = {0x73, 0x37, 0x35, 0x34, 0x36, 0x32, 0x34, 0xff};
+    uint8_t ekey[] = {0x73, 0x37, 0x35, 0x34, 0x36, 0x32, 0x34, 0xff, 0xff};
+
+    sprintf(valuebuf, "key1");
+    s = fdb_set_kv(db, key1, 11, valuebuf, 4);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    sprintf(valuebuf, "key2");
+    s = fdb_set_kv(db, key2, 11, valuebuf, 4);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    s = fdb_commit(dbfile, FDB_COMMIT_MANUAL_WAL_FLUSH);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    s = fdb_iterator_init(db, &fit, skey, 8, ekey, 9,
+                          FDB_ITR_NO_DELETES | FDB_ITR_SKIP_MAX_KEY);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    i = 0;
+    do {
+        rdoc = NULL;
+        s = fdb_iterator_get(fit, &rdoc);
+        if (s != FDB_RESULT_SUCCESS) break;
+        i++;
+        fdb_doc_free(rdoc);
+    } while (fdb_iterator_next(fit) == FDB_RESULT_SUCCESS);
+    TEST_CHK(i == 2);
+
+    s = fdb_iterator_close(fit);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    s = fdb_close(dbfile);
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+    s = fdb_shutdown();
+    TEST_CHK(s == FDB_RESULT_SUCCESS);
+
+    memleak_end();
+
+    TEST_RESULT("iterator init using substring test");
+}
+
 int main(){
     int i, j;
 
@@ -3970,5 +4043,6 @@ int main(){
     iterator_offset_access_test();
     iterator_deleted_doc_right_before_the_end_test();
     iterator_uncommited_seeks();
+    iterator_init_using_substring_test();
     return 0;
 }