From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4DBB5445320 for ; Mon, 13 Jul 2020 17:42:57 +0300 (MSK) Date: Mon, 13 Jul 2020 14:42:55 +0000 From: Nikita Pettik Message-ID: <20200713144255.GD15396@tarantool.org> References: <965068a9fb4a38da3c64e15ab531df57f879ec14.1594618005.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <965068a9fb4a38da3c64e15ab531df57f879ec14.1594618005.git.imeevma@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v4 4/5] sql: change implicit cast for assignment List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org On 13 Jul 08:33, imeevma@tarantool.org wrote: > + > * Synopsis: type(r[P1@P2]) > * > - * Apply types to a range of P2 registers starting with P1. > - * > - * P4 is a string that is P2 characters long. The nth character of the > - * string indicates the column type that should be used for the nth > - * memory cell in the range. > + * Check that types of P2 registers starting from register P1 are > + * compatible with given field types in P4. If the MEM_type of the > + * value and the given type are incompatible according to > + * field_mp_plain_type_is_compatible(), but both are numeric, > + * this opcode attempts to convert the value to the type. > */ > case OP_ApplyType: { > enum field_type *types = pOp->p4.types; > @@ -2762,7 +2900,13 @@ case OP_ApplyType: { > while((type = *(types++)) != field_type_MAX) { > assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]); > assert(memIsValid(pIn1)); > - if (mem_apply_type(pIn1, type) != 0) { > + if (mem_is_type_compatible(pIn1, type)) { > + pIn1++; > + continue; > + } > + if (!mp_type_is_numeric(mem_mp_type(pIn1)) || > + !sql_type_is_numeric(type) || > + mem_convert_to_numeric(pIn1, type, false) != 0) { Consider refactoring: diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 863f38f5d..41a4750da 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2900,19 +2900,23 @@ case OP_ApplyType: { while((type = *(types++)) != field_type_MAX) { assert(pIn1 <= &p->aMem[(p->nMem+1 - p->nCursor)]); assert(memIsValid(pIn1)); - if (mem_is_type_compatible(pIn1, type)) { - pIn1++; - continue; - } - if (!mp_type_is_numeric(mem_mp_type(pIn1)) || - !sql_type_is_numeric(type) || - mem_convert_to_numeric(pIn1, type, false) != 0) { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), - field_type_strs[type]); - goto abort_due_to_error; + if (!mem_is_type_compatible(pIn1, type)) { + /* Implicit cast is allowed only to numeric type. */ + if (!sql_type_is_numeric(type)) + goto type_mismatch; + /* Implicit cast is allowed only from numeric type. */ + if (!mp_type_is_numeric(mem_mp_type(pIn1))) + goto type_mismatch; + /* Try to convert numeric-to-numeric. */ + if (mem_convert_to_numeric(pIn1, type, false) != 0) + goto type_mismatch; } pIn1++; + continue; +type_mismatch: + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn1), field_type_strs[type]); + goto abort_due_to_error; } break; } Otherwise LGTM > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > sql_value_to_diag_str(pIn1), > field_type_strs[type]); > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h > index 44c27bdb7..ad46ab129 100644 > --- a/src/box/sql/vdbeInt.h > +++ b/src/box/sql/vdbeInt.h > @@ -566,6 +566,10 @@ mem_mp_type(struct Mem *mem); > */ > #define mp_type_is_bloblike(X) ((X) == MP_BIN || (X) == MP_ARRAY || (X) == MP_MAP) > > +/** Return TRUE if MP_type of X is numeric, FALSE otherwise. */ > +#define mp_type_is_numeric(X) ((X) == MP_INT || (X) == MP_UINT ||\ > + (X) == MP_DOUBLE)