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 C61756EC55; Mon, 19 Jul 2021 13:07:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C61756EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626689236; bh=+O2ZOgZOMLXpPScaCtSyxQ5ZKwp14YXxO+nqYnVPb8o=; 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=xfAt8W8yfIa+UTBPAvRvqcmK1dQA3A1H0LsbCfm0nNxY5YNFm7UE7FGpxuPLlA2tE GxOGrahMGeLgffDK09Er5jTYqtVRE7T4dcO3gm42pxVUb4u4l/SIknYpM8nio/2mFj 9Xtb/8ZXEJxv9fi66rn2M9r5tWvj/OfzqgYMSQ/o= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (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 BF05E6EC55 for ; Mon, 19 Jul 2021 13:07:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BF05E6EC55 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1m5QB7-0006Wx-RV; Mon, 19 Jul 2021 13:07:14 +0300 To: Timur Safin Cc: tarantool-patches@dev.tarantool.org References: <189d01d77c7e$bbab2cc0$33018640$@tarantool.org> Message-ID: <28dd1848-e29d-c416-1f38-9ff8da2d3fd2@tarantool.org> Date: Mon, 19 Jul 2021 13:07:13 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <189d01d77c7e$bbab2cc0$33018640$@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD941C43E597735A9C36A98DBA789EBB6AEFB5A483DCE5E7579182A05F5380850404F0F9211F6CD0033E4A388E0ABD9020082021A47E2542B25DC350BF641AD2992 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE72424932217190DC2EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637968EC5F77C2942FE8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D86241522C80A6C1BD29A0225F2F4E0E7E117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18F04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6300D3B61E77C8D3B089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A593BAD40A86E71BAF43CAAB5D56F9D2CFAD23E435BE3609D6D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7501A9DF589746230F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D342B1F2AD168155B069DBBF1E276BE3B9B972511D0C9440C92C61240D325ECC3B0238E5E72F16407241D7E09C32AA3244CD1091954575D86F1E828CB48BA9888BAF2F5F14F68F1805B729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj+8+KVR9NZrEcEpgc52dBuA== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D328D1497500B4EA6DCFFC102AC9CCFAB83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack() 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: Mergen Imeev via Tarantool-patches Reply-To: Mergen Imeev Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thank you for the review. My answer below. On 19.07.2021 12:16, Timur Safin wrote: > LGTM, but with 1 question... > > : From: imeevma@tarantool.org > : Subject: [PATCH v2 4/4] sql: introduce mem_cmp_msgpack() > : > : This patch introduces the mem_cmp_msgpack() function that compares MEM > : and packed to msgpack value using SCALAR rules. MEM and packed value > : must be scalars. Prior to this patch, there was a function that used > : SCALAR rules to compare MEM and packed value, but its design became > : overly complex as new types appeared. > : > : Closes #6164 > : --- > : src/box/sql.c | 9 +- > : src/box/sql/mem.c | 289 +++++------------- > : src/box/sql/mem.h | 24 +- > : test/sql-tap/gh-6164-uuid-follow-ups.test.lua | 14 +- > : 4 files changed, 107 insertions(+), 229 deletions(-) > : > : diff --git a/src/box/sql.c b/src/box/sql.c > : index 790ca7f70..7471c3832 100644 > : --- a/src/box/sql.c > : +++ b/src/box/sql.c > : @@ -765,10 +765,13 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor, > : } > : } > : next_fieldno = fieldno + 1; > : - rc = sqlVdbeCompareMsgpack(&p, unpacked, i); > : + struct key_part *part = &unpacked->key_def->parts[i]; > : + struct Mem *mem = unpacked->aMem + i; > : + struct coll *coll = part->coll; > : + if (mem_cmp_msgpack(mem, &p, &rc, coll) != 0) > : + rc = 0; > : if (rc != 0) { > : - if (unpacked->key_def->parts[i].sort_order != > : - SORT_ORDER_ASC) > : + if (part->sort_order == SORT_ORDER_ASC) > : rc = -rc; > : goto out; > : } > : diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c > : index a8cde6769..fa80f6d5a 100644 > : --- a/src/box/sql/mem.c > : +++ b/src/box/sql/mem.c > : @@ -2098,35 +2098,77 @@ mem_cmp_scalar(const struct Mem *a, const struct Mem > : *b, int *result, > : return 0; > : } > : > : -/* > : - * Both *pMem1 and *pMem2 contain string values. Compare the two values > : - * using the collation sequence pColl. As usual, return a negative , zero > : - * or positive value if *pMem1 is less than, equal to or greater than > : - * *pMem2, respectively. Similar in spirit to "rc = (*pMem1) - (*pMem2);". > : - * > : - * Strungs assume to be UTF-8 encoded > : - */ > : -static int > : -vdbeCompareMemString(const Mem * pMem1, const Mem * pMem2, > : - const struct coll * pColl) > : -{ > : - return pColl->cmp(pMem1->z, (size_t)pMem1->n, > : - pMem2->z, (size_t)pMem2->n, pColl); > : -} > : - > : -/* > : - * The input pBlob is guaranteed to be a Blob that is not marked > : - * with MEM_Zero. Return true if it could be a zero-blob. > : - */ > : -static int > : -isAllZero(const char *z, int n) > : -{ > : - int i; > : - for (i = 0; i < n; i++) { > : - if (z[i]) > : - return 0; > : +int > : +mem_cmp_msgpack(const struct Mem *a, const char **b, int *result, > : + const struct coll *coll) > : +{ > : + struct Mem mem; > : + switch (mp_typeof(**b)) { > : + case MP_NIL: > : + mem.type = MEM_TYPE_NULL; > : + mp_decode_nil(b); > : + break; > : + case MP_BOOL: > : + mem.type = MEM_TYPE_BOOL; > : + mem.u.b = mp_decode_bool(b); > : + break; > : + case MP_UINT: > : + mem.type = MEM_TYPE_UINT; > : + mem.u.u = mp_decode_uint(b); > : + break; > : + case MP_INT: > : + mem.type = MEM_TYPE_INT; > : + mem.u.i = mp_decode_int(b); > : + break; > : + case MP_FLOAT: > : + mem.type = MEM_TYPE_DOUBLE; > : + mem.u.r = mp_decode_float(b); > : + break; > : + case MP_DOUBLE: > : + mem.type = MEM_TYPE_DOUBLE; > : + mem.u.r = mp_decode_double(b); > : + break; > : + case MP_STR: > : + mem.type = MEM_TYPE_STR; > : + mem.n = mp_decode_strl(b); > : + mem.z = (char *)*b; > : + *b += mem.n; > : + mem.flags = MEM_Ephem; > : + break; > : + case MP_BIN: > : + mem.type = MEM_TYPE_BIN; > : + mem.n = mp_decode_binl(b); > : + mem.z = (char *)*b; > : + *b += mem.n; > : + mem.flags = MEM_Ephem; > : + break; > : + case MP_ARRAY: > : + case MP_MAP: > : + mp_next(b); > : + *result = -1; > : + return 0; > : + case MP_EXT: { > : + int8_t type; > : + const char *buf = *b; > : + uint32_t len = mp_decode_extl(b, &type); > > Decimals are coming here? Yes? Yes. If you want I can give you branch where DECIMAL is partially done. > : + if (type == MP_UUID) { > : + assert(len == UUID_LEN); > : + mem.type = MEM_TYPE_UUID; > : + if (uuid_unpack(b, len, &mem.u.uuid) == NULL) > : + return -1; > : + break; > : + } > : + *b += len; > : + mem.type = MEM_TYPE_BIN; > : + mem.z = (char *)buf; > : + mem.n = *b - buf; > : + mem.flags = MEM_Ephem; > : + break; > : } > : - return 1; > : + default: > : + unreachable(); > : + } > : + return mem_cmp_scalar(a, &mem, result, coll); > : } > : > : char * > > Regards, > Timur >