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 EFCE86EC5D; Fri, 9 Apr 2021 22:47:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EFCE86EC5D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617997659; bh=y6qtDm1V4BeR4sPcOBRLfKBnO2fFwVL1sVCoYOPzfUE=; 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=zG8cnnO+UzVu8T//gjfY6YQ4ZxnyHmgTajLCvawtd/6Xov73pq4lFwRRmXPyADrMs sfw1szbl/FxQU7U/Sg7zdlAH8lZq1ST1O2O2W2CWkg+dj7piCWPB6ISp1fbJWc6G05 1S0pVv/EpBjAIZP+MyfycWCI4aptqaFjfB1kWuNQ= 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 27C476EC5D for ; Fri, 9 Apr 2021 22:45:45 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 27C476EC5D Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1lUx4a-0006ek-Fu; Fri, 09 Apr 2021 22:45:44 +0300 To: v.shpilevoy@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.tarantool.org Date: Fri, 9 Apr 2021 22:45:44 +0300 Message-Id: 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: 4F1203BC0FB41BD92FFCB8E6708E7480B1C8842CE613979772AC4B34408C6525182A05F538085040B67161FE7ADF8FD194CE5AB8500CDABB41C245A09493D3CDEFF4C9BD74148494 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7DB7B102DCB413779EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379A6B93796C91DCB58638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2242E878E95EC6295E871793E91E93EED3999716826408816D2E47CDBA5A96583C09775C1D3CA48CFED8438A78DFE0A9E117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE77FFCE1C639F4728C9FA2833FD35BB23DF004C906525384302BEBFE083D3B9BA73A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735C6EABA9B74D0DA47B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: C20DE7B7AB408E4181F030C43753B8186998911F362727C414F749A5E30D975CD0035DD76F8A8A4FF11730708CE550360F159C3576D2A9699C2B6934AE262D3EE7EAB7254005DCED7532B743992DF240BDC6A1CF3F042BAD6DF99611D93F60EF0417BEADF48D1460699F904B3F4130E343918A1A30D5E7FCCB5012B2E24CD356 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D345B8C2BEC800D1021EEA7288A8D8EC734A136ABECFFD66EF850381E4C46C81AB0A49CB63B27AA9E911D7E09C32AA3244C5088106D1C50A26124455A17EC39D0758A6D4CC6FBFAC251FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyO2lHpuZu4RNMXTvnHrBUw== X-Mailru-Sender: 689FA8AB762F73936BC43F508A0638222BE49F7190A5C6FF212F966BF9654C4583D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: [Tarantool-patches] [PATCH v5 30/52] sql: introduce mem_copy_bin() 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:05, Vladislav Shpilevoy wrote: > Thanks for the patch! > > On 23.03.2021 10:36, Mergen Imeev via Tarantool-patches wrote: >> This patch introduces mem_copy_binary() function. Function >> mem_copy_binary() clears MEM, allocates enough memory and copies given >> binary to allocated memory. >> >> Part of #5818 >> --- >> src/box/sql/mem.c | 14 ++++++++++++++ >> src/box/sql/mem.h | 3 +++ >> src/box/sql/vdbe.c | 7 ++----- >> src/box/sql/vdbeapi.c | 4 ++-- >> src/box/sql/vdbesort.c | 6 +----- >> 5 files changed, 22 insertions(+), 12 deletions(-) >> >> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c >> index 5ee49cdca..99beec9ad 100644 >> --- a/src/box/sql/mem.c >> +++ b/src/box/sql/mem.c >> @@ -465,6 +465,20 @@ mem_set_allocated_binary(struct Mem *mem, char *value, uint32_t size) >> mem_set_dyn_bin(mem, value, size, 0); >> } >> >> +int >> +mem_copy_binary(struct Mem *mem, const char *value, uint32_t size) >> +{ > > What if mem is not a binary now? What if it is a frame? Why don't you clear it? > Thanks! Fixed here and in mem_copy_str(), mem_copy_str0() functions. >> + bool is_own_value = (mem->flags & MEM_Blob) != 0 && mem->z == value; >> + if (sqlVdbeMemGrow(mem, size, is_own_value) != 0) >> + return -1; >> + if (!is_own_value) >> + memcpy(mem->z, value, size); >> + mem->n = size; >> + mem->flags = MEM_Blob; >> + mem->field_type = FIELD_TYPE_VARBINARY; >> + return 0; >> +} New patch: commit ab44b535847f591f638826e19277b85bab9f6189 Author: Mergen Imeev Date: Tue Mar 16 11:55:05 2021 +0300 sql: introduce mem_copy_bin() This patch introduces mem_copy_bin() function. This function copies given binary value to a newly allocated by MEM memory. Part of #5818 diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 2622cdd82..e30795de5 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -40,6 +40,19 @@ #include "lua/utils.h" #include "lua/msgpack.h" +/* + * Make sure pMem->z points to a writable allocation of at least + * min(n,32) bytes. + * + * If the bPreserve argument is true, then copy of the content of + * pMem->z into the new allocation. pMem must be either a string or + * blob if bPreserve is true. If bPreserve is false, any prior content + * in pMem->z is discarded. + */ +static int +sqlVdbeMemGrow(struct Mem *pMem, int n, int preserve); + + bool mem_is_null(const struct Mem *mem) { @@ -477,6 +490,23 @@ mem_set_bin_allocated(struct Mem *mem, char *value, uint32_t size) set_bin_dynamic(mem, value, size, 0); } +int +mem_copy_bin(struct Mem *mem, const char *value, uint32_t size) +{ + if ((mem->flags & (MEM_Agg | MEM_Frame)) != 0) + mem_clear(mem); + bool is_own_value = (mem->flags & (MEM_Str | MEM_Blob)) != 0 && + mem->z == value; + if (sqlVdbeMemGrow(mem, size, is_own_value) != 0) + return -1; + if (!is_own_value) + memcpy(mem->z, value, size); + mem->n = size; + mem->flags = MEM_Blob; + mem->field_type = FIELD_TYPE_VARBINARY; + return 0; +} + int mem_copy(struct Mem *to, const struct Mem *from) { @@ -2046,17 +2076,8 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type) return mem_convert_to_integer(mem); } -/* - * Make sure pMem->z points to a writable allocation of at least - * min(n,32) bytes. - * - * If the bPreserve argument is true, then copy of the content of - * pMem->z into the new allocation. pMem must be either a string or - * blob if bPreserve is true. If bPreserve is false, any prior content - * in pMem->z is discarded. - */ -SQL_NOINLINE int -sqlVdbeMemGrow(Mem * pMem, int n, int bPreserve) +static int +sqlVdbeMemGrow(struct Mem *pMem, int n, int bPreserve) { assert(sqlVdbeCheckMemInvariants(pMem)); testcase(pMem->db == 0); diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h index 532860ff7..e17a722f4 100644 --- a/src/box/sql/mem.h +++ b/src/box/sql/mem.h @@ -274,6 +274,13 @@ mem_set_bin_dynamic(struct Mem *mem, char *value, uint32_t size); void mem_set_bin_allocated(struct Mem *mem, char *value, uint32_t size); +/** + * Copy binary value to a newly allocated memory. The MEM type becomes + * VARBINARY. + */ +int +mem_copy_bin(struct Mem *mem, const char *value, uint32_t size); + /** * Copy content of MEM from one MEM to another. In case source MEM contains * string or binary and allocation type is not STATIC, this value is copied to @@ -541,7 +548,6 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type); /** Setters = Change MEM value. */ -int sqlVdbeMemGrow(struct Mem * pMem, int n, int preserve); int sqlVdbeMemClearAndResize(struct Mem * pMem, int n); /** diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 85fb7a26d..19a0b041c 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -2196,11 +2196,8 @@ case OP_MakeRecord: { * routine. */ if (bIsEphemeral) { - if (sqlVdbeMemClearAndResize(pOut, tuple_size) != 0) + if (mem_copy_bin(pOut, tuple, tuple_size) != 0) goto abort_due_to_error; - pOut->flags = MEM_Blob; - pOut->n = tuple_size; - memcpy(pOut->z, tuple, tuple_size); region_truncate(region, used); } else { /* Allocate memory on the region for the tuple @@ -2540,7 +2537,7 @@ case OP_SequenceTest: { * Open a new cursor that points to a fake table that contains a single * row of data. The content of that one row is the content of memory * register P2. In other words, cursor P1 becomes an alias for the - * MEM_Blob content contained in register P2. + * MEM with binary content contained in register P2. * * A pseudo-table created by this opcode is used to hold a single * row output from the sorter so that the row can be decomposed into diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index 8c7a0b6cd..d16b92cf6 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -188,7 +188,7 @@ sql_result_blob(sql_context * pCtx, mem_set_bin_allocated(pCtx->pOut, (char *)z, n); else if (xDel != SQL_TRANSIENT) mem_set_bin_dynamic(pCtx->pOut, (char *)z, n); - else if (sqlVdbeMemSetStr(pCtx->pOut, z, n, 0, xDel) != 0) + else if (mem_copy_bin(pCtx->pOut, z, n) != 0) pCtx->is_aborted = true; } @@ -843,7 +843,7 @@ sql_bind_blob(sql_stmt * pStmt, mem_set_bin_allocated(var, (char *)zData, nData); else if (xDel != SQL_TRANSIENT) mem_set_bin_dynamic(var, (char *)zData, nData); - else if (sqlVdbeMemSetStr(var, zData, nData, 0, xDel) != 0) + else if (mem_copy_bin(var, zData, nData) != 0) return -1; return sql_bind_type(p, i, "varbinary"); } diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 171cb8946..c294a0286 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -1305,19 +1305,26 @@ sqlVdbeList(Vdbe * p) * has not already been seen. */ if (pOp->p4type == P4_SUBPROGRAM) { - int nByte = (nSub + 1) * sizeof(SubProgram *); int j; for (j = 0; j < nSub; j++) { if (apSub[j] == pOp->p4.pProgram) break; } - if (j == nSub && - sqlVdbeMemGrow(pSub, nByte, - nSub != 0) == 0) { - apSub = (SubProgram **) pSub->z; - apSub[nSub++] = pOp->p4.pProgram; - pSub->flags |= MEM_Blob; - pSub->n = nSub * sizeof(SubProgram *); + if (nSub == 0) { + uint32_t size = sizeof(SubProgram *); + char *bin = (char *)&pOp->p4.pProgram; + if (mem_copy_bin(pSub, bin, size) != 0) + return -1; + } else if (j == nSub) { + struct Mem tmp; + mem_create(&tmp); + uint32_t size = sizeof(SubProgram *); + char *bin = (char *)&pOp->p4.pProgram; + mem_set_bin_ephemeral(&tmp, bin, size); + int rc = mem_concat(pSub, &tmp, pSub); + mem_destroy(&tmp); + if (rc != 0) + return -1; } } } diff --git a/src/box/sql/vdbesort.c b/src/box/sql/vdbesort.c index a9a5f45af..3da425be4 100644 --- a/src/box/sql/vdbesort.c +++ b/src/box/sql/vdbesort.c @@ -2164,12 +2164,8 @@ sqlVdbeSorterRowkey(const VdbeCursor * pCsr, Mem * pOut) assert(pCsr->eCurType == CURTYPE_SORTER); pSorter = pCsr->uc.pSorter; pKey = vdbeSorterRowkey(pSorter, &nKey); - if (sqlVdbeMemClearAndResize(pOut, nKey)) { + if (mem_copy_bin(pOut, pKey, nKey) != 0) return -1; - } - pOut->n = nKey; - MemSetTypeFlag(pOut, MEM_Blob); - memcpy(pOut->z, pKey, nKey); return 0; }