[Tarantool-patches] [PATCH v5 30/52] sql: introduce mem_copy_bin()
Mergen Imeev
imeevma at tarantool.org
Wed Apr 14 01:06:39 MSK 2021
Thank you for the review! My answers and new patch below. Patch was a bit
changed due to merge conflicts with new version of
"sql: introduce mem_is_*() functions".
On Tue, Apr 13, 2021 at 01:36:15AM +0200, Vladislav Shpilevoy wrote:
> I appreciate the work you did here!
>
> See 2 comments below.
>
> > 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);
> > +
> > +
>
> 1. Double empty line.
>
Fixed during resolving merge conflicts with new version of
"sql: introduce mem_is_*() functions".
> > 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);
>
> 2. The same comment as for copy_str.
>
Same as there, in case copied value is value of the MEM it copied to and its
allocation type being MEM_Dyn, it will be lost during clear().
> > + 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;
> > +}
New patch:
commit 8788956575ecf095d86d63de5eeea2e932fb0bac
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 1e7adfcb0..190311d80 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -40,6 +40,18 @@
#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);
+
enum {
BUF_SIZE = 32,
};
@@ -335,6 +347,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)
{
@@ -1869,17 +1898,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 617966cf7..197c16461 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -456,6 +456,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
@@ -699,7 +706,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 cd7811846..d573e1e8f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2240,11 +2240,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
@@ -2584,7 +2581,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 7114f4351..7656d8d67 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -168,7 +168,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;
}
@@ -807,7 +807,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 dad9cab03..ffc85b91d 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