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 192616EC55; Sat, 10 Jul 2021 17:35:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 192616EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1625927707; bh=g1gWcH5mMEMWdG3fDwugrZRqBLb8auvci7sjz0lIjSQ=; 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=KiWJxolqvP3lJQu8gFly3rRsyECkQoaiKWy2dm1TPXRoD0eEe07D3v7Nt/0avFXoj F+56MM4UD//BVdiPW5byWOThfZFRDubyrOfOuzwFPU025u/IuZOZLsSJlF2910kNx3 xwRlg8VtxrFhsujhKaqnaK6TyQKwE4+XNG3YYGpA= 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 59FF86EC6E for ; Sat, 10 Jul 2021 17:33:41 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 59FF86EC6E Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1m2E32-0007im-NJ; Sat, 10 Jul 2021 17:33:41 +0300 To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Sat, 10 Jul 2021 17:33:40 +0300 Message-Id: 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: 4F1203BC0FB41BD954DFF1DC42D673FB0C620705B15DE32D6B8174B3EFF2BD9A182A05F538085040779EEB3B8F93F038F0FC702E1BEA909F08912BBCCAAFCCAB8FB3B1E6D02540E7 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7922E451CE6E839B1EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063718A0EC59585977138638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8DE54D3B83FDD42962E9E7BE6AFAD0731117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18F04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B62CFFCC7B69C47339089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975C425F95675CFBD49F89FE0977AB20813ABA844FADF3101AB79C2B6934AE262D3EE7EAB7254005DCED114C52B35DBB74F4E7EAB7254005DCEDA5DF9383870C0FED1E0A4E2319210D9B64D260DF9561598FCFFBF5018520E39817A45118377F5F9E8E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34C5CF6F4B28551E3FB5254CF277EC86B75E5994E4BF34D289B93D28493D8B252548B6A9857227A01F1D7E09C32AA3244CF620F261075B11BE856CCF9144FE7BEBB4DF56057A86259F729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojwCsty+r9xmB619+FtDTiuQ== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5DFD86E7DB887EB175B37BD9E2710AAC5B83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v2 3/4] sql: introduce mem_cmp_scalar() 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" 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 | 14 +- src/box/sql/mem.c | 140 ++++++++---------- src/box/sql/mem.h | 10 +- src/box/sql/vdbe.c | 2 +- src/box/sql/where.c | 6 +- test/sql-tap/gh-6164-uuid-follow-ups.test.lua | 45 +++++- 6 files changed, 128 insertions(+), 89 deletions(-) diff --git a/src/box/sql/func.c b/src/box/sql/func.c index aa565277c..dc99bd390 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -144,11 +144,10 @@ 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); + int res; + mem_cmp_scalar(argv[iBest], argv[i], &res, pColl); + if ((res ^ mask) >= 0) iBest = i; - } } sql_result_value(context, argv[iBest]); } @@ -1057,9 +1056,10 @@ 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) { + int res; + mem_cmp_scalar(argv[0], argv[1], &res, pColl); + if (res != 0) sql_result_value(context, argv[0]); - } } /** @@ -1827,7 +1827,7 @@ 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); + mem_cmp_scalar(pBest, pArg, &cmp, pColl); 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 2595e2fd4..576596c9f 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -59,6 +59,40 @@ enum { BUF_SIZE = 32, }; +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) { @@ -2009,6 +2043,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 @@ -2440,82 +2504,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 b3cd5c545..bbb99c4d2 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -696,6 +696,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 @@ -961,8 +969,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 32d02d96e..220e8b269 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1828,7 +1828,7 @@ 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); + mem_cmp_scalar(&aMem[p1+idx], &aMem[p2+idx], &iCompare, coll); if (iCompare) { if (is_rev) iCompare = -iCompare; diff --git a/src/box/sql/where.c b/src/box/sql/where.c index e5f35fbf8..dadc6d4a2 100644 --- a/src/box/sql/where.c +++ b/src/box/sql/where.c @@ -1272,12 +1272,14 @@ 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); + int res; + mem_cmp_scalar(p1, pVal, &res, coll); if (res >= 0) nLower++; } if (rc == 0 && p2 != NULL) { - int res = sqlMemCompare(p2, pVal, coll); + int res; + mem_cmp_scalar(p2, pVal, &res, coll); if (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