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 804486EC5F; Fri, 9 Apr 2021 20:37:25 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 804486EC5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617989845; bh=Yr+0W7AYOZe/oWN251TjFOOMYG7fVHIDIFxLGKSnpG4=; 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=olrgTw1yNi35FlRkFghO8/PbtCOXFK09NwkCXoqwaYaPvb1+SET2ZiZ3PSQrLWRY4 LkJT74zhyKbFm+cVylAibeEAxPauFNIkWdwRdEsSI8v0oXMesbJctojps0ajUzxZTA WLfIe5GwRUf8vc0Xp2gnjj7vys8BVBuKhD3w5wbU= Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 CB3EC6EC5F for ; Fri, 9 Apr 2021 20:36:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CB3EC6EC5F Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lUv3u-0007VK-NL; Fri, 09 Apr 2021 20:36:55 +0300 To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Fri, 9 Apr 2021 20:36:54 +0300 Message-Id: <6724ff5d04df88fc611d4f921e6bb0b541cd5863.1617984948.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-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E7480EBD5CA77A668ECB87DA2124B0A8E6609182A05F53808504093DA91266C0FAF327BFD00D621CB76EA51F57DCB9EA1BBB10F820CDA6C6BB3B4 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7519D5B419DD3416AEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006372521E7C1CE72986C8638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2E176D2FD0289E6984FC4227D82450B3B3EADAC849D614696D2E47CDBA5A96583C09775C1D3CA48CFCA5A41EBD8A3A0199FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C317B107DEF921CE79117882F4460429728AD0CFFFB425014E868A13BD56FB6657E2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8BDC0F6C5B2EEF3D0C75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975CD0035DD76F8A8A4F1FD798B41F673AF1FE8E90E255BA1B919C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF0417BEADF48D1460699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34BF5454112BD5BFD7A58AA53A68F17ECC60BE3887EB220E0C2A6306D8FE356BC68883A99ED7A971BF1D7E09C32AA3244C45F4418C8180C95EC4D9F143AC49317D24AF4FAF06DA24FDFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyO2lHpuZu4SowrqkU+xg8A== X-Mailru-Sender: 689FA8AB762F73936BC43F508A06382210098BA2A126DE0727868F5017D4322C83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v5 12/52] sql: introduce mem_is_*() functions() 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 and new patch below. On 30.03.2021 02:01, Vladislav Shpilevoy wrote: > Thanks for working on this! > > I have a general comment affecting this entire naming schema. > The names seem too long. Besides, we already had some mem_is_*() > and mem_set_*() functions before this patch, which used short > names for the types. > > What I mean is 'integer' -> 'int', 'boolean' -> 'bool', > 'string' -> 'str', 'binary' -> 'bin', and so on. > I believe I fixed for most of mem_is_*(), mem_set_*() and mem_get_*() functions. Some functions still have long names, for example mem_is_ephemeral(). I didn't come to proper short name for these functions. > For the integers we have several functions because we split > unsigned, signed, and always negative integers. So we would > need more int-like names. For instance, > > mem_set_uint(uint64_t) - for MEM_UInt. > mem_set_nint(int64_t) - for MEM_Int. > mem_set_int(int64_t) - for both, checks the sign inside. > mem_set_sint(int64_t, bool) - for both, takes the sign flag > in the second argument > > This can be discussed. The main point - shorter is better IMO. > I do not hink that splitting is needed. I see it more like field_type -> name of function + some functions for internal use. > See 14 comments below. > > On 23.03.2021 10:35, Mergen Imeev via Tarantool-patches wrote: >> This patch introduces mem_is_*() functions that allows to check current >> MEM state. >> >> Part of #5818 >> --- >> diff --git a/src/box/sql/func.c b/src/box/sql/func.c >> index e600a9800..81b537d9b 100644 >> --- a/src/box/sql/func.c >> +++ b/src/box/sql/func.c >> @@ -736,8 +736,8 @@ substrFunc(sql_context * context, int argc, sql_value ** argv) >> context->is_aborted = true; >> return; >> } >> - if (sql_value_is_null(argv[1]) >> - || (argc == 3 && sql_value_is_null(argv[2])) >> + if (mem_is_null(argv[1]) >> + || (argc == 3 && mem_is_null(argv[2])) > > 1. This is not movement of huge code blocks, it is rather new code > now. And in the new code better use our code style. Such as > > - || goes in the end of line, not in the beginning of a next line; > - Unary operators don't have a whitespace after them; > - Comparison with NULL should be explicit, no implicit boot casts. > > I mark the places below which I was able to find after a swift look. > Thanks, fixed. >> ) { >> return; >> } >> @@ -1578,13 +1576,13 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv) >> assert(zStr == sql_value_text(argv[0])); /* No encoding change */ >> zPattern = sql_value_text(argv[1]); >> if (zPattern == 0) { >> - assert(sql_value_is_null(argv[1]) >> + assert(mem_is_null(argv[1]) >> || sql_context_db_handle(context)->mallocFailed); >> return; >> } >> nPattern = sql_value_bytes(argv[1]); >> if (nPattern == 0) { >> - assert(! sql_value_is_null(argv[1])); >> + assert(! mem_is_null(argv[1])); > > 2. Whitespace after unary operator. > Fixed. >> sql_result_value(context, argv[0]); >> return; >> } >> @@ -2039,7 +2035,7 @@ countStep(sql_context * context, int argc, sql_value ** argv) >> return; >> } >> p = sql_aggregate_context(context, sizeof(*p)); >> - if ((argc == 0 || ! sql_value_is_null(argv[0])) && p) { >> + if ((argc == 0 || ! mem_is_null(argv[0])) && p) { > > 3. Ditto. > Fixed. >> p->n++; >> } >> } >> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c >> index ec6aaab64..abc9291ef 100644 >> --- a/src/box/sql/mem.c >> +++ b/src/box/sql/mem.c >> @@ -37,6 +37,142 @@ >> #include "box/tuple.h" >> #include "mpstream/mpstream.h" >> >> +bool >> +mem_is_null(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Null) != 0; >> +} > > 4. Maybe better move them all to mem.h. These one-liners easily > can be inlined (the ones which are <= 3 lines long could be moved). > In one of the patches I move MEM types to mem.c so they are not visible from outside anymore. I think it is right way, at least for now. We may return MEM types back after we convert them to enum, so there won't be a possiblity of setting two or more MEM types at the same moment. If we do so, then these mem_is_*() functions won't be as importans as now and we may make them inlined or even get rid of them. >> + >> +bool >> +mem_is_unsigned(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_UInt) != 0; >> +} >> + >> +bool >> +mem_is_string(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Str) != 0; >> +} >> + >> +bool >> +mem_is_number(const struct Mem *mem) >> +{ >> + return (mem->flags & (MEM_Real | MEM_Int |MEM_UInt)) != 0; > > 5. Missed whitespace after the last '|'. > Fixed. >> +} >> + >> +bool >> +mem_is_double(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Real) != 0; >> +} >> + >> +bool >> +mem_is_integer(const struct Mem *mem) >> +{ >> + return (mem->flags & (MEM_Int | MEM_UInt)) != 0; >> +} >> + >> +bool >> +mem_is_boolean(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Bool) != 0; >> +} >> + >> +bool >> +mem_is_binary(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Blob) != 0; >> +} >> + >> +bool >> +mem_is_map(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Blob) != 0 && >> + (mem->flags & MEM_Subtype) != 0 && >> + mem->subtype == SQL_SUBTYPE_MSGPACK && >> + mp_typeof(*mem->z) == MP_MAP; >> +} >> + >> +bool >> +mem_is_array(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Blob) != 0 && >> + (mem->flags & MEM_Subtype) != 0 && >> + mem->subtype == SQL_SUBTYPE_MSGPACK && >> + mp_typeof(*mem->z) == MP_ARRAY; >> +} >> + >> +bool >> +mem_is_aggregate(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Agg) != 0; >> +} >> + >> +bool >> +mem_is_varstring(const struct Mem *mem) >> +{ >> + return (mem->flags & (MEM_Blob | MEM_Str)) != 0; > > 6. It does not look right to call it varstring if it includes > binary. A string is always binary, but not each binary object > is a string. > > Maybe mem_is_bytes()? mem_is_bytearray()? > Thank you. I changed 'varstring' to 'bytes' everywhere. >> +} >> + >> +bool >> +mem_is_frame(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Frame) != 0; >> +} >> + >> +bool >> +mem_is_undefined(const struct Mem *mem) >> +{ >> + return (mem->flags & MEM_Undefined) != 0; >> +} >> + >> +bool >> +mem_is_static(const struct Mem *mem) >> +{ >> + return (mem->flags & (MEM_Str | MEM_Blob)) != 0 && >> + (mem->flags & MEM_Static) != 0; >> +} >> + >> +bool >> +mem_is_ephemeral(const struct Mem *mem) >> +{ >> + return (mem->flags & (MEM_Str | MEM_Blob)) != 0 && >> + (mem->flags & MEM_Ephem) != 0; > > 7. How can it be that MEM_Ephem is set, but Str/Blob are not? > There is actually a possiblity. After sqlVdbeMemAboutToChange() is called MEM may become invalid after which MEM_Undefined is changed to MEM_Ephem and then MEM become valid again. For now I disable this SCopyFrom mechanism, but did not remove it completely. May be we will enable it later. >> +} >> + >> +bool >> +mem_is_dynamic(const struct Mem *mem) >> +{ >> + return (mem->flags & (MEM_Str | MEM_Blob)) != 0 && >> + (mem->flags & MEM_Dyn) != 0; >> +} >> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c >> index 12712efb4..05e0f78c1 100644 >> --- a/src/box/sql/vdbe.c >> +++ b/src/box/sql/vdbe.c >> @@ -1088,10 +1086,10 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ >> * Concatenation operation can be applied only to >> * strings and blobs. >> */ >> - uint32_t str_type_p1 = pIn1->flags & (MEM_Blob | MEM_Str); >> - uint32_t str_type_p2 = pIn2->flags & (MEM_Blob | MEM_Str); >> - if (str_type_p1 == 0 || str_type_p2 == 0) { >> - char *inconsistent_type = str_type_p1 == 0 ? >> + bool str_type_p1 = mem_is_varstring(pIn1); >> + bool str_type_p2 = mem_is_varstring(pIn2); > > 8. They are not types now. Only flags. Should be renamed to something > more appropriate. > I removed them since they were used in two times only. And the second time is in case of an error. Fixed, now they are proper types. >> + if (!str_type_p1 || !str_type_p2) { >> + char *inconsistent_type = !str_type_p1 ? >> mem_type_to_str(pIn1) : >> mem_type_to_str(pIn2); >> diag_set(ClientError, ER_INCONSISTENT_TYPES, >> @@ -1100,7 +1098,7 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ >> } >> >> /* Moreover, both operands must be of the same type. */ >> - if (str_type_p1 != str_type_p2) { >> + if (mem_is_string(pIn1) != mem_is_string(pIn2)) { > > 9. I would recommend mem_is_same_type(). Up to you. > Thanks, fixed by using mem_is_same_type(). >> diag_set(ClientError, ER_INCONSISTENT_TYPES, >> mem_type_to_str(pIn2), mem_type_to_str(pIn1)); >> goto abort_due_to_error; >> @@ -1186,14 +1183,16 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ >> pIn2 = &aMem[pOp->p2]; >> type2 = numericType(pIn2); >> pOut = vdbe_prepare_null_out(p, pOp->p3); >> - flags = pIn1->flags | pIn2->flags; >> - if ((flags & MEM_Null)!=0) goto arithmetic_result_is_null; >> + if (mem_is_null(pIn1) || mem_is_null(pIn2)) >> + goto arithmetic_result_is_null; >> if ((type1 & (MEM_Int | MEM_UInt)) != 0 && >> (type2 & (MEM_Int | MEM_UInt)) != 0) { >> iA = pIn1->u.i; >> iB = pIn2->u.i; >> - bool is_lhs_neg = pIn1->flags & MEM_Int; >> - bool is_rhs_neg = pIn2->flags & MEM_Int; >> + bool is_lhs_neg = mem_is_integer(pIn1) && >> + !mem_is_unsigned(pIn1); >> + bool is_rhs_neg = mem_is_integer(pIn2) && >> + !mem_is_unsigned(pIn2); > > 10. The checks look overcomplicated. Worth adding mem_is_nint()? > Still not sure. Also, this part will be moved to mem.c in a few patches where we do not have any problems with determining sign of INTEGER. >> bool is_res_neg; >> switch( pOp->opcode) { >> case OP_Add: { >> @@ -1509,7 +1508,7 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ >> pIn1 = &aMem[pOp->p1]; >> pIn2 = &aMem[pOp->p2]; >> pOut = vdbe_prepare_null_out(p, pOp->p3); >> - if ((pIn1->flags | pIn2->flags) & MEM_Null) { >> + if (mem_is_null(pIn1) || mem_is_null(pIn2)) { > > 11. This is at least third time I see the check of kind "one of them is null". > And I see more below. Probably worth adding a function which would do it more > efficient, in one check: mem_is_any_null(mem1, mem2) or something. Up to you. > Thanks, added mem_is_any_null(). >> @@ -1757,11 +1756,10 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ >> * or not both operands are null. >> */ >> assert(pOp->opcode==OP_Eq || pOp->opcode==OP_Ne); >> - assert((flags1 & MEM_Cleared)==0); >> + assert(!mem_is_cleared(pIn1)); >> assert((pOp->p5 & SQL_JUMPIFNULL)==0); >> - if ((flags1&flags3&MEM_Null)!=0 >> - && (flags3&MEM_Cleared)==0 >> - ) { >> + if (mem_is_null(pIn1) && mem_is_null(pIn3) && > > 12. You already know pIn1 or pIn3 is NULL from the 'if' above. So it > would be just a bit faster and easier to do mem_is_same_type(). Only > one branch. > Thanks, fixed. >> + !mem_is_cleared(pIn3)) { >> res = 0; /* Operands are equal */ >> } else { >> res = 1; /* Operands are not equal */ >> @@ -2982,18 +2965,18 @@ case OP_SeekGT: { /* jump, in3 */ >> * the seek, so convert it. >> */ >> pIn3 = &aMem[int_field]; >> - if ((pIn3->flags & MEM_Null) != 0) >> + if (mem_is_null(pIn3)) >> goto skip_truncate; >> - if ((pIn3->flags & MEM_Str) != 0) >> + if (mem_is_string(pIn3)) >> mem_apply_numeric_type(pIn3); >> int64_t i; >> - if ((pIn3->flags & MEM_Int) == MEM_Int) { >> + if (mem_is_integer(pIn3) && !mem_is_unsigned(pIn3)) { > > 13. Better be mem_is_nint(), mentioned in one of the previous > comments. > Again, not sure about this. Also, this part too will be moved to mem.c in a few patches. >> @@ -3352,11 +3336,11 @@ case OP_FCopy: { /* out2 */ >> pIn1 = &aMem[pOp->p1]; >> } >> >> - if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) && (pIn1->flags & MEM_Null)) { >> + if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) && mem_is_null(pIn1)) { > > 14. Should be explicit != 0. > Fixed. >> pOut = vdbe_prepare_null_out(p, pOp->p2); >> } else { >> assert(memIsValid(pIn1)); >> - assert((pIn1->flags & (MEM_Int | MEM_UInt)) != 0); >> + assert(mem_is_integer(pIn1)); New patch: commit 6724ff5d04df88fc611d4f921e6bb0b541cd5863 Author: Mergen Imeev Date: Tue Mar 2 11:27:48 2021 +0300 sql: introduce mem_is_*() functions() This patch introduces mem_is_*() functions that allows to check current MEM state. Part of #5818 diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 13d8dd32c..a0108220f 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -92,10 +92,10 @@ minmaxFunc(sql_context * context, int argc, sql_value ** argv) pColl = sqlGetFuncCollSeq(context); assert(mask == -1 || mask == 0); iBest = 0; - if (sql_value_is_null(argv[0])) + if (mem_is_null(argv[0])) return; for (i = 1; i < argc; i++) { - if (sql_value_is_null(argv[i])) + if (mem_is_null(argv[i])) return; if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) >= 0) { @@ -430,11 +430,8 @@ substrFunc(sql_context * context, int argc, sql_value ** argv) context->is_aborted = true; return; } - if (sql_value_is_null(argv[1]) - || (argc == 3 && sql_value_is_null(argv[2])) - ) { + if (mem_is_null(argv[1]) || (argc == 3 && mem_is_null(argv[2]))) return; - } p0type = sql_value_type(argv[0]); p1 = sql_value_int(argv[1]); if (p0type == MP_BIN) { @@ -532,16 +529,15 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) return; } if (argc == 2) { - if (sql_value_is_null(argv[1])) + if (mem_is_null(argv[1])) return; n = sql_value_int(argv[1]); if (n < 0) n = 0; } - if (sql_value_is_null(argv[0])) + if (mem_is_null(argv[0])) return; - enum mp_type mp_type = sql_value_type(argv[0]); - if (mp_type_is_bloblike(mp_type)) { + if (mem_is_bin(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "numeric"); context->is_aborted = true; @@ -601,8 +597,7 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \ const char *z2; \ int n; \ UNUSED_PARAMETER(argc); \ - int arg_type = sql_value_type(argv[0]); \ - if (mp_type_is_bloblike(arg_type)) { \ + if (mem_is_bin(argv[0])) { \ diag_set(ClientError, ER_INCONSISTENT_TYPES, "text", \ "varbinary"); \ context->is_aborted = true; \ @@ -683,7 +678,7 @@ randomBlob(sql_context * context, int argc, sql_value ** argv) unsigned char *p; assert(argc == 1); UNUSED_PARAMETER(argc); - if (mp_type_is_bloblike(sql_value_type(argv[0]))) { + if (mem_is_bin(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "numeric"); context->is_aborted = true; @@ -1133,7 +1128,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv) break; } default:{ - assert(sql_value_is_null(argv[0])); + assert(mem_is_null(argv[0])); sql_result_text(context, "NULL", 4, SQL_STATIC); break; } @@ -1272,13 +1267,13 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv) assert(zStr == sql_value_text(argv[0])); /* No encoding change */ zPattern = sql_value_text(argv[1]); if (zPattern == 0) { - assert(sql_value_is_null(argv[1]) + assert(mem_is_null(argv[1]) || sql_context_db_handle(context)->mallocFailed); return; } nPattern = sql_value_bytes(argv[1]); if (nPattern == 0) { - assert(! sql_value_is_null(argv[1])); + assert(!mem_is_null(argv[1])); sql_result_value(context, argv[0]); return; } @@ -1442,10 +1437,9 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg) { /* In case of VARBINARY type default trim octet is X'00'. */ const unsigned char *default_trim; - enum mp_type val_type = sql_value_type(arg); - if (val_type == MP_NIL) + if (mem_is_null(arg)) return; - if (mp_type_is_bloblike(val_type)) + if (mem_is_bin(arg)) default_trim = (const unsigned char *) "\0"; else default_trim = (const unsigned char *) " "; @@ -1574,8 +1568,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); - enum mp_type mp_type = sql_value_type(argv[0]); - if (mp_type_is_bloblike(mp_type)) { + if (mem_is_bin(argv[0])) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(argv[0]), "text"); context->is_aborted = true; @@ -1733,9 +1726,8 @@ countStep(sql_context * context, int argc, sql_value ** argv) return; } p = sql_aggregate_context(context, sizeof(*p)); - if ((argc == 0 || ! sql_value_is_null(argv[0])) && p) { + if ((argc == 0 || !mem_is_null(argv[0])) && p != NULL) p->n++; - } } static void @@ -1762,7 +1754,7 @@ minmaxStep(sql_context * context, int NotUsed, sql_value ** argv) if (!pBest) return; - if (sql_value_is_null(argv[0])) { + if (mem_is_null(argv[0])) { if (pBest->flags) sqlSkipAccumulatorLoad(context); } else if (pBest->flags) { @@ -1816,7 +1808,7 @@ groupConcatStep(sql_context * context, int argc, sql_value ** argv) context->is_aborted = true; return; } - if (sql_value_is_null(argv[0])) + if (mem_is_null(argv[0])) return; pAccum = (StrAccum *) sql_aggregate_context(context, sizeof(*pAccum)); diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 805dc7054..25b2e75ee 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -40,6 +40,149 @@ #include "lua/utils.h" #include "lua/msgpack.h" +bool +mem_is_null(const struct Mem *mem) +{ + return (mem->flags & MEM_Null) != 0; +} + +bool +mem_is_uint(const struct Mem *mem) +{ + return (mem->flags & MEM_UInt) != 0; +} + +bool +mem_is_str(const struct Mem *mem) +{ + return (mem->flags & MEM_Str) != 0; +} + +bool +mem_is_num(const struct Mem *mem) +{ + return (mem->flags & (MEM_Real | MEM_Int | MEM_UInt)) != 0; +} + +bool +mem_is_double(const struct Mem *mem) +{ + return (mem->flags & MEM_Real) != 0; +} + +bool +mem_is_int(const struct Mem *mem) +{ + return (mem->flags & (MEM_Int | MEM_UInt)) != 0; +} + +bool +mem_is_bool(const struct Mem *mem) +{ + return (mem->flags & MEM_Bool) != 0; +} + +bool +mem_is_bin(const struct Mem *mem) +{ + return (mem->flags & MEM_Blob) != 0; +} + +bool +mem_is_map(const struct Mem *mem) +{ + return (mem->flags & MEM_Blob) != 0 && + (mem->flags & MEM_Subtype) != 0 && + mem->subtype == SQL_SUBTYPE_MSGPACK && + mp_typeof(*mem->z) == MP_MAP; +} + +bool +mem_is_array(const struct Mem *mem) +{ + return (mem->flags & MEM_Blob) != 0 && + (mem->flags & MEM_Subtype) != 0 && + mem->subtype == SQL_SUBTYPE_MSGPACK && + mp_typeof(*mem->z) == MP_ARRAY; +} + +bool +mem_is_agg(const struct Mem *mem) +{ + return (mem->flags & MEM_Agg) != 0; +} + +bool +mem_is_bytes(const struct Mem *mem) +{ + return (mem->flags & (MEM_Blob | MEM_Str)) != 0; +} + +bool +mem_is_frame(const struct Mem *mem) +{ + return (mem->flags & MEM_Frame) != 0; +} + +bool +mem_is_invalid(const struct Mem *mem) +{ + return (mem->flags & MEM_Undefined) != 0; +} + +bool +mem_is_static(const struct Mem *mem) +{ + assert((mem->flags & (MEM_Str | MEM_Blob)) != 0); + return (mem->flags & MEM_Static) != 0; +} + +bool +mem_is_ephemeral(const struct Mem *mem) +{ + assert((mem->flags & (MEM_Str | MEM_Blob)) != 0); + return (mem->flags & MEM_Ephem) != 0; +} + +bool +mem_is_dynamic(const struct Mem *mem) +{ + assert((mem->flags & (MEM_Str | MEM_Blob)) != 0); + return (mem->flags & MEM_Dyn) != 0; +} + +bool +mem_is_allocated(const struct Mem *mem) +{ + return (mem->flags & (MEM_Str | MEM_Blob)) != 0 && + mem->z == mem->zMalloc; +} + +bool +mem_is_cleared(const struct Mem *mem) +{ + return (mem->flags & MEM_Null) != 0 && (mem->flags & MEM_Cleared) != 0; +} + +bool +mem_is_zerobin(const struct Mem *mem) +{ + return (mem->flags & MEM_Blob) != 0 && (mem->flags & MEM_Zero) != 0; +} + +bool +mem_is_same_type(const struct Mem *mem1, const struct Mem *mem2) +{ + return (mem1->flags & MEM_PURE_TYPE_MASK) == + (mem2->flags & MEM_PURE_TYPE_MASK); +} + +bool +mem_is_any_null(const struct Mem *mem1, const struct Mem *mem2) +{ + return ((mem1->flags | mem2->flags) & MEM_Null) != 0; +} + enum { BUF_SIZE = 32, }; @@ -1043,8 +1186,7 @@ mem_convert_to_integer(struct Mem *mem) int mem_convert_to_numeric(struct Mem *mem, enum field_type type) { - assert(mp_type_is_numeric(mem_mp_type(mem)) && - sql_type_is_numeric(type)); + assert(mem_is_num(mem) && sql_type_is_numeric(type)); assert(type != FIELD_TYPE_NUMBER); if (type == FIELD_TYPE_DOUBLE) return mem_convert_to_double(mem); diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 82b3084fb..e4e586d4d 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -87,6 +87,72 @@ struct Mem { */ #define MEMCELLSIZE offsetof(Mem,zMalloc) +bool +mem_is_null(const struct Mem *mem); + +bool +mem_is_uint(const struct Mem *mem); + +bool +mem_is_str(const struct Mem *mem); + +bool +mem_is_num(const struct Mem *mem); + +bool +mem_is_double(const struct Mem *mem); + +bool +mem_is_int(const struct Mem *mem); + +bool +mem_is_bool(const struct Mem *mem); + +bool +mem_is_bin(const struct Mem *mem); + +bool +mem_is_map(const struct Mem *mem); + +bool +mem_is_array(const struct Mem *mem); + +bool +mem_is_agg(const struct Mem *mem); + +bool +mem_is_bytes(const struct Mem *mem); + +bool +mem_is_frame(const struct Mem *mem); + +bool +mem_is_invalid(const struct Mem *mem); + +bool +mem_is_static(const struct Mem *mem); + +bool +mem_is_ephemeral(const struct Mem *mem); + +bool +mem_is_dynamic(const struct Mem *mem); + +bool +mem_is_allocated(const struct Mem *mem); + +bool +mem_is_cleared(const struct Mem *mem); + +bool +mem_is_zerobin(const struct Mem *mem); + +bool +mem_is_same_type(const struct Mem *mem1, const struct Mem *mem2); + +bool +mem_is_any_null(const struct Mem *mem1, const struct Mem *mem2); + /** * Return a string that represent content of MEM. String is either allocated * using static_alloc() of just a static variable. @@ -375,12 +441,6 @@ columnNullValue(void); /** Checkers. */ -static inline bool -sql_value_is_null(struct Mem *value) -{ - return sql_value_type(value) == MP_NIL; -} - int sqlVdbeMemTooBig(Mem *); /* Return TRUE if Mem X contains dynamically allocated content - anything diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 0fa388ae9..b9107fccc 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1092,7 +1092,7 @@ selectInnerLoop(Parse * pParse, /* The parser context */ * re-use second for Null op-code. * * Change to an OP_Null sets the - * MEM_Cleared bit on the first + * Cleared flag on the first * register of the previous value. * This will cause the OP_Ne below * to always fail on the first diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7cc72dc38..f054a0f43 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -120,8 +120,8 @@ int sql_sort_count = 0; #endif /* - * The next global variable records the size of the largest MEM_Blob - * or MEM_Str that has been used by a VDBE opcode. The test procedures + * The next global variable records the size of the largest varbinary + * or string that has been used by a VDBE opcode. The test procedures * use this information to make sure that the zero-blob functionality * is working correctly. This variable has no function other than to * help verify the correct operation of the library. @@ -131,9 +131,8 @@ int sql_max_blobsize = 0; static void updateMaxBlobsize(Mem *p) { - if ((p->flags & (MEM_Str|MEM_Blob))!=0 && p->n>sql_max_blobsize) { + if (mem_is_bytes(p) && p->n > sql_max_blobsize) sql_max_blobsize = p->n; - } } #endif @@ -425,8 +424,7 @@ vdbe_field_ref_fetch(struct vdbe_field_ref *field_ref, uint32_t fieldno, * Add 0 termination (at most for strings) * Not sure why do we check MEM_Ephem */ - if ((dest_mem->flags & (MEM_Ephem | MEM_Str)) == - (MEM_Ephem | MEM_Str)) { + if (mem_is_str(dest_mem) && mem_is_ephemeral(dest_mem)) { int len = dest_mem->n; if (dest_mem->szMalloc < len + 1) { if (sqlVdbeMemGrow(dest_mem, len + 1, 1) != 0) @@ -659,7 +657,7 @@ case OP_Gosub: { /* jump */ */ case OP_Return: { /* in1 */ pIn1 = &aMem[pOp->p1]; - assert(pIn1->flags==MEM_UInt); + assert(mem_is_uint(pIn1)); pOp = &aOp[pIn1->u.u]; pIn1->flags = MEM_Undefined; break; @@ -698,7 +696,7 @@ case OP_InitCoroutine: { /* jump */ case OP_EndCoroutine: { /* in1 */ VdbeOp *pCaller; pIn1 = &aMem[pOp->p1]; - assert(pIn1->flags == MEM_UInt); + assert(mem_is_uint(pIn1)); assert(pIn1->u.u < (uint64_t) p->nOp); pCaller = &aOp[pIn1->u.u]; assert(pCaller->opcode==OP_Yield); @@ -1108,7 +1106,7 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ pIn2 = &aMem[pOp->p2]; pOut = vdbe_prepare_null_out(p, pOp->p3); assert(pIn1!=pOut); - if ((pIn1->flags | pIn2->flags) & MEM_Null) { + if (mem_is_any_null(pIn1, pIn2)) { /* Force NULL be of type STRING. */ pOut->field_type = FIELD_TYPE_STRING; break; @@ -1117,10 +1115,8 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ * Concatenation operation can be applied only to * strings and blobs. */ - uint32_t str_type_p1 = pIn1->flags & (MEM_Blob | MEM_Str); - uint32_t str_type_p2 = pIn2->flags & (MEM_Blob | MEM_Str); - if (str_type_p1 == 0 || str_type_p2 == 0) { - char *inconsistent_type = str_type_p1 == 0 ? + if (!mem_is_bytes(pIn1) || !mem_is_bytes(pIn2)) { + char *inconsistent_type = !mem_is_bytes(pIn1) ? mem_type_to_str(pIn1) : mem_type_to_str(pIn2); diag_set(ClientError, ER_INCONSISTENT_TYPES, @@ -1129,7 +1125,7 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ } /* Moreover, both operands must be of the same type. */ - if (str_type_p1 != str_type_p2) { + if (!mem_is_same_type(pIn1, pIn2)) { diag_set(ClientError, ER_INCONSISTENT_TYPES, mem_type_to_str(pIn2), mem_type_to_str(pIn1)); goto abort_due_to_error; @@ -1143,7 +1139,7 @@ case OP_Concat: { /* same as TK_CONCAT, in1, in2, out3 */ if (sqlVdbeMemGrow(pOut, (int)nByte+2, pOut==pIn2)) { goto no_mem; } - if (pIn1->flags & MEM_Str) + if (mem_is_str(pIn1)) MemSetTypeFlag(pOut, MEM_Str); else MemSetTypeFlag(pOut, MEM_Blob); @@ -1202,7 +1198,6 @@ case OP_Subtract: /* same as TK_MINUS, in1, in2, out3 */ case OP_Multiply: /* same as TK_STAR, in1, in2, out3 */ case OP_Divide: /* same as TK_SLASH, in1, in2, out3 */ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ - u32 flags; /* Combined MEM_* flags from both inputs */ u16 type1; /* Numeric type of left operand */ u16 type2; /* Numeric type of right operand */ i64 iA; /* Integer value of left operand */ @@ -1215,14 +1210,14 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ pIn2 = &aMem[pOp->p2]; type2 = numericType(pIn2); pOut = vdbe_prepare_null_out(p, pOp->p3); - flags = pIn1->flags | pIn2->flags; - if ((flags & MEM_Null)!=0) goto arithmetic_result_is_null; + if (mem_is_any_null(pIn1, pIn2)) + goto arithmetic_result_is_null; if ((type1 & (MEM_Int | MEM_UInt)) != 0 && (type2 & (MEM_Int | MEM_UInt)) != 0) { iA = pIn1->u.i; iB = pIn2->u.i; - bool is_lhs_neg = pIn1->flags & MEM_Int; - bool is_rhs_neg = pIn2->flags & MEM_Int; + bool is_lhs_neg = mem_is_int(pIn1) && !mem_is_uint(pIn1); + bool is_rhs_neg = mem_is_int(pIn2) && !mem_is_uint(pIn2); bool is_res_neg; switch( pOp->opcode) { case OP_Add: { @@ -1429,7 +1424,7 @@ case OP_BuiltinFunction: { goto abort_due_to_error; /* Copy the result of the function into register P3 */ - if (pOut->flags & (MEM_Str|MEM_Blob)) { + if (mem_is_bytes(pOut)) { if (sqlVdbeMemTooBig(pCtx->pOut)) goto too_big; } @@ -1488,7 +1483,7 @@ case OP_FunctionByName: { * Copy the result of the function invocation into * register P3. */ - if ((pOut->flags & (MEM_Str | MEM_Blob)) != 0) + if (mem_is_bytes(pOut)) if (sqlVdbeMemTooBig(pOut)) goto too_big; REGISTER_TRACE(p, pOp->p3, pOut); @@ -1538,7 +1533,7 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ pIn1 = &aMem[pOp->p1]; pIn2 = &aMem[pOp->p2]; pOut = vdbe_prepare_null_out(p, pOp->p3); - if ((pIn1->flags | pIn2->flags) & MEM_Null) { + if (mem_is_any_null(pIn1, pIn2)) { /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; break; @@ -1597,7 +1592,7 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ case OP_AddImm: { /* in1 */ pIn1 = &aMem[pOp->p1]; memAboutToChange(p, pIn1); - assert((pIn1->flags & MEM_UInt) != 0 && pOp->p2 >= 0); + assert(mem_is_uint(pIn1) && pOp->p2 >= 0); pIn1->u.u += pOp->p2; break; } @@ -1611,9 +1606,9 @@ case OP_AddImm: { /* in1 */ */ case OP_MustBeInt: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; - if ((pIn1->flags & (MEM_Int | MEM_UInt)) == 0) { + if (!mem_is_int(pIn1)) { mem_apply_type(pIn1, FIELD_TYPE_INTEGER); - if ((pIn1->flags & (MEM_Int | MEM_UInt)) == 0) { + if (!mem_is_int(pIn1)) { if (pOp->p2==0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(pIn1), "integer"); @@ -1637,7 +1632,7 @@ case OP_MustBeInt: { /* jump, in1 */ */ case OP_Realify: { /* in1 */ pIn1 = &aMem[pOp->p1]; - if ((pIn1->flags & (MEM_Int | MEM_UInt)) != 0) { + if (mem_is_int(pIn1)) { sqlVdbeMemRealify(pIn1); } break; @@ -1778,7 +1773,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ flags3 = pIn3->flags; enum field_type ft_p1 = pIn1->field_type; enum field_type ft_p3 = pIn3->field_type; - if ((flags1 | flags3)&MEM_Null) { + if (mem_is_any_null(pIn1, pIn3)) { /* One or both operands are NULL */ if (pOp->p5 & SQL_NULLEQ) { /* If SQL_NULLEQ is set (which will only happen if the operator is @@ -1786,11 +1781,10 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ * or not both operands are null. */ assert(pOp->opcode==OP_Eq || pOp->opcode==OP_Ne); - assert((flags1 & MEM_Cleared)==0); + assert(!mem_is_cleared(pIn1)); assert((pOp->p5 & SQL_JUMPIFNULL)==0); - if ((flags1&flags3&MEM_Null)!=0 - && (flags3&MEM_Cleared)==0 - ) { + if (mem_is_same_type(pIn1, pIn3) && + !mem_is_cleared(pIn3)) { res = 0; /* Operands are equal */ } else { res = 1; /* Operands are not equal */ @@ -1812,22 +1806,17 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ } break; } - } else if (((flags1 | flags3) & MEM_Bool) != 0 || - ((flags1 | flags3) & MEM_Blob) != 0) { - /* - * If one of values is of type BOOLEAN or VARBINARY, - * then the second one must be of the same type as - * well. Otherwise an error is raised. - */ - int type_arg1 = flags1 & (MEM_Bool | MEM_Blob); - int type_arg3 = flags3 & (MEM_Bool | MEM_Blob); - if (type_arg1 != type_arg3) { - char *inconsistent_type = type_arg1 != 0 ? + } else if (mem_is_bool(pIn1) || mem_is_bool(pIn3) || + mem_is_bin(pIn1) || mem_is_bin(pIn3)) { + if (!mem_is_same_type(pIn1, pIn3)) { + char *inconsistent_type = mem_is_bool(pIn1) || + mem_is_bin(pIn1) ? mem_type_to_str(pIn3) : mem_type_to_str(pIn1); - char *expected_type = type_arg1 != 0 ? - mem_type_to_str(pIn1) : - mem_type_to_str(pIn3); + char *expected_type = mem_is_bool(pIn1) || + mem_is_bin(pIn1) ? + mem_type_to_str(pIn1) : + mem_type_to_str(pIn3); diag_set(ClientError, ER_SQL_TYPE_MISMATCH, inconsistent_type, expected_type); goto abort_due_to_error; @@ -1836,28 +1825,24 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ } else { enum field_type type = pOp->p5 & FIELD_TYPE_MASK; if (sql_type_is_numeric(type)) { - if ((flags1 | flags3)&MEM_Str) { - if ((flags1 & MEM_Str) == MEM_Str) { - mem_apply_numeric_type(pIn1); - testcase( flags3!=pIn3->flags); /* Possible if pIn1==pIn3 */ - flags3 = pIn3->flags; - } - if ((flags3 & MEM_Str) == MEM_Str) { - if (mem_apply_numeric_type(pIn3) != 0) { - diag_set(ClientError, - ER_SQL_TYPE_MISMATCH, - mem_str(pIn3), - "numeric"); - goto abort_due_to_error; - } - + if (mem_is_str(pIn1)) { + mem_apply_numeric_type(pIn1); + flags3 = pIn3->flags; + } + if (mem_is_str(pIn3)) { + if (mem_apply_numeric_type(pIn3) != 0) { + diag_set(ClientError, + ER_SQL_TYPE_MISMATCH, + mem_str(pIn3), + "numeric"); + goto abort_due_to_error; } } /* Handle the common case of integer comparison here, as an * optimization, to avoid a call to sqlMemCompare() */ - if ((pIn1->flags & pIn3->flags & (MEM_Int | MEM_UInt)) != 0) { - if ((pIn1->flags & pIn3->flags & MEM_Int) != 0) { + if (mem_is_int(pIn1) && mem_is_int(pIn3)) { + if (!mem_is_uint(pIn1) && !mem_is_uint(pIn3)) { if (pIn3->u.i > pIn1->u.i) res = +1; else if (pIn3->u.i < pIn1->u.i) @@ -1866,7 +1851,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ res = 0; goto compare_op; } - if ((pIn1->flags & pIn3->flags & MEM_UInt) != 0) { + if (mem_is_uint(pIn1) && mem_is_uint(pIn3)) { if (pIn3->u.u > pIn1->u.u) res = +1; else if (pIn3->u.u < pIn1->u.u) @@ -1875,8 +1860,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ res = 0; goto compare_op; } - if ((pIn1->flags & MEM_UInt) != 0 && - (pIn3->flags & MEM_Int) != 0) { + if (mem_is_uint(pIn1) && !mem_is_uint(pIn3)) { res = -1; goto compare_op; } @@ -1884,21 +1868,13 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ goto compare_op; } } else if (type == FIELD_TYPE_STRING) { - if ((flags1 & MEM_Str) == 0 && - (flags1 & (MEM_Int | MEM_UInt | MEM_Real)) != 0) { - testcase( pIn1->flags & MEM_Int); - testcase( pIn1->flags & MEM_Real); + if (!mem_is_str(pIn1) && mem_is_num(pIn1)) { sqlVdbeMemStringify(pIn1); - testcase( (flags1&MEM_Dyn) != (pIn1->flags&MEM_Dyn)); flags1 = (pIn1->flags & ~MEM_TypeMask) | (flags1 & MEM_TypeMask); assert(pIn1!=pIn3); } - if ((flags3 & MEM_Str) == 0 && - (flags3 & (MEM_Int | MEM_UInt | MEM_Real)) != 0) { - testcase( pIn3->flags & MEM_Int); - testcase( pIn3->flags & MEM_Real); + if (!mem_is_str(pIn3) && mem_is_num(pIn3)) { sqlVdbeMemStringify(pIn3); - testcase( (flags3&MEM_Dyn) != (pIn3->flags&MEM_Dyn)); flags3 = (pIn3->flags & ~MEM_TypeMask) | (flags3 & MEM_TypeMask); } } @@ -2106,9 +2082,9 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */ int v2; /* Right operand: 0==FALSE, 1==TRUE, 2==UNKNOWN or NULL */ pIn1 = &aMem[pOp->p1]; - if (pIn1->flags & MEM_Null) { + if (mem_is_null(pIn1)) { v1 = 2; - } else if ((pIn1->flags & MEM_Bool) != 0) { + } else if (mem_is_bool(pIn1)) { v1 = pIn1->u.b; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, @@ -2116,9 +2092,9 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */ goto abort_due_to_error; } pIn2 = &aMem[pOp->p2]; - if (pIn2->flags & MEM_Null) { + if (mem_is_null(pIn2)) { v2 = 2; - } else if ((pIn2->flags & MEM_Bool) != 0) { + } else if (mem_is_bool(pIn2)) { v2 = pIn2->u.b; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, @@ -2149,8 +2125,8 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ pIn1 = &aMem[pOp->p1]; pOut = vdbe_prepare_null_out(p, pOp->p2); pOut->field_type = FIELD_TYPE_BOOLEAN; - if ((pIn1->flags & MEM_Null)==0) { - if ((pIn1->flags & MEM_Bool) == 0) { + if (!mem_is_null(pIn1)) { + if (!mem_is_bool(pIn1)) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(pIn1), "boolean"); goto abort_due_to_error; @@ -2172,7 +2148,7 @@ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ pOut = vdbe_prepare_null_out(p, pOp->p2); /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; - if ((pIn1->flags & MEM_Null)==0) { + if (!mem_is_null(pIn1)) { int64_t i; bool is_neg; if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { @@ -2217,9 +2193,9 @@ case OP_If: /* jump, in1 */ case OP_IfNot: { /* jump, in1 */ int c; pIn1 = &aMem[pOp->p1]; - if (pIn1->flags & MEM_Null) { + if (mem_is_null(pIn1)) { c = pOp->p3; - } else if ((pIn1->flags & MEM_Bool) != 0) { + } else if (mem_is_bool(pIn1)) { c = pOp->opcode == OP_IfNot ? ! pIn1->u.b : pIn1->u.b; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, @@ -2240,8 +2216,8 @@ case OP_IfNot: { /* jump, in1 */ */ case OP_IsNull: { /* same as TK_ISNULL, jump, in1 */ pIn1 = &aMem[pOp->p1]; - VdbeBranchTaken( (pIn1->flags & MEM_Null)!=0, 2); - if ((pIn1->flags & MEM_Null)!=0) { + VdbeBranchTaken(mem_is_null(pIn1), 2); + if (mem_is_null(pIn1)) { goto jump_to_p2; } break; @@ -2254,8 +2230,8 @@ case OP_IsNull: { /* same as TK_ISNULL, jump, in1 */ */ case OP_NotNull: { /* same as TK_NOTNULL, jump, in1 */ pIn1 = &aMem[pOp->p1]; - VdbeBranchTaken( (pIn1->flags & MEM_Null)==0, 2); - if ((pIn1->flags & MEM_Null)==0) { + VdbeBranchTaken(!mem_is_null(pIn1), 2); + if (!mem_is_null(pIn1)) { goto jump_to_p2; } break; @@ -2309,7 +2285,7 @@ case OP_Column: { if (pC->eCurType==CURTYPE_PSEUDO) { assert(pC->uc.pseudoTableReg>0); pReg = &aMem[pC->uc.pseudoTableReg]; - assert(pReg->flags & MEM_Blob); + assert(mem_is_bin(pReg)); assert(memIsValid(pReg)); vdbe_field_ref_prepare_data(&pC->field_ref, pReg->z, pReg->n); @@ -2338,7 +2314,7 @@ case OP_Column: { if (vdbe_field_ref_fetch(&pC->field_ref, p2, pDest) != 0) goto abort_due_to_error; - if ((pDest->flags & MEM_Null) && + if (mem_is_null(pDest) && (uint32_t) p2 >= pC->field_ref.field_count && default_val_mem != NULL) { sqlVdbeMemShallowCopy(pDest, default_val_mem, MEM_Static); @@ -2393,7 +2369,7 @@ case OP_ApplyType: { if (!sql_type_is_numeric(type)) goto type_mismatch; /* Implicit cast is allowed only from numeric type. */ - if (!mp_type_is_numeric(mem_mp_type(pIn1))) + if (!mem_is_num(pIn1)) goto type_mismatch; /* Try to convert numeric-to-numeric. */ if (mem_convert_to_numeric(pIn1, type) != 0) @@ -3011,18 +2987,18 @@ case OP_SeekGT: { /* jump, in3 */ * the seek, so convert it. */ pIn3 = &aMem[int_field]; - if ((pIn3->flags & MEM_Null) != 0) + if (mem_is_null(pIn3)) goto skip_truncate; - if ((pIn3->flags & MEM_Str) != 0) + if (mem_is_str(pIn3)) mem_apply_numeric_type(pIn3); int64_t i; - if ((pIn3->flags & MEM_Int) == MEM_Int) { - i = pIn3->u.i; - is_neg = true; - } else if ((pIn3->flags & MEM_UInt) == MEM_UInt) { + if (mem_is_uint(pIn3)) { i = pIn3->u.u; is_neg = false; - } else if ((pIn3->flags & MEM_Real) == MEM_Real) { + } else if (mem_is_int(pIn3)) { + i = pIn3->u.i; + is_neg = true; + } else if (mem_is_double(pIn3)) { if (pIn3->u.r > (double)INT64_MAX) i = INT64_MAX; else if (pIn3->u.r < (double)INT64_MIN) @@ -3040,8 +3016,8 @@ case OP_SeekGT: { /* jump, in3 */ /* If the P3 value could not be converted into an integer without * loss of information, then special processing is required... */ - if ((pIn3->flags & (MEM_Int | MEM_UInt)) == 0) { - if ((pIn3->flags & MEM_Real)==0) { + if (!mem_is_int(pIn3)) { + if (!mem_is_double(pIn3)) { /* If the P3 value cannot be converted into any kind of a number, * then the seek is not possible, so jump to P2 */ @@ -3247,7 +3223,8 @@ case OP_Found: { /* jump, in3 */ #ifdef SQL_DEBUG for(ii=0; iip3+ii, &r.aMem[ii]); } @@ -3257,7 +3234,7 @@ case OP_Found: { /* jump, in3 */ } else { pFree = pIdxKey = sqlVdbeAllocUnpackedRecord(db, pC->key_def); if (pIdxKey==0) goto no_mem; - assert(pIn3->flags & MEM_Blob ); + assert(mem_is_bin(pIn3)); (void)ExpandBlob(pIn3); sqlVdbeRecordUnpackMsgpack(pC->key_def, pIn3->z, pIdxKey); @@ -3271,7 +3248,7 @@ case OP_Found: { /* jump, in3 */ * conflict */ for(ii=0; iinField; ii++) { - if (pIdxKey->aMem[ii].flags & MEM_Null) { + if (mem_is_null(&pIdxKey->aMem[ii])) { takeJump = 1; break; } @@ -3381,11 +3358,11 @@ case OP_FCopy: { /* out2 */ pIn1 = &aMem[pOp->p1]; } - if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) && (pIn1->flags & MEM_Null)) { + if ((pOp->p3 & OPFLAG_NOOP_IF_NULL) != 0 && mem_is_null(pIn1)) { pOut = vdbe_prepare_null_out(p, pOp->p2); } else { assert(memIsValid(pIn1)); - assert((pIn1->flags & (MEM_Int | MEM_UInt)) != 0); + assert(mem_is_int(pIn1)); pOut = vdbe_prepare_null_out(p, pOp->p2); mem_set_int(pOut, pIn1->u.i, pIn1->flags == MEM_Int); @@ -3520,7 +3497,7 @@ case OP_SorterData: { assert(isSorter(pC)); if (sqlVdbeSorterRowkey(pC, pOut) != 0) goto abort_due_to_error; - assert(pOut->flags & MEM_Blob); + assert(mem_is_bin(pOut)); assert(pOp->p1>=0 && pOp->p1nCursor); p->apCsr[pOp->p3]->cacheStatus = CACHE_STALE; break; @@ -3878,7 +3855,7 @@ case OP_SorterInsert: { /* in2 */ assert(cursor != NULL); assert(isSorter(cursor)); pIn2 = &aMem[pOp->p2]; - assert((pIn2->flags & MEM_Blob) != 0); + assert(mem_is_bin(pIn2)); if (ExpandBlob(pIn2) != 0 || sqlVdbeSorterWrite(cursor, pIn2) != 0) goto abort_due_to_error; @@ -3912,7 +3889,7 @@ case OP_SorterInsert: { /* in2 */ case OP_IdxReplace: case OP_IdxInsert: { pIn2 = &aMem[pOp->p1]; - assert((pIn2->flags & MEM_Blob) != 0); + assert(mem_is_bin(pIn2)); if (ExpandBlob(pIn2) != 0) goto abort_due_to_error; struct space *space; @@ -3923,7 +3900,7 @@ case OP_IdxInsert: { assert(space != NULL); if (space->def->id != 0) { /* Make sure that memory has been allocated on region. */ - assert(aMem[pOp->p1].flags & MEM_Ephem); + assert(mem_is_ephemeral(&aMem[pOp->p1])); if (pOp->opcode == OP_IdxInsert) { rc = tarantoolsqlInsert(space, pIn2->z, pIn2->z + pIn2->n); @@ -3957,7 +3934,7 @@ case OP_IdxInsert: { } if ((pOp->p5 & OPFLAG_NCHANGE) != 0) p->nChange++; - if (pOp->p3 > 0 && ((aMem[pOp->p3].flags) & MEM_Null) != 0) { + if (pOp->p3 > 0 && mem_is_null(&aMem[pOp->p3])) { assert(space->sequence != NULL); int64_t value; if (sequence_get_value(space->sequence, &value) != 0) @@ -4003,10 +3980,10 @@ case OP_Update: { assert(pOp->p4type == P4_SPACEPTR); struct Mem *key_mem = &aMem[pOp->p2]; - assert((key_mem->flags & MEM_Blob) != 0); + assert(mem_is_bin(key_mem)); struct Mem *upd_fields_mem = &aMem[pOp->p3]; - assert((upd_fields_mem->flags & MEM_Blob) != 0); + 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); @@ -4438,7 +4415,7 @@ case OP_Program: { /* jump */ * the trigger program. If this trigger has been fired before, then pRt * is already allocated. Otherwise, it must be initialized. */ - if ((pRt->flags&MEM_Frame)==0) { + if (!mem_is_frame(pRt)) { /* SubProgram.nMem is set to the number of memory cells used by the * program stored in SubProgram.aOp. As well as these, one memory * cell is required for each cursor used by the program. Set local @@ -4578,8 +4555,8 @@ case OP_FkIfZero: { /* jump */ */ case OP_IfPos: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; - assert((pIn1->flags & (MEM_Int | MEM_UInt)) != 0); - if ((pIn1->flags & MEM_UInt) != 0 && pIn1->u.u != 0) { + assert(mem_is_int(pIn1)); + if (mem_is_uint(pIn1) && pIn1->u.u != 0) { assert(pOp->p3 >= 0); uint64_t res = pIn1->u.u - (uint64_t) pOp->p3; /* @@ -4613,8 +4590,8 @@ case OP_OffsetLimit: { /* in1, out2, in3 */ pIn3 = &aMem[pOp->p3]; pOut = vdbe_prepare_null_out(p, pOp->p2); - assert((pIn1->flags & MEM_UInt) != 0); - assert((pIn3->flags & MEM_UInt) != 0); + assert(mem_is_uint(pIn1)); + assert(mem_is_uint(pIn3)); uint64_t x = pIn1->u.u; uint64_t rhs = pIn3->u.u; bool unused; @@ -4637,7 +4614,7 @@ case OP_OffsetLimit: { /* in1, out2, in3 */ */ case OP_IfNotZero: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; - assert((pIn1->flags & MEM_UInt) != 0); + assert(mem_is_uint(pIn1)); if (pIn1->u.u > 0) { pIn1->u.u--; goto jump_to_p2; @@ -4653,7 +4630,7 @@ case OP_IfNotZero: { /* jump, in1 */ */ case OP_DecrJumpZero: { /* jump, in1 */ pIn1 = &aMem[pOp->p1]; - assert((pIn1->flags & MEM_UInt) != 0); + assert(mem_is_uint(pIn1)); if (pIn1->u.u > 0) pIn1->u.u--; if (pIn1->u.u == 0) goto jump_to_p2; @@ -4750,7 +4727,7 @@ case OP_AggStep: { mem_destroy(&t); goto abort_due_to_error; } - assert(t.flags==MEM_Null); + assert(mem_is_null(&t)); if (pCtx->skipFlag) { assert(pOp[-1].opcode==OP_CollSeq); i = pOp[-1].p1; @@ -4776,7 +4753,7 @@ case OP_AggFinal: { Mem *pMem; assert(pOp->p1>0 && pOp->p1<=(p->nMem+1 - p->nCursor)); pMem = &aMem[pOp->p1]; - assert((pMem->flags & ~(MEM_Null|MEM_Agg))==0); + assert(mem_is_null(pMem) || mem_is_agg(pMem)); if (sql_vdbemem_finalize(pMem, pOp->p4.func) != 0) goto abort_due_to_error; UPDATE_MAX_BLOBSIZE(pMem); @@ -4903,7 +4880,7 @@ case OP_SetSession: { struct session_setting *setting = &session_settings[sid]; switch (setting->field_type) { case FIELD_TYPE_BOOLEAN: { - if ((pIn1->flags & MEM_Bool) == 0) + if (!mem_is_bool(pIn1)) goto invalid_type; bool value = pIn1->u.b; size_t size = mp_sizeof_bool(value); @@ -4914,7 +4891,7 @@ case OP_SetSession: { break; } case FIELD_TYPE_STRING: { - if ((pIn1->flags & MEM_Str) == 0) + if (!mem_is_str(pIn1)) goto invalid_type; const char *str = pIn1->z; uint32_t size = mp_sizeof_str(pIn1->n); diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index cbf32ccdf..b4ad8b774 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -333,17 +333,6 @@ int sqlVdbeList(Vdbe *); int sqlVdbeHalt(Vdbe *); -/** - * In terms of VDBE memory cell type, _BIN, _ARRAY and _MAP - * messagepacks are stored as binary string (i.e. featuring - * MEM_Blob internal type). - */ -#define mp_type_is_bloblike(X) ((X) == MP_BIN || (X) == MP_ARRAY || (X) == MP_MAP) - -/** Return TRUE if MP_type of X is numeric, FALSE otherwise. */ -#define mp_type_is_numeric(X) ((X) == MP_INT || (X) == MP_UINT ||\ - (X) == MP_DOUBLE) - const char *sqlOpcodeName(int); int sqlVdbeCloseStatement(Vdbe *, int); void sqlVdbeFrameDelete(VdbeFrame *); diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index a195f8dfd..2a3561d42 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -380,7 +380,7 @@ static SQL_NOINLINE void * createAggContext(sql_context * p, int nByte) { Mem *pMem = p->pMem; - assert((pMem->flags & MEM_Agg) == 0); + assert(!mem_is_agg(pMem)); if (nByte <= 0) { sqlVdbeMemSetNull(pMem); pMem->z = 0; @@ -407,7 +407,7 @@ sql_aggregate_context(sql_context * p, int nByte) assert(p->func->def->language == FUNC_LANGUAGE_SQL_BUILTIN); assert(p->func->def->aggregate == FUNC_AGGREGATE_GROUP); testcase(nByte < 0); - if ((p->pMem->flags & MEM_Agg) == 0) { + if (!mem_is_agg(p->pMem)) { return createAggContext(p, nByte); } else { return (void *)p->pMem->z; diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 850158572..52e100454 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -1253,7 +1253,7 @@ sqlVdbeList(Vdbe * p) */ assert(p->nMem > 9); pSub = &p->aMem[9]; - if (pSub->flags & MEM_Blob) { + if (mem_is_bin(pSub)) { /* On the first call to sql_step(), pSub will hold a NULL. It is * initialized to a BLOB by the P4_SUBPROGRAM processing logic below */ @@ -1722,7 +1722,7 @@ Cleanup(Vdbe * p) assert(p->apCsr[i] == 0); if (p->aMem) { for (i = 0; i < p->nMem; i++) - assert(p->aMem[i].flags == MEM_Undefined); + assert(mem_is_invalid(&p->aMem[i])); } #endif @@ -2330,7 +2330,7 @@ sqlVdbeGetBoundValue(Vdbe * v, int iVar, u8 aff) assert(iVar > 0); if (v) { Mem *pMem = &v->aVar[iVar - 1]; - if (0 == (pMem->flags & MEM_Null)) { + if (!mem_is_null(pMem)) { sql_value *pRet = sqlValueNew(v->db); if (pRet) { sqlVdbeMemCopy((Mem *) pRet, pMem); diff --git a/src/box/sql/vdbesort.c b/src/box/sql/vdbesort.c index 927f85559..a9a5f45af 100644 --- a/src/box/sql/vdbesort.c +++ b/src/box/sql/vdbesort.c @@ -2218,7 +2218,7 @@ sqlVdbeSorterCompare(const VdbeCursor * pCsr, /* Sorter cursor */ pKey = vdbeSorterRowkey(pSorter, &nKey); sqlVdbeRecordUnpackMsgpack(pCsr->key_def, pKey, r2); for (i = 0; i < nKeyCol; i++) { - if (r2->aMem[i].flags & MEM_Null) { + if (mem_is_null(&r2->aMem[i])) { *pRes = -1; return 0; } diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 0c002dbee..93de722cb 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -312,9 +312,8 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix, pVal = sqlVdbeGetBoundValue(pReprepare, iCol, FIELD_TYPE_SCALAR); - if (pVal && sql_value_type(pVal) == MP_STR) { + if (pVal != NULL && mem_is_str(pVal)) z = (char *)sql_value_text(pVal); - } assert(pRight->op == TK_VARIABLE || pRight->op == TK_REGISTER); } else if (op == TK_STRING) { z = pRight->u.zToken;