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 A6B776C1AE; Mon, 17 May 2021 15:05:26 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A6B776C1AE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1621253126; bh=B9jE1hCvp77ZjhKpmPoCxfSKp3LOWZqt0aqgGD1DxMc=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=rESwA/zL88CPt2ZY4JtNguGg/4FMoZt3sr3oPgDJijwAnCT16sSnEtTz4Clte/K40 lvK9vTW0WQfu7SURTisNCzR2rM80YHAlf/x8ajkCSVyzRqIH/CQuQTSy9BiwfkrAr1 CYiSDfNQFv/EWql/ErY4bmy/2dcj5X8DgrGIkFas= Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [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 D47CB6C1AE for ; Mon, 17 May 2021 15:05:24 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D47CB6C1AE Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1libzv-00040y-T3; Mon, 17 May 2021 15:05:24 +0300 Date: Mon, 17 May 2021 15:05:22 +0300 To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210517120522.GB177059@tarantool.org> References: <73769b1c3a358ebdbdb603592bda2353c0e6b800.1619542456.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD95978C26455E69BE0FB16CA32063744B94D060C3A2F6991A4182A05F53808504025F86E431C40A3A51728C24660A613DAB0B902CDCA1C2A793C1F32467E67427C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE74C265300876DF183C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7E628FE8A185FCFBEEA1F7E6F0F101C67CDEEF6D7F21E0D1D9295C2E9FA3191EE1B59CA4C82EFA6583B46908D1BE4B8128A30E75CE89CAACEF6B57BC7E64490618DEB871D839B73339E8FC8737B5C22498424CA1AAF98A6958941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B68424CA1AAF98A6958941B15DA834481F9449624AB7ADAF372E808ACE2090B5E14AD6D5ED66289B5259CC434672EE63711DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C309A7649CC036878F35872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A5D2EA3A726E2A690971FA62BF5EB4E793874A7BE24A5DF241D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA752546FE575EB473F1410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34F1ADD4D8CD3C81CEDFD85085C3E41E7160EA405A0FEF651D84973F27667534AC571A3283BDDAE1EA1D7E09C32AA3244C26AC2D4E954ED23962F907450A7A0F276C24832127668422729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojlIWKWc0dtJ2DtH5Knu7Z4g== X-Mailru-Sender: 689FA8AB762F73936BC43F508A0638222A1A54DD37BBE800FB5A00C3515CD54983D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 2/3] sql: make mem_is_bin() to check only for VARBINARY 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: Mergen Imeev Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thank you for the review! My answers, diff and new patch below. On Thu, Apr 29, 2021 at 11:05:19PM +0200, Vladislav Shpilevoy wrote: > Thanks for working on this! > > See 11 comments below! > > On 27.04.2021 18:55, Mergen Imeev via Tarantool-patches wrote: > > After this patch, the mem_is_bin() function will return 'true' only if > > the value that the MEM contains is of type VARBINARY. This patch also > > adds the mem_is_bin_ext() function, which is used to check if a MEM > > contains value of type VARBINARY or value of types that are currently > > considered VARBINARY extensions - MAP and ARRAY. > > > > Part of #4906 > > --- > > src/box/sql/func.c | 10 +++++----- > > src/box/sql/mem.h | 8 +++++++- > > src/box/sql/vdbe.c | 16 ++++++++-------- > > 3 files changed, 20 insertions(+), 14 deletions(-) > > > > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > > index d282b2cea..bed7e8488 100644 > > --- a/src/box/sql/func.c > > +++ b/src/box/sql/func.c > > @@ -553,7 +553,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) > > } > > if (mem_is_null(argv[0])) > > return; > > - if (mem_is_bin(argv[0])) { > > + if (mem_is_bin_ext(argv[0])) { > > 1. The name is quite confusing. I spent some time thinking about a > better one, but can't find any. Maybe because such a function looks > very strange in its purpose, as well as some of its usages. > > Maybe better add a function mem_is_nested() to check for MP_ARRAY > and MP_MAP. Or mem_is_scalar(), which checks it is not MP_ARRAY and > not MP_MAP and you call it with negation. Or add special checks > for array and map. mem_is_bin_ext() would be either > > mem_is_bin() || mem_is_nested() > > or > > mem_is_bin() || !mem_is_scalar() > > or > > mem_is_bin() || mem_is_array() || mem_is_map() > Fixed. However, it turned out that you suggestion made diff even less, though you suggested to use two or three functions instead of one. > Then you would see something strange below. > > Why in ROUND() we check for types we do not accept instead of > checking the types we do accept? I mean, why not > > if (!mem_is_number() && !mem_is_str()) > return error; > > instead of > > if (mem_is_bin_ext()) > return error; > > What will happen when you will add UUID, date? Will you change > this on each new type like that?: > > if (mem_is_bin_ext() || mem_is_uuid() || mem_is_date() || ...) > return error; > > IMO would be easier to maintain a list of supported types than a > list of not supported types. Worth changing now? > I changed here, however I do not think that it should be done this way, since we have to take into account our implicit cast rules and change this code every time we will add something, that can be converted to value of accepted type. Instead, all check and convertions should be done before the function is called. This will be done in issue #4159, which was already partially done last year. Last time I wasn't able to complete this issue, but we were able to come to a conclusion about how should it be done. I think I will return to the issue in Q3 or Q4. > > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > > mem_str(argv[0]), "numeric"); > > context->is_aborted = true; > > @@ -613,7 +613,7 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \ > > const char *z2; \ > > int n; \ > > UNUSED_PARAMETER(argc); \ > > - if (mem_is_bin(argv[0])) { \ > > + if (mem_is_bin_ext(argv[0])) { \ > > 2. Ditto. > Fixed. > > diag_set(ClientError, ER_INCONSISTENT_TYPES, "text", \ > > "varbinary"); \ > > context->is_aborted = true; \ > > @@ -694,7 +694,7 @@ randomBlob(sql_context * context, int argc, sql_value ** argv) > > unsigned char *p; > > assert(argc == 1); > > UNUSED_PARAMETER(argc); > > - if (mem_is_bin(argv[0])) { > > + if (mem_is_bin_ext(argv[0])) { > > 3. Ditto. > Fixed. > > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > > mem_str(argv[0]), "numeric"); > > context->is_aborted = true; > > @@ -1455,7 +1455,7 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg) > > const unsigned char *default_trim; > > if (mem_is_null(arg)) > > return; > > - if (mem_is_bin(arg)) > > + if (mem_is_bin_ext(arg)) > > 4. TBH, I would ban MP_ARRAY and MP_MAP from TRIM(). It makes 0 > sense for them. I am not sure it is even tested. So we are not > talking about a notable 'behaviour change' really. > In current design TRIM() accepts any argument. I reverted this change, however it should not affect MAP and ARRAY values. > > default_trim = (const unsigned char *) "\0"; > > else > > default_trim = (const unsigned char *) " "; > > @@ -1584,7 +1584,7 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv) > > 1, 2, 6, 2, 3, 0, 1, 0, 2, 0, 2, 0, 0, 0, 0, 0, > > }; > > assert(argc == 1); > > - if (mem_is_bin(argv[0])) { > > + if (mem_is_bin_ext(argv[0])) { > > 5. Ditto. > Fixed. > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > > index 2308587e7..bedfa87af 100644 > > --- a/src/box/sql/vdbe.c > > +++ b/src/box/sql/vdbe.c > > @@ -1634,9 +1634,9 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ > > "boolean"); > > goto abort_due_to_error; > > } > > - } else if (mem_is_bin(pIn3) || mem_is_bin(pIn1)) { > > + } else if (mem_is_bin_ext(pIn3) || mem_is_bin_ext(pIn1)) { > > if (mem_cmp_bin(pIn3, pIn1, &res) != 0) { > > 6. Arrays and maps are not comparable by memcmp(). The same numbers > can be encoded in MessagePack as 8 bytes and as 1 byte (you can do that > legally, just not widely used). In 2 maps elements might be in different > order. And obviously you can't compare map and array. Such places would > draw more attention for future fixes if we wouldn't mask them behind > _ext() suffix but would rather use more explicit checks. > I added an error on attempt to compare MAP or ARRAY to anything. > > - char *str = !mem_is_bin(pIn3) ? > > + char *str = !mem_is_bin_ext(pIn3) ? > > mem_type_to_str(pIn3) : > > mem_type_to_str(pIn1); > > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str, > > @@ -2991,7 +2991,7 @@ case OP_Found: { /* jump, in3 */ > > } else { > > pFree = pIdxKey = sqlVdbeAllocUnpackedRecord(db, pC->key_def); > > if (pIdxKey==0) goto no_mem; > > - assert(mem_is_bin(pIn3)); > > + assert(mem_is_bin_ext(pIn3)); > > 7. This would be mem_is_array() AFAIU. Because it is a tuple, right? > It is true, that MEM contains MP_ARRAY. However, currently this MEM has VARBINARY type. I tried to change type to ARRAY, but found unexpected for me error: ARRAY cannot be inserted in SCALAR field, but VARBINARY can. Also, we cannot create an index that contains MAP or ARRAY.It means that after changing type to ARRAY, value created by OP_MakeRecord cannot be inserted to most of ephemeral spaces, since all their fields have VARBINARY type and their PK consist of all fields. I decided to drop my attempt and accept that currently all internally created arrays are of VARBINARY type. Also, this means that arrays and maps have undefined behaviour in our code, since their behaviour depends on the source of the value. So, for now most of maps and arrays we can just use mem_is_bin(), at least for now. I think that we have to rework out ephemeral spaces if we want to introduce support of MAP and ARRAY in SQL. > > (void)ExpandBlob(pIn3); > > sqlVdbeRecordUnpackMsgpack(pC->key_def, > > pIn3->z, pIdxKey); > > @@ -3253,7 +3253,7 @@ case OP_SorterData: { > > assert(isSorter(pC)); > > if (sqlVdbeSorterRowkey(pC, pOut) != 0) > > goto abort_due_to_error; > > - assert(mem_is_bin(pOut)); > > + assert(mem_is_bin_ext(pOut)); > > 8. Ditto. > Reverted. > > assert(pOp->p1>=0 && pOp->p1nCursor); > > p->apCsr[pOp->p3]->cacheStatus = CACHE_STALE; > > break; > > @@ -3616,7 +3616,7 @@ case OP_SorterInsert: { /* in2 */ > > assert(cursor != NULL); > > assert(isSorter(cursor)); > > pIn2 = &aMem[pOp->p2]; > > - assert(mem_is_bin(pIn2)); > > + assert(mem_is_bin_ext(pIn2)); > > 9. Ditto. > Reverted. > > if (ExpandBlob(pIn2) != 0 || > > sqlVdbeSorterWrite(cursor, pIn2) != 0) > > goto abort_due_to_error; > > @@ -3650,7 +3650,7 @@ case OP_SorterInsert: { /* in2 */ > > case OP_IdxReplace: > > case OP_IdxInsert: { > > pIn2 = &aMem[pOp->p1]; > > - assert(mem_is_bin(pIn2)); > > + assert(mem_is_bin_ext(pIn2)); > > 10. Ditto. > Reverted. > > if (ExpandBlob(pIn2) != 0) > > goto abort_due_to_error; > > struct space *space; > > @@ -3741,10 +3741,10 @@ case OP_Update: { > > assert(pOp->p4type == P4_SPACEPTR); > > > > struct Mem *key_mem = &aMem[pOp->p2]; > > - assert(mem_is_bin(key_mem)); > > + assert(mem_is_bin_ext(key_mem)); > > 11. Ditto here and below. > Reverted. > > struct Mem *upd_fields_mem = &aMem[pOp->p3]; > > - assert(mem_is_bin(upd_fields_mem)); > > + assert(mem_is_bin_ext(upd_fields_mem)); > > uint32_t *upd_fields = (uint32_t *)upd_fields_mem->z; > > uint32_t upd_fields_cnt = upd_fields_mem->n / sizeof(uint32_t); Diff: diff --git a/src/box/sql/func.c b/src/box/sql/func.c index ae5ead67e..90e8e152f 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -553,7 +553,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) } if (mem_is_null(argv[0])) return; - if (mem_is_bin_ext(argv[0])) { + if (!mem_is_num(argv[0]) && !mem_is_str(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "numeric"); context->is_aborted = true; @@ -613,7 +613,8 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \ const char *z2; \ int n; \ UNUSED_PARAMETER(argc); \ - if (mem_is_bin_ext(argv[0])) { \ + if (mem_is_bin(argv[0]) || mem_is_map(argv[0]) || \ + mem_is_array(argv[0])) { \ diag_set(ClientError, ER_INCONSISTENT_TYPES, "text", \ "varbinary"); \ context->is_aborted = true; \ @@ -694,7 +695,8 @@ randomBlob(sql_context * context, int argc, sql_value ** argv) unsigned char *p; assert(argc == 1); UNUSED_PARAMETER(argc); - if (mem_is_bin_ext(argv[0])) { + if (mem_is_bin(argv[0]) || mem_is_map(argv[0]) || + mem_is_array(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "numeric"); context->is_aborted = true; @@ -1455,7 +1457,7 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg) const unsigned char *default_trim; if (mem_is_null(arg)) return; - if (mem_is_bin_ext(arg)) + if (mem_is_bin(arg)) default_trim = (const unsigned char *) "\0"; else default_trim = (const unsigned char *) " "; @@ -1584,7 +1586,8 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv) 1, 2, 6, 2, 3, 0, 1, 0, 2, 0, 2, 0, 0, 0, 0, 0, }; assert(argc == 1); - if (mem_is_bin_ext(argv[0])) { + if (mem_is_bin(argv[0]) || mem_is_map(argv[0]) || + mem_is_array(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "text"); context->is_aborted = true; diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 6fc15617d..526b6bf3e 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -206,12 +206,6 @@ mem_is_array(const struct Mem *mem) mp_typeof(*mem->z) == MP_ARRAY; } -static inline bool -mem_is_bin_ext(const struct Mem *mem) -{ - return (mem->flags & MEM_Blob) != 0; -} - static inline bool mem_is_agg(const struct Mem *mem) { diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index bedfa87af..12ec703a2 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1634,15 +1634,20 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ "boolean"); goto abort_due_to_error; } - } else if (mem_is_bin_ext(pIn3) || mem_is_bin_ext(pIn1)) { + } else if (mem_is_bin(pIn3) || mem_is_bin(pIn1)) { if (mem_cmp_bin(pIn3, pIn1, &res) != 0) { - char *str = !mem_is_bin_ext(pIn3) ? + char *str = !mem_is_bin(pIn3) ? mem_type_to_str(pIn3) : mem_type_to_str(pIn1); diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str, "varbinary"); goto abort_due_to_error; } + } else if (mem_is_map(pIn3) || mem_is_map(pIn1) || mem_is_array(pIn3) || + mem_is_array(pIn1)) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_type_to_str(pIn3), mem_type_to_str(pIn1)); + goto abort_due_to_error; } else if (type == FIELD_TYPE_STRING) { if (mem_cmp_str(pIn3, pIn1, &res, pOp->p4.pColl) != 0) { const char *str = @@ -2991,7 +2996,7 @@ case OP_Found: { /* jump, in3 */ } else { pFree = pIdxKey = sqlVdbeAllocUnpackedRecord(db, pC->key_def); if (pIdxKey==0) goto no_mem; - assert(mem_is_bin_ext(pIn3)); + assert(mem_is_bin(pIn3)); (void)ExpandBlob(pIn3); sqlVdbeRecordUnpackMsgpack(pC->key_def, pIn3->z, pIdxKey); @@ -3253,7 +3258,7 @@ case OP_SorterData: { assert(isSorter(pC)); if (sqlVdbeSorterRowkey(pC, pOut) != 0) goto abort_due_to_error; - assert(mem_is_bin_ext(pOut)); + assert(mem_is_bin(pOut)); assert(pOp->p1>=0 && pOp->p1nCursor); p->apCsr[pOp->p3]->cacheStatus = CACHE_STALE; break; @@ -3616,7 +3621,7 @@ case OP_SorterInsert: { /* in2 */ assert(cursor != NULL); assert(isSorter(cursor)); pIn2 = &aMem[pOp->p2]; - assert(mem_is_bin_ext(pIn2)); + assert(mem_is_bin(pIn2)); if (ExpandBlob(pIn2) != 0 || sqlVdbeSorterWrite(cursor, pIn2) != 0) goto abort_due_to_error; @@ -3650,7 +3655,7 @@ case OP_SorterInsert: { /* in2 */ case OP_IdxReplace: case OP_IdxInsert: { pIn2 = &aMem[pOp->p1]; - assert(mem_is_bin_ext(pIn2)); + assert(mem_is_bin(pIn2)); if (ExpandBlob(pIn2) != 0) goto abort_due_to_error; struct space *space; @@ -3741,10 +3746,10 @@ case OP_Update: { assert(pOp->p4type == P4_SPACEPTR); struct Mem *key_mem = &aMem[pOp->p2]; - assert(mem_is_bin_ext(key_mem)); + assert(mem_is_bin(key_mem)); struct Mem *upd_fields_mem = &aMem[pOp->p3]; - assert(mem_is_bin_ext(upd_fields_mem)); + assert(mem_is_bin(upd_fields_mem)); uint32_t *upd_fields = (uint32_t *)upd_fields_mem->z; uint32_t upd_fields_cnt = upd_fields_mem->n / sizeof(uint32_t); Patch: commit f6ad3ac1c548a2ffbfbe4b103b533f48db6458fa Author: Mergen Imeev Date: Tue Apr 27 15:12:33 2021 +0300 sql: make mem_is_bin() to check only for VARBINARY After this patch, the mem_is_bin() function will return 'true' only if the value that the MEM contains is of type VARBINARY. This patch also adds the mem_is_bin_ext() function, which is used to check if a MEM contains value of type VARBINARY or value of types that are currently considered VARBINARY extensions - MAP and ARRAY. Part of #4906 diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 94302cc56..90e8e152f 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -553,7 +553,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) } if (mem_is_null(argv[0])) return; - if (mem_is_bin(argv[0])) { + if (!mem_is_num(argv[0]) && !mem_is_str(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "numeric"); context->is_aborted = true; @@ -613,7 +613,8 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \ const char *z2; \ int n; \ UNUSED_PARAMETER(argc); \ - if (mem_is_bin(argv[0])) { \ + if (mem_is_bin(argv[0]) || mem_is_map(argv[0]) || \ + mem_is_array(argv[0])) { \ diag_set(ClientError, ER_INCONSISTENT_TYPES, "text", \ "varbinary"); \ context->is_aborted = true; \ @@ -694,7 +695,8 @@ randomBlob(sql_context * context, int argc, sql_value ** argv) unsigned char *p; assert(argc == 1); UNUSED_PARAMETER(argc); - if (mem_is_bin(argv[0])) { + if (mem_is_bin(argv[0]) || mem_is_map(argv[0]) || + mem_is_array(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "numeric"); context->is_aborted = true; @@ -1584,7 +1586,8 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv) 1, 2, 6, 2, 3, 0, 1, 0, 2, 0, 2, 0, 0, 0, 0, 0, }; assert(argc == 1); - if (mem_is_bin(argv[0])) { + if (mem_is_bin(argv[0]) || mem_is_map(argv[0]) || + mem_is_array(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "text"); context->is_aborted = true; diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 1db7f4deb..526b6bf3e 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -184,7 +184,7 @@ mem_is_bool(const struct Mem *mem) static inline bool mem_is_bin(const struct Mem *mem) { - return (mem->flags & MEM_Blob) != 0; + return (mem->flags & MEM_Blob) != 0 && (mem->flags & MEM_Subtype) == 0; } static inline bool diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 2308587e7..12ec703a2 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1643,6 +1643,11 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ "varbinary"); goto abort_due_to_error; } + } else if (mem_is_map(pIn3) || mem_is_map(pIn1) || mem_is_array(pIn3) || + mem_is_array(pIn1)) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + mem_type_to_str(pIn3), mem_type_to_str(pIn1)); + goto abort_due_to_error; } else if (type == FIELD_TYPE_STRING) { if (mem_cmp_str(pIn3, pIn1, &res, pOp->p4.pColl) != 0) { const char *str =