From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Imeev Mergen <imeevma@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: Fix UPDATE for types unknown to SQL. Date: Tue, 2 Jul 2019 02:29:14 +0300 [thread overview] Message-ID: <F5517B43-5586-418B-A564-75A3006CA36B@tarantool.org> (raw) In-Reply-To: <132ca83597d0e9c2b4ef75bc8f0d03d22cdf27dd.1561736006.git.imeevma@gmail.com> > On 28 Jun 2019, at 18:34, imeevma@tarantool.org wrote: > > Before this patch, UPDATE throwed an error when executed on a > tuple with types unknown to SQL. This patch fixes the problem in > case the fields with unknown types are not updated. I’ve fixed commit message a bit: sql: add ARRAY, MAP and ANY types to mem_apply_type() mem_apply_type() implements implicit type conversion. As a rule, tuple to be inserted to the space is exposed to this conversion which is invoked during execution of OP_MakeRecord opcode (which in turn forms tuple). This function was not adjusted to operate on ARRAY, MAP and ANY field types since they are poorly supported in current SQL implementation. Hence, when tuple to be inserted in space having mentioned field types reaches this function, it results in error. Note that we can't set ARRAY or MAP types in SQL, but such situation may appear during UPDATE operation on space created via Lua interface. This problem is solved by extending implicit type conversions with obvious casts: array field can be casted to array, map to map and any to any. > Closes #4189 > --- > https://github.com/tarantool/tarantool/issues/4189 > https://github.com/tarantool/tarantool/tree/imeevma/gh-4189-field-type-conversion-error I suggest follow refactoring: diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 8ddc29d14..f8cf1afc7 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -292,7 +292,9 @@ mem_apply_numeric_type(struct Mem *record) * Convert mem to a string representation. * * SCALAR: - * Mem is unchanged, but flag is set to BLOB. + * Mem is unchanged, but flag is set to BLOB in case of + * scalar-like type. Otherwise, (MAP, ARRAY) conversion + * is impossible. * * BOOLEAN: * If memory holds BOOLEAN no actions take place. @@ -300,13 +302,9 @@ mem_apply_numeric_type(struct Mem *record) * ANY: * Mem is unchanged, no actions take place. * - * MAP: - * If memory holds value with SQL_SUBTYPE_MSGPACK subtype and - * data has MP_MAP type, no actions take place. - * - * ARRAY: - * If memory holds value with SQL_SUBTYPE_MSGPACK subtype and - * data has MP_ARRAY type, no actions take place. + * MAP/ARRAY: + * These types can't be casted to scalar ones, or to each + * other. So the only valid conversion is to type itself. * * @param record The value to apply type to. * @param type The type to be applied. @@ -351,8 +349,13 @@ mem_apply_type(struct Mem *record, enum field_type type) } record->flags &= ~(MEM_Real | MEM_Int); return 0; - case FIELD_TYPE_ANY: case FIELD_TYPE_SCALAR: + /* Can't cast MAP and ARRAY to scalar types. */ + if (record->subtype == SQL_SUBTYPE_MSGPACK) { + assert(mp_typeof(*record->z) == MP_MAP || + mp_typeof(*record->z) == MP_ARRAY); + return -1; + } return 0; case FIELD_TYPE_MAP: if (record->subtype == SQL_SUBTYPE_MSGPACK && @@ -364,6 +367,8 @@ mem_apply_type(struct Mem *record, enum field_type type) mp_typeof(*record->z) == MP_ARRAY) return 0; return -1; + case FIELD_TYPE_ANY: + return 0; default: return -1; } diff --git a/test/sql/types.result b/test/sql/types.result index 5cc7f1659..32588d38c 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -967,8 +967,8 @@ box.execute('SELECT ?', {true}) - [true] ... -- --- gh-4189: Update throws an error when executed on a tuple with --- types unknown to SQL. +-- gh-4189: make sure that update doesn't throw an error if format +-- of table features map/array field types. -- format = {} --- diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index d33314a0b..69af421d2 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -236,8 +236,8 @@ box.execute('SELECT \'9223372036854\' + 1;') box.execute('SELECT ?', {true}) -- --- gh-4189: Update throws an error when executed on a tuple with --- types unknown to SQL. +-- gh-4189: make sure that update doesn't throw an error if format +-- of table features map/array field types. -- format = {} format[1] = {type = 'integer', name = 'I'}
next prev parent reply other threads:[~2019-07-01 23:29 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-28 15:34 [tarantool-patches] " imeevma 2019-07-01 23:29 ` n.pettik [this message] 2019-07-08 10:13 ` [tarantool-patches] " Mergen Imeev 2019-07-08 13:32 ` n.pettik 2019-07-09 9:14 ` Imeev Mergen 2019-07-15 13:32 ` n.pettik 2019-07-16 14:19 ` Mergen Imeev 2019-07-18 15:59 ` n.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=F5517B43-5586-418B-A564-75A3006CA36B@tarantool.org \ --to=korablev@tarantool.org \ --cc=imeevma@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH v1 1/1] sql: Fix UPDATE for types unknown to SQL.' \ /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