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 0FAF16EC55; Wed, 14 Jul 2021 09:51:41 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0FAF16EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1626245501; bh=NCiThiuxLJ3vW8n10w3GigKI0zabxLaOrnCQx8+RHC0=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=sPJO7io/hzFs3LGOyF+wpelSoILRkYTntK75CWFCYsPquCrWLBaJdMoXLVxE4IUaa WW0KOHtncAYSmpUPKCKr2WtZpuRX+gT3hWYs+nprpyxQY3s+C696SLAj0whcbpNk4Q Ochth/iz95KwMegFOr6pNTe7THn71cUvAkuvmfXs= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (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 0F85D6EC55 for ; Wed, 14 Jul 2021 09:51:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0F85D6EC55 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1m3Yk7-0005jC-1r; Wed, 14 Jul 2021 09:51:39 +0300 Date: Wed, 14 Jul 2021 09:51:37 +0300 To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210714065137.GA7470@tarantool.org> References: <28d370911c40124da91441be2f25cad49e4973b4.1625926838.git.imeevma@gmail.com> <20c6c37d-6364-8fdb-0aa5-c7c82407b143@tarantool.org> <20210711175916.GB99369@tarantool.org> <913595cc-0d71-1a69-7997-ca4a136f97a1@tarantool.org> <20210713081007.GB104775@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD97BB0EF39AD2B33D598226807B8A1E9DC331E3F9997896515182A05F5380850407872AB666140001B866133E6A4D77754475414D49B19F0136BE32AA200B82C34 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E439DB85B4CC0F53EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379A70878BADDF00B98638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8146B8D67D03414124476ABE6A79EB0A3117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B3A703B70628EAD7BA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CD1D040B6C1ECEA3F43847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5CA5D95E2DDE1F596659DCE4A84A9D2310BC57BEB7D1959F1D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA753753CEE10E4ED4A7410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3480665FBD8F49180C46A77A25A05F811D93FCBED66A8A7BBB0E7BB441302FFBAB352894561E2E421A1D7E09C32AA3244C2B2C5721BA77D1A4B5751F459F4C9A7A3A76366E8A9DE7CA729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojDdSFIg49M1QPItXYLA9H+w== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D94267CF1B60434412B3FB94AE1EC16CA83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B 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 answers and diff below. On Tue, Jul 13, 2021 at 10:39:04PM +0200, Vladislav Shpilevoy wrote: > Thanks for the fixes! > > > diff --git a/src/box/sql.c b/src/box/sql.c > > index 790ca7f70..c1271ee0a 100644 > > --- a/src/box/sql.c > > +++ b/src/box/sql.c > > @@ -765,10 +765,14 @@ 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; > > + /* In case of fail make rc equal to 0. */ > > The comment isn't really necessary I would say. Does not > help much. The same in the other place below. > Dropped. > > + 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) > > Why did you change != to ==? > I did this because function sqlVdbeCompareMsgpack() compared packed value as left operand and MEM as right operand. In mem_cmp_msgpack() order was reversed, now MEM is left operand and packed value is right operand. > > diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c > > index da27cd191..8e176e418 100644 > > --- a/src/box/sql/mem.c > > +++ b/src/box/sql/mem.c > > @@ -2721,13 +2586,16 @@ sqlVdbeRecordCompareMsgpack(const void *key1, > > n = MIN(n, key2->nField); > > > > for (i = 0; i != n; i++) { > > - rc = sqlVdbeCompareMsgpack((const char**)&key1, key2, i); > > + struct key_part *part = &key2->key_def->parts[i]; > > + struct Mem *mem = key2->aMem + i; > > + struct coll *coll = part->coll; > > + /* In case of fail make rc equal to 0. */ > > + if (mem_cmp_msgpack(mem, (const char **)&key1, &rc, coll) != 0) > > + rc = 0; Diff: diff --git a/src/box/sql.c b/src/box/sql.c index c1271ee0a..7471c3832 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -768,7 +768,6 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor, struct key_part *part = &unpacked->key_def->parts[i]; struct Mem *mem = unpacked->aMem + i; struct coll *coll = part->coll; - /* In case of fail make rc equal to 0. */ if (mem_cmp_msgpack(mem, &p, &rc, coll) != 0) rc = 0; if (rc != 0) { diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 8e176e418..62993fa4f 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -2589,7 +2589,6 @@ sqlVdbeRecordCompareMsgpack(const void *key1, struct key_part *part = &key2->key_def->parts[i]; struct Mem *mem = key2->aMem + i; struct coll *coll = part->coll; - /* In case of fail make rc equal to 0. */ if (mem_cmp_msgpack(mem, (const char **)&key1, &rc, coll) != 0) rc = 0; if (rc != 0) {