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 8BC586EC55; Thu, 9 Sep 2021 10:40:01 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8BC586EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1631173201; bh=3s2vSbLgOrbU4p1No5tKLZBcdRM5F6s+sjlA8UHeZ8Q=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=xuCV5a8hydTK3uV3Rux3NCsvmkgPrbkXOsl/BaRkDRJ5uZFdqC5uLF428TwH08OE6 wszlGbMp4/Xay4jLU1n2iA5TEEAVqYt0+CF/6Gajr5oPFtax3kTwjJe1Ang9n9KEt0 xvIHIZUR1kyiSpVoFkLWtDxJS16iVHtlmfURKvyg= 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 397A16EC55 for ; Thu, 9 Sep 2021 10:40:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 397A16EC55 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mOEf8-0001r9-TK; Thu, 09 Sep 2021 10:39:59 +0300 Date: Thu, 9 Sep 2021 10:39:57 +0300 To: Igor Munkin Message-ID: <20210909073957.GA41015@tarantool.org> References: <6229320676324201d74e78ac1f2832b79fd159cb.1630303937.git.imeevma@gmail.com> <20210907114025.GX5743@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210907114025.GX5743@tarantool.org> X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD91AE02D33A9C88A2F7631404A3C5D35349DE4D58F47F062C400894C459B0CD1B91936A595ABF7789E78311D68898E29488561B97AA39936857C6799903D76029A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78E88BD1CA827EF00C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE745FB935E07B76C84EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BBCA57AF85F7723F2D2641D2766CC006859A34EFEDED0FECECC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8B861051D4BA689FCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDCE939D40DBB93CA75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A58DE8D380F10F3B5DEB1157AA77337622111A1B9AE2F8D595D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75E3127721F5A72C97410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34128DEC38EFF4BE5AB948219E553439A6ECC856FCB001DFD879B63562657370C5AA20CB1ED74A33481D7E09C32AA3244CEE56A39141C368E9767BB98232F0406255E75C8D0ED9F6EEFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojV8xpaxME9p+Pe1NIPuYx9g== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5DB348DE2C760ED6D35A6CAA6AEBF0840E83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix comparison between DECIMAL and big DOUBLE 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 Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thank you for the review! My answers and new patch below. On Tue, Sep 07, 2021 at 02:40:25PM +0300, Igor Munkin wrote: > Mergen, > > Thanks for the patch! LGTM, with a few nits regarding the commit > message. > > On 30.08.21, Mergen Imeev via Tarantool-patches wrote: > > This patch fixes comparison between DECIMAL value and DOUBLE values > > greater or equal to 1e38 or less or equal to -1e38. Now any DOUBLE value > > greater or equal to 1e38 is more than any DECIMAL value and DOUBLE > > value less or equal to -1e38 is less than any DECIMAL value. > > Minor: Considering Vlad's review[1], I would explicitly mention the > order of the operands to be compared (DECIMAL is the left one, DOUBLE is > the right one). The opposite case works fine even prior to the patch. > Thanks, fixed. > Minor: It would be clearer, if you mention here that there are only 38 > decimal digits in DECIMAL representation (hence, 1e38). > Fixed. > > > > Closes #6376 > > --- > > https://github.com/tarantool/tarantool/issues/6376 > > https://github.com/tarantool/tarantool/tree/imeevma/gh-6376-fix-cmp-between-big-double-and-dec > > > > .../gh-6376-fix-incorrect-dec-inf-cmp.md | 3 ++ > > src/box/sql/mem.c | 4 +- > > test/sql-tap/engine.cfg | 1 + > > .../gh-6376-wrong-double-to-dec-cmp.test.lua | 38 +++++++++++++++++++ > > 4 files changed, 44 insertions(+), 2 deletions(-) > > create mode 100644 changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md > > create mode 100755 test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua > > > > > > > -- > > 2.25.1 > > > > [1]: https://lists.tarantool.org/tarantool-patches/003f11f0-5ecc-2e69-dc89-47a34b5f24ac@tarantool.org/ > > -- > Best regards, > IM commit 04ce137be225ea4ee4ef83dd973a099a8cf385f6 Author: Mergen Imeev Date: Mon Aug 23 09:34:23 2021 +0300 sql: fix comparison between DECIMAL and big DOUBLE This patch fixes the comparison between DECIMAL as a left value and DOUBLE greater than or equal to 1e38 or less than or equal to -1e38 as a right value. Any DOUBLE value greater than or equal to 1e38 is now greater than any DECIMAL value, and a DOUBLE value less than or equal to -1e38 is less than any DECIMAL value. This is because our decimal cannot contain more than 38 digits. Closes #6376 diff --git a/changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md b/changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md new file mode 100644 index 000000000..70de655f1 --- /dev/null +++ b/changelogs/unreleased/gh-6376-fix-incorrect-dec-inf-cmp.md @@ -0,0 +1,3 @@ +## bugfix/sql + +* Fixed wrong comparison between DECIMAL and large DOUBLE values (gh-6376). diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 48755a017..b7af00723 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -2452,9 +2452,9 @@ mem_cmp_num(const struct Mem *a, const struct Mem *b) } case MEM_TYPE_DOUBLE: { if (b->u.r >= 1e38) - return 1; - if (b->u.r <= -1e38) return -1; + if (b->u.r <= -1e38) + return 1; decimal_t dec; decimal_t *d = decimal_from_double(&dec, b->u.r); assert(d != NULL && d == &dec); diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg index c35d1dced..a6f03307f 100644 --- a/test/sql-tap/engine.cfg +++ b/test/sql-tap/engine.cfg @@ -36,6 +36,7 @@ "memtx": {"engine": "memtx"} }, "gh-6157-unnecessary-free-on-string.test.lua": {}, + "gh-6376-wrong-double-to-dec-cmp.test.lua": {}, "gh-4077-iproto-execute-no-bind.test.lua": {}, "gh-6375-assert-on-unsupported-ext.test.lua": {}, "*": { diff --git a/test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua b/test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua new file mode 100755 index 000000000..edfc851a2 --- /dev/null +++ b/test/sql-tap/gh-6376-wrong-double-to-dec-cmp.test.lua @@ -0,0 +1,38 @@ +#!/usr/bin/env tarantool +local test = require("sqltester") +test:plan(4) + +-- Make sure that the comparison between DECIMAL and large DOUBLE is correct. +test:do_execsql_test( + "gh-6376-1", + [[ + SELECT CAST(1 AS DECIMAL) < -1e40; + ]], { + false + }) + +test:do_execsql_test( + "gh-6376-2", + [[ + SELECT CAST(-1 AS DECIMAL) > -1e400; + ]], { + true + }) + +test:do_execsql_test( + "gh-6376-3", + [[ + SELECT CAST(1 AS DECIMAL) <= 1e40; + ]], { + true + }) + +test:do_execsql_test( + "gh-6376-4", + [[ + SELECT CAST(1 AS DECIMAL) >= 1e400; + ]], { + false + }) + +test:finish_test()