From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 BFF9B469719 for ; Mon, 28 Sep 2020 19:10:42 +0300 (MSK) Date: Mon, 28 Sep 2020 19:10:40 +0300 From: Mergen Imeev Message-ID: <20200928161040.GC77246@tarantool.org> References: <96a8dec05827608e00503fd33d2cb0d2a1076052.1598000242.git.imeevma@gmail.com> <20200918080821.GI10599@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200918080821.GI10599@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v5 3/6] sql: change comparison between numbers using index List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Thank you for review! My answers and diff below. On Fri, Sep 18, 2020 at 08:08:21AM +0000, Nikita Pettik wrote: > On 21 Aug 12:19, imeevma@tarantool.org wrote: > > This patch disables number conversions in ApplyType in wherecode.c. This > > allows conversions between numbers introduced in previous commit to be > > used. > > > > Part of #4230 > > --- > > src/box/sql/sqlInt.h | 2 + > > src/box/sql/vdbe.c | 3 +- > > src/box/sql/wherecode.c | 76 +--------------------------- > > test/sql-tap/in4.test.lua | 4 +- > > test/sql-tap/join.test.lua | 4 +- > > test/sql-tap/tkt-9a8b09f8e6.test.lua | 8 +-- > > test/sql/types.result | 54 ++++++++++++++++++++ > > test/sql/types.test.lua | 12 +++++ > > 8 files changed, 79 insertions(+), 84 deletions(-) > > > > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > > index adf90d824..1e6f0f41f 100644 > > --- a/src/box/sql/sqlInt.h > > +++ b/src/box/sql/sqlInt.h > > @@ -2309,6 +2309,8 @@ struct Parse { > > #define OPFLAG_SYSTEMSP 0x20 /* OP_Open**: set if space pointer > > * points to system space. > > */ > > +/** OP_ApplyType: Do not convert numbers. */ > > Useless comment. I do not think so, since it shows that this MACRO should be used only in OP_ApplyType. > > > +#define OPFLAG_DO_NOT_CONVERT_NUMBERS 0x01 > > diff --git a/test/sql/types.result b/test/sql/types.result > > index 442245186..8810a9f82 100644 > > --- a/test/sql/types.result > > +++ b/test/sql/types.result > > @@ -2795,3 +2795,57 @@ box.execute([[DROP TABLE ts;]]) > > --- > > - row_count: 1 > > ... > > +-- > > +-- gh-4230: Make sure the comparison between numbers that use > > +-- index is working correctly. > > +-- > > +box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE);]]) > > +--- > > +- row_count: 1 > > +... > > +box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60}) > > I'd place also eqp statements to make sure that index is really used. Done. It helped to find that there wasn't an index in column 'a'. Also fixed. > > > +--- > > +- row_count: 1 > > +... Diff: diff --git a/test/sql/types.result b/test/sql/types.result index 8810a9f82..9a7578d75 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -2799,7 +2796,7 @@ box.execute([[DROP TABLE ts;]]) -- gh-4230: Make sure the comparison between numbers that use -- index is working correctly. -- -box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE);]]) +box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE UNIQUE);]]) --- - row_count: 1 ... @@ -2807,6 +2804,20 @@ box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60}) --- - row_count: 1 ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {2^70}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING PRIMARY KEY (I>?) (~262144 rows)'] +... box.execute([[SELECT * FROM t WHERE i > ?]], {2^70}) --- - metadata: @@ -2816,6 +2827,20 @@ box.execute([[SELECT * FROM t WHERE i > ?]], {2^70}) type: double rows: [] ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {-2^70}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING PRIMARY KEY (I>?) (~262144 rows)'] +... box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70}) --- - metadata: @@ -2826,6 +2851,20 @@ box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70}) rows: - [1, 1152921504606846976] ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING COVERING INDEX unique_unnamed_T_2 (A=?) (~1 row)'] +... box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) --- - metadata: @@ -2835,6 +2874,21 @@ box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) type: double rows: [] ... +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) +--- +- metadata: + - name: selectid + type: integer + - name: order + type: integer + - name: from + type: integer + - name: detail + type: text + rows: + - [0, 0, 0, 'SEARCH TABLE T USING COVERING INDEX unique_unnamed_T_2 (A>?) (~262144 + rows)'] +... box.execute([[SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) --- - metadata: diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index a23b12801..ae19167bd 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -628,10 +628,14 @@ box.execute([[DROP TABLE ts;]]) -- gh-4230: Make sure the comparison between numbers that use -- index is working correctly. -- -box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE);]]) +box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, a DOUBLE UNIQUE);]]) box.execute([[INSERT INTO t VALUES (1, ?);]], {2^60}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {2^70}) box.execute([[SELECT * FROM t WHERE i > ?]], {2^70}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE i > ?]], {-2^70}) box.execute([[SELECT * FROM t WHERE i > ?]], {-2^70}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) box.execute([[SELECT * FROM t WHERE a = ?]], {2ULL^60ULL - 1ULL}) +box.execute([[EXPLAIN QUERY PLAN SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) box.execute([[SELECT * FROM t WHERE a > ?]], {2ULL^60ULL - 1ULL}) box.execute([[DROP TABLE t;]])