Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type

[Mergen Imeev <imeevma@tarantool.org>]

Thank you for review! My answers and new commit-message
below.

On Wed, Dec 25, 2019 at 01:50:46AM +0300, Nikita Pettik wrote:
> On 21 Dec 19:03, imeevma@tarantool.org wrote:
> > This patch introduces type DOUBLE in SQL.
> > 
> > Closes #3812
> > Needed for #4233
> > 
> > @TarantoolBot document
> > Title: Tarantool DOUBLE field type and DOUBLE type in SQL
> > The DOUBLE field type was added to Tarantool mainly for adding the
> > DOUBLE type to SQL.
> > 
> > In Lua, only non-integer numbers and CDATA of type DOUBLE can be
> > inserted in this field. You cannot insert integers of type Lua
> > NUMBER or CDATA of type int64 or uint64 in this field.
> 
> It would be nice to see justification for this ban.
> 
Added: "It was done this way to avoid unwanted implicit
casts that could affect performance."

> > The same
> > rules apply to key in get(), select(), update() and upsert()
> > methods.
> > 
> > It is important to note that you can use the ffi.cast() function
> > to cast numbers to CDATA of type DOUBLE. An example of this can be
> > seen below.
> > 
> > Another very important point is that CDATA of type DOUBLE in lua
> > can be used in arithmetic, but arithmetic for them does not work
> > correctly. This comes from LuaJIT and most likely will not be
> > fixed.
> > 
> > Example of usage in Lua:
> > s = box.schema.space.create('s', {format = {{'d', 'double'}}})
> > _ = s:create_index('ii')
> > s:insert({1.1})
> > ffi = require('ffi')
> > s:insert({ffi.cast('double', 1)})
> > s:insert({ffi.cast('double', tonumber('123'))})
> > s:select(1.1)
> > s:select({ffi.cast('double', 1)})
> 
> I'd also mention the way how double values are stored (their format:
> mp_float or mp_double). It would allow to provide correct storage size
> calculations.
> 
Added: "Values of this type are stored as MP_DOUBLE in
msgpack. The size of the encoded value is always 9 bytes."

> > In SQL, DOUBLE type behavior is different due to implicit casting.
> > In a column of type DOUBLE, the number of any supported type can
> > be inserted. However, it is possible that the number that will be
> > inserted will be different from that which is inserted due to the
> > rules for casting to DOUBLE.
> 
> In addition, this patch makes type of floating point literals
> be double (not number).
> 
Added: "In addition, after this patch, all floating point
literals will be recognized as DOUBLE. Prior to that, they
were considered as NUMBER."

> > Example of usage in SQL:
> > box.execute('CREATE TABLE t (d DOUBLE PRIMARY KEY);')
> > box.execute('INSERT INTO t VALUES (10), (-2.0), (3.3);')
> > box.execute('SELECT * FROM t;')
> > box.execute('SELECT d / 100 FROM t;')
> > box.execute('SELECT * from t WHERE d < 15;')
> > box.execute('SELECT * from t WHERE d = 3.3;')
> > ---
> > diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> > index 407b42e..df3f0d8 100644
> > --- a/src/box/sql/vdbemem.c
> > +++ b/src/box/sql/vdbemem.c
> > @@ -741,6 +741,7 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
> >  		    (pMem->flags & MEM_UInt) == 0)
> >  			return -1;
> >  		return 0;
> > +	case FIELD_TYPE_DOUBLE:
> >  	case FIELD_TYPE_NUMBER:
> >  		return sqlVdbeMemRealify(pMem);
> >  	case FIELD_TYPE_VARBINARY:
> 
> Are you going to fix CAST TO NUMBER in a separate follow-up?
> 
Yes, I will fix this as part of #4233 issue.

> PS quite brief and meanwhile nice patch. Thanks.
> 
Thank you :)

New commit-message:

sql: introduce DOUBLE type

This patch introduces type DOUBLE in SQL.

Closes #3812
Needed for #4233

@TarantoolBot document
Title: Tarantool DOUBLE field type and DOUBLE type in SQL
The DOUBLE field type was added to Tarantool mainly for adding the
DOUBLE type to SQL. Values of this type are stored as MP_DOUBLE in
msgpack. The size of the encoded value is always 9 bytes.

In Lua, only non-integer numbers and CDATA of type DOUBLE can be
inserted in this field. You cannot insert integers of type Lua
NUMBER or CDATA of type int64 or uint64 in this field. The same
rules apply to key in get(), select(), update() and upsert()
methods. It was done this way to avoid unwanted implicit casts
that could affect performance.

It is important to note that you can use the ffi.cast() function
to cast numbers to CDATA of type DOUBLE. An example of this can be
seen below.

Another very important point is that CDATA of type DOUBLE in lua
can be used in arithmetic, but arithmetic for them does not work
correctly. This comes from LuaJIT and most likely will not be
fixed.

Example of usage in Lua:
s = box.schema.space.create('s', {format = {{'d', 'double'}}})
_ = s:create_index('ii')
s:insert({1.1})
ffi = require('ffi')
s:insert({ffi.cast('double', 1)})
s:insert({ffi.cast('double', tonumber('123'))})
s:select(1.1)
s:select({ffi.cast('double', 1)})

In SQL, DOUBLE type behavior is different due to implicit casting.
In a column of type DOUBLE, the number of any supported type can
be inserted. However, it is possible that the number that will be
inserted will be different from that which is inserted due to the
rules for casting to DOUBLE. In addition, after this patch, all
floating point literals will be recognized as DOUBLE. Prior to
that, they were considered as NUMBER.

Example of usage in SQL:
box.execute('CREATE TABLE t (d DOUBLE PRIMARY KEY);')
box.execute('INSERT INTO t VALUES (10), (-2.0), (3.3);')
box.execute('SELECT * FROM t;')
box.execute('SELECT d / 100 FROM t;')
box.execute('SELECT * from t WHERE d < 15;')
box.execute('SELECT * from t WHERE d = 3.3;')