From 345dd14aad89e95758ea0cdb47bece148ee58fa9 Mon Sep 17 00:00:00 2001
From: Roman Tsisyk <roman@tsisyk.com>
Date: Tue, 1 Jul 2014 14:33:22 +0400
Subject: [PATCH] Fix SPLICE operation to support one-based indexing

See #347
---
 src/box/tuple_update.cc  | 24 +++++++++++++++---------
 src/box/tuple_update.h   |  2 +-
 test/box/update.result   | 32 ++++++++++++++++++++++++++------
 test/box/update.test.lua | 13 +++++++++----
 4 files changed, 51 insertions(+), 20 deletions(-)

diff --git a/src/box/tuple_update.cc b/src/box/tuple_update.cc
index 5ce86d6365..226966ba42 100644
--- a/src/box/tuple_update.cc
+++ b/src/box/tuple_update.cc
@@ -95,6 +95,7 @@ struct tuple_update
 	struct rope *rope;
 	struct update_op *ops;
 	uint32_t op_count;
+	int index_base; /* 0 for C and 1 for Lua */
 };
 
 /** Argument of SET operation. */
@@ -330,12 +331,16 @@ do_op_splice(struct tuple_update *update, struct update_op *op,
 	in = mp_decode_str(&in, &str_len);
 
 	if (arg->offset < 0) {
-		if (-arg->offset > str_len)
+		if (-arg->offset > str_len + 1)
 			tnt_raise(ClientError, ER_SPLICE,
 				  "offset is out of bound");
-		arg->offset = arg->offset + str_len;
-	} else if (arg->offset > str_len) {
-		arg->offset = str_len;
+		arg->offset = arg->offset + str_len + 1;
+	} else if (arg->offset >= update->index_base){
+		arg->offset -= update->index_base;
+		if (arg->offset > str_len)
+			arg->offset = str_len;
+	} else {
+		tnt_raise(ClientError, ER_SPLICE, "offset is out of bound");
 	}
 
 	assert(arg->offset >= 0 && arg->offset <= str_len);
@@ -547,7 +552,7 @@ update_write_tuple(struct tuple_update *update, char *buffer, char *buffer_end)
 
 static void
 update_read_ops(struct tuple_update *update, const char *expr,
-		const char *expr_end, int field_base)
+		const char *expr_end)
 {
 	/* number of operations */
 	update->op_count = mp_decode_array(&expr);
@@ -601,11 +606,11 @@ update_read_ops(struct tuple_update *update, const char *expr,
 		op->field_no = mp_read_int(&expr, "expected a field no (integer)");
 		if (op->field_no != UINT32_MAX) {
 			/* Check that field_no is not zero for Lua (base = 1) */
-			if (op->field_no < field_base) {
+			if (op->field_no < update->index_base) {
 				tnt_raise(ClientError, ER_NO_SUCH_FIELD,
 					  op->field_no);
 			}
-			op->field_no -= field_base;
+			op->field_no -= update->index_base;
 		}
 		op->meta->do_op(update, op, &expr);
 	}
@@ -619,7 +624,7 @@ const char *
 tuple_update_execute(region_alloc_func alloc, void *alloc_ctx,
 		     const char *expr,const char *expr_end,
 		     const char *old_data, const char *old_data_end,
-		     uint32_t *p_tuple_len, int field_base)
+		     uint32_t *p_tuple_len, int index_base)
 {
 	struct tuple_update *update = (struct tuple_update *)
 			alloc(alloc_ctx, sizeof(*update));
@@ -627,9 +632,10 @@ tuple_update_execute(region_alloc_func alloc, void *alloc_ctx,
 	memset(update, 0, sizeof(*update));
 	update->alloc = alloc;
 	update->alloc_ctx = alloc_ctx;
+	update->index_base = index_base;
 
 	update_create_rope(update, old_data, old_data_end);
-	update_read_ops(update, expr, expr_end, field_base);
+	update_read_ops(update, expr, expr_end);
 	uint32_t tuple_len = update_calc_tuple_length(update);
 
 	char *buffer = (char *) alloc(alloc_ctx, tuple_len);
diff --git a/src/box/tuple_update.h b/src/box/tuple_update.h
index 17348c70cd..c6873f1f2a 100644
--- a/src/box/tuple_update.h
+++ b/src/box/tuple_update.h
@@ -43,6 +43,6 @@ const char *
 tuple_update_execute(region_alloc_func alloc, void *alloc_ctx,
 		     const char *expr,const char *expr_end,
 		     const char *old_data, const char *old_data_end,
-		     uint32_t *p_new_size, int field_base);
+		     uint32_t *p_new_size, int index_base);
 
 #endif /* TARANTOOL_BOX_TUPLE_UPDATE_H_INCLUDED */
diff --git a/test/box/update.result b/test/box/update.result
index 89329cfdc8..fce073b92d 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -178,19 +178,15 @@ s:update({1}, {{'+', 3, 16}, {'&', 4, 0xffff0000}, {'|', 4, 0x0000a0a0}, {'^', 4
 - error: 'Field 3 UPDATE error: double update of the same field'
 ...
 -- test update splice operation
-s:update({1}, {{':', 2, 0, 3, 'the newest'}})
----
-- [1, 'the newest field string value', 42, 3735928559]
-...
 s:replace{1953719668, 'something to splice'}
 ---
 - [1953719668, 'something to splice']
 ...
-s:update(1953719668, {{':', 2, 0, 4, 'no'}})
+s:update(1953719668, {{':', 2, 1, 4, 'no'}})
 ---
 - [1953719668, 'nothing to splice']
 ...
-s:update(1953719668, {{':', 2, 0, 2, 'every'}})
+s:update(1953719668, {{':', 2, 1, 2, 'every'}})
 ---
 - [1953719668, 'everything to splice']
 ...
@@ -228,6 +224,30 @@ s:delete{1953719668}
 ---
 - [1953719668, 'bye, world']
 ...
+s:replace({10, 'abcde'})
+---
+- [10, 'abcde']
+...
+s:update(10,  {{':', 2, 0, 0, '!'}})
+---
+- error: 'Field SPLICE error: offset is out of bound'
+...
+s:update(10,  {{':', 2, 1, 0, '('}})
+---
+- [10, '(abcde']
+...
+s:update(10,  {{':', 2, 2, 0, '({'}})
+---
+- [10, '(({abcde']
+...
+s:update(10,  {{':', 2, -1, 0, ')'}})
+---
+- [10, '(({abcde)']
+...
+s:update(10,  {{':', 2, -2, 0, '})'}})
+---
+- [10, '(({abcde}))']
+...
 -- test update delete operations
 s:update({1}, {{'#', 4, 1}, {'#', 3, 1}})
 ---
diff --git a/test/box/update.test.lua b/test/box/update.test.lua
index 4eec6c4af7..57112954f8 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -57,11 +57,9 @@ s:update({1}, {{'=', 2, 'new field string value'}, {'=', 3, 42}, {'=', 4, 0xdead
 s:update({1}, {{'+', 3, 16}, {'&', 4, 0xffff0000}, {'|', 4, 0x0000a0a0}, {'^', 4, 0xffff00aa}})
 
 -- test update splice operation
-s:update({1}, {{':', 2, 0, 3, 'the newest'}})
-
 s:replace{1953719668, 'something to splice'}
-s:update(1953719668, {{':', 2, 0, 4, 'no'}})
-s:update(1953719668, {{':', 2, 0, 2, 'every'}})
+s:update(1953719668, {{':', 2, 1, 4, 'no'}})
+s:update(1953719668, {{':', 2, 1, 2, 'every'}})
 -- check an incorrect offset
 s:update(1953719668, {{':', 2, 100, 2, 'every'}})
 s:update(1953719668, {{':', 2, -100, 2, 'every'}})
@@ -72,6 +70,13 @@ s:insert{1953719668, 'hello world'}
 s:update(1953719668, {{'=', 2, 'bye, world'}})
 s:delete{1953719668}
 
+s:replace({10, 'abcde'})
+s:update(10,  {{':', 2, 0, 0, '!'}})
+s:update(10,  {{':', 2, 1, 0, '('}})
+s:update(10,  {{':', 2, 2, 0, '({'}})
+s:update(10,  {{':', 2, -1, 0, ')'}})
+s:update(10,  {{':', 2, -2, 0, '})'}})
+
 -- test update delete operations
 s:update({1}, {{'#', 4, 1}, {'#', 3, 1}})
 
-- 
GitLab