From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (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 C91AF46970E for ; Thu, 26 Dec 2019 23:34:49 +0300 (MSK) Date: Thu, 26 Dec 2019 22:34:46 +0200 From: Nikita Pettik Message-ID: <20191226203446.GF18639@tarantool.org> References: <6faa3090e13cd9317baf619aa17716251c7faa27.1576944064.git.imeevma@gmail.com> <20191224225037.GA16168@tarantool.org> <20191226163836.GA32521@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20191226163836.GA32521@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org On 26 Dec 19:38, Mergen Imeev wrote: > Thank you for review! My answers and diff below. LGTM > On Wed, Dec 25, 2019 at 01:50:37AM +0300, Nikita Pettik wrote: > > On 21 Dec 19:03, imeevma@tarantool.org wrote: > > > diff --git a/test/engine/insert.result b/test/engine/insert.result > > > index 1015b6a..60c31a7 100644 > > > --- a/test/engine/insert.result > > > +++ b/test/engine/insert.result > > > @@ -730,3 +730,154 @@ s:drop() > > > fiber = nil > > > --- > > > ... > > > +-- Integers of Lua type NUMBER and CDATA of type int64 or uint64 > > > +-- cannot be inserted into this field. > > > +-- > > > +s:insert({4, 1}) > > > +--- > > > +- error: 'Tuple field 2 type does not match one required by operation: expected double' > > > +... > > > > Could you please add a follow-up or file an issue to make this error > > more clear and display actually passed type? Like > > '..operatiion: expected double, got integer' > > > Filled an issue: > https://github.com/tarantool/tarantool/issues/4707 > > > > + > > > +-- > > > +-- To insert an integer, we must cast it to a CDATA of type DOUBLE > > > +-- using ffi.cast(). Non-integers can also be inserted this way. > > > +-- > > > +s:insert({7, ffi.cast('double', 1)}) > > > +s:insert({8, ffi.cast('double', -9223372036854775808)}) > > > +s:insert({9, ffi.cast('double', tonumber('123'))}) > > > +s:insert({10, ffi.cast('double', tonumber64('18000000000000000000'))}) > > > +s:insert({11, ffi.cast('double', 1.1)}) > > > +s:insert({12, ffi.cast('double', -3.0009)}) > > > > Inserts are OK, but let's also test delete, update and upsert > > operations (a few tests just in case). > > > Fixed, diff below. > > > The rest is OK. > > > > Diff: > > diff --git a/test/engine/insert.result b/test/engine/insert.result > index 60c31a7..13ffe8c 100644 > --- a/test/engine/insert.result > +++ b/test/engine/insert.result > @@ -878,6 +878,113 @@ dd:select(1.1, {iterator = 'req'}) > - - [11, 1.1] > - [1, 1.1] > ... > +s:delete(11) > +--- > +- [11, 1.1] > +... > +s:delete(12) > +--- > +- [12, -3.0009] > +... > +-- Make sure that other operations are working correctly: > +ddd = s:create_index('ddd', {parts = {{2, 'double'}}}) > +--- > +... > +s:update(1, {{'=', 2, 2}}) > +--- > +- error: 'Tuple field 2 type does not match one required by operation: expected double' > +... > +s:insert({22, 22}) > +--- > +- error: 'Tuple field 2 type does not match one required by operation: expected double' > +... > +s:upsert({10, 100}, {{'=', 2, 2}}) > +--- > +- error: 'Tuple field 2 type does not match one required by operation: expected double' > +... > +s:upsert({100, 100}, {{'=', 2, 2}}) > +--- > +- error: 'Tuple field 2 type does not match one required by operation: expected double' > +... > +ddd:update(1, {{'=', 1, 70}}) > +--- > +- error: 'Supplied key type of part 0 does not match index part type: expected double' > +... > +ddd:delete(1) > +--- > +- error: 'Supplied key type of part 0 does not match index part type: expected double' > +... > +s:update(2, {{'=', 2, 2.55}}) > +--- > +- [2, 2.55] > +... > +s:replace({22, 22.22}) > +--- > +- [22, 22.22] > +... > +s:upsert({100, 100.5}, {{'=', 2, 2}}) > +--- > +... > +s:get(100) > +--- > +- [100, 100.5] > +... > +s:upsert({10, 100.5}, {{'=', 2, 2.2}}) > +--- > +... > +s:get(10) > +--- > +- [10, 2.2] > +... > +ddd:update(1.1, {{'=', 3, 111}}) > +--- > +- [1, 1.1, 111] > +... > +ddd:delete(1.1) > +--- > +- [1, 1.1, 111] > +... > +s:update(2, {{'=', 2, ffi.cast('double', 255)}}) > +--- > +- [2, 255] > +... > +s:replace({22, ffi.cast('double', 22)}) > +--- > +- [22, 22] > +... > +s:upsert({200, ffi.cast('double', 200)}, {{'=', 2, 222}}) > +--- > +... > +s:get(200) > +--- > +- [200, 200] > +... > +s:upsert({200, ffi.cast('double', 200)}, {{'=', 2, ffi.cast('double', 222)}}) > +--- > +... > +s:get(200) > +--- > +- [200, 222] > +... > +ddd:update(ffi.cast('double', 1), {{'=', 3, 7}}) > +--- > +- [7, 1, 7] > +... > +ddd:delete(ffi.cast('double', 123)) > +--- > +- [9, 123] > +... > +s:select() > +--- > +- - [2, 255] > + - [3, -3.0009] > + - [7, 1, 7] > + - [8, -9223372036854775808] > + - [10, 2.2] > + - [22, 22] > + - [100, 100.5] > + - [200, 222] > +... > s:drop() > --- > ... > diff --git a/test/engine/insert.test.lua b/test/engine/insert.test.lua > index a5d8a4c..2ffc8cd 100644 > --- a/test/engine/insert.test.lua > +++ b/test/engine/insert.test.lua > @@ -173,4 +173,40 @@ dd:select(1.1, {iterator = 'all'}) > dd:select(1.1, {iterator = 'eq'}) > dd:select(1.1, {iterator = 'req'}) > > +s:delete(11) > +s:delete(12) > + > +-- Make sure that other operations are working correctly: > +ddd = s:create_index('ddd', {parts = {{2, 'double'}}}) > + > +s:update(1, {{'=', 2, 2}}) > +s:insert({22, 22}) > +s:upsert({10, 100}, {{'=', 2, 2}}) > +s:upsert({100, 100}, {{'=', 2, 2}}) > + > +ddd:update(1, {{'=', 1, 70}}) > +ddd:delete(1) > + > +s:update(2, {{'=', 2, 2.55}}) > +s:replace({22, 22.22}) > +s:upsert({100, 100.5}, {{'=', 2, 2}}) > +s:get(100) > +s:upsert({10, 100.5}, {{'=', 2, 2.2}}) > +s:get(10) > + > +ddd:update(1.1, {{'=', 3, 111}}) > +ddd:delete(1.1) > + > +s:update(2, {{'=', 2, ffi.cast('double', 255)}}) > +s:replace({22, ffi.cast('double', 22)}) > +s:upsert({200, ffi.cast('double', 200)}, {{'=', 2, 222}}) > +s:get(200) > +s:upsert({200, ffi.cast('double', 200)}, {{'=', 2, ffi.cast('double', 222)}}) > +s:get(200) > + > +ddd:update(ffi.cast('double', 1), {{'=', 3, 7}}) > +ddd:delete(ffi.cast('double', 123)) > + > +s:select() > + > s:drop() >