From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id E31BD6EC5B; Wed, 14 Apr 2021 01:59:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E31BD6EC5B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618354774; bh=EwNFjto0qmSFJgashy/s7pZk4z5YvhP+bDIKIY8WFbc=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Y13X9OJosRv7dfZjuQSwTb9QJDg/HUWIlazl4zFyTgXItvTDmlBkLjF4XBDDEoYZT C3ltM9kkIeLP1qA6eYPySacZ/ytt6m8XH13gBGsn7yk92TPfKrQebiBlYbxEEuFRoM PD39nCRkUY2YrPTZCrRcQ9RKPetc6izNrmFlbXLk= Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 E7CB16EC5B for ; Wed, 14 Apr 2021 01:59:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E7CB16EC5B Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1lWS0K-0006Da-Us; Wed, 14 Apr 2021 01:59:33 +0300 To: imeevma@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org References: <83460913f3fdab50eb49ee0b3d84a69da6e29128.1618000037.git.imeevma@gmail.com> Message-ID: Date: Wed, 14 Apr 2021 00:59:31 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: <83460913f3fdab50eb49ee0b3d84a69da6e29128.1618000037.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E74807BAE725B9AE625DE765B0E193B5B7687182A05F538085040ABFAEFA66705DDA4C8579FE94AC00E020FD72D762ED4E39514400DFD0BCB21EA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE783C1FBFE215D363AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063790A3F9E01CE65FDC8638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B238692527177D38A9DE3EDFD4846894618F4427C359EA06A0D2E47CDBA5A96583C09775C1D3CA48CFCA5A41EBD8A3A0199FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3CA5A41EBD8A3A0199FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E73542F54486E6D6388DC4224003CC83647689D4C264860C145E X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2AD77751E876CB595E8F7B195E1C97831AB0D4F42DD743D231D9CBE3238DE1566 X-C1DE0DAB: 0D63561A33F958A5AC88F651A9C912EE1FFDF48AA3EDCEC21B6132A3957C6281D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D347215713CF3EEE9D1BAF2D352344EF9E4378564A6C36FC9762F6FFE71733F459E067931254FB03AE41D7E09C32AA3244CA0E3EFC1F3D0B38D9094253A96E0F2B0B018FE5BB746DCD1FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnA7/qPBUIXFuMLGKxQvmSw== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822B6F783CF2564FE8A6ACDB0EBFC1B78CE3841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v5 43/52] sql: introduce mem_cast_explicit() X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for the patch! See 2 comments below. > diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c > index be7b47e76..45d2d5fe3 100644 > --- a/src/box/sql/mem.c > +++ b/src/box/sql/mem.c > @@ -946,6 +946,131 @@ mem_to_str(struct Mem *mem) > return -1; > } > > +static inline int > +bytes_to_uint(struct Mem *mem) > +{ > + bool is_neg; > + int64_t i; > + if (sql_atoi64(mem->z, &i, &is_neg, mem->n) != 0) > + return -1; > + if (is_neg) > + return -1; > + mem_set_uint(mem, (uint64_t)i); > + return 0; > +} > + > +static inline int > +str_to_bool(struct Mem *mem) > +{ > + char *str = mem->z; > + bool b; > + const char *str_true = "TRUE"; > + const char *str_false = "FALSE"; > + uint32_t len_true = strlen(str_true); > + uint32_t len_false = strlen(str_false); > + > + for (; str[0] == ' '; str++); > + if (strncasecmp(str, str_true, len_true) == 0) { > + b = true; > + str += len_true; > + } else if (strncasecmp(str, str_false, len_false) == 0) { > + b = false; > + str += len_false; > + } else { > + return -1; > + } > + for (; str[0] == ' '; str++); > + if (str[0] != '\0') > + return -1; > + mem_set_bool(mem, b); > + return 0; > +} > + > +static inline int > +int_to_bool(struct Mem *mem) > +{ > + mem->u.b = mem->u.i != 0; > + mem->flags = MEM_Bool; > + mem->field_type = FIELD_TYPE_BOOLEAN; > + return 0; > +} > + > +static inline int > +double_to_bool(struct Mem *mem) > +{ > + mem->u.b = mem->u.r != 0.; > + mem->flags = MEM_Bool; > + mem->field_type = FIELD_TYPE_BOOLEAN; > + return 0; > +} > + > +static inline int > +str_to_bin(struct Mem *mem) > +{ > + mem->flags = (mem->flags & (MEM_Dyn | MEM_Static | MEM_Ephem)) | > + MEM_Blob; > + mem->field_type = FIELD_TYPE_VARBINARY; > + return 0; > +} 1. You have tons of _to_ converters. I propose you to group them. For example, str_to_* all together, double_to_* all together, and so on. It would simplify reading and search. > + > +int > +mem_cast_explicit(struct Mem *mem, enum field_type type) > +{ > + if ((mem->flags & MEM_Null) != 0) { > + mem->field_type = type; > + return 0; > + } > + switch (type) { > + case FIELD_TYPE_UNSIGNED: > + if ((mem->flags & MEM_UInt) != 0) > + return 0; > + if ((mem->flags & MEM_Int) != 0) > + return -1; > + if ((mem->flags & MEM_Blob) != 0 && > + (mem->flags & MEM_Subtype) != 0) > + return -1; > + if ((mem->flags & (MEM_Str | MEM_Blob)) != 0) > + return bytes_to_uint(mem); > + if ((mem->flags & MEM_Real) != 0) > + return double_to_int(mem); 2. tarantool> box.execute('SELECT CAST(-1.1 AS UNSIGNED);') --- - metadata: - name: COLUMN_1 type: unsigned rows: - [-1] ... That looks quite broken. Is this a known issue? From the code I see the issue existed before your patch (but I was too lazy to try it). > @@ -2018,113 +2107,6 @@ registerTrace(int iReg, Mem *p) { > } > #endif > > -/* > - * Cast the datatype of the value in pMem according to the type > - * @type. Casting is different from applying type in that a cast > - * is forced. In other words, the value is converted into the desired > - * type even if that results in loss of data. This routine is > - * used (for example) to implement the SQL "cast()" operator. > - */ > -int > -sqlVdbeMemCast(Mem * pMem, enum field_type type) > -{ > - assert(type < field_type_MAX); > - if (pMem->flags & MEM_Null) > - return 0; > - switch (type) { > - case FIELD_TYPE_SCALAR: > - return 0; > - case FIELD_TYPE_BOOLEAN: > - if ((pMem->flags & MEM_Int) != 0) { > - mem_set_bool(pMem, pMem->u.i); > - return 0; > - } > - if ((pMem->flags & MEM_UInt) != 0) { > - mem_set_bool(pMem, pMem->u.u); > - return 0; > - } > - if ((pMem->flags & MEM_Real) != 0) { > - mem_set_bool(pMem, pMem->u.r); > - return 0; > - } > - if ((pMem->flags & MEM_Str) != 0) { > - bool value; > - if (str_cast_to_boolean(pMem->z, &value) != 0) > - return -1; > - mem_set_bool(pMem, value); > - return 0; > - } > - if ((pMem->flags & MEM_Bool) != 0) > - return 0; > - return -1; > - case FIELD_TYPE_INTEGER: > - case FIELD_TYPE_UNSIGNED: > - if ((pMem->flags & (MEM_Blob | MEM_Str)) != 0) { > - bool is_neg; > - int64_t val; > - if (sql_atoi64(pMem->z, &val, &is_neg, pMem->n) != 0) > - return -1; > - if (type == FIELD_TYPE_UNSIGNED && is_neg) > - return -1; > - mem_set_int(pMem, val, is_neg); > - return 0; > - } > - if ((pMem->flags & MEM_Bool) != 0) { > - pMem->u.u = (uint64_t)pMem->u.b; > - pMem->flags = MEM_UInt; > - pMem->field_type = FIELD_TYPE_UNSIGNED; > - return 0; > - } > - if ((pMem->flags & MEM_Real) != 0) { > - double d = pMem->u.r; > - if (d < 0. && d >= (double)INT64_MIN) { > - pMem->u.i = (int64_t)d; > - pMem->flags = MEM_Int; > - pMem->field_type = FIELD_TYPE_INTEGER; > - return 0; > - } > - if (d >= 0. && d < (double)UINT64_MAX) { > - pMem->u.u = (uint64_t)d; > - pMem->flags = MEM_UInt; > - pMem->field_type = FIELD_TYPE_UNSIGNED; > - return 0; > - } > - return -1; > - } > - if (type == FIELD_TYPE_UNSIGNED && > - (pMem->flags & MEM_UInt) == 0) > - return -1; > - return 0; > - case FIELD_TYPE_DOUBLE: > - return mem_to_double(pMem); > - case FIELD_TYPE_NUMBER: > - return mem_to_number(pMem); > - case FIELD_TYPE_VARBINARY: > - if ((pMem->flags & MEM_Blob) != 0) > - return 0; > - if ((pMem->flags & MEM_Str) != 0) { > - MemSetTypeFlag(pMem, MEM_Str); > - return 0; > - } > - return -1; > - default: > - assert(type == FIELD_TYPE_STRING); > - assert(MEM_Str == (MEM_Blob >> 3)); > - if ((pMem->flags & MEM_Bool) != 0) { > - const char *str_bool = SQL_TOKEN_BOOLEAN(pMem->u.b); > - if (mem_copy_str0(pMem, str_bool) != 0) > - return -1; > - return 0; > - } > - pMem->flags |= (pMem->flags & MEM_Blob) >> 3; > - sql_value_apply_type(pMem, FIELD_TYPE_STRING); > - assert(pMem->flags & MEM_Str || pMem->db->mallocFailed); > - pMem->flags &= > - ~(MEM_Int | MEM_UInt | MEM_Real | MEM_Blob | MEM_Zero); > - return 0; > - } > -} It is fascinating how a good code structure allows to get rid of all of that old garbage mess almost naturally, and reveals some issues.