Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL
@ 2019-12-21 16:03 imeevma
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type imeevma
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: imeevma @ 2019-12-21 16:03 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch-set adds the DOUBLE type to SQL. In the first patch of
the set, the field type DOUBLE is added to Tarantool. In the
second patch, the DOUBLE type is added to SQL.

https://github.com/tarantool/tarantool/issues/3812
https://github.com/tarantool/tarantool/tree/imeevma/gh-3812-add-double-type

Mergen Imeev (2):
  box: introduce DOUBLE field type
  sql: introduce DOUBLE type

 extra/mkkeywordhash.c                      |   2 +-
 src/box/field_def.c                        |  28 ++-
 src/box/field_def.h                        |   1 +
 src/box/sql/expr.c                         |   6 +-
 src/box/sql/parse.y                        |   3 +-
 src/box/sql/sqlInt.h                       |   3 +-
 src/box/sql/vdbe.c                         |   4 +
 src/box/sql/vdbemem.c                      |  15 +-
 src/box/tuple_compare.cc                   |  24 ++
 test/engine/insert.result                  | 151 +++++++++++
 test/engine/insert.test.lua                |  51 ++++
 test/sql/gh-3888-values-blob-assert.result |   4 +-
 test/sql/misc.result                       |   4 +-
 test/sql/types.result                      | 390 ++++++++++++++++++++++++++++-
 test/sql/types.test.lua                    |  66 +++++
 15 files changed, 716 insertions(+), 36 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type
  2019-12-21 16:03 [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL imeevma
@ 2019-12-21 16:03 ` imeevma
  2019-12-24 22:50   ` Nikita Pettik
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type imeevma
  2019-12-23 19:16 ` [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL Vladislav Shpilevoy
  2 siblings, 1 reply; 11+ messages in thread
From: imeevma @ 2019-12-21 16:03 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch creates DOUBLE field type in Tarantool. The main
purpose of this field type is to add DOUBLE type to SQL.

Part of #3812
---
 src/box/field_def.c         |  28 ++++----
 src/box/field_def.h         |   1 +
 src/box/tuple_compare.cc    |  24 +++++++
 test/engine/insert.result   | 151 ++++++++++++++++++++++++++++++++++++++++++++
 test/engine/insert.test.lua |  51 +++++++++++++++
 5 files changed, 243 insertions(+), 12 deletions(-)

diff --git a/src/box/field_def.c b/src/box/field_def.c
index da766d5..fde4e5a 100644
--- a/src/box/field_def.c
+++ b/src/box/field_def.c
@@ -59,6 +59,7 @@ const uint32_t field_mp_type[] = {
 	/* [FIELD_TYPE_STRING]   =  */ 1U << MP_STR,
 	/* [FIELD_TYPE_NUMBER]   =  */ (1U << MP_UINT) | (1U << MP_INT) |
 		(1U << MP_FLOAT) | (1U << MP_DOUBLE),
+	/* [FIELD_TYPE_DOUBLE]   =  */ 1U << MP_DOUBLE,
 	/* [FIELD_TYPE_INTEGER]  =  */ (1U << MP_UINT) | (1U << MP_INT),
 	/* [FIELD_TYPE_BOOLEAN]  =  */ 1U << MP_BOOL,
 	/* [FIELD_TYPE_VARBINARY] =  */ 1U << MP_BIN,
@@ -75,6 +76,7 @@ const uint32_t field_ext_type[] = {
 	/* [FIELD_TYPE_UNSIGNED]  = */ 0,
 	/* [FIELD_TYPE_STRING]    = */ 0,
 	/* [FIELD_TYPE_NUMBER]    = */ 1U << MP_DECIMAL,
+	/* [FIELD_TYPE_DOUBLE]    = */ 0,
 	/* [FIELD_TYPE_INTEGER]   = */ 0,
 	/* [FIELD_TYPE_BOOLEAN]   = */ 0,
 	/* [FIELD_TYPE_VARBINARY] = */ 0,
@@ -89,6 +91,7 @@ const char *field_type_strs[] = {
 	/* [FIELD_TYPE_UNSIGNED] = */ "unsigned",
 	/* [FIELD_TYPE_STRING]   = */ "string",
 	/* [FIELD_TYPE_NUMBER]   = */ "number",
+	/* [FIELD_TYPE_DOUBLE]   = */ "double",
 	/* [FIELD_TYPE_INTEGER]  = */ "integer",
 	/* [FIELD_TYPE_BOOLEAN]  = */ "boolean",
 	/* [FIELD_TYPE_VARBINARY] = */"varbinary",
@@ -120,18 +123,19 @@ field_type_by_name_wrapper(const char *str, uint32_t len)
  * values can be stored in the j type.
  */
 static const bool field_type_compatibility[] = {
-	   /*   ANY   UNSIGNED  STRING   NUMBER  INTEGER  BOOLEAN VARBINARY SCALAR  DECIMAL  ARRAY    MAP  */
-/*   ANY    */ true,   false,   false,   false,   false,   false,   false,  false,  false,  false,   false,
-/* UNSIGNED */ true,   true,    false,   true,    true,    false,   false,  true,   false,  false,   false,
-/*  STRING  */ true,   false,   true,    false,   false,   false,   false,  true,   false,  false,   false,
-/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,  true,   false,  false,   false,
-/*  INTEGER */ true,   false,   false,   true,    true,    false,   false,  true,   false,  false,   false,
-/*  BOOLEAN */ true,   false,   false,   false,   false,   true,    false,  true,   false,  false,   false,
-/* VARBINARY*/ true,   false,   false,   false,   false,   false,   true,   true,   false,  false,   false,
-/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,  true,   false,  false,   false,
-/*  DECIMAL */ true,   false,   false,   true,    false,   false,   false,  true,   true,   false,   false,
-/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,  false,  false,  true,    false,
-/*    MAP   */ true,   false,   false,   false,   false,   false,   false,  false,  false,  false,   true,
+	   /*   ANY   UNSIGNED  STRING   NUMBER  DOUBLE  INTEGER  BOOLEAN VARBINARY SCALAR  DECIMAL  ARRAY    MAP  */
+/*   ANY    */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   false,
+/* UNSIGNED */ true,   true,    false,   true,    false,   true,    false,   false,  true,   false,  false,   false,
+/*  STRING  */ true,   false,   true,    false,   false,   false,   false,   false,  true,   false,  false,   false,
+/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,   false,  true,   false,  false,   false,
+/*  DOUBLE  */ true,   false,   false,   true,    true,    false,   false,   false,  true,   false,  false,   false,
+/*  INTEGER */ true,   false,   false,   true,    false,   true,    false,   false,  true,   false,  false,   false,
+/*  BOOLEAN */ true,   false,   false,   false,   false,   false,   true,    false,  true,   false,  false,   false,
+/* VARBINARY*/ true,   false,   false,   false,   false,   false,   false,   true,   true,   false,  false,   false,
+/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,   false,  true,   false,  false,   false,
+/*  DECIMAL */ true,   false,   false,   true,    false,   false,   false,   false,  true,   true,   false,   false,
+/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  true,    false,
+/*    MAP   */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   true,
 };
 
 bool
diff --git a/src/box/field_def.h b/src/box/field_def.h
index 49c2998..8e82369 100644
--- a/src/box/field_def.h
+++ b/src/box/field_def.h
@@ -54,6 +54,7 @@ enum field_type {
 	FIELD_TYPE_UNSIGNED,
 	FIELD_TYPE_STRING,
 	FIELD_TYPE_NUMBER,
+	FIELD_TYPE_DOUBLE,
 	FIELD_TYPE_INTEGER,
 	FIELD_TYPE_BOOLEAN,
 	FIELD_TYPE_VARBINARY,
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index 9573ff1..3f8a0ce 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -122,6 +122,14 @@ mp_compare_bool(const char *field_a, const char *field_b)
 }
 
 static int
+mp_compare_double(const char *field_a, const char *field_b)
+{
+	double a_val = mp_decode_double(&field_a);
+	double b_val = mp_decode_double(&field_b);
+	return COMPARE_RESULT(a_val, b_val);
+}
+
+static int
 mp_compare_integer_with_type(const char *field_a, enum mp_type a_type,
 			     const char *field_b, enum mp_type b_type)
 {
@@ -443,6 +451,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
 						    mp_typeof(*field_b));
 	case FIELD_TYPE_NUMBER:
 		return mp_compare_number(field_a, field_b);
+	case FIELD_TYPE_DOUBLE:
+		return mp_compare_double(field_a, field_b);
 	case FIELD_TYPE_BOOLEAN:
 		return mp_compare_bool(field_a, field_b);
 	case FIELD_TYPE_VARBINARY:
@@ -477,6 +487,8 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
 	case FIELD_TYPE_NUMBER:
 		return mp_compare_number_with_type(field_a, a_type,
 						   field_b, b_type);
+	case FIELD_TYPE_DOUBLE:
+		return mp_compare_double(field_a, field_b);
 	case FIELD_TYPE_BOOLEAN:
 		return mp_compare_bool(field_a, field_b);
 	case FIELD_TYPE_VARBINARY:
@@ -1631,6 +1643,13 @@ field_hint_integer(const char *field)
 }
 
 static inline hint_t
+field_hint_double(const char *field)
+{
+	assert(mp_typeof(*field) == MP_DOUBLE);
+	return hint_double(mp_decode_double(&field));
+}
+
+static inline hint_t
 field_hint_number(const char *field)
 {
 	switch (mp_typeof(*field)) {
@@ -1753,6 +1772,8 @@ field_hint(const char *field, struct coll *coll)
 		return field_hint_integer(field);
 	case FIELD_TYPE_NUMBER:
 		return field_hint_number(field);
+	case FIELD_TYPE_DOUBLE:
+		return field_hint_double(field);
 	case FIELD_TYPE_STRING:
 		return field_hint_string(field, coll);
 	case FIELD_TYPE_VARBINARY:
@@ -1857,6 +1878,9 @@ key_def_set_hint_func(struct key_def *def)
 	case FIELD_TYPE_NUMBER:
 		key_def_set_hint_func<FIELD_TYPE_NUMBER>(def);
 		break;
+	case FIELD_TYPE_DOUBLE:
+		key_def_set_hint_func<FIELD_TYPE_DOUBLE>(def);
+		break;
 	case FIELD_TYPE_STRING:
 		key_def_set_hint_func<FIELD_TYPE_STRING>(def);
 		break;
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
 ---
 ...
+-- gh-3812: Make sure that DOUBLE field type works properly.
+ffi = require('ffi')
+---
+...
+s = box.schema.space.create('s', {format = {{'i', 'integer'}, {'d', 'double'}}})
+---
+...
+_ = s:create_index('ii')
+---
+...
+--
+-- If number of Lua type NUMBER is not integer, than it could be
+-- inserted in DOUBLE field.
+--
+s:insert({1, 1.1})
+---
+- [1, 1.1]
+...
+s:insert({2, 2.5})
+---
+- [2, 2.5]
+...
+s:insert({3, -3.0009})
+---
+- [3, -3.0009]
+...
+--
+-- 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'
+...
+s:insert({5, -9223372036854775800ULL})
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected double'
+...
+s:insert({6, 18000000000000000000ULL})
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected double'
+...
+--
+-- 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)})
+---
+- [7, 1]
+...
+s:insert({8, ffi.cast('double', -9223372036854775808)})
+---
+- [8, -9223372036854775808]
+...
+s:insert({9, ffi.cast('double', tonumber('123'))})
+---
+- [9, 123]
+...
+s:insert({10, ffi.cast('double', tonumber64('18000000000000000000'))})
+---
+- [10, 18000000000000000000]
+...
+s:insert({11, ffi.cast('double', 1.1)})
+---
+- [11, 1.1]
+...
+s:insert({12, ffi.cast('double', -3.0009)})
+---
+- [12, -3.0009]
+...
+s:select()
+---
+- - [1, 1.1]
+  - [2, 2.5]
+  - [3, -3.0009]
+  - [7, 1]
+  - [8, -9223372036854775808]
+  - [9, 123]
+  - [10, 18000000000000000000]
+  - [11, 1.1]
+  - [12, -3.0009]
+...
+-- The same rules apply to the key of this field:
+dd = s:create_index('dd', {unique = false, parts = {{2, 'double'}}})
+---
+...
+dd:select(1.1)
+---
+- - [1, 1.1]
+  - [11, 1.1]
+...
+dd:select(1)
+---
+- error: 'Supplied key type of part 0 does not match index part type: expected double'
+...
+dd:select(ffi.cast('double', 1))
+---
+- - [7, 1]
+...
+-- Make sure the comparisons work correctly.
+dd:select(1.1, {iterator = 'ge'})
+---
+- - [1, 1.1]
+  - [11, 1.1]
+  - [2, 2.5]
+  - [9, 123]
+  - [10, 18000000000000000000]
+...
+dd:select(1.1, {iterator = 'le'})
+---
+- - [11, 1.1]
+  - [1, 1.1]
+  - [7, 1]
+  - [12, -3.0009]
+  - [3, -3.0009]
+  - [8, -9223372036854775808]
+...
+dd:select(ffi.cast('double', 1.1), {iterator = 'gt'})
+---
+- - [2, 2.5]
+  - [9, 123]
+  - [10, 18000000000000000000]
+...
+dd:select(ffi.cast('double', 1.1), {iterator = 'lt'})
+---
+- - [7, 1]
+  - [12, -3.0009]
+  - [3, -3.0009]
+  - [8, -9223372036854775808]
+...
+dd:select(1.1, {iterator = 'all'})
+---
+- - [1, 1.1]
+  - [11, 1.1]
+  - [2, 2.5]
+  - [9, 123]
+  - [10, 18000000000000000000]
+...
+dd:select(1.1, {iterator = 'eq'})
+---
+- - [1, 1.1]
+  - [11, 1.1]
+...
+dd:select(1.1, {iterator = 'req'})
+---
+- - [11, 1.1]
+  - [1, 1.1]
+...
+s:drop()
+---
+...
diff --git a/test/engine/insert.test.lua b/test/engine/insert.test.lua
index 39b1501..a5d8a4c 100644
--- a/test/engine/insert.test.lua
+++ b/test/engine/insert.test.lua
@@ -123,3 +123,54 @@ s:select{}
 s:drop()
 fiber = nil
 
+-- gh-3812: Make sure that DOUBLE field type works properly.
+ffi = require('ffi')
+
+s = box.schema.space.create('s', {format = {{'i', 'integer'}, {'d', 'double'}}})
+_ = s:create_index('ii')
+
+--
+-- If number of Lua type NUMBER is not integer, than it could be
+-- inserted in DOUBLE field.
+--
+s:insert({1, 1.1})
+s:insert({2, 2.5})
+s:insert({3, -3.0009})
+
+--
+-- Integers of Lua type NUMBER and CDATA of type int64 or uint64
+-- cannot be inserted into this field.
+--
+s:insert({4, 1})
+s:insert({5, -9223372036854775800ULL})
+s:insert({6, 18000000000000000000ULL})
+
+--
+-- 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)})
+
+s:select()
+
+-- The same rules apply to the key of this field:
+dd = s:create_index('dd', {unique = false, parts = {{2, 'double'}}})
+dd:select(1.1)
+dd:select(1)
+dd:select(ffi.cast('double', 1))
+
+-- Make sure the comparisons work correctly.
+dd:select(1.1, {iterator = 'ge'})
+dd:select(1.1, {iterator = 'le'})
+dd:select(ffi.cast('double', 1.1), {iterator = 'gt'})
+dd:select(ffi.cast('double', 1.1), {iterator = 'lt'})
+dd:select(1.1, {iterator = 'all'})
+dd:select(1.1, {iterator = 'eq'})
+dd:select(1.1, {iterator = 'req'})
+
+s:drop()
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type
  2019-12-21 16:03 [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL imeevma
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type imeevma
@ 2019-12-21 16:03 ` imeevma
  2019-12-24 22:50   ` Nikita Pettik
  2019-12-23 19:16 ` [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL Vladislav Shpilevoy
  2 siblings, 1 reply; 11+ messages in thread
From: imeevma @ 2019-12-21 16:03 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

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. 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)})

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.

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;')
---
 extra/mkkeywordhash.c                      |   2 +-
 src/box/sql/expr.c                         |   6 +-
 src/box/sql/parse.y                        |   3 +-
 src/box/sql/sqlInt.h                       |   3 +-
 src/box/sql/vdbe.c                         |   4 +
 src/box/sql/vdbemem.c                      |  15 +-
 test/sql/gh-3888-values-blob-assert.result |   4 +-
 test/sql/misc.result                       |   4 +-
 test/sql/types.result                      | 390 ++++++++++++++++++++++++++++-
 test/sql/types.test.lua                    |  66 +++++
 10 files changed, 473 insertions(+), 24 deletions(-)

diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index 3c3de4c..0062856 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -202,7 +202,7 @@ static Keyword aKeywordTable[] = {
   { "DENSE_RANK",             "TK_STANDARD",    true  },
   { "DESCRIBE",               "TK_STANDARD",    true  },
   { "DETERMINISTIC",          "TK_STANDARD",    true  },
-  { "DOUBLE",                 "TK_STANDARD",    true  },
+  { "DOUBLE",                 "TK_DOUBLE",      true  },
   { "ELSEIF",                 "TK_STANDARD",    true  },
   { "ENABLE",                 "TK_ENABLE",      false },
   { "FETCH",                  "TK_STANDARD",    true  },
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 0bdcfe5..f76f822 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -388,9 +388,11 @@ sql_type_result(enum field_type lhs, enum field_type rhs)
 	if (sql_type_is_numeric(lhs) || sql_type_is_numeric(rhs)) {
 		if (lhs == FIELD_TYPE_NUMBER || rhs == FIELD_TYPE_NUMBER)
 			return FIELD_TYPE_NUMBER;
+		if (lhs == FIELD_TYPE_DOUBLE || rhs == FIELD_TYPE_DOUBLE)
+			return FIELD_TYPE_DOUBLE;
 		if (lhs == FIELD_TYPE_INTEGER || rhs == FIELD_TYPE_INTEGER)
 			return FIELD_TYPE_INTEGER;
-		assert(lhs == FIELD_TYPE_UNSIGNED ||
+		assert(lhs == FIELD_TYPE_UNSIGNED &&
 		       rhs == FIELD_TYPE_UNSIGNED);
 		return FIELD_TYPE_UNSIGNED;
 	}
@@ -2260,7 +2262,7 @@ sql_expr_needs_no_type_change(const struct Expr *p, enum field_type type)
 	case TK_INTEGER:
 		return type == FIELD_TYPE_INTEGER;
 	case TK_FLOAT:
-		return type == FIELD_TYPE_NUMBER;
+		return type == FIELD_TYPE_DOUBLE;
 	case TK_STRING:
 		return type == FIELD_TYPE_STRING;
 	case TK_BLOB:
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 1d0c95f..c5f6e47 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -981,7 +981,7 @@ idlist(A) ::= nm(Y). {
         p->type = FIELD_TYPE_INTEGER;
         break;
       case TK_FLOAT:
-        p->type = FIELD_TYPE_NUMBER;
+        p->type = FIELD_TYPE_DOUBLE;
         break;
       case TK_TRUE:
       case TK_FALSE:
@@ -1846,6 +1846,7 @@ typedef(A) ::= VARCHAR char_len(B) . {
 %type number_typedef {struct type_def}
 typedef(A) ::= number_typedef(A) .
 number_typedef(A) ::= NUMBER . { A.type = FIELD_TYPE_NUMBER; }
+number_typedef(A) ::= DOUBLE . { A.type = FIELD_TYPE_DOUBLE; }
 number_typedef(A) ::= INT|INTEGER_KW . { A.type = FIELD_TYPE_INTEGER; }
 number_typedef(A) ::= UNSIGNED . { A.type = FIELD_TYPE_UNSIGNED; }
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 2594b73..6c50572 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1278,7 +1278,8 @@ enum trim_side_mask {
 
 #define sql_type_is_numeric(X)  ((X) == FIELD_TYPE_INTEGER || \
 				 (X) == FIELD_TYPE_NUMBER || \
-				 (X) == FIELD_TYPE_UNSIGNED)
+				 (X) == FIELD_TYPE_UNSIGNED || \
+				 (X) == FIELD_TYPE_DOUBLE)
 
 /*
  * Additional bit values that can be ORed with an type without
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 6ebc5f0..eccb265 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -347,6 +347,10 @@ mem_apply_type(struct Mem *record, enum field_type type)
 		if ((record->flags & (MEM_Real | MEM_Int | MEM_UInt)) != 0)
 			return 0;
 		return sqlVdbeMemRealify(record);
+	case FIELD_TYPE_DOUBLE:
+		if ((record->flags & MEM_Real) != 0)
+			return 0;
+		return sqlVdbeMemRealify(record);
 	case FIELD_TYPE_STRING:
 		/*
 		 * Only attempt the conversion to TEXT if there is
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:
@@ -1820,26 +1821,12 @@ mpstream_encode_vdbe_mem(struct mpstream *stream, struct Mem *var)
 	if (var->flags & MEM_Null) {
 		mpstream_encode_nil(stream);
 	} else if (var->flags & MEM_Real) {
-		/*
-		 * We can't pass to INT iterator float
-		 * value. Hence, if floating point value
-		 * lacks fractional component, we can
-		 * encode it as INT and successfully
-		 * pass to INT iterator.
-		 */
-		i = var->u.r;
-		if (i == var->u.r && i < 0)
-			goto encode_int;
-		if (i == var->u.r && i >= 0)
-			goto encode_uint;
 		mpstream_encode_double(stream, var->u.r);
 	} else if (var->flags & MEM_Int) {
 		i = var->u.i;
-encode_int:
 		mpstream_encode_int(stream, i);
 	} else if (var->flags & MEM_UInt) {
 		i = var->u.u;
-encode_uint:
 		mpstream_encode_uint(stream, i);
 	} else if (var->flags & MEM_Str) {
 		mpstream_encode_strn(stream, var->z, var->n);
diff --git a/test/sql/gh-3888-values-blob-assert.result b/test/sql/gh-3888-values-blob-assert.result
index 4b8e7ed..edc5035 100644
--- a/test/sql/gh-3888-values-blob-assert.result
+++ b/test/sql/gh-3888-values-blob-assert.result
@@ -53,7 +53,7 @@ box.execute('VALUES(-0.5e-2)')
 ---
 - metadata:
   - name: column1
-    type: number
+    type: double
   rows:
   - [-0.005]
 ...
@@ -70,7 +70,7 @@ box.execute('SELECT 3.14')
 ---
 - metadata:
   - name: '3.14'
-    type: number
+    type: double
   rows:
   - [3.14]
 ...
diff --git a/test/sql/misc.result b/test/sql/misc.result
index a157ddb..1fc39a8 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -83,7 +83,7 @@ box.execute('SELECT 1.5;')
 ---
 - metadata:
   - name: '1.5'
-    type: number
+    type: double
   rows:
   - [1.5]
 ...
@@ -91,7 +91,7 @@ box.execute('SELECT 1.0;')
 ---
 - metadata:
   - name: '1.0'
-    type: number
+    type: double
   rows:
   - [1]
 ...
diff --git a/test/sql/types.result b/test/sql/types.result
index 1ad52e8..d2cc3aa 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -309,7 +309,7 @@ box.execute('SELECT 1 + 1.1;')
 ---
 - metadata:
   - name: 1 + 1.1
-    type: number
+    type: double
   rows:
   - [2.1]
 ...
@@ -1691,3 +1691,391 @@ box.execute('DROP TABLE t1;')
 ---
 - row_count: 1
 ...
+-- gh-3812: Make sure DOUBLE type works correctly.
+box.execute("SELECT 1.0;")
+---
+- metadata:
+  - name: '1.0'
+    type: double
+  rows:
+  - [1]
+...
+box.execute("SELECT .01;")
+---
+- metadata:
+  - name: '.01'
+    type: double
+  rows:
+  - [0.01]
+...
+box.execute("SELECT CAST(1 AS DOUBLE);")
+---
+- metadata:
+  - name: CAST(1 AS DOUBLE)
+    type: double
+  rows:
+  - [1]
+...
+box.execute("SELECT CAST(1.123 AS DOUBLE);")
+---
+- metadata:
+  - name: CAST(1.123 AS DOUBLE)
+    type: double
+  rows:
+  - [1.123]
+...
+box.execute("SELECT CAST(true AS DOUBLE);")
+---
+- null
+- 'Type mismatch: can not convert TRUE to double'
+...
+box.execute("SELECT CAST('asd' AS DOUBLE);")
+---
+- null
+- 'Type mismatch: can not convert asd to double'
+...
+box.execute("SELECT CAST('1' AS DOUBLE);")
+---
+- metadata:
+  - name: CAST('1' AS DOUBLE)
+    type: double
+  rows:
+  - [1]
+...
+box.execute("SELECT CAST('1.123' AS DOUBLE);")
+---
+- metadata:
+  - name: CAST('1.123' AS DOUBLE)
+    type: double
+  rows:
+  - [1.123]
+...
+box.execute("SELECT CAST(x'' AS DOUBLE);")
+---
+- null
+- 'Type mismatch: can not convert varbinary to double'
+...
+box.execute("SELECT CAST(x'35' AS DOUBLE);")
+---
+- null
+- 'Type mismatch: can not convert varbinary to double'
+...
+box.execute("SELECT CAST(CAST(x'35' AS STRING) AS DOUBLE);")
+---
+- metadata:
+  - name: CAST(CAST(x'35' AS STRING) AS DOUBLE)
+    type: double
+  rows:
+  - [5]
+...
+box.execute('CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT, d DOUBLE);')
+---
+- row_count: 1
+...
+box.execute('INSERT INTO t(d) VALUES (10), (-2.0), (3.3), (18000000000000000000);')
+---
+- autoincrement_ids:
+  - 1
+  - 2
+  - 3
+  - 4
+  row_count: 4
+...
+box.execute('SELECT * FROM t;')
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: D
+    type: double
+  rows:
+  - [1, 10]
+  - [2, -2]
+  - [3, 3.3]
+  - [4, 18000000000000000000]
+...
+box.execute('SELECT d / 100 FROM t;')
+---
+- metadata:
+  - name: d / 100
+    type: double
+  rows:
+  - [0.1]
+  - [-0.02]
+  - [0.033]
+  - [180000000000000000]
+...
+box.execute('SELECT * from t WHERE d < 15;')
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: D
+    type: double
+  rows:
+  - [1, 10]
+  - [2, -2]
+  - [3, 3.3]
+...
+box.execute('SELECT * from t WHERE d = 3.3;')
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: D
+    type: double
+  rows:
+  - [3, 3.3]
+...
+box.execute("SELECT sum(d) FROM t;")
+---
+- metadata:
+  - name: sum(d)
+    type: number
+  rows:
+  - [18000000000000000000]
+...
+box.execute("SELECT avg(d) FROM t;")
+---
+- metadata:
+  - name: avg(d)
+    type: number
+  rows:
+  - [4500000000000000000]
+...
+box.execute("SELECT total(d) FROM t;")
+---
+- metadata:
+  - name: total(d)
+    type: number
+  rows:
+  - [18000000000000000000]
+...
+box.execute("SELECT min(d) FROM t;")
+---
+- metadata:
+  - name: min(d)
+    type: scalar
+  rows:
+  - [-2]
+...
+box.execute("SELECT max(d) FROM t;")
+---
+- metadata:
+  - name: max(d)
+    type: scalar
+  rows:
+  - [18000000000000000000]
+...
+box.execute("SELECT count(d) FROM t;")
+---
+- metadata:
+  - name: count(d)
+    type: integer
+  rows:
+  - [4]
+...
+box.execute("SELECT group_concat(d) FROM t;")
+---
+- metadata:
+  - name: group_concat(d)
+    type: string
+  rows:
+  - ['10.0,-2.0,3.3,1.8e+19']
+...
+box.execute("SELECT lower(d) FROM t;")
+---
+- metadata:
+  - name: lower(d)
+    type: string
+  rows:
+  - ['10.0']
+  - ['-2.0']
+  - ['3.3']
+  - ['1.8e+19']
+...
+box.execute("SELECT upper(d) FROM t;")
+---
+- metadata:
+  - name: upper(d)
+    type: string
+  rows:
+  - ['10.0']
+  - ['-2.0']
+  - ['3.3']
+  - ['1.8E+19']
+...
+box.execute("SELECT abs(d) FROM t;")
+---
+- metadata:
+  - name: abs(d)
+    type: number
+  rows:
+  - [10]
+  - [2]
+  - [3.3]
+  - [18000000000000000000]
+...
+box.execute("SELECT typeof(d) FROM t;")
+---
+- metadata:
+  - name: typeof(d)
+    type: string
+  rows:
+  - ['double']
+  - ['double']
+  - ['double']
+  - ['double']
+...
+box.execute("SELECT quote(d) FROM t;")
+---
+- metadata:
+  - name: quote(d)
+    type: string
+  rows:
+  - ['10.0']
+  - ['-2.0']
+  - ['3.3']
+  - ['1.8e+19']
+...
+box.execute("SELECT LEAST(d, 0) FROM t;")
+---
+- metadata:
+  - name: LEAST(d, 0)
+    type: scalar
+  rows:
+  - [0]
+  - [-2]
+  - [0]
+  - [0]
+...
+box.execute("CREATE INDEX dd ON t(d);")
+---
+- row_count: 1
+...
+box.execute("SELECT d FROM t WHERE d < 0;")
+---
+- metadata:
+  - name: D
+    type: double
+  rows:
+  - [-2]
+...
+box.execute("SELECT d FROM t ORDER BY d;")
+---
+- metadata:
+  - name: D
+    type: double
+  rows:
+  - [-2]
+  - [3.3]
+  - [10]
+  - [18000000000000000000]
+...
+box.execute("UPDATE t SET d = 1 WHERE d = 10;")
+---
+- row_count: 1
+...
+box.execute("SELECT d FROM t;")
+---
+- metadata:
+  - name: D
+    type: double
+  rows:
+  - [1]
+  - [-2]
+  - [3.3]
+  - [18000000000000000000]
+...
+box.execute("DROP TABLE t;")
+---
+- row_count: 1
+...
+box.execute("CREATE TABLE t1 (d DOUBLE PRIMARY KEY);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t1 VALUES (1), (2.2), (3.5);")
+---
+- row_count: 3
+...
+box.execute("INSERT INTO t1 VALUES (1);")
+---
+- null
+- Duplicate key exists in unique index 'pk_unnamed_T1_1' in space 'T1'
+...
+box.execute("CREATE TABLE t2 (i INT PRIMARY KEY, d DOUBLE REFERENCES t1);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t2 VALUES (1,1), (2,2.2), (100, 3.5), (4, 1);")
+---
+- row_count: 4
+...
+box.execute("INSERT INTO t2 VALUES (5,10);")
+---
+- null
+- 'Failed to execute SQL statement: FOREIGN KEY constraint failed'
+...
+box.execute("DROP TABLE t2;")
+---
+- row_count: 1
+...
+box.execute("DROP TABLE t1;")
+---
+- row_count: 1
+...
+box.execute("CREATE TABLE t3 (i INT PRIMARY KEY, d DOUBLE CHECK (d < 10));")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t3 VALUES (1, 1);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t3 VALUES (2, 9.999999);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t3 VALUES (3, 10.0000001);")
+---
+- null
+- 'Check constraint failed ''ck_unnamed_T3_1'': d < 10'
+...
+box.execute("SELECT * FROM t3;")
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: D
+    type: double
+  rows:
+  - [1, 1]
+  - [2, 9.999999]
+...
+box.execute("DROP TABLE t3;")
+---
+- row_count: 1
+...
+box.execute("CREATE TABLE t4 (i INT PRIMARY KEY, d DOUBLE DEFAULT 1.2345);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t4(i) VALUES (1);")
+---
+- row_count: 1
+...
+box.execute("SELECT * FROM t4;")
+---
+- metadata:
+  - name: I
+    type: integer
+  - name: D
+    type: double
+  rows:
+  - [1, 1.2345]
+...
+box.execute("DROP TABLE t4;")
+---
+- row_count: 1
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index f6fb64c..24bfa42 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -399,3 +399,69 @@ s:insert({1, {b = 1}})
 box.execute('INSERT INTO t1(a) SELECT a FROM t2;')
 s:drop()
 box.execute('DROP TABLE t1;')
+
+-- gh-3812: Make sure DOUBLE type works correctly.
+box.execute("SELECT 1.0;")
+box.execute("SELECT .01;")
+
+box.execute("SELECT CAST(1 AS DOUBLE);")
+box.execute("SELECT CAST(1.123 AS DOUBLE);")
+box.execute("SELECT CAST(true AS DOUBLE);")
+box.execute("SELECT CAST('asd' AS DOUBLE);")
+box.execute("SELECT CAST('1' AS DOUBLE);")
+box.execute("SELECT CAST('1.123' AS DOUBLE);")
+box.execute("SELECT CAST(x'' AS DOUBLE);")
+box.execute("SELECT CAST(x'35' AS DOUBLE);")
+box.execute("SELECT CAST(CAST(x'35' AS STRING) AS DOUBLE);")
+
+box.execute('CREATE TABLE t (i INT PRIMARY KEY AUTOINCREMENT, d DOUBLE);')
+box.execute('INSERT INTO t(d) VALUES (10), (-2.0), (3.3), (18000000000000000000);')
+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;')
+
+box.execute("SELECT sum(d) FROM t;")
+box.execute("SELECT avg(d) FROM t;")
+box.execute("SELECT total(d) FROM t;")
+box.execute("SELECT min(d) FROM t;")
+box.execute("SELECT max(d) FROM t;")
+box.execute("SELECT count(d) FROM t;")
+box.execute("SELECT group_concat(d) FROM t;")
+
+box.execute("SELECT lower(d) FROM t;")
+box.execute("SELECT upper(d) FROM t;")
+box.execute("SELECT abs(d) FROM t;")
+box.execute("SELECT typeof(d) FROM t;")
+box.execute("SELECT quote(d) FROM t;")
+box.execute("SELECT LEAST(d, 0) FROM t;")
+
+box.execute("CREATE INDEX dd ON t(d);")
+box.execute("SELECT d FROM t WHERE d < 0;")
+box.execute("SELECT d FROM t ORDER BY d;")
+
+box.execute("UPDATE t SET d = 1 WHERE d = 10;")
+box.execute("SELECT d FROM t;")
+box.execute("DROP TABLE t;")
+
+box.execute("CREATE TABLE t1 (d DOUBLE PRIMARY KEY);")
+box.execute("INSERT INTO t1 VALUES (1), (2.2), (3.5);")
+box.execute("INSERT INTO t1 VALUES (1);")
+
+box.execute("CREATE TABLE t2 (i INT PRIMARY KEY, d DOUBLE REFERENCES t1);")
+box.execute("INSERT INTO t2 VALUES (1,1), (2,2.2), (100, 3.5), (4, 1);")
+box.execute("INSERT INTO t2 VALUES (5,10);")
+box.execute("DROP TABLE t2;")
+box.execute("DROP TABLE t1;")
+
+box.execute("CREATE TABLE t3 (i INT PRIMARY KEY, d DOUBLE CHECK (d < 10));")
+box.execute("INSERT INTO t3 VALUES (1, 1);")
+box.execute("INSERT INTO t3 VALUES (2, 9.999999);")
+box.execute("INSERT INTO t3 VALUES (3, 10.0000001);")
+box.execute("SELECT * FROM t3;")
+box.execute("DROP TABLE t3;")
+
+box.execute("CREATE TABLE t4 (i INT PRIMARY KEY, d DOUBLE DEFAULT 1.2345);")
+box.execute("INSERT INTO t4(i) VALUES (1);")
+box.execute("SELECT * FROM t4;")
+box.execute("DROP TABLE t4;")
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL
  2019-12-21 16:03 [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL imeevma
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type imeevma
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type imeevma
@ 2019-12-23 19:16 ` Vladislav Shpilevoy
  2019-12-27 12:37   ` Nikita Pettik
  2 siblings, 1 reply; 11+ messages in thread
From: Vladislav Shpilevoy @ 2019-12-23 19:16 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Thanks for the patch! LGTM.

Nikita, please, do a second review.

On 21/12/2019 17:03, imeevma@tarantool.org wrote:
> This patch-set adds the DOUBLE type to SQL. In the first patch of
> the set, the field type DOUBLE is added to Tarantool. In the
> second patch, the DOUBLE type is added to SQL.
> 
> https://github.com/tarantool/tarantool/issues/3812
> https://github.com/tarantool/tarantool/tree/imeevma/gh-3812-add-double-type
> 
> Mergen Imeev (2):
>   box: introduce DOUBLE field type
>   sql: introduce DOUBLE type
> 
>  extra/mkkeywordhash.c                      |   2 +-
>  src/box/field_def.c                        |  28 ++-
>  src/box/field_def.h                        |   1 +
>  src/box/sql/expr.c                         |   6 +-
>  src/box/sql/parse.y                        |   3 +-
>  src/box/sql/sqlInt.h                       |   3 +-
>  src/box/sql/vdbe.c                         |   4 +
>  src/box/sql/vdbemem.c                      |  15 +-
>  src/box/tuple_compare.cc                   |  24 ++
>  test/engine/insert.result                  | 151 +++++++++++
>  test/engine/insert.test.lua                |  51 ++++
>  test/sql/gh-3888-values-blob-assert.result |   4 +-
>  test/sql/misc.result                       |   4 +-
>  test/sql/types.result                      | 390 ++++++++++++++++++++++++++++-
>  test/sql/types.test.lua                    |  66 +++++
>  15 files changed, 716 insertions(+), 36 deletions(-)
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type imeevma
@ 2019-12-24 22:50   ` Nikita Pettik
  2019-12-26 16:38     ` Mergen Imeev
  0 siblings, 1 reply; 11+ messages in thread
From: Nikita Pettik @ 2019-12-24 22:50 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches, v.shpilevoy

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'

> +
> +--
> +-- 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). 

The rest is OK.
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type
  2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type imeevma
@ 2019-12-24 22:50   ` Nikita Pettik
  2019-12-26 16:42     ` Mergen Imeev
  0 siblings, 1 reply; 11+ messages in thread
From: Nikita Pettik @ 2019-12-24 22:50 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches, v.shpilevoy

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.

> 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.

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

> 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?

PS quite brief and meanwhile nice patch. Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type
  2019-12-24 22:50   ` Nikita Pettik
@ 2019-12-26 16:38     ` Mergen Imeev
  2019-12-26 20:34       ` Nikita Pettik
  0 siblings, 1 reply; 11+ messages in thread
From: Mergen Imeev @ 2019-12-26 16:38 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type
  2019-12-24 22:50   ` Nikita Pettik
@ 2019-12-26 16:42     ` Mergen Imeev
  2019-12-26 20:37       ` Nikita Pettik
  0 siblings, 1 reply; 11+ messages in thread
From: Mergen Imeev @ 2019-12-26 16:42 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy

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;')

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type
  2019-12-26 16:38     ` Mergen Imeev
@ 2019-12-26 20:34       ` Nikita Pettik
  0 siblings, 0 replies; 11+ messages in thread
From: Nikita Pettik @ 2019-12-26 20:34 UTC (permalink / raw)
  To: Mergen Imeev; +Cc: tarantool-patches, v.shpilevoy

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type
  2019-12-26 16:42     ` Mergen Imeev
@ 2019-12-26 20:37       ` Nikita Pettik
  0 siblings, 0 replies; 11+ messages in thread
From: Nikita Pettik @ 2019-12-26 20:37 UTC (permalink / raw)
  To: Mergen Imeev; +Cc: tarantool-patches, v.shpilevoy

On 26 Dec 19:42, Mergen Imeev wrote:
> Thank you for review! My answers and new commit-message
> below.

LGTM
 
> 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;')
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL
  2019-12-23 19:16 ` [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL Vladislav Shpilevoy
@ 2019-12-27 12:37   ` Nikita Pettik
  0 siblings, 0 replies; 11+ messages in thread
From: Nikita Pettik @ 2019-12-27 12:37 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On 23 Dec 20:16, Vladislav Shpilevoy wrote:
> Thanks for the patch! LGTM.
> 
> Nikita, please, do a second review.

Mergen fixed nits I had noticed. Pushed to master.
 
> On 21/12/2019 17:03, imeevma@tarantool.org wrote:
> > This patch-set adds the DOUBLE type to SQL. In the first patch of
> > the set, the field type DOUBLE is added to Tarantool. In the
> > second patch, the DOUBLE type is added to SQL.
> > 
> > https://github.com/tarantool/tarantool/issues/3812
> > https://github.com/tarantool/tarantool/tree/imeevma/gh-3812-add-double-type
> > 
> > Mergen Imeev (2):
> >   box: introduce DOUBLE field type
> >   sql: introduce DOUBLE type
> > 
> >  extra/mkkeywordhash.c                      |   2 +-
> >  src/box/field_def.c                        |  28 ++-
> >  src/box/field_def.h                        |   1 +
> >  src/box/sql/expr.c                         |   6 +-
> >  src/box/sql/parse.y                        |   3 +-
> >  src/box/sql/sqlInt.h                       |   3 +-
> >  src/box/sql/vdbe.c                         |   4 +
> >  src/box/sql/vdbemem.c                      |  15 +-
> >  src/box/tuple_compare.cc                   |  24 ++
> >  test/engine/insert.result                  | 151 +++++++++++
> >  test/engine/insert.test.lua                |  51 ++++
> >  test/sql/gh-3888-values-blob-assert.result |   4 +-
> >  test/sql/misc.result                       |   4 +-
> >  test/sql/types.result                      | 390 ++++++++++++++++++++++++++++-
> >  test/sql/types.test.lua                    |  66 +++++
> >  15 files changed, 716 insertions(+), 36 deletions(-)
> > 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-12-27 12:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-21 16:03 [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL imeevma
2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 1/2] box: introduce DOUBLE field type imeevma
2019-12-24 22:50   ` Nikita Pettik
2019-12-26 16:38     ` Mergen Imeev
2019-12-26 20:34       ` Nikita Pettik
2019-12-21 16:03 ` [Tarantool-patches] [PATCH v1 2/2] sql: introduce DOUBLE type imeevma
2019-12-24 22:50   ` Nikita Pettik
2019-12-26 16:42     ` Mergen Imeev
2019-12-26 20:37       ` Nikita Pettik
2019-12-23 19:16 ` [Tarantool-patches] [PATCH v1 0/2] Add DOUBLE type to SQL Vladislav Shpilevoy
2019-12-27 12:37   ` Nikita Pettik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox