Re: [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type

[Nikita Pettik <korablev@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()
>