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 219896EC5D; Fri, 9 Apr 2021 20:01:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 219896EC5D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617987680; bh=1WmTXI5ndTrCbiFQ6eawC3AiCI5F0C93K+GOjm9tMX4=; 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=M3QYsfRlhDe25+hVrKFADIyTxGlhfQ1vCyCumh/k5ukVQ7zt6sLDe8mIccJesKFOv wvOIfkvGJ7Tudl+cgxFu2iGE1TEAkpaRLwTJu9085wz43212WfyS6kSzuX/kCnRb7N ztljExuSn55I7AEjSi1ehMitKkebvFOgQKytuVBY= 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 B2ACC6EC5D for ; Fri, 9 Apr 2021 19:59:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B2ACC6EC5D Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lUuU5-0000le-Q2; Fri, 09 Apr 2021 19:59:54 +0300 To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Fri, 9 Apr 2021 19:59:53 +0300 Message-Id: <2ce8b15b5c1869dec995b4a9f348fe7649f6af47.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: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E74807BAE725B9AE625DE765B0E193B5B7687182A05F538085040537CE8BD0BD39A7D4F476F3BEB12296656255495E269A6B5CE293DEB958B72DF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE71D6E7AC8529A326CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AAF0FBE7E77B7ED78638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2D635E198A763FB2EC3D6079AE823E403CB629EEF1311BF91D2E47CDBA5A96583C09775C1D3CA48CF17B107DEF921CE79117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE71AE4D56B06699BBC9FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735E4A630A5B664A4FFC4224003CC83647689D4C264860C145E X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C4C7A0BC55FA0FE5FC0D06ED59D0318C35E6DAFEAEB58294E6DF71D3546CC44454B1881A6453793CE9C32612AADDFBE061C61BE10805914D3804EBA3D8E7E5B87ABF8C51168CD8EBDB63AF70AF8205D7DCDC48ACC2A39D04F89CDFB48F4795C241BDAD6C7F3747799A X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3498910055B812BD9C92078C349E40AC5544FF2457FDF41CA66A329F728D4170C6FA9B63A3A6B5E66C1D7E09C32AA3244CAE1FE52CCCB8C798952FF7DA94E5D7BD97FE24653F78E668FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyO2lHpuZu4S4GjMbAFnEIw== X-Mailru-Sender: 689FA8AB762F73936BC43F508A0638228DAC2DA785A5EBBB164B8EFC0C9E09A283D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v5 09/52] sql: introduce mem_str() 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 01:58, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 2 comments below. > >> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c >> index 62338e1db..0ed5e38d4 100644 >> --- a/src/box/sql/mem.c >> +++ b/src/box/sql/mem.c >> @@ -37,6 +37,39 @@ >> #include "box/tuple.h" >> #include "mpstream/mpstream.h" >> >> +enum { >> + BUF_SIZE = 32, >> +}; >> + >> +const char * >> +mem_str(const struct Mem *mem) >> +{ >> + char buf[BUF_SIZE]; >> + switch (mem->flags & MEM_PURE_TYPE_MASK) { >> + case MEM_Null: >> + return "NULL"; >> + case MEM_Str: >> + return tt_sprintf("%.*s", mem->n, mem->z); >> + case MEM_Int: >> + return tt_sprintf("%lld", mem->u.i); >> + case MEM_UInt: >> + return tt_sprintf("%llu", mem->u.u); >> + case MEM_Real: >> + sql_snprintf(BUF_SIZE, &buf[0], "%!.15g", mem->u.r); >> + return tt_sprintf("%s", buf); >> + case MEM_Blob: >> + if ((mem->flags & MEM_Subtype) == 0) >> + return "varbinary"; >> + assert(mem->subtype == SQL_SUBTYPE_MSGPACK); >> + return mp_str(mem->z); >> + case MEM_Bool: >> + return mem->u.b ? "TRUE" : "FALSE"; > > 1. Why are they capital? > We already have macro SQL_TOKEN_BOOLEAN which returns capital version of these two words. I didn't want to use this macro, however decided that in this function these words also should be capital. >> + default: >> + break; >> + } >> + return "unknown"; >> +} >> diff --git a/src/box/sql/vdbetrace.c b/src/box/sql/vdbetrace.c >> index e84bb3192..eceaa953b 100644 >> --- a/src/box/sql/vdbetrace.c >> +++ b/src/box/sql/vdbetrace.c >> @@ -147,33 +145,10 @@ sqlVdbeExpandSql(Vdbe * p, /* The prepared statement being evaluated */ >> zRawSql += nToken; >> nextIndex = idx + 1; >> assert(idx > 0 && idx <= p->nVar); >> - pVar = &p->aVar[idx - 1]; >> - if (pVar->flags & MEM_Null) { >> - sqlStrAccumAppend(&out, "NULL", 4); >> - } else if (pVar->flags & MEM_Int) { >> - sqlXPrintf(&out, "%lld", pVar->u.i); >> - } else if (pVar->flags & MEM_UInt) { >> - sqlXPrintf(&out, "%llu", pVar->u.u); >> - } else if (pVar->flags & MEM_Real) { >> - sqlXPrintf(&out, "%!.15g", pVar->u.r); >> - } else if (pVar->flags & MEM_Str) { >> - int nOut; /* Number of bytes of the string text to include in output */ >> - nOut = pVar->n; >> - sqlXPrintf(&out, "'%.*q'", nOut, pVar->z); >> - } else if (pVar->flags & MEM_Zero) { >> - sqlXPrintf(&out, "zeroblob(%d)", >> - pVar->u.nZero); >> - } else { >> - int nOut; /* Number of bytes of the blob to include in output */ >> - assert(pVar->flags & MEM_Blob); >> - sqlStrAccumAppend(&out, "x'", 2); >> - nOut = pVar->n; >> - for (i = 0; i < nOut; i++) { >> - sqlXPrintf(&out, "%02x", >> - pVar->z[i] & 0xff); >> - } >> - sqlStrAccumAppend(&out, "'", 1); >> - } >> + const char *value = mem_str(&p->aVar[idx - 1]); >> + uint32_t size = MIN(strlen(value), sizeof(zBase) - 1); > > 2. strlen() might be called twice because MIN is a macro. > Fixed. >> + memcpy(zBase, value, size); >> + zBase[size] = '\0'; >> } >> } >> if (out.accError) New patch: commit 2ce8b15b5c1869dec995b4a9f348fe7649f6af47 Author: Mergen Imeev Date: Tue Mar 2 13:52:11 2021 +0300 sql: introduce mem_str() This patch introduces mem_str() which allows to receive value of MEM as a string. Due to the limitations of static_alloc(), this function cannot be used to safely retrieve a value of MEM converted to string. This function is suitable for debugging, displaying the value in an error message, etc. Part of #5818 diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 46814f341..99ce938d5 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -543,7 +543,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv) enum mp_type mp_type = sql_value_type(argv[0]); if (mp_type_is_bloblike(mp_type)) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(argv[0]), "numeric"); + mem_str(argv[0]), "numeric"); context->is_aborted = true; return; } @@ -685,7 +685,7 @@ randomBlob(sql_context * context, int argc, sql_value ** argv) UNUSED_PARAMETER(argc); if (mp_type_is_bloblike(sql_value_type(argv[0]))) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(argv[0]), "numeric"); + mem_str(argv[0]), "numeric"); context->is_aborted = true; return; } @@ -1577,7 +1577,7 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv) enum mp_type mp_type = sql_value_type(argv[0]); if (mp_type_is_bloblike(mp_type)) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(argv[0]), "text"); + mem_str(argv[0]), "text"); context->is_aborted = true; return; } @@ -1650,7 +1650,7 @@ sum_step(struct sql_context *context, int argc, sql_value **argv) if (type != MP_DOUBLE && type != MP_INT && type != MP_UINT) { if (mem_apply_numeric_type(argv[0]) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(argv[0]), "number"); + mem_str(argv[0]), "number"); context->is_aborted = true; return; } diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 25f5f2f2d..a2a0fc33e 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -40,6 +40,39 @@ #include "lua/utils.h" #include "lua/msgpack.h" +enum { + BUF_SIZE = 32, +}; + +const char * +mem_str(const struct Mem *mem) +{ + char buf[BUF_SIZE]; + switch (mem->flags & MEM_PURE_TYPE_MASK) { + case MEM_Null: + return "NULL"; + case MEM_Str: + return tt_sprintf("%.*s", mem->n, mem->z); + case MEM_Int: + return tt_sprintf("%lld", mem->u.i); + case MEM_UInt: + return tt_sprintf("%llu", mem->u.u); + case MEM_Real: + sql_snprintf(BUF_SIZE, &buf[0], "%!.15g", mem->u.r); + return tt_sprintf("%s", buf); + case MEM_Blob: + if ((mem->flags & MEM_Subtype) == 0) + return "varbinary"; + assert(mem->subtype == SQL_SUBTYPE_MSGPACK); + return mp_str(mem->z); + case MEM_Bool: + return mem->u.b ? "TRUE" : "FALSE"; + default: + break; + } + return "unknown"; +} + static inline bool mem_has_msgpack_subtype(struct Mem *mem) { @@ -1592,18 +1625,6 @@ sqlValueText(sql_value * pVal) return valueToText(pVal); } -const char * -sql_value_to_diag_str(sql_value *value) -{ - enum mp_type mp_type = sql_value_type(value); - if (mp_type_is_bloblike(mp_type)) { - if (mem_has_msgpack_subtype(value)) - return sqlValueText(value); - return "varbinary"; - } - return sqlValueText(value); -} - enum sql_subtype sql_value_subtype(sql_value * pVal) { diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index acc8ce054..7b9456426 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -87,6 +87,13 @@ struct Mem { */ #define MEMCELLSIZE offsetof(Mem,zMalloc) +/** + * Return a string that represent content of MEM. String is either allocated + * using static_alloc() of just a static variable. + */ +const char * +mem_str(const struct Mem *mem); + /* One or more of the following flags are set to indicate the validOK * representations of the value stored in the Mem struct. * @@ -358,15 +365,6 @@ sql_value_text(struct Mem *); const void *sqlValueText(struct Mem *); -/** - * Return pointer to a string with the data type in the case of - * binary data stored in @a value. Otherwise, return the result - * of sql_value_text(). It is used due to the fact that not all - * binary strings can be displayed correctly (e.g. contain - * unprintable symbols). - */ -const char * -sql_value_to_diag_str(struct Mem *value); #define VdbeFrameMem(p) ((Mem *)&((u8 *)p)[ROUND8(sizeof(VdbeFrame))]) enum sql_subtype diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index e269857ea..ec3d23cb2 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1265,12 +1265,12 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ } else { if (sqlVdbeRealValue(pIn1, &rA) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "numeric"); + mem_str(pIn1), "numeric"); goto abort_due_to_error; } if (sqlVdbeRealValue(pIn2, &rB) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn2), "numeric"); + mem_str(pIn2), "numeric"); goto abort_due_to_error; } assert(((type1 | type2) & MEM_Real) != 0); @@ -1546,12 +1546,12 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ bool unused; if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn2), "integer"); + mem_str(pIn2), "integer"); goto abort_due_to_error; } if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); + mem_str(pIn1), "integer"); goto abort_due_to_error; } op = pOp->opcode; @@ -1616,7 +1616,7 @@ case OP_MustBeInt: { /* jump, in1 */ if ((pIn1->flags & (MEM_Int | MEM_UInt)) == 0) { if (pOp->p2==0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); + mem_str(pIn1), "integer"); goto abort_due_to_error; } else { goto jump_to_p2; @@ -1673,7 +1673,7 @@ case OP_Cast: { /* in1 */ UPDATE_MAX_BLOBSIZE(pIn1); if (rc == 0) break; - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn1), + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(pIn1), field_type_strs[pOp->p2]); goto abort_due_to_error; } @@ -1846,7 +1846,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if (mem_apply_numeric_type(pIn3) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn3), + mem_str(pIn3), "numeric"); goto abort_due_to_error; } @@ -2112,7 +2112,7 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */ v1 = pIn1->u.b; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "boolean"); + mem_str(pIn1), "boolean"); goto abort_due_to_error; } pIn2 = &aMem[pOp->p2]; @@ -2122,7 +2122,7 @@ case OP_Or: { /* same as TK_OR, in1, in2, out3 */ v2 = pIn2->u.b; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn2), "boolean"); + mem_str(pIn2), "boolean"); goto abort_due_to_error; } if (pOp->opcode==OP_And) { @@ -2152,7 +2152,7 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ if ((pIn1->flags & MEM_Null)==0) { if ((pIn1->flags & MEM_Bool) == 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "boolean"); + mem_str(pIn1), "boolean"); goto abort_due_to_error; } mem_set_bool(pOut, ! pIn1->u.b); @@ -2177,7 +2177,7 @@ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ bool is_neg; if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); + mem_str(pIn1), "integer"); goto abort_due_to_error; } mem_set_i64(pOut, ~i); @@ -2223,7 +2223,7 @@ case OP_IfNot: { /* jump, in1 */ c = pOp->opcode == OP_IfNot ? ! pIn1->u.b : pIn1->u.b; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "boolean"); + mem_str(pIn1), "boolean"); goto abort_due_to_error; } VdbeBranchTaken(c!=0, 2); @@ -2403,7 +2403,7 @@ case OP_ApplyType: { continue; type_mismatch: diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), field_type_strs[type]); + mem_str(pIn1), field_type_strs[type]); goto abort_due_to_error; } break; @@ -3032,7 +3032,7 @@ case OP_SeekGT: { /* jump, in3 */ is_neg = i < 0; } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn3), "integer"); + mem_str(pIn3), "integer"); goto abort_due_to_error; } iKey = i; diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 79c3d60e0..b7e148422 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -1108,21 +1108,10 @@ displayP4(Op * pOp, char *zTemp, int nTemp) break; } case P4_MEM:{ - Mem *pMem = pOp->p4.pMem; - if (pMem->flags & MEM_Str) { - zP4 = pMem->z; - } else if (pMem->flags & MEM_Int) { - sqlXPrintf(&x, "%lld", pMem->u.i); - } else if (pMem->flags & MEM_UInt) { - sqlXPrintf(&x, "%llu", pMem->u.u); - } else if (pMem->flags & MEM_Real) { - sqlXPrintf(&x, "%.16g", pMem->u.r); - } else if (pMem->flags & MEM_Null) { - zP4 = "NULL"; - } else { - assert(pMem->flags & MEM_Blob); - zP4 = "(binary string)"; - } + const char *value = mem_str(pOp->p4.pMem); + uint32_t size = MIN((int)strlen(value), nTemp - 1); + memcpy(zP4, value, size); + zP4[size] = '\0'; break; } case P4_INTARRAY:{ diff --git a/src/box/sql/vdbetrace.c b/src/box/sql/vdbetrace.c index e84bb3192..4ca56865d 100644 --- a/src/box/sql/vdbetrace.c +++ b/src/box/sql/vdbetrace.c @@ -97,8 +97,6 @@ sqlVdbeExpandSql(Vdbe * p, /* The prepared statement being evaluated */ int nextIndex = 1; /* Index of next ? host parameter */ int n; /* Length of a token prefix */ int nToken; /* Length of the parameter token */ - int i; /* Loop counter */ - Mem *pVar; /* Value of a host parameter */ StrAccum out; /* Accumulate the output here */ char zBase[100]; /* Initial working space */ @@ -147,33 +145,11 @@ sqlVdbeExpandSql(Vdbe * p, /* The prepared statement being evaluated */ zRawSql += nToken; nextIndex = idx + 1; assert(idx > 0 && idx <= p->nVar); - pVar = &p->aVar[idx - 1]; - if (pVar->flags & MEM_Null) { - sqlStrAccumAppend(&out, "NULL", 4); - } else if (pVar->flags & MEM_Int) { - sqlXPrintf(&out, "%lld", pVar->u.i); - } else if (pVar->flags & MEM_UInt) { - sqlXPrintf(&out, "%llu", pVar->u.u); - } else if (pVar->flags & MEM_Real) { - sqlXPrintf(&out, "%!.15g", pVar->u.r); - } else if (pVar->flags & MEM_Str) { - int nOut; /* Number of bytes of the string text to include in output */ - nOut = pVar->n; - sqlXPrintf(&out, "'%.*q'", nOut, pVar->z); - } else if (pVar->flags & MEM_Zero) { - sqlXPrintf(&out, "zeroblob(%d)", - pVar->u.nZero); - } else { - int nOut; /* Number of bytes of the blob to include in output */ - assert(pVar->flags & MEM_Blob); - sqlStrAccumAppend(&out, "x'", 2); - nOut = pVar->n; - for (i = 0; i < nOut; i++) { - sqlXPrintf(&out, "%02x", - pVar->z[i] & 0xff); - } - sqlStrAccumAppend(&out, "'", 1); - } + const char *value = mem_str(&p->aVar[idx - 1]); + uint32_t len = strlen(value); + uint32_t size = MIN(len, sizeof(zBase) - 1); + memcpy(zBase, value, size); + zBase[size] = '\0'; } } if (out.accError)