From 77ccb7b1f2fee093076c573cf1a926e852673263 Mon Sep 17 00:00:00 2001
From: Konstantin Osipov <kostja.osipov@gmail.com>
Date: Thu, 9 Feb 2012 21:50:09 +0400
Subject: [PATCH] A fix and a test case for Bug#929654

A fix and a test case for Bu#929654 "
Regression with secondary HASH indexes"

Secodnary HASH indexes where skipped by build_indexes()
when starting from a snapshot.

Add a way to reserve space for as many values as necessary
and enable HASH indexes in build_indexes().

Add a test.
---
 include/mhash.h            | 18 +++++++++++---
 mod/box/box.m              |  5 +---
 mod/box/index.h            |  1 +
 mod/box/index.m            | 51 ++++++++++++++++++++++++++++++++++++++
 mod/box/tree.h             |  1 -
 test/box_big/sql.result    | 26 +++++++++++++++++++
 test/box_big/sql.test      | 12 +++++++++
 test/box_big/tarantool.cfg | 12 +++++++++
 8 files changed, 117 insertions(+), 9 deletions(-)

diff --git a/include/mhash.h b/include/mhash.h
index c277095b8c..172d94f3cb 100644
--- a/include/mhash.h
+++ b/include/mhash.h
@@ -118,12 +118,14 @@ struct _mh(t) {
 #define mh_begin(h)		({ 0;			})
 #define mh_end(h)		({ (h)->n_buckets;	})
 
+#define MH_DENSITY 0.7
 
 struct _mh(t) * _mh(init)();
 void _mh(clear)(struct _mh(t) *h);
 void _mh(destroy)(struct _mh(t) *h);
 void _mh(resize)(struct _mh(t) *h);
 mh_int_t _mh(start_resize)(struct _mh(t) *h, mh_int_t buckets, mh_int_t batch);
+void _mh(reserve)(struct _mh(t) *h, mh_int_t size);
 void __attribute__((noinline)) _mh(put_resize)(struct _mh(t) *h, mh_key_t key, mh_val_t val);
 void __attribute__((noinline)) _mh(del_resize)(struct _mh(t) *h, mh_int_t x);
 void _mh(dump)(struct _mh(t) *h);
@@ -280,7 +282,7 @@ _mh(init)()
 	h->n_buckets = __ac_prime_list[h->prime];
 	h->p = calloc(h->n_buckets, sizeof(struct _mh(pair)));
 	h->b = calloc(h->n_buckets / 16 + 1, sizeof(unsigned));
-	h->upper_bound = h->n_buckets * 0.7;
+	h->upper_bound = h->n_buckets * MH_DENSITY;
 	return h;
 }
 
@@ -291,7 +293,7 @@ _mh(clear)(struct _mh(t) *h)
 	h->prime = 0;
 	h->n_buckets = __ac_prime_list[h->prime];
 	h->p = calloc(h->n_buckets, sizeof(struct _mh(pair)));
-	h->upper_bound = h->n_buckets * 0.7;
+	h->upper_bound = h->n_buckets * MH_DENSITY;
 }
 
 void
@@ -352,7 +354,8 @@ _mh(start_resize)(struct _mh(t) *h, mh_int_t buckets, mh_int_t batch)
 	if (h->batch < 256) {
 		/*
 		 * Minimal batch must be greater or equal to
-		 * 1 / (1 - f), where f is upper bound percent = 0.7
+		 * 1 / (1 - f), where f is upper bound percent
+		 * = MH_DENSITY
 		 */
 		h->batch = 256;
 	}
@@ -361,7 +364,7 @@ _mh(start_resize)(struct _mh(t) *h, mh_int_t buckets, mh_int_t batch)
 	memcpy(s, h, sizeof(*h));
 	s->resize_position = 0;
 	s->n_buckets = __ac_prime_list[h->prime];
-	s->upper_bound = s->n_buckets * 0.7;
+	s->upper_bound = s->n_buckets * MH_DENSITY;
 	s->n_dirty = 0;
 	s->p = malloc(s->n_buckets * sizeof(struct _mh(pair)));
 	s->b = calloc(s->n_buckets / 16 + 1, sizeof(unsigned));
@@ -370,6 +373,12 @@ _mh(start_resize)(struct _mh(t) *h, mh_int_t buckets, mh_int_t batch)
 	return h->n_buckets;
 }
 
+void
+_mh(reserve)(struct _mh(t) *h, mh_int_t size)
+{
+	_mh(start_resize)(h, size/MH_DENSITY, h->size);
+}
+
 #ifndef mh_stat
 #define mh_stat(buf, h) ({						\
                 tbuf_printf(buf, "  n_buckets: %"PRIu32 CRLF		\
@@ -425,6 +434,7 @@ _mh(dump)(struct _mh(t) *h)
 #undef mh_unlikely
 #undef slot
 #undef slot_and_dirty
+#undef MH_DENSITY
 #endif
 
 #undef mh_cat
diff --git a/mod/box/box.m b/mod/box/box.m
index 9150e9f26e..3673757696 100644
--- a/mod/box/box.m
+++ b/mod/box/box.m
@@ -47,7 +47,6 @@
 #include <mod/box/tuple.h>
 #include "memcached.h"
 #include "box_lua.h"
-#include "tree.h"
 
 static void box_process_ro(u32 op, struct tbuf *request_data);
 static void box_process_rw(u32 op, struct tbuf *request_data);
@@ -1534,9 +1533,7 @@ build_indexes(void)
 		Index *pk = space[n].index[0];
 		for (int i = 1; i < space[n].key_count; i++) {
 			Index *index = space[n].index[i];
-			if ([index isKindOf: [TreeIndex class]]) {
-				[(TreeIndex *) index build: pk];
-			}
+			[index build: pk];
 		}
 
 		say_info("Space %"PRIu32": done", n);
diff --git a/mod/box/index.h b/mod/box/index.h
index caf0eb95ca..e3cb401c39 100644
--- a/mod/box/index.h
+++ b/mod/box/index.h
@@ -111,6 +111,7 @@ struct key_def {
  * Finish index construction.
  */
 - (void) enable;
+- (void) build: (Index *) pk;
 - (size_t) size;
 - (struct box_tuple *) min;
 - (struct box_tuple *) max;
diff --git a/mod/box/index.m b/mod/box/index.m
index 5b3883c6dc..c1bab7ea15 100644
--- a/mod/box/index.m
+++ b/mod/box/index.m
@@ -107,6 +107,13 @@ iterator_first_equal(struct iterator *it)
 	[self subclassResponsibility: _cmd];
 }
 
+- (void) build: (Index *) pk
+{
+	(void) pk;
+	[self subclassResponsibility: _cmd];
+}
+
+
 - (size_t) size
 {
 	[self subclassResponsibility: _cmd];
@@ -180,6 +187,7 @@ iterator_first_equal(struct iterator *it)
 /* {{{ HashIndex -- base class for all hashes. ********************/
 
 @interface HashIndex: Index
+	- (void) reserve: (u32) n_tuples;
 @end
 
 struct hash_iterator {
@@ -218,6 +226,33 @@ hash_iterator_free(struct iterator *iterator)
 
 
 @implementation HashIndex
+
+- (void) reserve: (u32) n_tuples
+{
+	(void) n_tuples;
+	[self subclassResponsibility: _cmd];
+}
+
+- (void) build: (Index *) pk
+{
+	u32 n_tuples = [pk size];
+
+	if (n_tuples == 0)
+		return;
+
+	[self reserve: n_tuples];
+
+	say_info("Adding %"PRIu32 " keys to HASH index %"
+		 PRIu32 "...", n_tuples, index_n(self));
+
+	struct iterator *it = pk->position;
+	struct box_tuple *tuple;
+	[pk initIterator: it];
+
+	while ((tuple = it->next(it)))
+	      [self replace: NULL :tuple];
+}
+
 - (void) free
 {
 	[super free];
@@ -267,6 +302,12 @@ hash_iterator_free(struct iterator *iterator)
 @end
 
 @implementation Hash32Index
+
+- (void) reserve: (u32) n_tuples
+{
+	mh_i32ptr_reserve(int_hash, n_tuples);
+}
+
 - (void) free
 {
 	mh_i32ptr_destroy(int_hash);
@@ -388,6 +429,11 @@ hash_iterator_free(struct iterator *iterator)
 @end
 
 @implementation Hash64Index
+- (void) reserve: (u32) n_tuples
+{
+	mh_i64ptr_reserve(int64_hash, n_tuples);
+}
+
 - (void) free
 {
 	mh_i64ptr_destroy(int64_hash);
@@ -509,6 +555,11 @@ hash_iterator_free(struct iterator *iterator)
 @end
 
 @implementation HashStrIndex
+- (void) reserve: (u32) n_tuples
+{
+	mh_lstrptr_reserve(str_hash, n_tuples);
+}
+
 - (void) free
 {
 	mh_lstrptr_destroy(str_hash);
diff --git a/mod/box/tree.h b/mod/box/tree.h
index e51f167861..19b58b28b7 100644
--- a/mod/box/tree.h
+++ b/mod/box/tree.h
@@ -42,7 +42,6 @@ typedef int (*tree_cmp_t)(const void *, const void *, void *);
 };
 
 + (Index *) alloc: (struct key_def *) key_def :(struct space *) space;
-- (void) build: (Index *) pk;
 
 /** To be defined in subclasses. */
 - (size_t) node_size;
diff --git a/test/box_big/sql.result b/test/box_big/sql.result
index ec1fe56abd..e9e87ba6fa 100644
--- a/test/box_big/sql.result
+++ b/test/box_big/sql.result
@@ -169,6 +169,32 @@ call box.select('5', '1', 'part1', 'part2')
 Found 2 tuples:
 ['01234567', 'part1', 'part2']
 ['11234567', 'part1', 'part2']
+insert into t7 values (1, 'hello')
+Insert OK, 1 row affected
+insert into t7 values (2, 'brave')
+Insert OK, 1 row affected
+insert into t7 values (3, 'new')
+Insert OK, 1 row affected
+insert into t7 values (4, 'world')
+Insert OK, 1 row affected
+#
+# Bug#929654 - secondary hash index is not built with build_indexes()
+#
+select * from t7 where k1='hello'
+Found 1 tuple:
+[1, 'hello']
+select * from t7 where k1='brave'
+Found 1 tuple:
+[2, 'brave']
+select * from t7 where k1='new'
+Found 1 tuple:
+[3, 'new']
+select * from t7 where k1='world'
+Found 1 tuple:
+[4, 'world']
+lua box.space[7]:truncate()
+---
+...
 select * from t1 where k0='key1'
 Found 1 tuple:
 [830039403, 'part1', 'part2']
diff --git a/test/box_big/sql.test b/test/box_big/sql.test
index 602ca6b3f9..65c21b649e 100644
--- a/test/box_big/sql.test
+++ b/test/box_big/sql.test
@@ -86,9 +86,21 @@ exec sql "select * from t5 where k1='part1_a'"
 exec sql "select * from t5 where k1='part_none'"
 exec sql "call box.select('5', '1', 'part1', 'part2')"
 sql.sort = False 
+exec sql "insert into t7 values (1, 'hello')"
+exec sql "insert into t7 values (2, 'brave')"
+exec sql "insert into t7 values (3, 'new')"
+exec sql "insert into t7 values (4, 'world')"
 # Check how build_idnexes() works
 server.stop()
 server.start()
+print """#
+# Bug#929654 - secondary hash index is not built with build_indexes()
+#"""
+exec sql "select * from t7 where k1='hello'"
+exec sql "select * from t7 where k1='brave'"
+exec sql "select * from t7 where k1='new'"
+exec sql "select * from t7 where k1='world'"
+exec admin "lua box.space[7]:truncate()"
 exec sql "select * from t1 where k0='key1'"
 exec sql "select * from t1 where k0='key2'"
 exec sql "select * from t1 where k0='key3'"
diff --git a/test/box_big/tarantool.cfg b/test/box_big/tarantool.cfg
index f298f2bdd0..6a3d5fea29 100644
--- a/test/box_big/tarantool.cfg
+++ b/test/box_big/tarantool.cfg
@@ -124,3 +124,15 @@ space[6].index[6].key_field[3].type = "STR"
 space[6].index[6].key_field[4].fieldno = 8
 space[6].index[6].key_field[4].type = "NUM"
 
+# Space #7, https://bugs.launchpad.net/tarantool/+bug/929654
+space[7].enabled = true
+
+space[7].index[0].type = "HASH"
+space[7].index[0].unique = true
+space[7].index[0].key_field[0].fieldno = 0
+space[7].index[0].key_field[0].type = "NUM"
+
+space[7].index[1].type = "HASH"
+space[7].index[1].unique = true
+space[7].index[1].key_field[0].fieldno = 1
+space[7].index[1].key_field[0].type = "STR"
-- 
GitLab