[Tarantool-patches] [PATCH v2 0/4] Follow ups for uuid introduction

[Tarantool-patches] [PATCH v2 0/4] Follow ups for uuid introduction

[Mergen Imeev via Tarantool-patches]

After addition of UUID to SQL a few new problems appeared. This patch fixes such
problems.

https://github.com/tarantool/tarantool/issues/6164
https://github.com/tarantool/tarantool/tree/imeevma/gh-6164-uuid-follow-ups

Changes in v2:
 - Fixed problem with wrong comparison in MIN()/MAX() functions.
 - Fixed problem with wrong comparison in ORDER BY.

Mergen Imeev (4):
  sql: introduce uuid to quote()
  sql: allow to bind uuid values
  sql: introduce mem_cmp_scalar()
  sql: introduce mem_cmp_msgpack()

 src/box/bind.c                                |   3 +
 src/box/bind.h                                |   5 +
 src/box/lua/execute.c                         |   5 +
 src/box/sql.c                                 |   9 +-
 src/box/sql/func.c                            |  54 ++-
 src/box/sql/mem.c                             | 427 ++++++------------
 src/box/sql/mem.h                             |  34 +-
 src/box/sql/sqlInt.h                          |   5 +
 src/box/sql/vdbe.c                            |   8 +-
 src/box/sql/vdbeapi.c                         |  10 +
 src/box/sql/where.c                           |  22 +-
 test/sql-tap/engine.cfg                       |   3 +
 test/sql-tap/gh-6164-uuid-follow-ups.test.lua |  93 ++++
 13 files changed, 354 insertions(+), 324 deletions(-)
 create mode 100755 test/sql-tap/gh-6164-uuid-follow-ups.test.lua

-- 
2.25.1

[Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote()

[Mergen Imeev via Tarantool-patches]

Prior to this patch, built-in SQL function quote() could not work with
uuid. It now returns a string representation of the received uuid.

Part of #6164
---
 src/box/sql/func.c                            | 24 ++++++++++++-------
 test/sql-tap/engine.cfg                       |  3 +++
 test/sql-tap/gh-6164-uuid-follow-ups.test.lua | 14 +++++++++++
 3 files changed, 32 insertions(+), 9 deletions(-)
 create mode 100755 test/sql-tap/gh-6164-uuid-follow-ups.test.lua

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 84768f17c..d2edda6d3 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1095,8 +1095,8 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 {
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
-	switch (sql_value_type(argv[0])) {
-	case MP_DOUBLE:{
+	switch (argv[0]->type) {
+	case MEM_TYPE_DOUBLE:{
 			double r1, r2;
 			char zBuf[50];
 			r1 = mem_get_double_unsafe(argv[0]);
@@ -1110,14 +1110,20 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 					    SQL_TRANSIENT);
 			break;
 		}
-	case MP_UINT:
-	case MP_INT:{
+	case MEM_TYPE_UUID: {
+		char buf[UUID_STR_LEN + 1];
+		tt_uuid_to_string(&argv[0]->u.uuid, &buf[0]);
+		sql_result_text(context, buf, UUID_STR_LEN, SQL_TRANSIENT);
+		break;
+	}
+	case MEM_TYPE_UINT:
+	case MEM_TYPE_INT: {
 			sql_result_value(context, argv[0]);
 			break;
 		}
-	case MP_BIN:
-	case MP_ARRAY:
-	case MP_MAP: {
+	case MEM_TYPE_BIN:
+	case MEM_TYPE_ARRAY:
+	case MEM_TYPE_MAP: {
 			char *zText = 0;
 			char const *zBlob = mem_as_bin(argv[0]);
 			int nBlob = mem_len_unsafe(argv[0]);
@@ -1143,7 +1149,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 			}
 			break;
 		}
-	case MP_STR:{
+	case MEM_TYPE_STR: {
 			int i, j;
 			u64 n;
 			const unsigned char *zArg = mem_as_ustr(argv[0]);
@@ -1171,7 +1177,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 			}
 			break;
 		}
-	case MP_BOOL: {
+	case MEM_TYPE_BOOL: {
 		sql_result_text(context,
 				SQL_TOKEN_BOOLEAN(mem_get_bool_unsafe(argv[0])),
 				-1, SQL_TRANSIENT);
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index 693a477b7..94b0bb1f6 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -20,6 +20,9 @@
     "gh-3332-tuple-format-leak.test.lua": {
         "memtx": {"engine": "memtx"}
     },
+    "gh-6164-uuid-follow-ups.test.lua": {
+        "memtx": {"engine": "memtx"}
+    },
     "gh-4077-iproto-execute-no-bind.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
new file mode 100755
index 000000000..a8f662f77
--- /dev/null
+++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
@@ -0,0 +1,14 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(1)
+
+-- Make sure that function quote() can work with uuid.
+test:do_execsql_test(
+    "gh-6164-1",
+    [[
+        SELECT quote(cast('11111111-1111-1111-1111-111111111111' as uuid));
+    ]], {
+        '11111111-1111-1111-1111-111111111111'
+    })
+
+test:finish_test()
-- 
2.25.1

[Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values

[Mergen Imeev via Tarantool-patches]

After this patch, uuid values can be bound like any other supported by
SQL values.

Part of #6164
---
 src/box/bind.c                                |  3 +++
 src/box/bind.h                                |  5 ++++
 src/box/lua/execute.c                         |  5 ++++
 src/box/sql/sqlInt.h                          |  5 ++++
 src/box/sql/vdbeapi.c                         | 10 +++++++
 test/sql-tap/gh-6164-uuid-follow-ups.test.lua | 26 ++++++++++++++++++-
 6 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/src/box/bind.c b/src/box/bind.c
index d45a0f9a7..734f65186 100644
--- a/src/box/bind.c
+++ b/src/box/bind.c
@@ -191,6 +191,9 @@ sql_bind_column(struct sql_stmt *stmt, const struct sql_bind *p,
 	case MP_BIN:
 		return sql_bind_blob64(stmt, pos, (const void *) p->s, p->bytes,
 				       SQL_STATIC);
+	case MP_EXT:
+		assert(p->ext_type == MP_UUID);
+		return sql_bind_uuid(stmt, pos, &p->uuid);
 	default:
 		unreachable();
 	}
diff --git a/src/box/bind.h b/src/box/bind.h
index 568c558f3..143f010ce 100644
--- a/src/box/bind.h
+++ b/src/box/bind.h
@@ -40,6 +40,8 @@ extern "C" {
 #include <stdlib.h>
 
 #include "msgpuck.h"
+#include "uuid/tt_uuid.h"
+#include "mp_extension_types.h"
 
 struct sql_stmt;
 
@@ -59,6 +61,8 @@ struct sql_bind {
 	uint32_t bytes;
 	/** MessagePack type of the value. */
 	enum mp_type type;
+	/** Subtype of MP_EXT type. */
+	enum mp_extension_type ext_type;
 	/** Bind value. */
 	union {
 		bool b;
@@ -67,6 +71,7 @@ struct sql_bind {
 		uint64_t u64;
 		/** For string or blob. */
 		const char *s;
+		struct tt_uuid uuid;
 	};
 };
 
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index 1b59b2e4a..62ccaf3f1 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -371,6 +371,10 @@ lua_sql_bind_decode(struct lua_State *L, struct sql_bind *bind, int idx, int i)
 		bind->s = mp_decode_bin(&field.sval.data, &bind->bytes);
 		break;
 	case MP_EXT:
+		if (field.ext_type == MP_UUID) {
+			bind->uuid = *field.uuidval;
+			break;
+		}
 		diag_set(ClientError, ER_SQL_BIND_TYPE, "USERDATA",
 			 sql_bind_name(bind));
 		return -1;
@@ -386,6 +390,7 @@ lua_sql_bind_decode(struct lua_State *L, struct sql_bind *bind, int idx, int i)
 		unreachable();
 	}
 	bind->type = field.type;
+	bind->ext_type = field.ext_type;
 	lua_pop(L, lua_gettop(L) - idx);
 	return 0;
 }
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index ef8dcd693..115c52f96 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -326,6 +326,8 @@ struct sql_vfs {
 #define SQL_LIMIT_LIKE_PATTERN_LENGTH       8
 #define SQL_LIMIT_TRIGGER_DEPTH             9
 
+struct tt_uuid;
+
 enum sql_ret_code {
 	/** sql_step() has another row ready. */
 	SQL_ROW = 1,
@@ -634,6 +636,9 @@ int
 sql_bind_zeroblob64(sql_stmt *, int,
 			sql_uint64);
 
+int
+sql_bind_uuid(struct sql_stmt *stmt, int i, const struct tt_uuid *uuid);
+
 /**
  * Return the number of wildcards that should be bound to.
  */
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index aaae12e41..8031ee0dc 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -840,6 +840,16 @@ sql_bind_zeroblob64(sql_stmt * pStmt, int i, sql_uint64 n)
 	return sql_bind_zeroblob(pStmt, i, n);
 }
 
+int
+sql_bind_uuid(struct sql_stmt *stmt, int i, const struct tt_uuid *uuid)
+{
+	struct Vdbe *p = (struct Vdbe *)stmt;
+	if (vdbeUnbind(p, i) != 0 || sql_bind_type(p, i, "uuid") != 0)
+		return -1;
+	mem_set_uuid(&p->aVar[i - 1], uuid);
+	return 0;
+}
+
 int
 sql_bind_parameter_count(const struct sql_stmt *stmt)
 {
diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
index a8f662f77..4fc5052d8 100755
--- a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
+++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(1)
+test:plan(4)
 
 -- Make sure that function quote() can work with uuid.
 test:do_execsql_test(
@@ -11,4 +11,28 @@ test:do_execsql_test(
         '11111111-1111-1111-1111-111111111111'
     })
 
+-- Make sure that uuid value can be binded.
+local uuid1 = require('uuid').fromstr('11111111-1111-1111-1111-111111111111')
+local uuid2 = require('uuid').fromstr('11111111-2222-1111-1111-111111111111')
+local uuid3 = require('uuid').fromstr('11111111-1111-3333-1111-111111111111')
+test:do_test(
+    "gh-6164-2",
+    function()
+        return box.execute([[SELECT ?;]], {uuid1}).rows[1][1]
+    end,
+    uuid1)
+test:do_test(
+    "gh-6164-3",
+    function()
+        return box.execute([[SELECT $2;]], {123, uuid2}).rows[1][1]
+    end,
+    uuid2)
+
+test:do_test(
+    "gh-6164-4",
+    function()
+        return box.execute([[SELECT :two;]], {{[":two"] = uuid3}}).rows[1][1]
+    end,
+    uuid3)
+
 test:finish_test()
-- 
2.25.1

[Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar()

[Mergen Imeev via Tarantool-patches]

This patch introduces the mem_cmp_scalar() function that compares two
MEMs using SCALAR rules. MEMs must be scalars. Prior to this patch, there was a
function that used SCALAR rules to compare two MEMs, but its design became
overly complex as new types appeared.

Part of #6164
---
 src/box/sql/func.c                            |  30 +++-
 src/box/sql/mem.c                             | 144 +++++++++---------
 src/box/sql/mem.h                             |  10 +-
 src/box/sql/vdbe.c                            |   8 +-
 src/box/sql/where.c                           |  22 ++-
 test/sql-tap/gh-6164-uuid-follow-ups.test.lua |  45 +++++-
 6 files changed, 168 insertions(+), 91 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index d2edda6d3..6ca852dec 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -144,11 +144,16 @@ minmaxFunc(sql_context * context, int argc, sql_value ** argv)
 	for (i = 1; i < argc; i++) {
 		if (mem_is_null(argv[i]))
 			return;
-		if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) >=
-		    0) {
-			testcase(mask == 0);
-			iBest = i;
+		int res;
+		if (mem_cmp_scalar(argv[iBest], argv[i], &res, pColl) != 0) {
+			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+				 mem_str(argv[i]),
+				 mem_type_to_str(argv[iBest]));
+			context->is_aborted = true;
+			return;
 		}
+		if ((res ^ mask) >= 0)
+			iBest = i;
 	}
 	sql_result_value(context, argv[iBest]);
 }
@@ -1054,9 +1059,15 @@ nullifFunc(sql_context * context, int NotUsed, sql_value ** argv)
 {
 	struct coll *pColl = sqlGetFuncCollSeq(context);
 	UNUSED_PARAMETER(NotUsed);
-	if (sqlMemCompare(argv[0], argv[1], pColl) != 0) {
-		sql_result_value(context, argv[0]);
+	int res;
+	if (mem_cmp_scalar(argv[0], argv[1], &res, pColl) != 0) {
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 mem_str(argv[1]), mem_type_to_str(argv[0]));
+		context->is_aborted = true;
+		return;
 	}
+	if (res != 0)
+		sql_result_value(context, argv[0]);
 }
 
 /**
@@ -1824,7 +1835,12 @@ minmaxStep(sql_context * context, int NotUsed, sql_value ** argv)
 		 * comparison is inverted.
 		 */
 		bool is_max = (func->flags & SQL_FUNC_MAX) != 0;
-		cmp = sqlMemCompare(pBest, pArg, pColl);
+		if (mem_cmp_scalar(pBest, pArg, &cmp, pColl) != 0) {
+			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+				 mem_str(pArg), mem_type_to_str(pBest));
+			context->is_aborted = true;
+			return;
+		}
 		if ((is_max && cmp < 0) || (!is_max && cmp > 0)) {
 			mem_copy(pBest, pArg);
 		} else {
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index cdb55f858..a8cde6769 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -60,6 +60,44 @@ enum {
 	STR_VALUE_MAX_LEN = 128,
 };
 
+/**
+ * Analogue of enum mp_class for enum mp_type. The order of the classes must be
+ * the same as in the enum mp_class.
+ */
+enum mem_class {
+	MEM_CLASS_NULL,
+	MEM_CLASS_BOOL,
+	MEM_CLASS_NUMBER,
+	MEM_CLASS_STR,
+	MEM_CLASS_BIN,
+	MEM_CLASS_UUID,
+	mem_class_max,
+};
+
+static inline enum mem_class
+mem_type_class(enum mem_type type)
+{
+	switch (type) {
+	case MEM_TYPE_NULL:
+		return MEM_CLASS_NULL;
+	case MEM_TYPE_UINT:
+	case MEM_TYPE_INT:
+	case MEM_TYPE_DOUBLE:
+		return MEM_CLASS_NUMBER;
+	case MEM_TYPE_STR:
+		return MEM_CLASS_STR;
+	case MEM_TYPE_BIN:
+		return MEM_CLASS_BIN;
+	case MEM_TYPE_BOOL:
+		return MEM_CLASS_BOOL;
+	case MEM_TYPE_UUID:
+		return MEM_CLASS_UUID;
+	default:
+		break;
+	}
+	return mem_class_max;
+}
+
 bool
 mem_is_field_compatible(const struct Mem *mem, enum field_type type)
 {
@@ -2030,6 +2068,36 @@ mem_cmp_uuid(const struct Mem *a, const struct Mem *b, int *result)
 	return 0;
 }
 
+int
+mem_cmp_scalar(const struct Mem *a, const struct Mem *b, int *result,
+	       const struct coll *coll)
+{
+	enum mem_class class_a = mem_type_class(a->type);
+	enum mem_class class_b = mem_type_class(b->type);
+	if (class_a != class_b) {
+		*result = class_a - class_b;
+		return 0;
+	}
+	switch (class_a) {
+	case MEM_CLASS_NULL:
+		*result = 0;
+		return 0;
+	case MEM_CLASS_BOOL:
+		return mem_cmp_bool(a, b, result);
+	case MEM_CLASS_NUMBER:
+		return mem_cmp_num(a, b, result);
+	case MEM_CLASS_STR:
+		return mem_cmp_str(a, b, result, coll);
+	case MEM_CLASS_BIN:
+		return mem_cmp_bin(a, b, result);
+	case MEM_CLASS_UUID:
+		return mem_cmp_uuid(a, b, result);
+	default:
+		unreachable();
+	}
+	return 0;
+}
+
 /*
  * Both *pMem1 and *pMem2 contain string values. Compare the two values
  * using the collation sequence pColl. As usual, return a negative , zero
@@ -2463,82 +2531,6 @@ sqlVdbeMemTooBig(Mem * p)
 	return 0;
 }
 
-/*
- * Compare the values contained by the two memory cells, returning
- * negative, zero or positive if pMem1 is less than, equal to, or greater
- * than pMem2. Sorting order is NULL's first, followed by numbers (integers
- * and reals) sorted numerically, followed by text ordered by the collating
- * sequence pColl and finally blob's ordered by memcmp().
- *
- * Two NULL values are considered equal by this function.
- */
-int
-sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll * pColl)
-{
-	int res;
-
-	enum mem_type type1 = pMem1->type;
-	enum mem_type type2 = pMem2->type;
-
-	/* If one value is NULL, it is less than the other. If both values
-	 * are NULL, return 0.
-	 */
-	if (((type1 | type2) & MEM_TYPE_NULL) != 0)
-		return (int)(type2 == MEM_TYPE_NULL) -
-		       (int)(type1 == MEM_TYPE_NULL);
-
-	if (((type1 | type2) & MEM_TYPE_BOOL) != 0) {
-		if ((type1 & type2 & MEM_TYPE_BOOL) != 0) {
-			if (pMem1->u.b == pMem2->u.b)
-				return 0;
-			if (pMem1->u.b)
-				return 1;
-			return -1;
-		}
-		if (type2 == MEM_TYPE_BOOL)
-			return +1;
-		return -1;
-	}
-
-	if (((type1 | type2) & MEM_TYPE_UUID) != 0) {
-		if (mem_cmp_uuid(pMem1, pMem2, &res) == 0)
-			return res;
-		if (type1 != MEM_TYPE_UUID)
-			return +1;
-		return -1;
-	}
-
-	/* At least one of the two values is a number
-	 */
-	if (((type1 | type2) &
-	     (MEM_TYPE_INT | MEM_TYPE_UINT | MEM_TYPE_DOUBLE)) != 0) {
-		if (!mem_is_num(pMem1))
-			return +1;
-		if (!mem_is_num(pMem2))
-			return -1;
-		mem_cmp_num(pMem1, pMem2, &res);
-		return res;
-	}
-
-	/* If one value is a string and the other is a blob, the string is less.
-	 * If both are strings, compare using the collating functions.
-	 */
-	if (((type1 | type2) & MEM_TYPE_STR) != 0) {
-		if (type1 != MEM_TYPE_STR) {
-			return 1;
-		}
-		if (type2 != MEM_TYPE_STR) {
-			return -1;
-		}
-		mem_cmp_str(pMem1, pMem2, &res, pColl);
-		return res;
-	}
-
-	/* Both values must be blobs.  Compare using memcmp().  */
-	mem_cmp_bin(pMem1, pMem2, &res);
-	return res;
-}
-
 int
 sql_vdbemem_finalize(struct Mem *mem, struct func *func)
 {
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 97aba9ab4..dd8666f53 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -698,6 +698,14 @@ mem_cmp_num(const struct Mem *a, const struct Mem *b, int *result);
 int
 mem_cmp_uuid(const struct Mem *left, const struct Mem *right, int *result);
 
+/**
+ * Compare two MEMs using SCALAR rules and return the result of comparison. MEMs
+ * should be scalars. Original MEMs are not changed.
+ */
+int
+mem_cmp_scalar(const struct Mem *a, const struct Mem *b, int *result,
+	       const struct coll *coll);
+
 /**
  * Convert the given MEM to INTEGER. This function and the function below define
  * the rules that are used to convert values of all other types to INTEGER. In
@@ -963,8 +971,6 @@ int sqlVdbeMemTooBig(Mem *);
 #define VdbeMemDynamic(X) (((X)->flags & MEM_Dyn) != 0 ||\
 			   ((X)->type & (MEM_TYPE_AGG | MEM_TYPE_FRAME)) != 0)
 
-int sqlMemCompare(const Mem *, const Mem *, const struct coll *);
-
 /** MEM manipulate functions. */
 
 /**
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index cc698b715..9e763ed85 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1825,7 +1825,13 @@ case OP_Compare: {
 		assert(i < (int)def->part_count);
 		struct coll *coll = def->parts[i].coll;
 		bool is_rev = def->parts[i].sort_order == SORT_ORDER_DESC;
-		iCompare = sqlMemCompare(&aMem[p1+idx], &aMem[p2+idx], coll);
+		struct Mem *a = &aMem[p1+idx];
+		struct Mem *b = &aMem[p2+idx];
+		if (mem_cmp_scalar(a, b, &iCompare, coll) != 0) {
+			diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(b),
+				 mem_type_to_str(a));
+			goto abort_due_to_error;
+		}
 		if (iCompare) {
 			if (is_rev)
 				iCompare = -iCompare;
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index e5f35fbf8..e2eb153fb 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -1272,13 +1272,27 @@ whereRangeSkipScanEst(Parse * pParse,		/* Parsing & code generating context */
 			rc = sql_stat4_column(db, samples[i].sample_key, nEq,
 					      &pVal);
 			if (rc == 0 && p1 != NULL) {
-				int res = sqlMemCompare(p1, pVal, coll);
-				if (res >= 0)
+				int res;
+				rc = mem_cmp_scalar(p1, pVal, &res, coll);
+				if (rc != 0) {
+					diag_set(ClientError,
+						 ER_SQL_TYPE_MISMATCH,
+						 mem_str(pVal),
+						 mem_type_to_str(p1));
+				}
+				if (rc == 0 && res >= 0)
 					nLower++;
 			}
 			if (rc == 0 && p2 != NULL) {
-				int res = sqlMemCompare(p2, pVal, coll);
-				if (res >= 0)
+				int res;
+				rc = mem_cmp_scalar(p2, pVal, &res, coll);
+				if (rc != 0) {
+					diag_set(ClientError,
+						 ER_SQL_TYPE_MISMATCH,
+						 mem_str(pVal),
+						 mem_type_to_str(p2));
+				}
+				if (rc == 0 && res >= 0)
 					nUpper++;
 			}
 		}
diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
index 4fc5052d8..6b4a811c3 100755
--- a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
+++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(4)
+test:plan(8)
 
 -- Make sure that function quote() can work with uuid.
 test:do_execsql_test(
@@ -35,4 +35,47 @@ test:do_test(
     end,
     uuid3)
 
+--
+-- Make sure a comparison that includes a UUID and follows the SCALAR rules is
+-- working correctly.
+--
+box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, s SCALAR);]])
+box.execute([[INSERT INTO t VALUES (1, ?)]], {uuid1})
+
+test:do_execsql_test(
+    "gh-6164-5",
+    [[
+        SELECT GREATEST(i, s, x'33', 'something') FROM t;
+    ]], {
+        uuid1
+    })
+
+test:do_execsql_test(
+    "gh-6164-6",
+    [[
+        SELECT LEAST(i, s, x'33', 'something') FROM t;
+    ]], {
+        1
+    })
+
+box.execute([[INSERT INTO t VALUES (2, 2);]])
+
+test:do_execsql_test(
+    "gh-6164-7",
+    [[
+        SELECT MAX(s) FROM t;
+    ]], {
+        uuid1
+    })
+
+test:do_execsql_test(
+    "gh-6164-8",
+    [[
+        SELECT MIN(s) FROM t;
+    ]], {
+        2
+    })
+
+box.execute([[DROP TABLE t;]])
+
 test:finish_test()
-- 
2.25.1

[Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()

[Mergen Imeev via Tarantool-patches]

This patch introduces the mem_cmp_msgpack() function that compares MEM
and packed to msgpack value using SCALAR rules. MEM and packed value
must be scalars. Prior to this patch, there was a function that used
SCALAR rules to compare MEM and packed value, but its design became
overly complex as new types appeared.

Closes #6164
---
 src/box/sql.c                                 |   9 +-
 src/box/sql/mem.c                             | 289 +++++-------------
 src/box/sql/mem.h                             |  24 +-
 test/sql-tap/gh-6164-uuid-follow-ups.test.lua |  14 +-
 4 files changed, 107 insertions(+), 229 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 790ca7f70..7471c3832 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -765,10 +765,13 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor,
 			}
 		}
 		next_fieldno = fieldno + 1;
-		rc = sqlVdbeCompareMsgpack(&p, unpacked, i);
+		struct key_part *part = &unpacked->key_def->parts[i];
+		struct Mem *mem = unpacked->aMem + i;
+		struct coll *coll = part->coll;
+		if (mem_cmp_msgpack(mem, &p, &rc, coll) != 0)
+			rc = 0;
 		if (rc != 0) {
-			if (unpacked->key_def->parts[i].sort_order !=
-			    SORT_ORDER_ASC)
+			if (part->sort_order == SORT_ORDER_ASC)
 				rc = -rc;
 			goto out;
 		}
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index a8cde6769..fa80f6d5a 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -2098,35 +2098,77 @@ mem_cmp_scalar(const struct Mem *a, const struct Mem *b, int *result,
 	return 0;
 }
 
-/*
- * Both *pMem1 and *pMem2 contain string values. Compare the two values
- * using the collation sequence pColl. As usual, return a negative , zero
- * or positive value if *pMem1 is less than, equal to or greater than
- * *pMem2, respectively. Similar in spirit to "rc = (*pMem1) - (*pMem2);".
- *
- * Strungs assume to be UTF-8 encoded
- */
-static int
-vdbeCompareMemString(const Mem * pMem1, const Mem * pMem2,
-		     const struct coll * pColl)
-{
-	return pColl->cmp(pMem1->z, (size_t)pMem1->n,
-			      pMem2->z, (size_t)pMem2->n, pColl);
-}
-
-/*
- * The input pBlob is guaranteed to be a Blob that is not marked
- * with MEM_Zero.  Return true if it could be a zero-blob.
- */
-static int
-isAllZero(const char *z, int n)
-{
-	int i;
-	for (i = 0; i < n; i++) {
-		if (z[i])
-			return 0;
+int
+mem_cmp_msgpack(const struct Mem *a, const char **b, int *result,
+		const struct coll *coll)
+{
+	struct Mem mem;
+	switch (mp_typeof(**b)) {
+	case MP_NIL:
+		mem.type = MEM_TYPE_NULL;
+		mp_decode_nil(b);
+		break;
+	case MP_BOOL:
+		mem.type = MEM_TYPE_BOOL;
+		mem.u.b = mp_decode_bool(b);
+		break;
+	case MP_UINT:
+		mem.type = MEM_TYPE_UINT;
+		mem.u.u = mp_decode_uint(b);
+		break;
+	case MP_INT:
+		mem.type = MEM_TYPE_INT;
+		mem.u.i = mp_decode_int(b);
+		break;
+	case MP_FLOAT:
+		mem.type = MEM_TYPE_DOUBLE;
+		mem.u.r = mp_decode_float(b);
+		break;
+	case MP_DOUBLE:
+		mem.type = MEM_TYPE_DOUBLE;
+		mem.u.r = mp_decode_double(b);
+		break;
+	case MP_STR:
+		mem.type = MEM_TYPE_STR;
+		mem.n = mp_decode_strl(b);
+		mem.z = (char *)*b;
+		*b += mem.n;
+		mem.flags = MEM_Ephem;
+		break;
+	case MP_BIN:
+		mem.type = MEM_TYPE_BIN;
+		mem.n = mp_decode_binl(b);
+		mem.z = (char *)*b;
+		*b += mem.n;
+		mem.flags = MEM_Ephem;
+		break;
+	case MP_ARRAY:
+	case MP_MAP:
+		mp_next(b);
+		*result = -1;
+		return 0;
+	case MP_EXT: {
+		int8_t type;
+		const char *buf = *b;
+		uint32_t len = mp_decode_extl(b, &type);
+		if (type == MP_UUID) {
+			assert(len == UUID_LEN);
+			mem.type = MEM_TYPE_UUID;
+			if (uuid_unpack(b, len, &mem.u.uuid) == NULL)
+				return -1;
+			break;
+		}
+		*b += len;
+		mem.type = MEM_TYPE_BIN;
+		mem.z = (char *)buf;
+		mem.n = *b - buf;
+		mem.flags = MEM_Ephem;
+		break;
 	}
-	return 1;
+	default:
+		unreachable();
+	}
+	return mem_cmp_scalar(a, &mem, result, coll);
 }
 
 char *
@@ -2557,183 +2599,6 @@ sql_vdbemem_finalize(struct Mem *mem, struct func *func)
 	return ctx.is_aborted ? -1 : 0;
 }
 
-int
-sqlVdbeCompareMsgpack(const char **key1,
-			  struct UnpackedRecord *unpacked, int key2_idx)
-{
-	const char *aKey1 = *key1;
-	Mem *pKey2 = unpacked->aMem + key2_idx;
-	Mem mem1;
-	int rc = 0;
-	switch (mp_typeof(*aKey1)) {
-	default:{
-			/* FIXME */
-			rc = -1;
-			break;
-		}
-	case MP_NIL:{
-			rc = -(pKey2->type != MEM_TYPE_NULL);
-			mp_decode_nil(&aKey1);
-			break;
-		}
-	case MP_BOOL:{
-			mem1.u.b = mp_decode_bool(&aKey1);
-			if (pKey2->type == MEM_TYPE_BOOL) {
-				if (mem1.u.b != pKey2->u.b)
-					rc = mem1.u.b ? 1 : -1;
-			} else {
-				rc = pKey2->type == MEM_TYPE_NULL ? 1 : -1;
-			}
-			break;
-		}
-	case MP_UINT:{
-			mem1.u.u = mp_decode_uint(&aKey1);
-			if (pKey2->type == MEM_TYPE_INT) {
-				rc = +1;
-			} else if (pKey2->type == MEM_TYPE_UINT) {
-				if (mem1.u.u < pKey2->u.u)
-					rc = -1;
-				else if (mem1.u.u > pKey2->u.u)
-					rc = +1;
-			} else if (pKey2->type == MEM_TYPE_DOUBLE) {
-				rc = double_compare_uint64(pKey2->u.r,
-							   mem1.u.u, -1);
-			} else if (pKey2->type == MEM_TYPE_NULL) {
-				rc = 1;
-			} else if (pKey2->type == MEM_TYPE_BOOL) {
-				rc = 1;
-			} else {
-				rc = -1;
-			}
-			break;
-		}
-	case MP_INT:{
-			mem1.u.i = mp_decode_int(&aKey1);
-			if (pKey2->type == MEM_TYPE_UINT) {
-				rc = -1;
-			} else if (pKey2->type == MEM_TYPE_INT) {
-				if (mem1.u.i < pKey2->u.i) {
-					rc = -1;
-				} else if (mem1.u.i > pKey2->u.i) {
-					rc = +1;
-				}
-			} else if (pKey2->type == MEM_TYPE_DOUBLE) {
-				rc = double_compare_nint64(pKey2->u.r, mem1.u.i,
-							   -1);
-			} else if (pKey2->type == MEM_TYPE_NULL) {
-				rc = 1;
-			} else if (pKey2->type == MEM_TYPE_BOOL) {
-				rc = 1;
-			} else {
-				rc = -1;
-			}
-			break;
-		}
-	case MP_FLOAT:{
-			mem1.u.r = mp_decode_float(&aKey1);
-			goto do_float;
-		}
-	case MP_DOUBLE:{
-			mem1.u.r = mp_decode_double(&aKey1);
- do_float:
-			if (pKey2->type == MEM_TYPE_INT) {
-				rc = double_compare_nint64(mem1.u.r, pKey2->u.i,
-							   1);
-			} else if (pKey2->type == MEM_TYPE_UINT) {
-				rc = double_compare_uint64(mem1.u.r,
-							   pKey2->u.u, 1);
-			} else if (pKey2->type == MEM_TYPE_DOUBLE) {
-				if (mem1.u.r < pKey2->u.r) {
-					rc = -1;
-				} else if (mem1.u.r > pKey2->u.r) {
-					rc = +1;
-				}
-			} else if (pKey2->type == MEM_TYPE_NULL) {
-				rc = 1;
-			} else if (pKey2->type == MEM_TYPE_BOOL) {
-				rc = 1;
-			} else {
-				rc = -1;
-			}
-			break;
-		}
-	case MP_STR:{
-			if (pKey2->type == MEM_TYPE_STR) {
-				struct key_def *key_def = unpacked->key_def;
-				mem1.n = mp_decode_strl(&aKey1);
-				mem1.z = (char *)aKey1;
-				aKey1 += mem1.n;
-				struct coll *coll =
-					key_def->parts[key2_idx].coll;
-				if (coll != NULL) {
-					mem1.type = MEM_TYPE_STR;
-					mem1.flags = 0;
-					rc = vdbeCompareMemString(&mem1, pKey2,
-								  coll);
-				} else {
-					goto do_bin_cmp;
-				}
-			} else {
-				rc = pKey2->type == MEM_TYPE_BIN ? -1 : +1;
-			}
-			break;
-		}
-	case MP_BIN:{
-			mem1.n = mp_decode_binl(&aKey1);
-			mem1.z = (char *)aKey1;
-			aKey1 += mem1.n;
- do_blob:
-			if (pKey2->type == MEM_TYPE_BIN) {
-				if (pKey2->flags & MEM_Zero) {
-					if (!isAllZero
-					    ((const char *)mem1.z, mem1.n)) {
-						rc = 1;
-					} else {
-						rc = mem1.n - pKey2->u.nZero;
-					}
-				} else {
-					int nCmp;
- do_bin_cmp:
-					nCmp = MIN(mem1.n, pKey2->n);
-					rc = memcmp(mem1.z, pKey2->z, nCmp);
-					if (rc == 0)
-						rc = mem1.n - pKey2->n;
-				}
-			} else {
-				rc = 1;
-			}
-			break;
-		}
-	case MP_ARRAY:
-	case MP_MAP: {
-			mem1.z = (char *)aKey1;
-			mp_next(&aKey1);
-			mem1.n = aKey1 - (char *)mem1.z;
-			goto do_blob;
-		}
-	case MP_EXT: {
-		int8_t type;
-		const char *buf = aKey1;
-		uint32_t len = mp_decode_extl(&aKey1, &type);
-		if (type == MP_UUID) {
-			assert(len == UUID_LEN);
-			mem1.type = MEM_TYPE_UUID;
-			aKey1 = buf;
-			if (mp_decode_uuid(&aKey1, &mem1.u.uuid) == NULL ||
-			    mem_cmp_uuid(&mem1, pKey2, &rc) != 0)
-				rc = 1;
-			break;
-		}
-		aKey1 += len;
-		mem1.z = (char *)buf;
-		mem1.n = aKey1 - buf;
-		goto do_blob;
-	}
-	}
-	*key1 = aKey1;
-	return rc;
-}
-
 int
 sqlVdbeRecordCompareMsgpack(const void *key1,
 				struct UnpackedRecord *key2)
@@ -2744,13 +2609,15 @@ sqlVdbeRecordCompareMsgpack(const void *key1,
 	n = MIN(n, key2->nField);
 
 	for (i = 0; i != n; i++) {
-		rc = sqlVdbeCompareMsgpack((const char**)&key1, key2, i);
+		struct key_part *part = &key2->key_def->parts[i];
+		struct Mem *mem = key2->aMem + i;
+		struct coll *coll = part->coll;
+		if (mem_cmp_msgpack(mem, (const char **)&key1, &rc, coll) != 0)
+			rc = 0;
 		if (rc != 0) {
-			if (key2->key_def->parts[i].sort_order !=
-			    SORT_ORDER_ASC) {
-				rc = -rc;
-			}
-			return rc;
+			if (part->sort_order != SORT_ORDER_ASC)
+				return rc;
+			return -rc;
 		}
 	}
 
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index dd8666f53..645d0ee27 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -706,6 +706,16 @@ int
 mem_cmp_scalar(const struct Mem *a, const struct Mem *b, int *result,
 	       const struct coll *coll);
 
+/**
+ * Compare MEM and packed to msgpack value using SCALAR rules and return the
+ * result of comparison. Both values should be scalars. Original MEM is not
+ * changed. If successful, the second argument will contain the address
+ * following the specified packed value.
+ */
+int
+mem_cmp_msgpack(const struct Mem *a, const char **b, int *result,
+		const struct coll *coll);
+
 /**
  * Convert the given MEM to INTEGER. This function and the function below define
  * the rules that are used to convert values of all other types to INTEGER. In
@@ -983,20 +993,6 @@ int sqlVdbeMemTooBig(Mem *);
 int
 sql_vdbemem_finalize(struct Mem *mem, struct func *func);
 
-/** MEM and msgpack functions. */
-
-/**
- * Perform comparison of two keys: one is packed and one is not.
- *
- * @param key1 Pointer to pointer to first key.
- * @param unpacked Pointer to unpacked tuple.
- * @param key2_idx index of key in umpacked record to compare.
- *
- * @retval +1 if key1 > pUnpacked[iKey2], -1 ptherwise.
- */
-int sqlVdbeCompareMsgpack(const char **key1,
-			      struct UnpackedRecord *unpacked, int key2_idx);
-
 /**
  * Perform comparison of two tuples: unpacked (key1) and packed (key2)
  *
diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
index 6b4a811c3..dd041c0b4 100755
--- a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
+++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(8)
+test:plan(9)
 
 -- Make sure that function quote() can work with uuid.
 test:do_execsql_test(
@@ -76,6 +76,18 @@ test:do_execsql_test(
         2
     })
 
+box.execute([[DELETE FROM t;]])
+
+box.execute([[INSERT INTO t VALUES (1, X'00'), (2, ?);]], {uuid1})
+
+test:do_execsql_test(
+    "gh-6164-9",
+    [[
+        SELECT i FROM t ORDER BY s;
+    ]], {
+        1, 2
+    })
+
 box.execute([[DROP TABLE t;]])
 
 test:finish_test()
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()

[Timur Safin via Tarantool-patches]

LGTM, but with 1 question...

: From: imeevma@tarantool.org <imeevma@tarantool.org>
: Subject: [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()
: 
: This patch introduces the mem_cmp_msgpack() function that compares MEM
: and packed to msgpack value using SCALAR rules. MEM and packed value
: must be scalars. Prior to this patch, there was a function that used
: SCALAR rules to compare MEM and packed value, but its design became
: overly complex as new types appeared.
: 
: Closes #6164
: ---
:  src/box/sql.c                                 |   9 +-
:  src/box/sql/mem.c                             | 289 +++++-------------
:  src/box/sql/mem.h                             |  24 +-
:  test/sql-tap/gh-6164-uuid-follow-ups.test.lua |  14 +-
:  4 files changed, 107 insertions(+), 229 deletions(-)
: 
: diff --git a/src/box/sql.c b/src/box/sql.c
: index 790ca7f70..7471c3832 100644
: --- a/src/box/sql.c
: +++ b/src/box/sql.c
: @@ -765,10 +765,13 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor,
:  			}
:  		}
:  		next_fieldno = fieldno + 1;
: -		rc = sqlVdbeCompareMsgpack(&p, unpacked, i);
: +		struct key_part *part = &unpacked->key_def->parts[i];
: +		struct Mem *mem = unpacked->aMem + i;
: +		struct coll *coll = part->coll;
: +		if (mem_cmp_msgpack(mem, &p, &rc, coll) != 0)
: +			rc = 0;
:  		if (rc != 0) {
: -			if (unpacked->key_def->parts[i].sort_order !=
: -			    SORT_ORDER_ASC)
: +			if (part->sort_order == SORT_ORDER_ASC)
:  				rc = -rc;
:  			goto out;
:  		}
: diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
: index a8cde6769..fa80f6d5a 100644
: --- a/src/box/sql/mem.c
: +++ b/src/box/sql/mem.c
: @@ -2098,35 +2098,77 @@ mem_cmp_scalar(const struct Mem *a, const struct Mem
: *b, int *result,
:  	return 0;
:  }
: 
: -/*
: - * Both *pMem1 and *pMem2 contain string values. Compare the two values
: - * using the collation sequence pColl. As usual, return a negative , zero
: - * or positive value if *pMem1 is less than, equal to or greater than
: - * *pMem2, respectively. Similar in spirit to "rc = (*pMem1) - (*pMem2);".
: - *
: - * Strungs assume to be UTF-8 encoded
: - */
: -static int
: -vdbeCompareMemString(const Mem * pMem1, const Mem * pMem2,
: -		     const struct coll * pColl)
: -{
: -	return pColl->cmp(pMem1->z, (size_t)pMem1->n,
: -			      pMem2->z, (size_t)pMem2->n, pColl);
: -}
: -
: -/*
: - * The input pBlob is guaranteed to be a Blob that is not marked
: - * with MEM_Zero.  Return true if it could be a zero-blob.
: - */
: -static int
: -isAllZero(const char *z, int n)
: -{
: -	int i;
: -	for (i = 0; i < n; i++) {
: -		if (z[i])
: -			return 0;
: +int
: +mem_cmp_msgpack(const struct Mem *a, const char **b, int *result,
: +		const struct coll *coll)
: +{
: +	struct Mem mem;
: +	switch (mp_typeof(**b)) {
: +	case MP_NIL:
: +		mem.type = MEM_TYPE_NULL;
: +		mp_decode_nil(b);
: +		break;
: +	case MP_BOOL:
: +		mem.type = MEM_TYPE_BOOL;
: +		mem.u.b = mp_decode_bool(b);
: +		break;
: +	case MP_UINT:
: +		mem.type = MEM_TYPE_UINT;
: +		mem.u.u = mp_decode_uint(b);
: +		break;
: +	case MP_INT:
: +		mem.type = MEM_TYPE_INT;
: +		mem.u.i = mp_decode_int(b);
: +		break;
: +	case MP_FLOAT:
: +		mem.type = MEM_TYPE_DOUBLE;
: +		mem.u.r = mp_decode_float(b);
: +		break;
: +	case MP_DOUBLE:
: +		mem.type = MEM_TYPE_DOUBLE;
: +		mem.u.r = mp_decode_double(b);
: +		break;
: +	case MP_STR:
: +		mem.type = MEM_TYPE_STR;
: +		mem.n = mp_decode_strl(b);
: +		mem.z = (char *)*b;
: +		*b += mem.n;
: +		mem.flags = MEM_Ephem;
: +		break;
: +	case MP_BIN:
: +		mem.type = MEM_TYPE_BIN;
: +		mem.n = mp_decode_binl(b);
: +		mem.z = (char *)*b;
: +		*b += mem.n;
: +		mem.flags = MEM_Ephem;
: +		break;
: +	case MP_ARRAY:
: +	case MP_MAP:
: +		mp_next(b);
: +		*result = -1;
: +		return 0;
: +	case MP_EXT: {
: +		int8_t type;
: +		const char *buf = *b;
: +		uint32_t len = mp_decode_extl(b, &type);

Decimals are coming here? Yes?

: +		if (type == MP_UUID) {
: +			assert(len == UUID_LEN);
: +			mem.type = MEM_TYPE_UUID;
: +			if (uuid_unpack(b, len, &mem.u.uuid) == NULL)
: +				return -1;
: +			break;
: +		}
: +		*b += len;
: +		mem.type = MEM_TYPE_BIN;
: +		mem.z = (char *)buf;
: +		mem.n = *b - buf;
: +		mem.flags = MEM_Ephem;
: +		break;
:  	}
: -	return 1;
: +	default:
: +		unreachable();
: +	}
: +	return mem_cmp_scalar(a, &mem, result, coll);
:  }
: 
:  char *

Regards,
Timur

Re: [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values

[Timur Safin via Tarantool-patches]

LGTM, and one minor comment we need to address eventually. 

: From: imeevma@tarantool.org <imeevma@tarantool.org>
: Subject: [PATCH v2 2/4] sql: allow to bind uuid values
: 
: After this patch, uuid values can be bound like any other supported by
: SQL values.
: 
: Part of #6164
: ---

: diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
: index ef8dcd693..115c52f96 100644
: --- a/src/box/sql/sqlInt.h
: +++ b/src/box/sql/sqlInt.h
: @@ -326,6 +326,8 @@ struct sql_vfs {
:  #define SQL_LIMIT_LIKE_PATTERN_LENGTH       8
:  #define SQL_LIMIT_TRIGGER_DEPTH             9
: 
: +struct tt_uuid;
: +
:  enum sql_ret_code {
:  	/** sql_step() has another row ready. */
:  	SQL_ROW = 1,
: @@ -634,6 +636,9 @@ int
:  sql_bind_zeroblob64(sql_stmt *, int,
:  			sql_uint64);
: 
: +int
: +sql_bind_uuid(struct sql_stmt *stmt, int i, const struct tt_uuid *uuid);
: +

As interface header I'd expect that every function put to sqlInt.h would be
properly documented, and wanted to complain that `sql_bind_uuid()` is not
accompanied with doxygen block ... but then discovered that majority of 
`sql_bind_*` functions (with exception of `sql_bind_boolean()`) were not 
documented at all, and actually this terse introduction looks more consistent
than if it would be documented :(

So, it looks like it's ok for today, but eventually we should return here and
make sure it's reasonably documented/commented out. 

:  /**
:   * Return the number of wildcards that should be bound to.
:   */
: diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
: index aaae12e41..8031ee0dc 100644
: --- a/src/box/sql/vdbeapi.c
: +++ b/src/box/sql/vdbeapi.c
: @@ -840,6 +840,16 @@ sql_bind_zeroblob64(sql_stmt * pStmt, int i, sql_uint64
: n)
:  	return sql_bind_zeroblob(pStmt, i, n);
:  }
: 
: +int
: +sql_bind_uuid(struct sql_stmt *stmt, int i, const struct tt_uuid *uuid)
: +{
: +	struct Vdbe *p = (struct Vdbe *)stmt;
: +	if (vdbeUnbind(p, i) != 0 || sql_bind_type(p, i, "uuid") != 0)
: +		return -1;
: +	mem_set_uuid(&p->aVar[i - 1], uuid);
: +	return 0;
: +}
: +
:  int
:  sql_bind_parameter_count(const struct sql_stmt *stmt)
:  {
: diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-
: tap/gh-6164-uuid-follow-ups.test.lua
: index a8f662f77..4fc5052d8 100755
: --- a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
: +++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
: @@ -1,6 +1,6 @@
:  #!/usr/bin/env tarantool
:  local test = require("sqltester")
: -test:plan(1)
: +test:plan(4)
: 
:  -- Make sure that function quote() can work with uuid.
:  test:do_execsql_test(
: @@ -11,4 +11,28 @@ test:do_execsql_test(
:          '11111111-1111-1111-1111-111111111111'
:      })
: 
: +-- Make sure that uuid value can be binded.
: +local uuid1 = require('uuid').fromstr('11111111-1111-1111-1111-
: 111111111111')
: +local uuid2 = require('uuid').fromstr('11111111-2222-1111-1111-
: 111111111111')
: +local uuid3 = require('uuid').fromstr('11111111-1111-3333-1111-
: 111111111111')
: +test:do_test(
: +    "gh-6164-2",
: +    function()
: +        return box.execute([[SELECT ?;]], {uuid1}).rows[1][1]
: +    end,
: +    uuid1)
: +test:do_test(
: +    "gh-6164-3",
: +    function()
: +        return box.execute([[SELECT $2;]], {123, uuid2}).rows[1][1]
: +    end,
: +    uuid2)
: +
: +test:do_test(
: +    "gh-6164-4",
: +    function()
: +        return box.execute([[SELECT :two;]], {{[":two"] =
: uuid3}}).rows[1][1]
: +    end,
: +    uuid3)
: +
:  test:finish_test()
: --
: 2.25.1

Regards,Timur

Re: [Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote()

[Timur Safin via Tarantool-patches]

LGTM

: From: imeevma@tarantool.org <imeevma@tarantool.org>
: Subject: [PATCH v2 1/4] sql: introduce uuid to quote()
: 
: Prior to this patch, built-in SQL function quote() could not work with
: uuid. It now returns a string representation of the received uuid.
: 
: Part of #6164
: ---
:  src/box/sql/func.c                            | 24 ++++++++++++-------
:  test/sql-tap/engine.cfg                       |  3 +++
:  test/sql-tap/gh-6164-uuid-follow-ups.test.lua | 14 +++++++++++
:  3 files changed, 32 insertions(+), 9 deletions(-)
:  create mode 100755 test/sql-tap/gh-6164-uuid-follow-ups.test.lua
: 
: diff --git a/src/box/sql/func.c b/src/box/sql/func.c
: index 84768f17c..d2edda6d3 100644
: --- a/src/box/sql/func.c
: +++ b/src/box/sql/func.c
: @@ -1095,8 +1095,8 @@ quoteFunc(sql_context * context, int argc, sql_value
: ** argv)
:  {
:  	assert(argc == 1);
:  	UNUSED_PARAMETER(argc);
: -	switch (sql_value_type(argv[0])) {
: -	case MP_DOUBLE:{
: +	switch (argv[0]->type) {
: +	case MEM_TYPE_DOUBLE:{
:  			double r1, r2;
:  			char zBuf[50];
:  			r1 = mem_get_double_unsafe(argv[0]);
: @@ -1110,14 +1110,20 @@ quoteFunc(sql_context * context, int argc, sql_value
: ** argv)
:  					    SQL_TRANSIENT);
:  			break;
:  		}
: -	case MP_UINT:
: -	case MP_INT:{
: +	case MEM_TYPE_UUID: {
: +		char buf[UUID_STR_LEN + 1];
: +		tt_uuid_to_string(&argv[0]->u.uuid, &buf[0]);
: +		sql_result_text(context, buf, UUID_STR_LEN, SQL_TRANSIENT);
: +		break;
: +	}
: +	case MEM_TYPE_UINT:
: +	case MEM_TYPE_INT: {
:  			sql_result_value(context, argv[0]);
:  			break;
:  		}
: -	case MP_BIN:
: -	case MP_ARRAY:
: -	case MP_MAP: {
: +	case MEM_TYPE_BIN:
: +	case MEM_TYPE_ARRAY:
: +	case MEM_TYPE_MAP: {
:  			char *zText = 0;
:  			char const *zBlob = mem_as_bin(argv[0]);
:  			int nBlob = mem_len_unsafe(argv[0]);
: @@ -1143,7 +1149,7 @@ quoteFunc(sql_context * context, int argc, sql_value
: ** argv)
:  			}
:  			break;
:  		}
: -	case MP_STR:{
: +	case MEM_TYPE_STR: {
:  			int i, j;
:  			u64 n;
:  			const unsigned char *zArg = mem_as_ustr(argv[0]);
: @@ -1171,7 +1177,7 @@ quoteFunc(sql_context * context, int argc, sql_value
: ** argv)
:  			}
:  			break;
:  		}
: -	case MP_BOOL: {
: +	case MEM_TYPE_BOOL: {
:  		sql_result_text(context,
:  				SQL_TOKEN_BOOLEAN(mem_get_bool_unsafe(argv[0])),
:  				-1, SQL_TRANSIENT);
: diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
: index 693a477b7..94b0bb1f6 100644
: --- a/test/sql-tap/engine.cfg
: +++ b/test/sql-tap/engine.cfg
: @@ -20,6 +20,9 @@
:      "gh-3332-tuple-format-leak.test.lua": {
:          "memtx": {"engine": "memtx"}
:      },
: +    "gh-6164-uuid-follow-ups.test.lua": {
: +        "memtx": {"engine": "memtx"}
: +    },
:      "gh-4077-iproto-execute-no-bind.test.lua": {},
:      "*": {
:          "memtx": {"engine": "memtx"},
: diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-
: tap/gh-6164-uuid-follow-ups.test.lua
: new file mode 100755
: index 000000000..a8f662f77
: --- /dev/null
: +++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
: @@ -0,0 +1,14 @@
: +#!/usr/bin/env tarantool
: +local test = require("sqltester")
: +test:plan(1)
: +
: +-- Make sure that function quote() can work with uuid.
: +test:do_execsql_test(
: +    "gh-6164-1",
: +    [[
: +        SELECT quote(cast('11111111-1111-1111-1111-111111111111' as uuid));
: +    ]], {
: +        '11111111-1111-1111-1111-111111111111'
: +    })
: +
: +test:finish_test()
: --
: 2.25.1

Re: [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar()

[Timur Safin via Tarantool-patches]

LGTM, yup, it's much simpler now!

: From: imeevma@tarantool.org <imeevma@tarantool.org>
: Subject: [PATCH v2 3/4] sql: introduce mem_cmp_scalar()
: 
: This patch introduces the mem_cmp_scalar() function that compares two
: MEMs using SCALAR rules. MEMs must be scalars. Prior to this patch, there
: was a
: function that used SCALAR rules to compare two MEMs, but its design became
: overly complex as new types appeared.
: 
: Part of #6164


: @@ -2030,6 +2068,36 @@ mem_cmp_uuid(const struct Mem *a, const struct Mem
: *b, int *result)
:  	return 0;
:  }
: 
: +int
: +mem_cmp_scalar(const struct Mem *a, const struct Mem *b, int *result,
: +	       const struct coll *coll)
: +{
: +	enum mem_class class_a = mem_type_class(a->type);
: +	enum mem_class class_b = mem_type_class(b->type);
: +	if (class_a != class_b) {
: +		*result = class_a - class_b;
: +		return 0;
: +	}
: +	switch (class_a) {
: +	case MEM_CLASS_NULL:
: +		*result = 0;
: +		return 0;
: +	case MEM_CLASS_BOOL:
: +		return mem_cmp_bool(a, b, result);
: +	case MEM_CLASS_NUMBER:
: +		return mem_cmp_num(a, b, result);
: +	case MEM_CLASS_STR:
: +		return mem_cmp_str(a, b, result, coll);
: +	case MEM_CLASS_BIN:
: +		return mem_cmp_bin(a, b, result);
: +	case MEM_CLASS_UUID:
: +		return mem_cmp_uuid(a, b, result);
: +	default:
: +		unreachable();
: +	}
: +	return 0;
: +}
: +
:  /*
:   * Both *pMem1 and *pMem2 contain string values. Compare the two values
:   * using the collation sequence pColl. As usual, return a negative , zero
: @@ -2463,82 +2531,6 @@ sqlVdbeMemTooBig(Mem * p)
:  	return 0;
:  }
: 
: -/*
: - * Compare the values contained by the two memory cells, returning
: - * negative, zero or positive if pMem1 is less than, equal to, or greater
: - * than pMem2. Sorting order is NULL's first, followed by numbers (integers
: - * and reals) sorted numerically, followed by text ordered by the collating
: - * sequence pColl and finally blob's ordered by memcmp().
: - *
: - * Two NULL values are considered equal by this function.
: - */
: -int
: -sqlMemCompare(const Mem * pMem1, const Mem * pMem2, const struct coll *
: pColl)
: -{
: -	int res;
: -
: -	enum mem_type type1 = pMem1->type;
: -	enum mem_type type2 = pMem2->type;
: -
: -	/* If one value is NULL, it is less than the other. If both values
: -	 * are NULL, return 0.
: -	 */
: -	if (((type1 | type2) & MEM_TYPE_NULL) != 0)
: -		return (int)(type2 == MEM_TYPE_NULL) -
: -		       (int)(type1 == MEM_TYPE_NULL);
: -
: -	if (((type1 | type2) & MEM_TYPE_BOOL) != 0) {
: -		if ((type1 & type2 & MEM_TYPE_BOOL) != 0) {
: -			if (pMem1->u.b == pMem2->u.b)
: -				return 0;
: -			if (pMem1->u.b)
: -				return 1;
: -			return -1;
: -		}
: -		if (type2 == MEM_TYPE_BOOL)
: -			return +1;
: -		return -1;
: -	}
: -
: -	if (((type1 | type2) & MEM_TYPE_UUID) != 0) {
: -		if (mem_cmp_uuid(pMem1, pMem2, &res) == 0)
: -			return res;
: -		if (type1 != MEM_TYPE_UUID)
: -			return +1;
: -		return -1;
: -	}
: -
: -	/* At least one of the two values is a number
: -	 */
: -	if (((type1 | type2) &
: -	     (MEM_TYPE_INT | MEM_TYPE_UINT | MEM_TYPE_DOUBLE)) != 0) {
: -		if (!mem_is_num(pMem1))
: -			return +1;
: -		if (!mem_is_num(pMem2))
: -			return -1;
: -		mem_cmp_num(pMem1, pMem2, &res);
: -		return res;
: -	}
: -
: -	/* If one value is a string and the other is a blob, the string is
: less.
: -	 * If both are strings, compare using the collating functions.
: -	 */
: -	if (((type1 | type2) & MEM_TYPE_STR) != 0) {
: -		if (type1 != MEM_TYPE_STR) {
: -			return 1;
: -		}
: -		if (type2 != MEM_TYPE_STR) {
: -			return -1;
: -		}
: -		mem_cmp_str(pMem1, pMem2, &res, pColl);
: -		return res;
: -	}
: -
: -	/* Both values must be blobs.  Compare using memcmp().  */
: -	mem_cmp_bin(pMem1, pMem2, &res);
: -	return res;
: -}
: -
:  int
:  sql_vdbemem_finalize(struct Mem *mem, struct func *func)
:  {

Re: [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()

[Mergen Imeev via Tarantool-patches]

Hi! Thank you for the review. My answer below.

On 19.07.2021 12:16, Timur Safin wrote:
> LGTM, but with 1 question...
>
> : From: imeevma@tarantool.org <imeevma@tarantool.org>
> : Subject: [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()
> :
> : This patch introduces the mem_cmp_msgpack() function that compares MEM
> : and packed to msgpack value using SCALAR rules. MEM and packed value
> : must be scalars. Prior to this patch, there was a function that used
> : SCALAR rules to compare MEM and packed value, but its design became
> : overly complex as new types appeared.
> :
> : Closes #6164
> : ---
> :  src/box/sql.c                                 |   9 +-
> :  src/box/sql/mem.c                             | 289 +++++-------------
> :  src/box/sql/mem.h                             |  24 +-
> :  test/sql-tap/gh-6164-uuid-follow-ups.test.lua |  14 +-
> :  4 files changed, 107 insertions(+), 229 deletions(-)
> :
> : diff --git a/src/box/sql.c b/src/box/sql.c
> : index 790ca7f70..7471c3832 100644
> : --- a/src/box/sql.c
> : +++ b/src/box/sql.c
> : @@ -765,10 +765,13 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor,
> :  			}
> :  		}
> :  		next_fieldno = fieldno + 1;
> : -		rc = sqlVdbeCompareMsgpack(&p, unpacked, i);
> : +		struct key_part *part = &unpacked->key_def->parts[i];
> : +		struct Mem *mem = unpacked->aMem + i;
> : +		struct coll *coll = part->coll;
> : +		if (mem_cmp_msgpack(mem, &p, &rc, coll) != 0)
> : +			rc = 0;
> :  		if (rc != 0) {
> : -			if (unpacked->key_def->parts[i].sort_order !=
> : -			    SORT_ORDER_ASC)
> : +			if (part->sort_order == SORT_ORDER_ASC)
> :  				rc = -rc;
> :  			goto out;
> :  		}
> : diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> : index a8cde6769..fa80f6d5a 100644
> : --- a/src/box/sql/mem.c
> : +++ b/src/box/sql/mem.c
> : @@ -2098,35 +2098,77 @@ mem_cmp_scalar(const struct Mem *a, const struct Mem
> : *b, int *result,
> :  	return 0;
> :  }
> :
> : -/*
> : - * Both *pMem1 and *pMem2 contain string values. Compare the two values
> : - * using the collation sequence pColl. As usual, return a negative , zero
> : - * or positive value if *pMem1 is less than, equal to or greater than
> : - * *pMem2, respectively. Similar in spirit to "rc = (*pMem1) - (*pMem2);".
> : - *
> : - * Strungs assume to be UTF-8 encoded
> : - */
> : -static int
> : -vdbeCompareMemString(const Mem * pMem1, const Mem * pMem2,
> : -		     const struct coll * pColl)
> : -{
> : -	return pColl->cmp(pMem1->z, (size_t)pMem1->n,
> : -			      pMem2->z, (size_t)pMem2->n, pColl);
> : -}
> : -
> : -/*
> : - * The input pBlob is guaranteed to be a Blob that is not marked
> : - * with MEM_Zero.  Return true if it could be a zero-blob.
> : - */
> : -static int
> : -isAllZero(const char *z, int n)
> : -{
> : -	int i;
> : -	for (i = 0; i < n; i++) {
> : -		if (z[i])
> : -			return 0;
> : +int
> : +mem_cmp_msgpack(const struct Mem *a, const char **b, int *result,
> : +		const struct coll *coll)
> : +{
> : +	struct Mem mem;
> : +	switch (mp_typeof(**b)) {
> : +	case MP_NIL:
> : +		mem.type = MEM_TYPE_NULL;
> : +		mp_decode_nil(b);
> : +		break;
> : +	case MP_BOOL:
> : +		mem.type = MEM_TYPE_BOOL;
> : +		mem.u.b = mp_decode_bool(b);
> : +		break;
> : +	case MP_UINT:
> : +		mem.type = MEM_TYPE_UINT;
> : +		mem.u.u = mp_decode_uint(b);
> : +		break;
> : +	case MP_INT:
> : +		mem.type = MEM_TYPE_INT;
> : +		mem.u.i = mp_decode_int(b);
> : +		break;
> : +	case MP_FLOAT:
> : +		mem.type = MEM_TYPE_DOUBLE;
> : +		mem.u.r = mp_decode_float(b);
> : +		break;
> : +	case MP_DOUBLE:
> : +		mem.type = MEM_TYPE_DOUBLE;
> : +		mem.u.r = mp_decode_double(b);
> : +		break;
> : +	case MP_STR:
> : +		mem.type = MEM_TYPE_STR;
> : +		mem.n = mp_decode_strl(b);
> : +		mem.z = (char *)*b;
> : +		*b += mem.n;
> : +		mem.flags = MEM_Ephem;
> : +		break;
> : +	case MP_BIN:
> : +		mem.type = MEM_TYPE_BIN;
> : +		mem.n = mp_decode_binl(b);
> : +		mem.z = (char *)*b;
> : +		*b += mem.n;
> : +		mem.flags = MEM_Ephem;
> : +		break;
> : +	case MP_ARRAY:
> : +	case MP_MAP:
> : +		mp_next(b);
> : +		*result = -1;
> : +		return 0;
> : +	case MP_EXT: {
> : +		int8_t type;
> : +		const char *buf = *b;
> : +		uint32_t len = mp_decode_extl(b, &type);
>
> Decimals are coming here? Yes?

Yes. If you want I can give you branch where DECIMAL is partially done.

> : +		if (type == MP_UUID) {
> : +			assert(len == UUID_LEN);
> : +			mem.type = MEM_TYPE_UUID;
> : +			if (uuid_unpack(b, len, &mem.u.uuid) == NULL)
> : +				return -1;
> : +			break;
> : +		}
> : +		*b += len;
> : +		mem.type = MEM_TYPE_BIN;
> : +		mem.z = (char *)buf;
> : +		mem.n = *b - buf;
> : +		mem.flags = MEM_Ephem;
> : +		break;
> :  	}
> : -	return 1;
> : +	default:
> : +		unreachable();
> : +	}
> : +	return mem_cmp_scalar(a, &mem, result, coll);
> :  }
> :
> :  char *
>
> Regards,
> Timur
>

[Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote()

[Mergen Imeev via Tarantool-patches]

Hi! Thank you for the review! My answers, diff and new patch below.

On 08.07.2021 00:41, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
>> diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
>> new file mode 100755
>> index 000000000..d8fa700ea
>> --- /dev/null
>> +++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
>
> 1. The test runs 2 times with 2 engines. Please, make it work only
> once. It does not depend on an engine anyway.
>
Fixed.

>> @@ -0,0 +1,16 @@
>> +#!/usr/bin/env tarantool
>> +local test = require("sqltester")
>> +test:plan(1)
>> +
>> +box.execute([[select quote(cast('11111111-1111-1111-1111-111111111111' as uuid));]])
>
> 2. Why do you need this select? Isn't it exactly the same as the SELECT below?
>
I believe I used it for debug and forgot to remove. Removed.

>> +
>> +-- Make sure that function quote() can work with uuid.
>> +test:do_execsql_test(
>> +    "gh-6164-1",
>> +    [[
>> +        SELECT quote(cast('11111111-1111-1111-1111-111111111111' as uuid));
>> +    ]], {
>> +        '11111111-1111-1111-1111-111111111111'
>> +    })
>> +
>> +test:finish_test()


Diff:

diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index 693a477b7..94b0bb1f6 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -20,6 +20,9 @@
     "gh-3332-tuple-format-leak.test.lua": {
         "memtx": {"engine": "memtx"}
     },
+    "gh-6164-uuid-follow-ups.test.lua": {
+        "memtx": {"engine": "memtx"}
+    },
     "gh-4077-iproto-execute-no-bind.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
index d8fa700ea..a8f662f77 100755
--- a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
+++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
@@ -2,8 +2,6 @@
 local test = require("sqltester")
 test:plan(1)
 
-box.execute([[select quote(cast('11111111-1111-1111-1111-111111111111' as uuid));]])
-
 -- Make sure that function quote() can work with uuid.
 test:do_execsql_test(
     "gh-6164-1",


New patch:

commit ec87733ca387c72957ac3202df1a6b0547e20fdc
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Mon Jul 5 11:09:48 2021 +0300

    sql: introduce uuid to quote()
    
    Prior to this patch, built-in SQL function quote() could not work with
    uuid. It now returns a string representation of the received uuid.
    
    Part of #6164

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index f93ae867d..aa565277c 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1098,8 +1098,8 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 {
    assert(argc == 1);
    UNUSED_PARAMETER(argc);
-   switch (sql_value_type(argv[0])) {
-   case MP_DOUBLE:{
+   switch (argv[0]->type) {
+   case MEM_TYPE_DOUBLE:{
            double r1, r2;
            char zBuf[50];
            r1 = mem_get_double_unsafe(argv[0]);
@@ -1113,14 +1113,20 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
                        SQL_TRANSIENT);
            break;
        }
-   case MP_UINT:
-   case MP_INT:{
+   case MEM_TYPE_UUID: {
+       char buf[UUID_STR_LEN + 1];
+       tt_uuid_to_string(&argv[0]->u.uuid, &buf[0]);
+       sql_result_text(context, buf, UUID_STR_LEN, SQL_TRANSIENT);
+       break;
+   }
+   case MEM_TYPE_UINT:
+   case MEM_TYPE_INT: {
            sql_result_value(context, argv[0]);
            break;
        }
-   case MP_BIN:
-   case MP_ARRAY:
-   case MP_MAP: {
+   case MEM_TYPE_BIN:
+   case MEM_TYPE_ARRAY:
+   case MEM_TYPE_MAP: {
            char *zText = 0;
            char const *zBlob = mem_as_bin(argv[0]);
            int nBlob = mem_len_unsafe(argv[0]);
@@ -1146,7 +1152,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
            }
            break;
        }
-   case MP_STR:{
+   case MEM_TYPE_STR: {
            int i, j;
            u64 n;
            const unsigned char *zArg = mem_as_ustr(argv[0]);
@@ -1174,7 +1180,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
            }
            break;
        }
-   case MP_BOOL: {
+   case MEM_TYPE_BOOL: {
        sql_result_text(context,
                SQL_TOKEN_BOOLEAN(mem_get_bool_unsafe(argv[0])),
                -1, SQL_TRANSIENT);
diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index 693a477b7..94b0bb1f6 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -20,6 +20,9 @@
     "gh-3332-tuple-format-leak.test.lua": {
         "memtx": {"engine": "memtx"}
     },
+    "gh-6164-uuid-follow-ups.test.lua": {
+        "memtx": {"engine": "memtx"}
+    },
     "gh-4077-iproto-execute-no-bind.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
diff --git a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
new file mode 100755
index 000000000..a8f662f77
--- /dev/null
+++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua
@@ -0,0 +1,14 @@
+#!/usr/bin/env tarantool
+local test = require("sqltester")
+test:plan(1)
+
+-- Make sure that function quote() can work with uuid.
+test:do_execsql_test(
+    "gh-6164-1",
+    [[
+        SELECT quote(cast('11111111-1111-1111-1111-111111111111' as uuid));
+    ]], {
+        '11111111-1111-1111-1111-111111111111'
+    })
+
+test:finish_test()