From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id E5E0F24B62 for ; Tue, 6 Aug 2019 15:36:19 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id csWaspail2cW for ; Tue, 6 Aug 2019 15:36:19 -0400 (EDT) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E1DD724B5D for ; Tue, 6 Aug 2019 15:36:18 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: [tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type From: "n.pettik" In-Reply-To: <9a397d31-1cae-0dd0-cdd6-733388cb01af@tarantool.org> Date: Tue, 6 Aug 2019 22:36:15 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <734EC309-6DCF-42C2-8041-135A8B68E935@tarantool.org> <9a397d31-1cae-0dd0-cdd6-733388cb01af@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy >> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c >> index 325c54c18..b6b5cd0bf 100644 >> --- a/src/box/sql/vdbeaux.c >> +++ b/src/box/sql/vdbeaux.c >> @@ -2887,43 +2887,50 @@ sqlBlobCompare(const Mem * pB1, const Mem * = pB2) >> return n1 - n2; >> } >>=20 >> -/* >> - * Do a comparison between a 64-bit signed integer and a 64-bit = floating-point >> - * number. Return negative, zero, or positive if the first (i64) is = less than, >> - * equal to, or greater than the second (double). >> +/** >> + * Do a comparison between a 64-bit unsigned/signed integer and a >> + * 64-bit floating-point number. Return negative, zero, or >> + * positive if the first (integer) is less than, equal to, or >> + * greater than the second (double). >> */ >> static int >> -sqlIntFloatCompare(i64 i, double r) >> +compare_uint_float(uint64_t u, double r) >=20 > Unfortunately, it is not as simple as you implemented it. > See mp_compare_double_uint64 in tuple_compare.cc for details. > In short, your function works wrong when numbers are near > uint maximum. Perhaps, it is worth moving this comparator > from tuple_compare.cc to a header. Like trivia/util.h. Hi, I remember your concern about compare_uint_float(). At the moment of review I said that integrating mp_compare_double_uint64() into SQL code results in test fails. In fact, that=E2=80=99s not true: I accidentally added bug when substituting them. Now I've carefully reviewed patch and it seems that all tests except for one (which exactly covers that border case) are passed. What is more, fix seems to be quite trivial: @@ -2895,7 +2897,7 @@ sqlBlobCompare(const Mem * pB1, const Mem * pB2) static int compare_uint_float(uint64_t u, double r) { - if (r > (double) UINT64_MAX) + if (r >=3D exp2(64)) return -1; if (r < 0.0) return +1; With this fix I=E2=80=99ve failed to come up with counter-example demonstrating different from mp_compare_double_uint64 results. Anyway, I replaced compare_uint_float/compare_int_float with analogue from comparators so that we have unified way of=20 int-float comparison. Patch is on its way.