* [Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote()
2021-07-16 8:57 [Tarantool-patches] [PATCH v2 0/4] Follow ups for uuid introduction Mergen Imeev via Tarantool-patches
@ 2021-07-16 8:57 ` Mergen Imeev via Tarantool-patches
2021-07-19 9:16 ` Timur Safin via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values Mergen Imeev via Tarantool-patches
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-16 8:57 UTC (permalink / raw)
To: tsafin; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote()
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote() Mergen Imeev via Tarantool-patches
@ 2021-07-19 9:16 ` Timur Safin via Tarantool-patches
0 siblings, 0 replies; 12+ messages in thread
From: Timur Safin via Tarantool-patches @ 2021-07-19 9:16 UTC (permalink / raw)
To: imeevma; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values
2021-07-16 8:57 [Tarantool-patches] [PATCH v2 0/4] Follow ups for uuid introduction Mergen Imeev via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote() Mergen Imeev via Tarantool-patches
@ 2021-07-16 8:57 ` Mergen Imeev via Tarantool-patches
2021-07-19 9:16 ` Timur Safin via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar() Mergen Imeev via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack() Mergen Imeev via Tarantool-patches
3 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-16 8:57 UTC (permalink / raw)
To: tsafin; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values Mergen Imeev via Tarantool-patches
@ 2021-07-19 9:16 ` Timur Safin via Tarantool-patches
0 siblings, 0 replies; 12+ messages in thread
From: Timur Safin via Tarantool-patches @ 2021-07-19 9:16 UTC (permalink / raw)
To: imeevma; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar()
2021-07-16 8:57 [Tarantool-patches] [PATCH v2 0/4] Follow ups for uuid introduction Mergen Imeev via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 1/4] sql: introduce uuid to quote() Mergen Imeev via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values Mergen Imeev via Tarantool-patches
@ 2021-07-16 8:57 ` Mergen Imeev via Tarantool-patches
2021-07-19 9:17 ` Timur Safin via Tarantool-patches
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack() Mergen Imeev via Tarantool-patches
3 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-16 8:57 UTC (permalink / raw)
To: tsafin; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar()
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar() Mergen Imeev via Tarantool-patches
@ 2021-07-19 9:17 ` Timur Safin via Tarantool-patches
0 siblings, 0 replies; 12+ messages in thread
From: Timur Safin via Tarantool-patches @ 2021-07-19 9:17 UTC (permalink / raw)
To: imeevma; +Cc: 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)
: {
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()
2021-07-16 8:57 [Tarantool-patches] [PATCH v2 0/4] Follow ups for uuid introduction Mergen Imeev via Tarantool-patches
` (2 preceding siblings ...)
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar() Mergen Imeev via Tarantool-patches
@ 2021-07-16 8:57 ` Mergen Imeev via Tarantool-patches
2021-07-19 9:16 ` Timur Safin via Tarantool-patches
3 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-16 8:57 UTC (permalink / raw)
To: tsafin; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()
2021-07-16 8:57 ` [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack() Mergen Imeev via Tarantool-patches
@ 2021-07-19 9:16 ` Timur Safin via Tarantool-patches
2021-07-19 10:07 ` Mergen Imeev via Tarantool-patches
0 siblings, 1 reply; 12+ messages in thread
From: Timur Safin via Tarantool-patches @ 2021-07-19 9:16 UTC (permalink / raw)
To: imeevma; +Cc: 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()
2021-07-19 9:16 ` Timur Safin via Tarantool-patches
@ 2021-07-19 10:07 ` Mergen Imeev via Tarantool-patches
0 siblings, 0 replies; 12+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-19 10:07 UTC (permalink / raw)
To: Timur Safin; +Cc: 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
>
^ permalink raw reply [flat|nested] 12+ messages in thread