[Tarantool-patches] [PATCH v5 30/52] sql: introduce mem_copy_bin()
imeevma at tarantool.org
imeevma at tarantool.org
Fri Apr 9 22:45:44 MSK 2021
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 <imeevma at gmail.com>
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;
}
More information about the Tarantool-patches
mailing list