From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 6E3D422CAD for ; Mon, 1 Jul 2019 19:29:18 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LT6UeDl3STDJ for ; Mon, 1 Jul 2019 19:29:18 -0400 (EDT) Received: from smtp43.i.mail.ru (smtp43.i.mail.ru [94.100.177.103]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B80F2205D8 for ; Mon, 1 Jul 2019 19:29:17 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: Fix UPDATE for types unknown to SQL. From: "n.pettik" In-Reply-To: <132ca83597d0e9c2b4ef75bc8f0d03d22cdf27dd.1561736006.git.imeevma@gmail.com> Date: Tue, 2 Jul 2019 02:29:14 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <132ca83597d0e9c2b4ef75bc8f0d03d22cdf27dd.1561736006.git.imeevma@gmail.com> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Imeev Mergen > On 28 Jun 2019, at 18:34, imeevma@tarantool.org wrote: >=20 > 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=E2=80=99ve 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 = =20 to be inserted to the space is exposed to this conversion which is = =20 invoked during execution of OP_MakeRecord opcode (which in turn forms = =20 tuple). This function was not adjusted to operate on ARRAY, MAP and ANY = =20 field types since they are poorly supported in current SQL = =20 implementation. Hence, when tuple to be inserted in space having = =20 mentioned field types reaches this function, it results in error. Note = =20 that we can't set ARRAY or MAP types in SQL, but such situation may = =20 appear during UPDATE operation on space created via Lua interface. This = =20 problem is solved by extending implicit type conversions with obvious = =20 casts: array field can be casted to array, map to map and any to any. =20= > Closes #4189 > --- > https://github.com/tarantool/tarantool/issues/4189 > = https://github.com/tarantool/tarantool/tree/imeevma/gh-4189-field-type-con= version-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 &=3D ~(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 =3D=3D SQL_SUBTYPE_MSGPACK) { + assert(mp_typeof(*record->z) =3D=3D MP_MAP || + mp_typeof(*record->z) =3D=3D MP_ARRAY); + return -1; + } return 0; case FIELD_TYPE_MAP: if (record->subtype =3D=3D SQL_SUBTYPE_MSGPACK && @@ -364,6 +367,8 @@ mem_apply_type(struct Mem *record, enum field_type = type) mp_typeof(*record->z) =3D=3D 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 =3D {} --- 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}) =20 -- --- 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 =3D {} format[1] =3D {type =3D 'integer', name =3D 'I'}