Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH v1 1/1] box: fix format of tuple produced with frommap()
@ 2019-03-20 13:31 Kirill Shcherbatov
  2019-03-21 13:36 ` [tarantool-patches] " Vladislav Shpilevoy
  2019-03-21 15:41 ` Kirill Yukhin
  0 siblings, 2 replies; 3+ messages in thread
From: Kirill Shcherbatov @ 2019-03-20 13:31 UTC (permalink / raw)
  To: tarantool-patches, v.shpilevoy; +Cc: Kirill Shcherbatov

Previously, all tuples created with frommap() used default format
table_format_runtime, and therefore the created data tuples were
not checked for the ability to be inserted into the target space.

Moreover frommap(...):tomap(...) procedures sequence also did not
work because tomap(..) routine assumes that the tuple has format
with field names.

Closes #4045

Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4045-frommap-tuple-format-fix
Issue: https://github.com/tarantool/tarantool/issues/4045
---
 src/box/lua/space.cc    |  2 +-
 src/box/lua/tuple.c     | 11 ++++++++---
 src/box/lua/tuple.h     |  3 ++-
 test/box/tuple.result   | 25 +++++++++++++++++++++----
 test/box/tuple.test.lua | 12 +++++++++---
 5 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 7aa0b6661..9dfc97b6a 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -510,7 +510,7 @@ lbox_space_frommap(struct lua_State *L)
 
 	lua_replace(L, 1);
 	lua_settop(L, 1);
-	return lbox_tuple_new(L);
+	return luaT_tuple_new(L, space->format);
 usage_error:
 	return luaL_error(L, "Usage: space:frommap(map, opts)");
 }
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 5f2bfc807..60c1a3999 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -94,7 +94,7 @@ luaT_istuple(struct lua_State *L, int narg)
 }
 
 int
-lbox_tuple_new(lua_State *L)
+luaT_tuple_new(struct lua_State *L, struct tuple_format *format)
 {
 	int argc = lua_gettop(L);
 	if (argc < 1) {
@@ -120,8 +120,7 @@ lbox_tuple_new(lua_State *L)
 	}
 	mpstream_flush(&stream);
 
-	box_tuple_format_t *fmt = box_tuple_format_default();
-	struct tuple *tuple = box_tuple_new(fmt, buf->buf,
+	struct tuple *tuple = box_tuple_new(format, buf->buf,
 					   buf->buf + ibuf_used(buf));
 	if (tuple == NULL)
 		return luaT_error(L);
@@ -131,6 +130,12 @@ lbox_tuple_new(lua_State *L)
 	return 1;
 }
 
+static int
+lbox_tuple_new(lua_State *L)
+{
+	return luaT_tuple_new(L, box_tuple_format_default());
+}
+
 static int
 lbox_tuple_gc(struct lua_State *L)
 {
diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
index 5d7062eb8..ba7f01f4b 100644
--- a/src/box/lua/tuple.h
+++ b/src/box/lua/tuple.h
@@ -37,6 +37,7 @@ extern "C" {
 #endif /* defined(__cplusplus) */
 
 struct tuple;
+struct tuple_format;
 typedef struct tuple box_tuple_t;
 struct lua_State;
 struct mpstream;
@@ -67,7 +68,7 @@ luaT_istuple(struct lua_State *L, int idx);
 /** \endcond public */
 
 int
-lbox_tuple_new(struct lua_State *L);
+luaT_tuple_new(struct lua_State *L, struct tuple_format *format);
 
 static inline int
 luaT_pushtupleornil(struct lua_State *L, struct tuple *tuple)
diff --git a/test/box/tuple.result b/test/box/tuple.result
index 16aa66b1a..82ad8404d 100644
--- a/test/box/tuple.result
+++ b/test/box/tuple.result
@@ -1069,13 +1069,13 @@ format = {}
 format[1] = {name = 'aaa', type = 'unsigned'}
 ---
 ...
-format[2] = {name = 'bbb', type = 'unsigned'}
+format[2] = {name = 'bbb', type = 'unsigned', is_nullable = true}
 ---
 ...
-format[3] = {name = 'ccc', type = 'unsigned'}
+format[3] = {name = 'ccc', type = 'unsigned', is_nullable = true}
 ---
 ...
-format[4] = {name = 'ddd', type = 'unsigned'}
+format[4] = {name = 'ddd', type = 'unsigned', is_nullable = true}
 ---
 ...
 s = box.schema.create_space('test', {format = format})
@@ -1100,7 +1100,7 @@ s:frommap()
 ...
 s:frommap({})
 ---
-- []
+- error: Tuple field 1 required by space format is missing
 ...
 s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true})
 ---
@@ -1211,6 +1211,23 @@ s2:frommap({a="1", k="11"})
 ---
 - ['1', null, null, null, null, null, null, null, null, null, '11']
 ...
+--
+-- gh-4045: space:frommap():tomap() conversion fail
+--
+s2:frommap({a="1", k="11"}):tomap({names_only = true})
+---
+- i: null
+  f: null
+  c: null
+  g: null
+  b: null
+  j: null
+  k: '11'
+  d: null
+  h: null
+  a: '1'
+  e: null
+...
 s2:drop()
 ---
 ...
diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
index 0c89feace..8030b0884 100644
--- a/test/box/tuple.test.lua
+++ b/test/box/tuple.test.lua
@@ -354,9 +354,9 @@ box.tuple.bsize == t.bsize
 --
 format = {}
 format[1] = {name = 'aaa', type = 'unsigned'}
-format[2] = {name = 'bbb', type = 'unsigned'}
-format[3] = {name = 'ccc', type = 'unsigned'}
-format[4] = {name = 'ddd', type = 'unsigned'}
+format[2] = {name = 'bbb', type = 'unsigned', is_nullable = true}
+format[3] = {name = 'ccc', type = 'unsigned', is_nullable = true}
+format[4] = {name = 'ddd', type = 'unsigned', is_nullable = true}
 s = box.schema.create_space('test', {format = format})
 s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
 s:frommap({ddd = 1, aaa = 2, bbb = 3})
@@ -409,4 +409,10 @@ s2:format({{name="a", type="str"}, {name="b", type="str", is_nullable=true},
            {name="k", type="str", is_nullable=true}});
 test_run:cmd("setopt delimiter ''");
 s2:frommap({a="1", k="11"})
+
+--
+-- gh-4045: space:frommap():tomap() conversion fail
+--
+s2:frommap({a="1", k="11"}):tomap({names_only = true})
+
 s2:drop()
-- 
2.21.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tarantool-patches] Re: [PATCH v1 1/1] box: fix format of tuple produced with frommap()
  2019-03-20 13:31 [tarantool-patches] [PATCH v1 1/1] box: fix format of tuple produced with frommap() Kirill Shcherbatov
@ 2019-03-21 13:36 ` Vladislav Shpilevoy
  2019-03-21 15:41 ` Kirill Yukhin
  1 sibling, 0 replies; 3+ messages in thread
From: Vladislav Shpilevoy @ 2019-03-21 13:36 UTC (permalink / raw)
  To: Kirill Shcherbatov, tarantool-patches, Kirill Yukhin

LGTM.

On 20/03/2019 16:31, Kirill Shcherbatov wrote:
> Previously, all tuples created with frommap() used default format
> table_format_runtime, and therefore the created data tuples were
> not checked for the ability to be inserted into the target space.
> 
> Moreover frommap(...):tomap(...) procedures sequence also did not
> work because tomap(..) routine assumes that the tuple has format
> with field names.
> 
> Closes #4045
> 
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4045-frommap-tuple-format-fix
> Issue: https://github.com/tarantool/tarantool/issues/4045
> ---
>   src/box/lua/space.cc    |  2 +-
>   src/box/lua/tuple.c     | 11 ++++++++---
>   src/box/lua/tuple.h     |  3 ++-
>   test/box/tuple.result   | 25 +++++++++++++++++++++----
>   test/box/tuple.test.lua | 12 +++++++++---
>   5 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
> index 7aa0b6661..9dfc97b6a 100644
> --- a/src/box/lua/space.cc
> +++ b/src/box/lua/space.cc
> @@ -510,7 +510,7 @@ lbox_space_frommap(struct lua_State *L)
>   
>   	lua_replace(L, 1);
>   	lua_settop(L, 1);
> -	return lbox_tuple_new(L);
> +	return luaT_tuple_new(L, space->format);
>   usage_error:
>   	return luaL_error(L, "Usage: space:frommap(map, opts)");
>   }
> diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
> index 5f2bfc807..60c1a3999 100644
> --- a/src/box/lua/tuple.c
> +++ b/src/box/lua/tuple.c
> @@ -94,7 +94,7 @@ luaT_istuple(struct lua_State *L, int narg)
>   }
>   
>   int
> -lbox_tuple_new(lua_State *L)
> +luaT_tuple_new(struct lua_State *L, struct tuple_format *format)
>   {
>   	int argc = lua_gettop(L);
>   	if (argc < 1) {
> @@ -120,8 +120,7 @@ lbox_tuple_new(lua_State *L)
>   	}
>   	mpstream_flush(&stream);
>   
> -	box_tuple_format_t *fmt = box_tuple_format_default();
> -	struct tuple *tuple = box_tuple_new(fmt, buf->buf,
> +	struct tuple *tuple = box_tuple_new(format, buf->buf,
>   					   buf->buf + ibuf_used(buf));
>   	if (tuple == NULL)
>   		return luaT_error(L);
> @@ -131,6 +130,12 @@ lbox_tuple_new(lua_State *L)
>   	return 1;
>   }
>   
> +static int
> +lbox_tuple_new(lua_State *L)
> +{
> +	return luaT_tuple_new(L, box_tuple_format_default());
> +}
> +
>   static int
>   lbox_tuple_gc(struct lua_State *L)
>   {
> diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
> index 5d7062eb8..ba7f01f4b 100644
> --- a/src/box/lua/tuple.h
> +++ b/src/box/lua/tuple.h
> @@ -37,6 +37,7 @@ extern "C" {
>   #endif /* defined(__cplusplus) */
>   
>   struct tuple;
> +struct tuple_format;
>   typedef struct tuple box_tuple_t;
>   struct lua_State;
>   struct mpstream;
> @@ -67,7 +68,7 @@ luaT_istuple(struct lua_State *L, int idx);
>   /** \endcond public */
>   
>   int
> -lbox_tuple_new(struct lua_State *L);
> +luaT_tuple_new(struct lua_State *L, struct tuple_format *format);
>   
>   static inline int
>   luaT_pushtupleornil(struct lua_State *L, struct tuple *tuple)
> diff --git a/test/box/tuple.result b/test/box/tuple.result
> index 16aa66b1a..82ad8404d 100644
> --- a/test/box/tuple.result
> +++ b/test/box/tuple.result
> @@ -1069,13 +1069,13 @@ format = {}
>   format[1] = {name = 'aaa', type = 'unsigned'}
>   ---
>   ...
> -format[2] = {name = 'bbb', type = 'unsigned'}
> +format[2] = {name = 'bbb', type = 'unsigned', is_nullable = true}
>   ---
>   ...
> -format[3] = {name = 'ccc', type = 'unsigned'}
> +format[3] = {name = 'ccc', type = 'unsigned', is_nullable = true}
>   ---
>   ...
> -format[4] = {name = 'ddd', type = 'unsigned'}
> +format[4] = {name = 'ddd', type = 'unsigned', is_nullable = true}
>   ---
>   ...
>   s = box.schema.create_space('test', {format = format})
> @@ -1100,7 +1100,7 @@ s:frommap()
>   ...
>   s:frommap({})
>   ---
> -- []
> +- error: Tuple field 1 required by space format is missing
>   ...
>   s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true})
>   ---
> @@ -1211,6 +1211,23 @@ s2:frommap({a="1", k="11"})
>   ---
>   - ['1', null, null, null, null, null, null, null, null, null, '11']
>   ...
> +--
> +-- gh-4045: space:frommap():tomap() conversion fail
> +--
> +s2:frommap({a="1", k="11"}):tomap({names_only = true})
> +---
> +- i: null
> +  f: null
> +  c: null
> +  g: null
> +  b: null
> +  j: null
> +  k: '11'
> +  d: null
> +  h: null
> +  a: '1'
> +  e: null
> +...
>   s2:drop()
>   ---
>   ...
> diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
> index 0c89feace..8030b0884 100644
> --- a/test/box/tuple.test.lua
> +++ b/test/box/tuple.test.lua
> @@ -354,9 +354,9 @@ box.tuple.bsize == t.bsize
>   --
>   format = {}
>   format[1] = {name = 'aaa', type = 'unsigned'}
> -format[2] = {name = 'bbb', type = 'unsigned'}
> -format[3] = {name = 'ccc', type = 'unsigned'}
> -format[4] = {name = 'ddd', type = 'unsigned'}
> +format[2] = {name = 'bbb', type = 'unsigned', is_nullable = true}
> +format[3] = {name = 'ccc', type = 'unsigned', is_nullable = true}
> +format[4] = {name = 'ddd', type = 'unsigned', is_nullable = true}
>   s = box.schema.create_space('test', {format = format})
>   s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4})
>   s:frommap({ddd = 1, aaa = 2, bbb = 3})
> @@ -409,4 +409,10 @@ s2:format({{name="a", type="str"}, {name="b", type="str", is_nullable=true},
>              {name="k", type="str", is_nullable=true}});
>   test_run:cmd("setopt delimiter ''");
>   s2:frommap({a="1", k="11"})
> +
> +--
> +-- gh-4045: space:frommap():tomap() conversion fail
> +--
> +s2:frommap({a="1", k="11"}):tomap({names_only = true})
> +
>   s2:drop()
> -- 
> 2.21.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tarantool-patches] Re: [PATCH v1 1/1] box: fix format of tuple produced with frommap()
  2019-03-20 13:31 [tarantool-patches] [PATCH v1 1/1] box: fix format of tuple produced with frommap() Kirill Shcherbatov
  2019-03-21 13:36 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2019-03-21 15:41 ` Kirill Yukhin
  1 sibling, 0 replies; 3+ messages in thread
From: Kirill Yukhin @ 2019-03-21 15:41 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Kirill Shcherbatov

Hello,

On 20 Mar 16:31, Kirill Shcherbatov wrote:
> Previously, all tuples created with frommap() used default format
> table_format_runtime, and therefore the created data tuples were
> not checked for the ability to be inserted into the target space.
> 
> Moreover frommap(...):tomap(...) procedures sequence also did not
> work because tomap(..) routine assumes that the tuple has format
> with field names.
> 
> Closes #4045
> 
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4045-frommap-tuple-format-fix
> Issue: https://github.com/tarantool/tarantool/issues/4045

I've checked yout patch into master, 2.1 and 1.10 branches.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-03-21 15:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 13:31 [tarantool-patches] [PATCH v1 1/1] box: fix format of tuple produced with frommap() Kirill Shcherbatov
2019-03-21 13:36 ` [tarantool-patches] " Vladislav Shpilevoy
2019-03-21 15:41 ` Kirill Yukhin

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