From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 7CD1B6EC55; Sat, 10 Jul 2021 17:34:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7CD1B6EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1625927677; bh=ONU6kOESLvHGPIcLZz4RbxsHWZNN6GZrPg5eNaivZRg=; h=To:Cc:Date:In-Reply-To:References:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=R47p1EDpxEdffTa6n4vlmqkf9xQljOa/NxWoB2ApVwC1osWMb3MLAJvG/fC+GF2sP PjlPsQLYRgFSi2cx806tNIgtYaWSDlwt5Rm4/+QeoPYBqkM/PmDeHSGgg7GzIlxuBH MBZ+RN/o1QlhXpEM+hJQpROz30FLPRiMyamwyhPc= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A1B556EC5C for ; Sat, 10 Jul 2021 17:33:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A1B556EC5C Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1m2E30-0007i5-Tz; Sat, 10 Jul 2021 17:33:39 +0300 To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Sat, 10 Jul 2021 17:33:38 +0300 Message-Id: <8faa40c89f027dbb9aa8c8d7551b6b1aa214ed88.1625926838.git.imeevma@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD954DFF1DC42D673FB0C620705B15DE32D6B8174B3EFF2BD9A182A05F538085040962EED4FD03AB9FCA065E33C60567307EBFC8F308FCD495F0D0824A6B6C2F41D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE755BE8F535441E38CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006376B6794A086D6ADA58638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D83AFD513C1395D6B8F248ACDC6AF69DCE117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B2EE5AD8F952D28FBA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249EC8D19AE6D49635B68655334FD4449CB9ECD01F8117BC8BEAAAE862A0553A39223F8577A6DFFEA7CD1D040B6C1ECEA3F43847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C425F95675CFBD49FDD1628BA2CD50F1444966634D87752EA9C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF309DFB797F6729CB699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3474B2583E51315984FC49063B3837A8E5AFB7A134159D5B1A4CA38ABCD5E1DDF8B6C1FB9192147DD21D7E09C32AA3244C8D26C70B83651FE3BB37D51F8ED5B2228580396430872480729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojwCsty+r9xmDvFc+XiFtjCQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D1D47921119A4EB8715EBF7EFED7C5F1B83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok X-PR7D5YW: Astghik Subject: [Tarantool-patches] [PATCH v2 2/4] sql: allow to bind uuid values X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: imeevma@tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thank you for the review! My answers, diff and new patch below. However, this diff not exactly right, since the patch also changed due to dropping of previous patch of patch-set. On 08.07.2021 00:44, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 4 comments below. > > On 05.07.2021 17:06, Mergen Imeev via Tarantool-patches wrote: >> After this patch, uuid values can be binded like any other supported by > > 1. binded -> bound. > Fixed. >> diff --git a/src/box/bind.h b/src/box/bind.h >> index 568c558f3..20f3e7942 100644 >> --- a/src/box/bind.h >> +++ b/src/box/bind.h >> @@ -40,6 +40,8 @@ extern "C" { >> #include >> >> #include "msgpuck.h" >> +#include "uuid/tt_uuid.h" >> +#include "lib/core/mp_extension_types.h" > > 2. lib/core is already in the paths. You can omit it. > Fixed. >> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c >> index aaae12e41..a9700350d 100644 >> --- a/src/box/sql/vdbeapi.c >> +++ b/src/box/sql/vdbeapi.c >> @@ -840,6 +840,17 @@ 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) >> + return -1; >> + int rc = sql_bind_type(p, i, "uuid"); >> + mem_set_uuid(&p->aVar[i - 1], uuid); > > 3. Why do you set UUID even if bind_type() failed? > It happened because I used sql_bind_uint64() as prototype and did not noticed that there were an error. Fixed. >> 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 8872f9f23..426717972 100755 >> --- a/test/sql-tap/gh-6164-uuid-follow-ups.test.lua >> +++ b/test/sql-tap/gh-6164-uuid-follow-ups.test.lua >> @@ -32,4 +32,18 @@ test:do_execsql_test( >> true >> }) >> >> +-- Make sure that uuid value can be binded. >> +box.execute('CREATE TABLE t(i INT PRIMARY KEY, a UUID);') >> +box.execute('INSERT INTO t VALUES(1, ?);', {uuid}); > > 4. Do you need a table for that? Can you just make 'SELECT ?' with > the uuid argument? > True, fixed. >> + >> +test:do_execsql_test( >> + "gh-6164-4", >> + [[ >> + SELECT * FROM t; >> + ]], { >> + 1, uuid >> + }) >> + >> +box.execute([[DROP TABLE t;]]) >> + >> test:finish_test() >> Diff: commit daf89852f18bc3dbb3255dfd785182da5ef81e93 Author: Mergen Imeev Date: Sat Jul 10 14:50:53 2021 +0300 Review fix diff --git a/src/box/bind.h b/src/box/bind.h index 20f3e7942..143f010ce 100644 --- a/src/box/bind.h +++ b/src/box/bind.h @@ -41,7 +41,7 @@ extern "C" { #include "msgpuck.h" #include "uuid/tt_uuid.h" -#include "lib/core/mp_extension_types.h" +#include "mp_extension_types.h" struct sql_stmt; diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index a9700350d..8031ee0dc 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -844,11 +844,10 @@ 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) + if (vdbeUnbind(p, i) != 0 || sql_bind_type(p, i, "uuid") != 0) return -1; - int rc = sql_bind_type(p, i, "uuid"); mem_set_uuid(&p->aVar[i - 1], uuid); - return rc; + return 0; } int 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 822311237..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,8 +1,6 @@ #!/usr/bin/env tarantool local test = require("sqltester") -test:plan(2) - -local uuid = require('uuid').fromstr('11111111-1111-1111-1111-111111111111') +test:plan(4) -- Make sure that function quote() can work with uuid. test:do_execsql_test( @@ -14,17 +12,27 @@ test:do_execsql_test( }) -- Make sure that uuid value can be binded. -box.execute('CREATE TABLE t(i INT PRIMARY KEY, a UUID);') -box.execute('INSERT INTO t VALUES(1, ?);', {uuid}); +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_execsql_test( +test:do_test( "gh-6164-4", - [[ - SELECT * FROM t; - ]], { - 1, uuid - }) - -box.execute([[DROP TABLE t;]]) + function() + return box.execute([[SELECT :two;]], {{[":two"] = uuid3}}).rows[1][1] + end, + uuid3) test:finish_test() New patch: commit 8faa40c89f027dbb9aa8c8d7551b6b1aa214ed88 Author: Mergen Imeev Date: Mon Jul 5 11:57:35 2021 +0300 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/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 #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()