Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes
@ 2019-02-19  8:28 Nikita Pettik
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 1/3] sql: fix value of mask to map VDBE memory type Nikita Pettik
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Nikita Pettik @ 2019-02-19  8:28 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

This patch makes concatenation operator follow ANSI rules.
For now, only operands of string-like types (TEXT or BLOB) are
accepted. What is more, fixed resulting type of concatenation:
if both operands are of BLOB type, then the resulting value is of
BLOB value too.

Branch: https://github.com/tarantool/tarantool/tree/np/gh-3544-fix-randomblob-type
Issue: https://github.com/tarantool/tarantool/issues/3544

Nikita Pettik (3):
  sql: fix value of mask to map VDBE memory type
  sql: allow only string-like arguments for concatenation
  sql: fix resulting type for concatenation operator

 src/box/errcode.h               |  1 +
 src/box/sql/vdbe.c              | 59 ++++++++++++++++++++++++++++++++++++++---
 src/box/sql/vdbeInt.h           |  9 +++++++
 src/box/sql/vdbeapi.c           |  2 +-
 test/box/misc.result            |  1 +
 test/sql-tap/autoinc.test.lua   | 12 ++++-----
 test/sql-tap/e_select1.test.lua |  4 +--
 test/sql-tap/func.test.lua      |  4 +--
 test/sql-tap/sort.test.lua      |  6 ++---
 test/sql-tap/tkt2192.test.lua   |  4 +--
 test/sql-tap/trigger5.test.lua  |  4 +--
 test/sql/types.result           | 42 +++++++++++++++++++++++++++++
 test/sql/types.test.lua         | 16 +++++++++++
 13 files changed, 143 insertions(+), 21 deletions(-)

-- 
2.15.1

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

* [tarantool-patches] [PATCH 1/3] sql: fix value of mask to map VDBE memory type
  2019-02-19  8:28 [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes Nikita Pettik
@ 2019-02-19  8:28 ` Nikita Pettik
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 2/3] sql: allow only string-like arguments for concatenation Nikita Pettik
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Nikita Pettik @ 2019-02-19  8:28 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Accidentally, mask which is used to map type of VDBE memory cell into
outer API types was replaced with MEM_TypeMask. But value of the latter
is larger then possible values of VDBE memory cells types. Hence, if it
is applied to memory cell, not only pure types is taken into
consideration, but some additional flags (such as MEM_Zero) as well.
Overall, it results in wrong type calculation for zeroed blobs, for
instance. Lets return back previous mask.

Follow-up #3698
Needed for #3544
---
 src/box/sql/vdbeInt.h | 9 +++++++++
 src/box/sql/vdbeapi.c | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index d003d6bb3..933867469 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -253,6 +253,15 @@ struct Mem {
 #define MEM_Zero 0x0000
 #endif
 
+/**
+ * In contrast to Mem_TypeMask, this one allows to get
+ * pure type of memory cell, i.e. without _Dyn/_Zero and other
+ * auxiliary flags.
+ */
+enum {
+	MEM_PURE_TYPE_MASK = 0x1f
+};
+
 
 /* Return TRUE if Mem X contains dynamically allocated content - anything
  * that needs to be deallocated to avoid a leak.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 7a11bcb0a..250afc20d 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -280,7 +280,7 @@ sql_value_type(sql_value * pVal)
 		SQL_INTEGER,	/* 0x1e */
 		SQL_NULL,	/* 0x1f */
 	};
-	return aType[pVal->flags & MEM_TypeMask];
+	return aType[pVal->flags & MEM_PURE_TYPE_MASK];
 }
 
 /* Make a copy of an sql_value object
-- 
2.15.1

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

* [tarantool-patches] [PATCH 2/3] sql: allow only string-like arguments for concatenation
  2019-02-19  8:28 [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes Nikita Pettik
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 1/3] sql: fix value of mask to map VDBE memory type Nikita Pettik
@ 2019-02-19  8:28 ` Nikita Pettik
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 3/3] sql: fix resulting type for concatenation operator Nikita Pettik
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Nikita Pettik @ 2019-02-19  8:28 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Original SQLite operator of concatenation accepts all types of
arguments. If type of parameter is not TEXT, it is implicitly converted
to TEXT (except for NULLs). That contradicts ANSI (it is regulated by
[1]), so lets allow only TEXT and BLOB as argument type for
concatenation.  Moreover, they both must be of the same type at the same
time (i.e. both TEXT or BLOB).

[1] SQL ANSI 2013, 9.5 Result of data type combination

Part of #3544
---
 src/box/errcode.h               |  1 +
 src/box/sql/vdbe.c              | 54 +++++++++++++++++++++++++++++++++++++++--
 test/box/misc.result            |  1 +
 test/sql-tap/autoinc.test.lua   | 12 ++++-----
 test/sql-tap/e_select1.test.lua |  4 +--
 test/sql-tap/func.test.lua      |  4 +--
 test/sql-tap/sort.test.lua      |  6 ++---
 test/sql-tap/tkt2192.test.lua   |  4 +--
 test/sql-tap/trigger5.test.lua  |  4 +--
 test/sql/types.result           | 32 ++++++++++++++++++++++++
 test/sql/types.test.lua         | 12 +++++++++
 11 files changed, 115 insertions(+), 19 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index f7dbb948e..8ef60e861 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -227,6 +227,7 @@ struct errcode_record {
 	/*172 */_(ER_ROWID_OVERFLOW,            "Rowid is overflowed: too many entries in ephemeral space") \
 	/*173 */_(ER_DROP_COLLATION,		"Can't drop collation %s : %s") \
 	/*174 */_(ER_ILLEGAL_COLLATION_MIX,	"Illegal mix of collations") \
+	/*175 */_(ER_INCONSISTENT_TYPES,	"Inconsistent types: expected %s got %s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c370b81ec..37955858d 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -619,6 +619,32 @@ vdbe_add_new_autoinc_id(struct Vdbe *vdbe, int64_t id)
 	return 0;
 }
 
+/**
+ * Simple type to str convertor. It is used to simplify
+ * error reporting.
+ */
+static char *
+mem_type_to_str(const struct Mem *p)
+{
+	assert(p != NULL);
+	switch (p->flags & MEM_PURE_TYPE_MASK) {
+	case MEM_Null:
+		return "NULL";
+	case MEM_Str:
+		return "TEXT";
+	case MEM_Int:
+		return "INTEGER";
+	case MEM_Real:
+		return "REAL";
+	case MEM_Blob:
+		return "BLOB";
+	case MEM_Bool:
+		return "BOOLEAN";
+	default:
+		unreachable();
+	}
+}
+
 /*
  * Execute as much of a VDBE program as we can.
  * This is the core of sql_step().
@@ -1515,6 +1541,9 @@ case OP_ResultRow: {
  * It is illegal for P1 and P3 to be the same register. Sometimes,
  * if P3 is the same register as P2, the implementation is able
  * to avoid a memcpy().
+ *
+ * Concatenation operator accepts only arguments of string-like
+ * types (i.e. TEXT and BLOB).
  */
 case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 	i64 nByte;
@@ -1527,9 +1556,30 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 		sqlVdbeMemSetNull(pOut);
 		break;
 	}
+	/*
+	 * Concatenation operation can be applied only to
+	 * strings and blobs.
+	 */
+	uint32_t str_type_p1 = pIn1->flags & (MEM_Blob | MEM_Str);
+	uint32_t str_type_p2 = pIn2->flags & (MEM_Blob | MEM_Str);
+	if (str_type_p1 == 0 || str_type_p2 == 0) {
+		char *inconsistent_type = str_type_p1 == 0 ?
+					  mem_type_to_str(pIn1) :
+					  mem_type_to_str(pIn2);
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB",
+			 inconsistent_type);
+		rc = SQL_TARANTOOL_ERROR;
+		goto abort_due_to_error;
+	}
+
+	/* Moreover, both operands must be of the same type. */
+	if (str_type_p1 != str_type_p2) {
+		diag_set(ClientError, ER_INCONSISTENT_TYPES,
+			 mem_type_to_str(pIn2), mem_type_to_str(pIn1));
+		rc = SQL_TARANTOOL_ERROR;
+		goto abort_due_to_error;
+	}
 	if (ExpandBlob(pIn1) || ExpandBlob(pIn2)) goto no_mem;
-	Stringify(pIn1);
-	Stringify(pIn2);
 	nByte = pIn1->n + pIn2->n;
 	if (nByte>db->aLimit[SQL_LIMIT_LENGTH]) {
 		goto too_big;
diff --git a/test/box/misc.result b/test/box/misc.result
index 97189ecbb..345593655 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -502,6 +502,7 @@ t;
   172: box.error.ROWID_OVERFLOW
   173: box.error.DROP_COLLATION
   174: box.error.ILLEGAL_COLLATION_MIX
+  175: box.error.INCONSISTENT_TYPES
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/autoinc.test.lua b/test/sql-tap/autoinc.test.lua
index 1110a2472..5ec9fd632 100755
--- a/test/sql-tap/autoinc.test.lua
+++ b/test/sql-tap/autoinc.test.lua
@@ -686,11 +686,11 @@ test:do_test(
             DROP TRIGGER t3928r2;
             CREATE TRIGGER t3928r3 BEFORE UPDATE ON t3928
               WHEN new.b=='456' BEGIN
-                 INSERT INTO t3928(b) VALUES('before-int-' || new.b);
+                 INSERT INTO t3928(b) VALUES('before-int-' || CAST(new.b AS TEXT));
             END;
             CREATE TRIGGER t3928r4 AFTER UPDATE ON t3928
               WHEN new.b=='456' BEGIN
-                 INSERT INTO t3928(b) VALUES('after-int-' || new.b);
+                 INSERT INTO t3928(b) VALUES('after-int-' || CAST(new.b AS TEXT));
             END;
             DELETE FROM t3928 WHERE a!=1;
             UPDATE t3928 SET b=456 WHERE a=1;
@@ -725,12 +725,12 @@ test:do_test(
             DELETE FROM t3928;
             CREATE TABLE t3928c(y INTEGER PRIMARY KEY AUTOINCREMENT, z TEXT);
             CREATE TRIGGER t3928br1 BEFORE DELETE ON t3928b BEGIN
-              INSERT INTO t3928(b) VALUES('before-del-'||old.x);
-              INSERT INTO t3928c(z) VALUES('before-del-'||old.x);
+              INSERT INTO t3928(b) VALUES('before-del-'|| CAST(old.x AS TEXT));
+              INSERT INTO t3928c(z) VALUES('before-del-'|| CAST(old.x AS TEXT));
             END;
             CREATE TRIGGER t3928br2 AFTER DELETE ON t3928b BEGIN
-              INSERT INTO t3928(b) VALUES('after-del-'||old.x);
-              INSERT INTO t3928c(z) VALUES('after-del-'||old.x);
+              INSERT INTO t3928(b) VALUES('after-del-'|| CAST(old.x AS TEXT));
+              INSERT INTO t3928c(z) VALUES('after-del-'|| CAST(old.x AS TEXT));
             END;
             DELETE FROM t3928b;
             SELECT * FROM t3928 ORDER BY a;
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index 271fa1547..cdd088209 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -875,7 +875,7 @@ test:do_select_tests(
                 0, -22.18, -2.2, -23.18, "suiters", 1, 68, "", 67, "quartets", 0, -31.3,
                 -1.04, -32.3, "aspen", 0, 1, 63, 0, "-26"}},
 
-        {"3", "SELECT 32*32, d||e FROM z2", {1024, "", 1024, "36.06.0"}},
+        {"3", "SELECT 32*32, CAST(d AS TEXT) || CAST(e AS TEXT) FROM z2", {1024, "", 1024, "36.06.0"}},
     })
 
 -- Test cases e_select-4.5.* and e_select-4.6.* together show that:
@@ -1136,7 +1136,7 @@ test:do_select_tests(
         {"1.1", "SELECT up FROM c1 GROUP BY up HAVING count(*)>3", {"x"}},
         {"1.2", "SELECT up FROM c1 GROUP BY up HAVING sum(down)>16", {"y"}},
         {"1.3", "SELECT up FROM c1 GROUP BY up HAVING sum(down)<16", {"x"}},
-        {"1.4", "SELECT up||down FROM c1 GROUP BY (down<5) HAVING max(down)<10", {"x4"}},
+        {"1.4", "SELECT up|| CAST(down AS TEXT) FROM c1 GROUP BY (down<5) HAVING max(down)<10", {"x4"}},
 
         {"2.1", "SELECT up FROM c1 GROUP BY up HAVING down>10", {"y"}},
         {"2.2", "SELECT up FROM c1 GROUP BY up HAVING up='y'", {"y"}},
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index e6b7956aa..495be7d29 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -475,7 +475,7 @@ test:do_catchsql_test(
 test:do_catchsql_test(
     "func-4.10",
     [[
-        SELECT 'x' || round(c,a) || 'y' FROM t1 ORDER BY a
+        SELECT 'x' || CAST(round(c,a) AS TEXT) || 'y' FROM t1 ORDER BY a
     ]], {
         -- <func-4.10>
         0, {"x3.0y", "x-12345.68y", "x-5.0y"}
@@ -863,7 +863,7 @@ test:do_test(
 test:do_execsql_test(
     "func-8.2",
     [[
-        SELECT max('z+'||a||'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP') FROM t2;
+        SELECT max('z+'|| CAST(a AS TEXT) ||'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP') FROM t2;
     ]], {
         -- <func-8.2>
         "z+67890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOP"
diff --git a/test/sql-tap/sort.test.lua b/test/sql-tap/sort.test.lua
index df666070f..a84c549cc 100755
--- a/test/sql-tap/sort.test.lua
+++ b/test/sql-tap/sort.test.lua
@@ -238,7 +238,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "sort-2.1.1",
     [[
-        UPDATE t1 SET v='x' || -flt;
+        UPDATE t1 SET v='x' || CAST(-flt AS TEXT);
         UPDATE t1 SET v='x-2b' where v=='x-0.123';
         SELECT v FROM t1 ORDER BY v;
     ]], {
@@ -338,7 +338,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "sort-4.2",
     [[
-        SELECT n||'' FROM t1 ORDER BY 1;
+        SELECT CAST(n AS TEXT) || '' FROM t1 ORDER BY 1;
     ]], {
         -- <sort-4.2>
         "1", "10", "11", "12", "2", "3", "4", "5", "6", "7", "8", "9"
@@ -358,7 +358,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "sort-4.4",
     [[
-        SELECT n||'' FROM t1 ORDER BY 1 DESC;
+        SELECT CAST(n AS TEXT) || '' FROM t1 ORDER BY 1 DESC;
     ]], {
         -- <sort-4.4>
         "9", "8", "7", "6", "5", "4", "3", "2", "12", "11", "10", "1"
diff --git a/test/sql-tap/tkt2192.test.lua b/test/sql-tap/tkt2192.test.lua
index eb53863d4..b118d6310 100755
--- a/test/sql-tap/tkt2192.test.lua
+++ b/test/sql-tap/tkt2192.test.lua
@@ -146,7 +146,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "tkt2192-2.3",
     [[
-        SELECT x.a || '/' || x.b || '/' || y.b
+        SELECT CAST(x.a AS TEXT) || '/' || CAST(x.b AS TEXT) || '/' || CAST(y.b AS TEXT)
           FROM v1 AS x JOIN v1 AS y ON x.a=y.a AND x.b<y.b
          ORDER BY x.a, x.b, y.b
     ]], {
@@ -159,7 +159,7 @@ test:do_execsql_test(
     "tkt2192-2.4",
     [[
         CREATE VIEW v2 AS
-        SELECT x.a || '/' || x.b || '/' || y.b AS z
+        SELECT CAST(x.a AS TEXT) || '/' || CAST(x.b AS TEXT) || '/' || CAST(y.b AS TEXT) AS z
           FROM v1 AS x JOIN v1 AS y ON x.a=y.a AND x.b<y.b
          ORDER BY x.a, x.b, y.b;
         SELECT * FROM v2;
diff --git a/test/sql-tap/trigger5.test.lua b/test/sql-tap/trigger5.test.lua
index 8d6fd8339..f71649925 100755
--- a/test/sql-tap/trigger5.test.lua
+++ b/test/sql-tap/trigger5.test.lua
@@ -30,8 +30,8 @@ test:do_execsql_test(
         BEGIN
           INSERT INTO Undo VALUES
              ((SELECT coalesce(max(id),0) + 1 FROM Undo),
-              (SELECT 'INSERT INTO Item (a,b,c) VALUES (' || coalesce(old.a,'NULL')
-                  || ',' || quote(old.b) || ',' || old.c || ');'));
+              (SELECT 'INSERT INTO Item (a,b,c) VALUES (' || CAST(coalesce(old.a,'NULL') AS TEXT)
+                  || ',' || quote(old.b) || ',' || CAST(old.c AS TEXT) || ');'));
         END;
         DELETE FROM Item WHERE a = 1;
         SELECT * FROM Undo;
diff --git a/test/sql/types.result b/test/sql/types.result
index 9043cbfd5..0cbbd1642 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -131,3 +131,35 @@ box.sql.execute("SELECT * FROM test")
 sp:drop()
 ---
 ...
+-- gh-3544: concatenation operator accepts only TEXT and BLOB.
+--
+box.sql.execute("SELECT 'abc' || 1;")
+---
+- error: 'Inconsistent types: expected TEXT or BLOB got INTEGER'
+...
+box.sql.execute("SELECT 'abc' || 1.123;")
+---
+- error: 'Inconsistent types: expected TEXT or BLOB got REAL'
+...
+box.sql.execute("SELECT 1 || 'abc';")
+---
+- error: 'Inconsistent types: expected TEXT or BLOB got INTEGER'
+...
+box.sql.execute("SELECT 1.123 || 'abc';")
+---
+- error: 'Inconsistent types: expected TEXT or BLOB got REAL'
+...
+box.sql.execute("SELECt 'a' || 'b' || 1;")
+---
+- error: 'Inconsistent types: expected TEXT or BLOB got INTEGER'
+...
+-- What is more, they must be of the same type.
+--
+box.sql.execute("SELECT 'abc' || CAST('x' AS BLOB);")
+---
+- error: 'Inconsistent types: expected TEXT got BLOB'
+...
+box.sql.execute("SELECT CAST('abc' AS BLOB) || 'x';")
+---
+- error: 'Inconsistent types: expected BLOB got TEXT'
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index 8eda36159..e0331bb8f 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -40,3 +40,15 @@ sp:insert({1, true})
 sp:insert({2, false})
 box.sql.execute("SELECT * FROM test")
 sp:drop()
+
+-- gh-3544: concatenation operator accepts only TEXT and BLOB.
+--
+box.sql.execute("SELECT 'abc' || 1;")
+box.sql.execute("SELECT 'abc' || 1.123;")
+box.sql.execute("SELECT 1 || 'abc';")
+box.sql.execute("SELECT 1.123 || 'abc';")
+box.sql.execute("SELECt 'a' || 'b' || 1;")
+-- What is more, they must be of the same type.
+--
+box.sql.execute("SELECT 'abc' || CAST('x' AS BLOB);")
+box.sql.execute("SELECT CAST('abc' AS BLOB) || 'x';")
-- 
2.15.1

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

* [tarantool-patches] [PATCH 3/3] sql: fix resulting type for concatenation operator
  2019-02-19  8:28 [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes Nikita Pettik
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 1/3] sql: fix value of mask to map VDBE memory type Nikita Pettik
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 2/3] sql: allow only string-like arguments for concatenation Nikita Pettik
@ 2019-02-19  8:28 ` Nikita Pettik
  2019-02-22 15:54 ` [tarantool-patches] Re: [PATCH 0/3] Concatenation operator type fixes Vladislav Shpilevoy
  2019-02-25 11:42 ` Kirill Yukhin
  4 siblings, 0 replies; 6+ messages in thread
From: Nikita Pettik @ 2019-02-19  8:28 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Before this patch, resulting type of concatenation operator always was
TEXT (i.e. type of memory cell containing result - MEM_Str). Lets fix it
and return type depending on type of concatenation arguments: if both
arguments are TEXT, then resulting type is TEXT; BLOB otherwise. Note
that other options of combining types of arguments are illegal.

Closes #3544
---
 src/box/sql/vdbe.c      |  5 ++++-
 test/sql/types.result   | 10 ++++++++++
 test/sql/types.test.lua |  4 ++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 37955858d..b647491bb 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1587,7 +1587,10 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 	if (sqlVdbeMemGrow(pOut, (int)nByte+2, pOut==pIn2)) {
 		goto no_mem;
 	}
-	MemSetTypeFlag(pOut, MEM_Str);
+	if (pIn1->flags & MEM_Str)
+		MemSetTypeFlag(pOut, MEM_Str);
+	else
+		MemSetTypeFlag(pOut, MEM_Blob);
 	if (pOut!=pIn2) {
 		memcpy(pOut->z, pIn2->z, pIn2->n);
 	}
diff --git a/test/sql/types.result b/test/sql/types.result
index 0cbbd1642..a0159d661 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -163,3 +163,13 @@ box.sql.execute("SELECT CAST('abc' AS BLOB) || 'x';")
 ---
 - error: 'Inconsistent types: expected BLOB got TEXT'
 ...
+-- Result of BLOBs concatenation must be BLOB.
+--
+box.sql.execute("SELECT TYPEOF(CAST('abc' AS BLOB) || CAST('cda' AS BLOB))")
+---
+- - ['blob']
+...
+box.sql.execute("VALUES (TYPEOF(randomblob(5) || zeroblob(5)));")
+---
+- - ['blob']
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index e0331bb8f..b944b081b 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -52,3 +52,7 @@ box.sql.execute("SELECt 'a' || 'b' || 1;")
 --
 box.sql.execute("SELECT 'abc' || CAST('x' AS BLOB);")
 box.sql.execute("SELECT CAST('abc' AS BLOB) || 'x';")
+-- Result of BLOBs concatenation must be BLOB.
+--
+box.sql.execute("SELECT TYPEOF(CAST('abc' AS BLOB) || CAST('cda' AS BLOB))")
+box.sql.execute("VALUES (TYPEOF(randomblob(5) || zeroblob(5)));")
-- 
2.15.1

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

* [tarantool-patches] Re: [PATCH 0/3] Concatenation operator type fixes
  2019-02-19  8:28 [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes Nikita Pettik
                   ` (2 preceding siblings ...)
  2019-02-19  8:28 ` [tarantool-patches] [PATCH 3/3] sql: fix resulting type for concatenation operator Nikita Pettik
@ 2019-02-22 15:54 ` Vladislav Shpilevoy
  2019-02-25 11:42 ` Kirill Yukhin
  4 siblings, 0 replies; 6+ messages in thread
From: Vladislav Shpilevoy @ 2019-02-22 15:54 UTC (permalink / raw)
  To: tarantool-patches, Nikita Pettik, Kirill Yukhin

LGTM.

On 19/02/2019 11:28, Nikita Pettik wrote:
> This patch makes concatenation operator follow ANSI rules.
> For now, only operands of string-like types (TEXT or BLOB) are
> accepted. What is more, fixed resulting type of concatenation:
> if both operands are of BLOB type, then the resulting value is of
> BLOB value too.
> 
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3544-fix-randomblob-type
> Issue: https://github.com/tarantool/tarantool/issues/3544
> 
> Nikita Pettik (3):
>    sql: fix value of mask to map VDBE memory type
>    sql: allow only string-like arguments for concatenation
>    sql: fix resulting type for concatenation operator
> 
>   src/box/errcode.h               |  1 +
>   src/box/sql/vdbe.c              | 59 ++++++++++++++++++++++++++++++++++++++---
>   src/box/sql/vdbeInt.h           |  9 +++++++
>   src/box/sql/vdbeapi.c           |  2 +-
>   test/box/misc.result            |  1 +
>   test/sql-tap/autoinc.test.lua   | 12 ++++-----
>   test/sql-tap/e_select1.test.lua |  4 +--
>   test/sql-tap/func.test.lua      |  4 +--
>   test/sql-tap/sort.test.lua      |  6 ++---
>   test/sql-tap/tkt2192.test.lua   |  4 +--
>   test/sql-tap/trigger5.test.lua  |  4 +--
>   test/sql/types.result           | 42 +++++++++++++++++++++++++++++
>   test/sql/types.test.lua         | 16 +++++++++++
>   13 files changed, 143 insertions(+), 21 deletions(-)
> 
> -- 
> 2.15.1
> 
> 

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

* [tarantool-patches] Re: [PATCH 0/3] Concatenation operator type fixes
  2019-02-19  8:28 [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes Nikita Pettik
                   ` (3 preceding siblings ...)
  2019-02-22 15:54 ` [tarantool-patches] Re: [PATCH 0/3] Concatenation operator type fixes Vladislav Shpilevoy
@ 2019-02-25 11:42 ` Kirill Yukhin
  4 siblings, 0 replies; 6+ messages in thread
From: Kirill Yukhin @ 2019-02-25 11:42 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Hello,

On 19 Feb 11:28, Nikita Pettik wrote:
> This patch makes concatenation operator follow ANSI rules.
> For now, only operands of string-like types (TEXT or BLOB) are
> accepted. What is more, fixed resulting type of concatenation:
> if both operands are of BLOB type, then the resulting value is of
> BLOB value too.
> 
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3544-fix-randomblob-type
> Issue: https://github.com/tarantool/tarantool/issues/3544

I've checked your patches into 2.1 branch.

--
Regards, Kirill Yukhin

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

end of thread, other threads:[~2019-02-25 11:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-19  8:28 [tarantool-patches] [PATCH 0/3] Concatenation operator type fixes Nikita Pettik
2019-02-19  8:28 ` [tarantool-patches] [PATCH 1/3] sql: fix value of mask to map VDBE memory type Nikita Pettik
2019-02-19  8:28 ` [tarantool-patches] [PATCH 2/3] sql: allow only string-like arguments for concatenation Nikita Pettik
2019-02-19  8:28 ` [tarantool-patches] [PATCH 3/3] sql: fix resulting type for concatenation operator Nikita Pettik
2019-02-22 15:54 ` [tarantool-patches] Re: [PATCH 0/3] Concatenation operator type fixes Vladislav Shpilevoy
2019-02-25 11:42 ` Kirill Yukhin

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