Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org
Cc: "v.shpilevoy@tarantool.org" <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 2/2] Fixed lost format on update and upsert operations.
Date: Sun, 15 Apr 2018 13:03:58 +0300	[thread overview]
Message-ID: <f724f6b0-9b99-385c-5830-8037663c0c5b@tarantool.org> (raw)
In-Reply-To: <4a4c5106-1d2d-553f-c8f1-5c031e91c9ac@tarantool.org>

From 1aa00690d6c246450e82454f414c89af709905b5 Mon Sep 17 00:00:00 2001
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Date: Sun, 15 Apr 2018 13:02:29 +0300
Subject: [PATCH] Accounted recommendations

---
 src/box/lua/tuple.c      | 25 +++++++---------------
 src/box/tuple.c          | 10 ++-------
 src/box/tuple.h          |  8 ++++---
 src/box/tuple_format.h   |  9 +++++---
 src/box/vy_stmt.c        |  5 ++++-
 src/box/vy_stmt.h        |  9 --------
 test/box/update.result   | 54 +++++++++---------------------------------------
 test/box/update.test.lua | 20 +++++-------------
 8 files changed, 40 insertions(+), 100 deletions(-)

diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index d3b8dac..c40bdb7 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -29,6 +29,7 @@
  * SUCH DAMAGE.
  */
 #include "box/lua/tuple.h"
+#include "box/tuple_update.h"
 
 #include "lua/utils.h" /* luaT_error() */
 #include "lua/msgpack.h" /* luamp_encode_XXX() */
@@ -36,7 +37,6 @@
 #include <small/ibuf.h>
 #include <small/region.h>
 #include <fiber.h>
-#include <box/tuple_update.h>
 
 #include "box/tuple.h"
 #include "box/tuple_convert.h"
@@ -397,30 +397,21 @@ lbox_tuple_transform(struct lua_State *L)
 	size_t used = region_used(region);
 	struct tuple *new_tuple = NULL;
 	const char *new_data = tuple_update_execute(region_aligned_alloc_cb,
-	                                            region,
-	                                            buf->buf,
+	                                            region, buf->buf,
 	                                            buf->buf + ibuf_used(buf),
-	                                            old_data,
-	                                            old_data + bsize,
+	                                            old_data, old_data + bsize,
 	                                            &new_size, 1, NULL);
-	if (new_data == NULL) {
-		region_truncate(region, used);
-		goto error;
-	}
-	new_tuple = tuple_new(box_tuple_format_default(),
-	                                    new_data, new_data + new_size);
+	if (new_data != NULL)
+		new_tuple = tuple_new(box_tuple_format_default(),
+		                      new_data, new_data + new_size);
 	region_truncate(region, used);
+
 	if (new_tuple == NULL)
-		goto error;
-	tuple_bless(new_tuple);
+		luaT_error(L);
 
 	luaT_pushtuple(L, new_tuple);
 	ibuf_reset(buf);
 	return 1;
-error:
-	if (new_tuple == NULL)
-		luaT_error(L);
-	return 0;
 }
 
 /**
diff --git a/src/box/tuple.c b/src/box/tuple.c
index cb57d81..34efc12 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -61,7 +61,7 @@ struct tuple_format *tuple_format_runtime;
 static void
 runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple);
 
-struct tuple *
+static struct tuple *
 runtime_tuple_new(struct tuple_format *format, const char *data, const char *end);
 
 /** A virtual method table for tuple_format_runtime */
@@ -70,7 +70,7 @@ static struct tuple_format_vtab tuple_format_runtime_vtab = {
 	runtime_tuple_new,
 };
 
-struct tuple *
+static struct tuple *
 runtime_tuple_new(struct tuple_format *format, const char *data, const char *end)
 {
 	assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete);
@@ -151,12 +151,6 @@ tuple_validate_raw(struct tuple_format *format, const char *tuple)
 	return 0;
 }
 
-struct tuple *
-tuple_new(struct tuple_format *format, const char *data, const char *end)
-{
-	return format->vtab.tuple_new(format, data, end);
-}
-
 /**
  * Incremented on every snapshot and is used to distinguish tuples
  * which were created after start of a snapshot (these tuples can
diff --git a/src/box/tuple.h b/src/box/tuple.h
index d8138e2..d119909 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -430,10 +430,12 @@ tuple_extra(const struct tuple *tuple)
  * \param end the end of \a data
  * \retval tuple on success
  * \retval NULL on out of memory
- * \sa \code box.tuple.new(data) \endcode
  */
-struct tuple *
-tuple_new(struct tuple_format *format, const char *data, const char *end);
+static inline struct tuple *
+tuple_new(struct tuple_format *format, const char *data, const char *end)
+{
+	return format->vtab.tuple_new(format, data, end);
+}
 
 /**
  * Free the tuple of any engine.
diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
index cc727ec..155d0b5 100644
--- a/src/box/tuple_format.h
+++ b/src/box/tuple_format.h
@@ -66,12 +66,15 @@ struct tuple_format;
 
 /** Engine-specific tuple format methods. */
 struct tuple_format_vtab {
-	/** Free allocated tuple using engine-specific memory allocator. */
+	/** Free allocated tuple using engine-specific
+	 * memory allocator. */
 	void
 	(*tuple_delete)(struct tuple_format *format, struct tuple *tuple);
-	/** Allocates a new tuple on the same allocator and with the same format. */
+	/** Allocates a new tuple on the same allocator
+	 * and with the same format. */
 	struct tuple*
-	(*tuple_new)(struct tuple_format *format, const char *data, const char *end);
+	(*tuple_new)(struct tuple_format *format, const char *data,
+	             const char *end);
 };
 
 /** Tuple field meta information for tuple_format. */
diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
index 3135c58..d02ffd2 100644
--- a/src/box/vy_stmt.c
+++ b/src/box/vy_stmt.c
@@ -45,6 +45,9 @@
 #include "xrow.h"
 #include "fiber.h"
 
+static struct tuple *
+vy_tuple_new(struct tuple_format *format, const char *data, const char *end);
+
 struct tuple_format_vtab vy_tuple_format_vtab = {
 	vy_tuple_delete,
 	vy_tuple_new,
@@ -70,7 +73,7 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple)
 	free(tuple);
 }
 
-struct tuple *
+static struct tuple *
 vy_tuple_new(struct tuple_format *format, const char *data, const char *end)
 {
 	return vy_stmt_new_insert(format, data, end);
diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
index efc99c8..a33739d 100644
--- a/src/box/vy_stmt.h
+++ b/src/box/vy_stmt.h
@@ -206,15 +206,6 @@ void
 vy_tuple_delete(struct tuple_format *format, struct tuple *tuple);
 
 /**
- * Create the tuple of a vinyl space.
- * @param format Target tuple format.
- * @param data Target tuple data.
- * @param size Size of the data.
- */
-struct tuple *
-vy_tuple_new(struct tuple_format *format, const char *data, const char *end);
-
-/**
  * Duplicate the statememnt.
  *
  * @param stmt statement
diff --git a/test/box/update.result b/test/box/update.result
index 7b9e295..c71cbd8 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -852,54 +852,25 @@ s:drop()
 --
 -- gh-3051 Lost format while tuple update
 --
-format = {}
----
-...
-format[1] = {name = 'KEY', type = 'unsigned'}
----
-...
-format[2] = {name = 'VAL', type = 'string'}
----
-...
-s = box.schema.create_space('tst_sample', {format = format})
----
-...
-pk = s:create_index('pk')
----
-...
-s:insert({1, 'sss'})
+test_run = require('test_run')
 ---
-- [1, 'sss']
 ...
-aa = box.space.tst_sample:get(1)
+inspector = test_run.new()
 ---
 ...
-aa.VAL
+engine = inspector:get_cfg('engine')
 ---
-- sss
 ...
-aa = aa:update({{'=',2,'ssss'}})
----
-...
-collectgarbage()
----
-- 0
-...
-aa.VAL
----
-- ssss
-...
-s:drop()
+format = {}
 ---
 ...
-aa = nil
+format[1] = {name = 'KEY', type = 'unsigned'}
 ---
 ...
-collectgarbage()
+format[2] = {name = 'VAL', type = 'string'}
 ---
-- 0
 ...
-s = box.schema.space.create('tst_sample', {engine = 'vinyl', format = format})
+s = box.schema.space.create('tst_sample', {engine = engine, format = format})
 ---
 ...
 pk = s:create_index('pk')
@@ -919,10 +890,6 @@ aa.VAL
 aa = aa:update({{'=',2,'ssss'}})
 ---
 ...
-collectgarbage()
----
-- 0
-...
 aa.VAL
 ---
 - ssss
@@ -933,10 +900,6 @@ s:drop()
 aa = nil
 ---
 ...
-collectgarbage()
----
-- 0
-...
 -- test transform integrity
 space = box.schema.space.create('tweedledum')
 ---
@@ -967,3 +930,6 @@ t:transform(1, n - 1)
 t = nil
 ---
 ...
+space:drop()
+---
+...
diff --git a/test/box/update.test.lua b/test/box/update.test.lua
index 1dc1751..b6271a6 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -273,32 +273,21 @@ s:drop()
 --
 -- gh-3051 Lost format while tuple update
 --
+test_run = require('test_run')
+inspector = test_run.new()
+engine = inspector:get_cfg('engine')
 format = {}
 format[1] = {name = 'KEY', type = 'unsigned'}
 format[2] = {name = 'VAL', type = 'string'}
-s = box.schema.create_space('tst_sample', {format = format})
+s = box.schema.space.create('tst_sample', {engine = engine, format = format})
 pk = s:create_index('pk')
 s:insert({1, 'sss'})
 aa = box.space.tst_sample:get(1)
 aa.VAL
 aa = aa:update({{'=',2,'ssss'}})
-collectgarbage()
 aa.VAL
 s:drop()
 aa = nil
-collectgarbage()
-
-s = box.schema.space.create('tst_sample', {engine = 'vinyl', format = format})
-pk = s:create_index('pk')
-s:insert({1, 'sss'})
-aa = box.space.tst_sample:get(1)
-aa.VAL
-aa = aa:update({{'=',2,'ssss'}})
-collectgarbage()
-aa.VAL
-s:drop()
-aa = nil
-collectgarbage()
 
 -- test transform integrity
 space = box.schema.space.create('tweedledum')
@@ -310,3 +299,4 @@ tab = {}; for i=1,n,1 do table.insert(tab, i) end
 t = box.tuple.new(tab)
 t:transform(1, n - 1)
 t = nil
+space:drop()
-- 
2.7.4



On 10.04.2018 13:44, Vladislav Shpilevoy wrote:
> Hello. See my 19 comments below.
> 
> 1. Build fails:
> 
> /Users/v.shpilevoy/Work/Repositories/tarantool/src/box/lua/tuple.c:405:6: 
> error: variable 'new_tuple' is used uninitialized whenever 'if' 
> condition is true [-Werror,-Wsometimes-uninitialized]
>          if (new_data == NULL) {
>              ^~~~~~~~~~~~~~~~
> /Users/v.shpilevoy/Work/Repositories/tarantool/src/box/lua/tuple.c:420:6: 
> note: uninitialized use occurs here
>          if (new_tuple == NULL)
>              ^~~~~~~~~
> /Users/v.shpilevoy/Work/Repositories/tarantool/src/box/lua/tuple.c:405:2: 
> note: remove the 'if' if its condition is always false
>          if (new_data == NULL) {
>          ^~~~~~~~~~~~~~~~~~~~~~~
> /Users/v.shpilevoy/Work/Repositories/tarantool/src/box/lua/tuple.c:409:2: 
> note: variable 'new_tuple' is declared here
>          struct tuple *new_tuple = tuple_new(box_tuple_format_default(),
>          ^
> 1 error generated.
> 
> 
> On 10/04/2018 13:23, Kirill Shcherbatov wrote:
>>  From c268dedd32ed124fb25c46e3fb0bf1cd507fda62 Mon Sep 17 00:00:00 2001
>> From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
>> Date: Fri, 6 Apr 2018 19:21:32 +0300
>> Subject: [PATCH] Fixed lost format on update and upsert operations.
> 
> 2. Please, describe a problem, which the patch solves.
> 
>>
>> Fixes #3051
>> ---
>>   src/box/lua/tuple.c      |  31 +++++++++++--
>>   src/box/memtx_tuple.cc   |   1 +
>>   src/box/tuple.c          |  31 ++++++++-----
>>   src/box/tuple.h          |   2 +-
>>   src/box/tuple_format.h   |   5 +-
>>   src/box/vy_stmt.c        |   7 +++
>>   src/box/vy_stmt.h        |   9 ++++
>>   test/box/update.result   | 118 +++++++++++++++++++++++++++++++++++++++++++++++
>>   test/box/update.test.lua |  41 ++++++++++++++++
>>   9 files changed, 226 insertions(+), 19 deletions(-)
>>
>> diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
>> index 47b33c9..9bb62b0 100644
>> --- a/src/box/lua/tuple.c
>> +++ b/src/box/lua/tuple.c
>> @@ -36,6 +36,7 @@
>>   #include <small/ibuf.h>
>>   #include <small/region.h>
>>   #include <fiber.h>
>> +#include <box/tuple_update.h>
> 
> 3. Do not put project headers in <>.
> 
>>   
>>   #include "box/tuple.h"
>>   #include "box/tuple_convert.h"
>> @@ -390,15 +391,35 @@ lbox_tuple_transform(struct lua_State *L)
>>   	}
>>   	mpstream_flush(&stream);
>>   
>> -	/* Execute tuple_update */
>> -	struct tuple *new_tuple =
>> -		box_tuple_update(tuple, buf->buf, buf->buf + ibuf_used(buf));
>> +	uint32_t new_size = 0, bsize;
>> +	const char *old_data = tuple_data_range(tuple, &bsize);
>> +	struct region *region = &fiber()->gc;
>> +	size_t used = region_used(region);
>> +	const char *new_data = tuple_update_execute(region_aligned_alloc_cb,
>> +	                                            region,
>> +	                                            buf->buf,
>> +	                                            buf->buf + ibuf_used(buf),
>> +	                                            old_data,
>> +	                                            old_data + bsize,
>> +	                                            &new_size, 1, NULL);
> 
> 4. Please, do not put each argument on a separate line when possible.
> 
>> +	if (new_data == NULL) {
>> +		region_truncate(region, used);
>> +		goto error;
> 
> 5. You can remove goto error, if instead of new_data == NULL with do
> 
> if new_data != NULL then new_tuple = tuple_new(...) end
> region_truncate(...)
> if new_tuple == NULL then lua_error(...) end
> 
>> +	}
>> +	struct tuple *new_tuple = tuple_new(box_tuple_format_default(),
>> +	                                    new_data, new_data + new_size);
>> +	region_truncate(region, used);
>>   	if (new_tuple == NULL)
>> -		luaT_error(L);
>> -	/* box_tuple_update() doesn't leak on exception, see public API doc */
>> +		goto error;
>> +	tuple_bless(new_tuple);
> 
> 6. AFAIK tuple_bless here is not needed, because lua_pushtuple already 
> does tuple reference count increment.
> 
>> +
>>   	luaT_pushtuple(L, new_tuple);
>>   	ibuf_reset(buf);
>>   	return 1;
>> +error:
>> +	if (new_tuple == NULL)
>> +		luaT_error(L);
>> +	return 0;
> 
> 7. Error label will be removed, but please answer yourself why error 
> label can return non-error code.
> 
>>   }
>>   
>>   /**
>> diff --git a/src/box/memtx_tuple.cc b/src/box/memtx_tuple.cc
>> index 70453bd..1990bdf 100644
>> --- a/src/box/memtx_tuple.cc
>> +++ b/src/box/memtx_tuple.cc
>> diff --git a/src/box/tuple.c b/src/box/tuple.c
>> index d4760f3..cb57d81 100644
>> --- a/src/box/tuple.c
>> +++ b/src/box/tuple.c
>> @@ -61,15 +61,19 @@ struct tuple_format *tuple_format_runtime;
>>   static void
>>   runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple);
>>   
>> +struct tuple *
> 
> 8. Please, make this function static.
> 
>> +runtime_tuple_new(struct tuple_format *format, const char *data, const char *end);
>> +
>>   /** A virtual method table for tuple_format_runtime */
>>   static struct tuple_format_vtab tuple_format_runtime_vtab = {
>>   	runtime_tuple_delete,
>> +	runtime_tuple_new,
>>   };
>>   
>>   struct tuple *
>> -tuple_new(struct tuple_format *format, const char *data, const char *end)
>> +runtime_tuple_new(struct tuple_format *format, const char *data, const char *end)
>>   {
>> -	assert(format->vtab.destroy == tuple_format_runtime_vtab.destroy);
>> +	assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete);
>>   
>>   	mp_tuple_assert(data, end);
>>   	size_t data_len = end - data;
>> @@ -102,7 +106,7 @@ tuple_new(struct tuple_format *format, const char *data, const char *end)
>>   static void
>>   runtime_tuple_delete(struct tuple_format *format, struct tuple *tuple)
>>   {
>> -	assert(format->vtab.destroy == tuple_format_runtime_vtab.destroy);
>> +	assert(format->vtab.tuple_delete == tuple_format_runtime_vtab.tuple_delete);
>>   	say_debug("%s(%p)", __func__, tuple);
>>   	assert(tuple->refs == 0);
>>   	size_t total = sizeof(struct tuple) + tuple_format_meta_size(format) +
>> @@ -147,6 +151,12 @@ tuple_validate_raw(struct tuple_format *format, const char *tuple)
>>   	return 0;
>>   }
>>   
>> +struct tuple *
> 
> 9. Please make this wrapper be in tuple.h, static inline and add a 
> comment. It is a common practice for our virtual methods wrappers. See 
> key_def.h for examples.
> 
>> +tuple_new(struct tuple_format *format, const char *data, const char *end)
>> +{
>> +	return format->vtab.tuple_new(format, data, end);
>> +}
>> +
>>   /**
>>    * Incremented on every snapshot and is used to distinguish tuples
>>    * which were created after start of a snapshot (these tuples can
>> @@ -396,22 +406,20 @@ box_tuple_t *
>>   box_tuple_update(const box_tuple_t *tuple, const char *expr,
>>   		 const char *expr_end)
>>   {
>> -	struct tuple_format *format = tuple_format_runtime;
>> -
>>   	uint32_t new_size = 0, bsize;
>>   	const char *old_data = tuple_data_range(tuple, &bsize);
>>   	struct region *region = &fiber()->gc;
>>   	size_t used = region_used(region);
>>   	const char *new_data =
>>   		tuple_update_execute(region_aligned_alloc_cb, region, expr,
>> -				     expr_end, old_data, old_data + bsize,
>> -				     &new_size, 1, NULL);
>> +		                     expr_end, old_data, old_data + bsize,
>> +		                     &new_size, 1, NULL);
> 
> 10. Garbage diff.
> 
>>   	if (new_data == NULL) {
>>   		region_truncate(region, used);
>>   		return NULL;
>>   	}
>> -
>> -	struct tuple *ret = tuple_new(format, new_data, new_data + new_size);
>> +	struct tuple *ret = tuple_new(tuple_format(tuple), new_data,
>> +	                              new_data + new_size);
> 
> 11. Same.
> 
>>   	region_truncate(region, used);
>>   	if (ret != NULL)
>>   		return tuple_bless(ret);
>> @@ -422,8 +430,6 @@ box_tuple_t *
>>   box_tuple_upsert(const box_tuple_t *tuple, const char *expr,
>>   		 const char *expr_end)
>>   {
>> -	struct tuple_format *format = tuple_format_runtime;
> 
> 12. Same.
> 
>> -
>>   	uint32_t new_size = 0, bsize;
>>   	const char *old_data = tuple_data_range(tuple, &bsize);
>>   	struct region *region = &fiber()->gc;
>> @@ -437,7 +443,8 @@ box_tuple_upsert(const box_tuple_t *tuple, const char *expr,
>>   		return NULL;
>>   	}
>>   
>> -	struct tuple *ret = tuple_new(format, new_data, new_data + new_size);
>> +	struct tuple *ret = tuple_new(tuple_format(tuple),
>> +	                              new_data, new_data + new_size);
> 
> 13. Same.
> 
>>   	region_truncate(region, used);
>>   	if (ret != NULL)
>>   		return tuple_bless(ret);
>> diff --git a/src/box/tuple.h b/src/box/tuple.h
>> index 6ebedf5..d8138e2 100644
>> --- a/src/box/tuple.h
>> +++ b/src/box/tuple.h
>> @@ -445,7 +445,7 @@ tuple_delete(struct tuple *tuple)
>>   	say_debug("%s(%p)", __func__, tuple);
>>   	assert(tuple->refs == 0);
>>   	struct tuple_format *format = tuple_format(tuple);
>> -	format->vtab.destroy(format, tuple);
>> +	format->vtab.tuple_delete(format, tuple);
>>   }
>>   
>>   /**
>> diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
>> index d35182d..cc727ec 100644
>> --- a/src/box/tuple_format.h
>> +++ b/src/box/tuple_format.h
>> @@ -68,7 +68,10 @@ struct tuple_format;
>>   struct tuple_format_vtab {
>>   	/** Free allocated tuple using engine-specific memory allocator. */
>>   	void
>> -	(*destroy)(struct tuple_format *format, struct tuple *tuple);
>> +	(*tuple_delete)(struct tuple_format *format, struct tuple *tuple);
>> +	/** Allocates a new tuple on the same allocator and with the same format. */
> 
> 14. Out of 66 symbols.
> 
>> +	struct tuple*
>> +	(*tuple_new)(struct tuple_format *format, const char *data, const char *end);
> 
> 15. Out of 80 symbols.
> 
>>   };
>>   
>>   /** Tuple field meta information for tuple_format. */
>> diff --git a/src/box/vy_stmt.c b/src/box/vy_stmt.c
>> index 84182e7..3135c58 100644
>> --- a/src/box/vy_stmt.c
>> +++ b/src/box/vy_stmt.c
>> @@ -47,6 +47,7 @@
>>   
>>   struct tuple_format_vtab vy_tuple_format_vtab = {
>>   	vy_tuple_delete,
>> +	vy_tuple_new,
>>   };
>>   
>>   size_t vy_max_tuple_size = 1024 * 1024;
>> @@ -69,6 +70,12 @@ vy_tuple_delete(struct tuple_format *format, struct tuple *tuple)
>>   	free(tuple);
>>   }
>>   
>> +struct tuple *
> 
> 16. Static.
> 
>> +vy_tuple_new(struct tuple_format *format, const char *data, const char *end)
>> +{
>> +	return vy_stmt_new_insert(format, data, end);
>> +}
>> +
>>   /**
>>    * Allocate a vinyl statement object on base of the struct tuple
>>    * with malloc() and the reference counter equal to 1.
>> diff --git a/src/box/vy_stmt.h b/src/box/vy_stmt.h
>> index a33739d..efc99c8 100644
>> --- a/src/box/vy_stmt.h
>> +++ b/src/box/vy_stmt.h
>> @@ -206,6 +206,15 @@ void
>>   vy_tuple_delete(struct tuple_format *format, struct tuple *tuple);
>>   
>>   /**
>> + * Create the tuple of a vinyl space.
>> + * @param format Target tuple format.
>> + * @param data Target tuple data.
>> + * @param size Size of the data.
>> + */
>> +struct tuple *
>> +vy_tuple_new(struct tuple_format *format, const char *data, const char *end);
> 
> 17. Used in vy_stmt.c only. You can remove it from the header.
> 
>> diff --git a/test/box/update.test.lua b/test/box/update.test.lua
>> index c455bc1..1dc1751 100644
>> --- a/test/box/update.test.lua
>> +++ b/test/box/update.test.lua
>> @@ -269,3 +269,44 @@ t:update({{'=', 3, map}})
>>   s:update(1, {{'=', 3, map}})
>>   
>>   s:drop()
>> +
>> +--
>> +-- gh-3051 Lost format while tuple update
>> +--
>> +format = {}
>> +format[1] = {name = 'KEY', type = 'unsigned'}
>> +format[2] = {name = 'VAL', type = 'string'}
>> +s = box.schema.create_space('tst_sample', {format = format})
>> +pk = s:create_index('pk')
>> +s:insert({1, 'sss'})
>> +aa = box.space.tst_sample:get(1)
>> +aa.VAL
>> +aa = aa:update({{'=',2,'ssss'}})
>> +collectgarbage()
> 
> 18. Even if a tuple leaks, this code does not detect it. Either create a 
> correct check via Lua weak references (see __mode = 'k'/'v' and 
> net.box.test.lua), or remove this code.
> 
>> +aa.VAL
>> +s:drop()
>> +aa = nil
>> +collectgarbage()
>> +
>> +s = box.schema.space.create('tst_sample', {engine = 'vinyl', format = format})
> 
> 19. When you need multiple engines for the same test, you must use 
> engine/ test suite.
> 
>> +pk = s:create_index('pk')
>> +s:insert({1, 'sss'})
>> +aa = box.space.tst_sample:get(1)
>> +aa.VAL
>> +aa = aa:update({{'=',2,'ssss'}})
>> +collectgarbage()
>> +aa.VAL
>> +s:drop()
>> +aa = nil
>> +collectgarbage()
>> +
>> +-- test transform integrity
>> +space = box.schema.space.create('tweedledum')
>> +tmp = space:create_index('primary', { type = 'hash', parts = {1, 'string'}, unique = true })
>> +t = space:insert{'1', '2', '3', '4', '5', '6', '7'}
>> +t:transform(-1, 1)
>> +n = 2000
>> +tab = {}; for i=1,n,1 do table.insert(tab, i) end
>> +t = box.tuple.new(tab)
>> +t:transform(1, n - 1)
>> +t = nil
>>
> 

  reply	other threads:[~2018-04-15 10:04 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-06 16:24 [tarantool-patches] [PATCH v2 0/2] Fix lost format on update operation Kirill Shcherbatov
2018-04-06 16:24 ` [tarantool-patches] [PATCH v2 1/2] Fixed invalid check in lbox_tuple_transform Kirill Shcherbatov
2018-04-06 16:24 ` [tarantool-patches] [PATCH v2 2/2] Fixed lost format on update and upsert operations Kirill Shcherbatov
2018-04-08 13:56   ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-10 10:23     ` Kirill Shcherbatov
2018-04-10 10:44       ` Vladislav Shpilevoy
2018-04-15 10:03         ` Kirill Shcherbatov [this message]
2018-04-15 13:18           ` Vladislav Shpilevoy
2018-04-16  7:47             ` Kirill Shcherbatov
2018-04-16 10:07               ` Vladislav Shpilevoy
2018-04-16 16:51                 ` Kirill Shcherbatov
2018-04-16 17:14                   ` Vladislav Shpilevoy
2018-04-18 12:28                     ` Vladimir Davydov
2018-04-18 12:55                       ` Kirill Shcherbatov
2018-04-22 15:15                         ` Vladislav Shpilevoy
2018-04-28  6:56                           ` Kirill Shcherbatov
2018-04-28  9:29 ` [tarantool-patches] Re: [PATCH v2 0/2] Fix lost format on update operation Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f724f6b0-9b99-385c-5830-8037663c0c5b@tarantool.org \
    --to=kshcherbatov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH v2 2/2] Fixed lost format on update and upsert operations.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox