[tarantool-patches] [PATCH 0/3] Fix CAST operation

[tarantool-patches] [PATCH 0/3] Fix CAST operation

[Nikita Pettik]

Branch: https://github.com/tarantool/tarantool/tree/np/gh-4229-adjust-cast
Issue: https://github.com/tarantool/tarantool/issues/4229

This patch-set allows explicit conversion using CAST operator from REAL
to BOOLEAN and from string value containing quoted floating point
literal to INTEGER. Detailed explanation is provided in commit messages
and in issue description.

Nikita Pettik (3):
  sql: remove redundant conversion from OP_AddImm
  sql: allow CAST operation from REAL to BOOLEAN
  sql: allow CAST operation from quoted float to int

 src/box/sql/vdbe.c      |  2 +-
 src/box/sql/vdbemem.c   | 12 ++++++++---
 test/sql/types.result   | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
 test/sql/types.test.lua | 11 ++++++++++
 4 files changed, 74 insertions(+), 5 deletions(-)

-- 
2.15.1

[tarantool-patches] [PATCH 1/3] sql: remove redundant conversion from OP_AddImm

[Nikita Pettik]

OP_AddImm adds constant defined by P2 argument to memory cell P1. Before
addition, content of memory cell is converted to MEM_Int. However,
according to the usages of this opcode in source code, memory cell
always initially contains integer value. Hence, conversion to integer
can be replaced with simple assertion.
---
 src/box/sql/vdbe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index a34395cdf..d083d3709 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1929,7 +1929,7 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 case OP_AddImm: {            /* in1 */
 	pIn1 = &aMem[pOp->p1];
 	memAboutToChange(p, pIn1);
-	sqlVdbeMemIntegerify(pIn1, false);
+	assert((pIn1->flags & MEM_Int) != 0);
 	pIn1->u.i += pOp->p2;
 	break;
 }
-- 
2.15.1

[tarantool-patches] [PATCH 2/3] sql: allow CAST operation from REAL to BOOLEAN

[Nikita Pettik]

It was decided that all explicit casts for which we can come up with
meaningful semantics should work. If a user requests an explicit cast,
he/she most probably knows what they are doing.
CAST from REAL to BOOLEAN is disallowed by ANSI rules. However, we allow
CAST from INT to BOOLEAN which is also prohibited by ANSI. So, basically
it is possible to covert REAL to BOOLEAN in two steps:

SELECT CAST(CAST(1.123 AS INT) AS BOOLEAN);

For the reason mentioned above, now we allow straight CAST from REAL to
BOOLEAN. Anything different from 0.0 is evaluated to TRUE.

Part of #4229
---
 src/box/sql/vdbemem.c   |  4 ++++
 test/sql/types.result   | 22 +++++++++++++++++++++-
 test/sql/types.test.lua |  2 ++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index f73ea0a71..da17bf601 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -664,6 +664,10 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 			mem_set_bool(pMem, pMem->u.i);
 			return 0;
 		}
+		if ((pMem->flags & MEM_Real) != 0) {
+			mem_set_bool(pMem, pMem->u.r);
+			return 0;
+		}
 		if ((pMem->flags & MEM_Str) != 0) {
 			bool value;
 			if (str_cast_to_boolean(pMem->z, &value) != 0)
diff --git a/test/sql/types.result b/test/sql/types.result
index bc4518c01..bde7a2b92 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -786,7 +786,27 @@ box.execute("SELECT CAST(1 AS BOOLEAN);")
 ...
 box.execute("SELECT CAST(1.123 AS BOOLEAN);")
 ---
-- error: 'Type mismatch: can not convert 1.123 to boolean'
+- metadata:
+  - name: CAST(1.123 AS BOOLEAN)
+    type: boolean
+  rows:
+  - [true]
+...
+box.execute("SELECT CAST(0.0 AS BOOLEAN);")
+---
+- metadata:
+  - name: CAST(0.0 AS BOOLEAN)
+    type: boolean
+  rows:
+  - [false]
+...
+box.execute("SELECT CAST(0.00000001 AS BOOLEAN);")
+---
+- metadata:
+  - name: CAST(0.00000001 AS BOOLEAN)
+    type: boolean
+  rows:
+  - [true]
 ...
 box.execute("SELECT CAST('abc' AS BOOLEAN);")
 ---
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index c51660cb9..8e1745e7c 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -186,6 +186,8 @@ box.execute("SELECT CAST(true AS FLOAT);")
 box.execute("SELECT CAST(true AS SCALAR);")
 box.execute("SELECT CAST(1 AS BOOLEAN);")
 box.execute("SELECT CAST(1.123 AS BOOLEAN);")
+box.execute("SELECT CAST(0.0 AS BOOLEAN);")
+box.execute("SELECT CAST(0.00000001 AS BOOLEAN);")
 box.execute("SELECT CAST('abc' AS BOOLEAN);")
 box.execute("SELECT CAST('  TrUe' AS BOOLEAN);")
 box.execute("SELECT CAST('  falsE    ' AS BOOLEAN);")
-- 
2.15.1

[tarantool-patches] [PATCH 3/3] sql: allow CAST operation from quoted float to int

[Nikita Pettik]

As previous commit says, we've decided to allow all meaningful explicit
casts. One of these - conversion from string consisting from quoted
float literal to integer. Before this patch, mentioned operation was not
allowed:

SELECT CAST('1.123' AS INTEGER);
---
- error: 'Type mismatch: can not convert 1.123 to integer'
...

But anyway it can be done in two steps:

SELECT CAST(CAST('1.123' AS REAL) AS INTEGER);

So now this cast can be done in one CAST operation.

Closes #4229
---
 src/box/sql/vdbemem.c   |  8 +++++---
 test/sql/types.result   | 32 ++++++++++++++++++++++++++++++++
 test/sql/types.test.lua |  9 +++++++++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index da17bf601..08b649926 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -546,9 +546,11 @@ sqlVdbeMemIntegerify(Mem * pMem, bool is_forced)
 	}
 
 	double d;
-	if (sqlVdbeRealValue(pMem, &d) || (int64_t) d != d) {
-		return SQL_ERROR;
-	}
+	if (sqlVdbeRealValue(pMem, &d) != 0)
+		return -1;
+	if (d >= INT64_MAX || d < INT64_MIN)
+		return -1;
+
 	pMem->u.i = (int64_t) d;
 	MemSetTypeFlag(pMem, MEM_Int);
 	return 0;
diff --git a/test/sql/types.result b/test/sql/types.result
index bde7a2b92..200cf8201 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -247,6 +247,38 @@ box.execute("SELECT NULL LIKE s FROM t1;")
 box.space.T1:drop()
 ---
 ...
+-- gh-4229: allow explicit cast from string to integer for string
+-- values containing quoted floating point literals.
+--
+box.execute("SELECT CAST('1.123' AS INTEGER);")
+---
+- metadata:
+  - name: CAST('1.123' AS INTEGER)
+    type: integer
+  rows:
+  - [1]
+...
+box.execute("CREATE TABLE t1 (f TEXT PRIMARY KEY);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t1 VALUES('0.0'), ('1.5'), ('3.9312453');")
+---
+- row_count: 3
+...
+box.execute("SELECT CAST(f AS INTEGER) FROM t1;")
+---
+- metadata:
+  - name: CAST(f AS INTEGER)
+    type: integer
+  rows:
+  - [0]
+  - [1]
+  - [3]
+...
+box.space.T1:drop()
+---
+...
 -- Test basic capabilities of boolean type.
 --
 box.execute("SELECT true;")
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index 8e1745e7c..39de18b82 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -71,6 +71,15 @@ box.execute("SELECT * FROM t1 WHERE 'int' LIKE 4;")
 box.execute("SELECT NULL LIKE s FROM t1;")
 box.space.T1:drop()
 
+-- gh-4229: allow explicit cast from string to integer for string
+-- values containing quoted floating point literals.
+--
+box.execute("SELECT CAST('1.123' AS INTEGER);")
+box.execute("CREATE TABLE t1 (f TEXT PRIMARY KEY);")
+box.execute("INSERT INTO t1 VALUES('0.0'), ('1.5'), ('3.9312453');")
+box.execute("SELECT CAST(f AS INTEGER) FROM t1;")
+box.space.T1:drop()
+
 -- Test basic capabilities of boolean type.
 --
 box.execute("SELECT true;")
-- 
2.15.1

[tarantool-patches] Re: [PATCH 0/3] Fix CAST operation

[Vladislav Shpilevoy]

LGTM.

On 21/05/2019 13:34, Nikita Pettik wrote:
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-4229-adjust-cast
> Issue: https://github.com/tarantool/tarantool/issues/4229
> 
> This patch-set allows explicit conversion using CAST operator from REAL
> to BOOLEAN and from string value containing quoted floating point
> literal to INTEGER. Detailed explanation is provided in commit messages
> and in issue description.
> 
> Nikita Pettik (3):
>   sql: remove redundant conversion from OP_AddImm
>   sql: allow CAST operation from REAL to BOOLEAN
>   sql: allow CAST operation from quoted float to int
> 
>  src/box/sql/vdbe.c      |  2 +-
>  src/box/sql/vdbemem.c   | 12 ++++++++---
>  test/sql/types.result   | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  test/sql/types.test.lua | 11 ++++++++++
>  4 files changed, 74 insertions(+), 5 deletions(-)
> 

[tarantool-patches] Re: [PATCH 0/3] Fix CAST operation

[Kirill Yukhin]

Hello,

On 21 May 13:34, Nikita Pettik wrote:
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-4229-adjust-cast
> Issue: https://github.com/tarantool/tarantool/issues/4229
> 
> This patch-set allows explicit conversion using CAST operator from REAL
> to BOOLEAN and from string value containing quoted floating point
> literal to INTEGER. Detailed explanation is provided in commit messages
> and in issue description.
> 
> Nikita Pettik (3):
>   sql: remove redundant conversion from OP_AddImm
>   sql: allow CAST operation from REAL to BOOLEAN
>   sql: allow CAST operation from quoted float to int

I've checked your patch into master.

--
Regards, Kirill Yukhin