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

[Mergen Imeev <imeevma@tarantool.org>]

Thank you for review! My answers and diff below.

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()