From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Imeev Mergen <imeevma@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v2 2/2] sql: fix error in case ARRAY/MAP converted to SCALAR Date: Wed, 7 Aug 2019 21:25:59 +0300 [thread overview] Message-ID: <FCD1728E-8FAC-4BAD-B780-2422918D68B0@tarantool.org> (raw) In-Reply-To: <20190727101643.GA10982@tarantool.org> > > New patch: > > From 5dde6f6398ca9845c3a17173b9e9625bc1b20d32 Mon Sep 17 00:00:00 2001 > From: Mergen Imeev <imeevma@gmail.com> > Date: Mon, 22 Jul 2019 12:54:34 +0300 > Subject: [PATCH] sql: fix error in case ARRAY/MAP converted to SCALAR > > Since ARRAY and MAP cannot be converted to a scalar, this > operation should throw an error. But when the error was throws > from SQL, the error was unreadable. The reason for this is that > the given array or map was not correctly converted to a string. > This patch fixes the problem by converting ARRAY or MAP to their > string representation. > For example: > > box.execute('CREATE TABLE t1(i INT PRIMARY KEY, a SCALAR);') > format = {} > format[1] = {type = 'integer', name = 'I'} > format[2] = {type = 'array', name = 'A'} > s = box.schema.space.create('T2', {format=format}) > i = s:create_index('ii') > s:insert({1, {1,2,3}}) > box.execute('INSERT INTO t1 SELECT * FROM t2;') > > Should return: > - error: 'Type mismatch: can not convert [1, 2, 3] to scalar' > > Follow-up #4189 Fixed a bit commit message: sql: make valueToText() operate on MAP/ARRAY values Since ARRAY and MAP cannot be converted to SCALAR type, this operation should throw an error. But when the error is raised in SQL, it is displayed in unreadable form. The reason for this is that the given array or map is not correctly converted to a string. This patch fixes the problem by converting ARRAY or MAP to their string representation. For example: box.execute('CREATE TABLE t1(i INT PRIMARY KEY, a SCALAR);') format = {} format[1] = {type = 'integer', name = 'I'} format[2] = {type = 'array', name = 'A'} s = box.schema.space.create('T2', {format=format}) i = s:create_index('ii') s:insert({1, {1,2,3}}) box.execute('INSERT INTO t1 SELECT * FROM t2;') Should return: - error: 'Type mismatch: can not convert [1, 2, 3] to scalar' Follow-up #4189 > diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c > index 847a6b0..8bea46b 100644 > --- a/src/box/sql/vdbemem.c > +++ b/src/box/sql/vdbemem.c > @@ -1135,6 +1135,19 @@ valueToText(sql_value * pVal) > { > assert(pVal != 0); > assert((pVal->flags & (MEM_Null)) == 0); > + if ((pVal->flags & MEM_Subtype) != 0 && > + pVal->subtype == SQL_SUBTYPE_MSGPACK) { > + const char *value = mp_str(pVal->z); > + size_t len = strlen(value) + 1; > + char *result = region_alloc(&fiber()->gc, len); > + if (result == NULL) { > + diag_set(OutOfMemory, len, "region_alloc", "result"); > + sqlOomFault(sql_get()); > + return NULL; > + } > + memcpy(result, value, len); > + return result; That’s not what we need IMHO. Firstly, for all other memory types valueToText() function returns pVal->z, i.e. this function firstly converts value to string and then returns it. Secondly, we don’t track value allocated on region: obviously if it is used after transaction commitment, it will lead to use-after-free bug. I can’t say whether this scenario is possible looking only on code. You should rather patch sqlVdbeMemStringify: reserve memory using malloc, memcpy string to mem, change type of memory to MEM_Str. So that make it work with ARRAY and MAP types in the same way as with other types.
next prev parent reply other threads:[~2019-08-07 18:26 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-24 8:11 [tarantool-patches] [PATCH v2 0/2] sql: add ARRAY, MAP and ANY types to mem_apply_type() imeevma 2019-07-24 8:12 ` [tarantool-patches] [PATCH v2 1/2] " imeevma 2019-07-24 8:12 ` [tarantool-patches] [PATCH v2 2/2] sql: fix error in case ARRAY/MAP converted to SCALAR imeevma 2019-07-24 12:24 ` [tarantool-patches] " n.pettik 2019-07-24 22:37 ` Konstantin Osipov 2019-07-24 23:30 ` n.pettik 2019-07-27 10:16 ` Mergen Imeev 2019-08-07 18:25 ` n.pettik [this message] 2019-08-28 13:30 ` Mergen Imeev 2019-08-29 12:19 ` Nikita Pettik 2019-09-02 13:53 ` Mergen Imeev 2019-09-10 13:54 ` korablev 2019-09-11 8:13 ` Mergen Imeev 2019-09-11 9:25 ` Nikita Pettik
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=FCD1728E-8FAC-4BAD-B780-2422918D68B0@tarantool.org \ --to=korablev@tarantool.org \ --cc=imeevma@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v2 2/2] sql: fix error in case ARRAY/MAP converted to SCALAR' \ /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