[Tarantool-patches] [PATCH v5 49/52] sql: introduce mem_get_str0() and mem_as_str0()
imeevma at tarantool.org
imeevma at tarantool.org
Sat Apr 10 00:08:23 MSK 2021
Thank you for the review! My answers and new patch below.
On 30.03.2021 02:08, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index d033dae86..78f4ec3b5 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -54,6 +54,24 @@
>> #include "lua/utils.h"
>> #include "mpstream/mpstream.h"
>>
>> +static const char *
>> +mem_get_str(struct Mem *mem)
>> +{
>> + const char *str;
>> + if (mem_convert_to_string0(mem) != 0 || mem_get_string0(mem, &str) != 0)
>
> 1. You have the same code in 2 other files. I think it is worth moving this
> to mem.h.
>
Done.
>> + return NULL;
>> + return str;
>> +}
>> +
>> +static const unsigned char *
>> +mem_get_ustr(struct Mem *mem)
>
> 2. Function called 'get' should not change the value. Maybe 'as'? mem_as_str().
>
Thanks! Fixed.
>> +{
>> + const char *str;
>> + if (mem_convert_to_string0(mem) != 0 || mem_get_string0(mem, &str) != 0)
>> + return NULL;
>> + return (const unsigned char *)str;
>> +}
New patch:
commit 056987bba3f23e3298201d14728452f3033a166c
Author: Mergen Imeev <imeevma at gmail.com>
Date: Thu Mar 18 12:41:41 2021 +0300
sql: introduce mem_get_str0() and mem_as_str0()
This patch introduces mem_get_str0() and mem_as_str0(). Function
mem_get_str0() is used to receive NULL-terminated string from MEM. If
value of MEM is not NULL-terminated string, it is converted to
NULL-terminated string if possible. MEM is not changed. Function
mem_as_str0() is also used to receive NULL-terminated string from MEM,
however if MEM does not contain NULL-terminated string it converts MEM
to MEM that contains NULL-terminated string and returns its value.
Part of #5818
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 6a6970647..c1db6f78a 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -48,6 +48,12 @@
#include "box/coll_id_cache.h"
#include "box/schema.h"
+static const unsigned char *
+mem_as_ustr(struct Mem *mem)
+{
+ return (const unsigned char *)mem_as_str0(mem);
+}
+
/*
* Return the collating function associated with a function.
*/
@@ -174,7 +180,7 @@ lengthFunc(sql_context * context, int argc, sql_value ** argv)
break;
}
case MP_STR:{
- const unsigned char *z = sql_value_text(argv[0]);
+ const unsigned char *z = mem_as_ustr(argv[0]);
if (z == 0)
return;
len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
@@ -329,8 +335,8 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
* Character size is equal to
* needle char size.
*/
- haystack_str = sql_value_text(haystack);
- needle_str = sql_value_text(needle);
+ haystack_str = mem_as_ustr(haystack);
+ needle_str = mem_as_ustr(needle);
int n_needle_chars =
sql_utf8_char_count(needle_str, n_needle_bytes);
@@ -392,8 +398,7 @@ printfFunc(sql_context * context, int argc, sql_value ** argv)
int n;
sql *db = sql_context_db_handle(context);
- if (argc >= 1
- && (zFormat = (const char *)sql_value_text(argv[0])) != 0) {
+ if (argc >= 1 && (zFormat = mem_as_str0(argv[0])) != NULL) {
x.nArg = argc - 1;
x.nUsed = 0;
x.apArg = argv + 1;
@@ -447,7 +452,7 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
return;
assert(len == sql_value_bytes(argv[0]));
} else {
- z = sql_value_text(argv[0]);
+ z = mem_as_ustr(argv[0]);
if (z == 0)
return;
len = 0;
@@ -610,13 +615,13 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv) \
context->is_aborted = true; \
return; \
} \
- z2 = (char *)sql_value_text(argv[0]); \
+ z2 = mem_as_str0(argv[0]); \
n = sql_value_bytes(argv[0]); \
/* \
* Verify that the call to _bytes() \
* does not invalidate the _text() pointer. \
*/ \
- assert(z2 == (char *)sql_value_text(argv[0])); \
+ assert(z2 == mem_as_str0(argv[0])); \
if (!z2) \
return; \
z1 = contextMalloc(context, ((i64) n) + 1); \
@@ -947,8 +952,8 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
context->is_aborted = true;
return;
}
- const char *zB = (const char *) sql_value_text(argv[0]);
- const char *zA = (const char *) sql_value_text(argv[1]);
+ const char *zB = mem_as_str0(argv[0]);
+ const char *zA = mem_as_str0(argv[1]);
const char *zB_end = zB + sql_value_bytes(argv[0]);
const char *zA_end = zA + sql_value_bytes(argv[1]);
@@ -967,7 +972,7 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
return;
}
/* Encoding did not change */
- assert(zB == (const char *) sql_value_text(argv[0]));
+ assert(zB == mem_as_str0(argv[0]));
if (argc == 3) {
/*
@@ -975,7 +980,7 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
* single UTF-8 character. Otherwise, return an
* error.
*/
- const unsigned char *zEsc = sql_value_text(argv[2]);
+ const unsigned char *zEsc = mem_as_ustr(argv[2]);
if (zEsc == 0)
return;
if (sql_utf8_char_count(zEsc, sql_value_bytes(argv[2])) != 1) {
@@ -1104,7 +1109,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
case MP_STR:{
int i, j;
u64 n;
- const unsigned char *zArg = sql_value_text(argv[0]);
+ const unsigned char *zArg = mem_as_ustr(argv[0]);
char *z;
if (zArg == 0)
@@ -1151,7 +1156,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
static void
unicodeFunc(sql_context * context, int argc, sql_value ** argv)
{
- const unsigned char *z = sql_value_text(argv[0]);
+ const unsigned char *z = mem_as_ustr(argv[0]);
(void)argc;
if (z && z[0])
sql_result_uint(context, sqlUtf8Read(&z));
@@ -1270,12 +1275,12 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
assert(argc == 3);
UNUSED_PARAMETER(argc);
- zStr = sql_value_text(argv[0]);
+ zStr = mem_as_ustr(argv[0]);
if (zStr == 0)
return;
nStr = sql_value_bytes(argv[0]);
- assert(zStr == sql_value_text(argv[0])); /* No encoding change */
- zPattern = sql_value_text(argv[1]);
+ assert(zStr == mem_as_ustr(argv[0])); /* No encoding change */
+ zPattern = mem_as_ustr(argv[1]);
if (zPattern == 0) {
assert(mem_is_null(argv[1])
|| sql_context_db_handle(context)->mallocFailed);
@@ -1287,12 +1292,12 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
sql_result_value(context, argv[0]);
return;
}
- assert(zPattern == sql_value_text(argv[1])); /* No encoding change */
- zRep = sql_value_text(argv[2]);
+ assert(zPattern == mem_as_ustr(argv[1])); /* No encoding change */
+ zRep = mem_as_ustr(argv[2]);
if (zRep == 0)
return;
nRep = sql_value_bytes(argv[2]);
- assert(zRep == sql_value_text(argv[2]));
+ assert(zRep == mem_as_ustr(argv[2]));
nOut = nStr + 1;
assert(nOut < SQL_MAX_LENGTH);
zOut = contextMalloc(context, (i64) nOut);
@@ -1454,7 +1459,7 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg)
else
default_trim = (const unsigned char *) " ";
int input_str_sz = sql_value_bytes(arg);
- const unsigned char *input_str = sql_value_text(arg);
+ const unsigned char *input_str = mem_as_ustr(arg);
uint8_t trim_char_len[1] = { 1 };
trim_procedure(context, TRIM_BOTH, default_trim, trim_char_len, 1,
input_str, input_str_sz);
@@ -1476,7 +1481,7 @@ trim_func_two_args(struct sql_context *context, sql_value *arg1,
sql_value *arg2)
{
const unsigned char *input_str, *trim_set;
- if ((input_str = sql_value_text(arg2)) == NULL)
+ if ((input_str = mem_as_ustr(arg2)) == NULL)
return;
int input_str_sz = sql_value_bytes(arg2);
@@ -1486,7 +1491,7 @@ trim_func_two_args(struct sql_context *context, sql_value *arg1,
mem_get_uint(arg1, &n);
trim_procedure(context, n, (const unsigned char *) " ",
&len_one, 1, input_str, input_str_sz);
- } else if ((trim_set = sql_value_text(arg1)) != NULL) {
+ } else if ((trim_set = mem_as_ustr(arg1)) != NULL) {
int trim_set_sz = sql_value_bytes(arg1);
uint8_t *char_len;
int char_cnt = trim_prepare_char_len(context, trim_set,
@@ -1512,8 +1517,8 @@ trim_func_three_args(struct sql_context *context, sql_value *arg1,
{
assert(sql_value_type(arg1) == MP_INT || sql_value_type(arg1) == MP_UINT);
const unsigned char *input_str, *trim_set;
- if ((input_str = sql_value_text(arg3)) == NULL ||
- (trim_set = sql_value_text(arg2)) == NULL)
+ if ((input_str = mem_as_ustr(arg3)) == NULL ||
+ (trim_set = mem_as_ustr(arg2)) == NULL)
return;
int trim_set_sz = sql_value_bytes(arg2);
@@ -1587,7 +1592,7 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv)
context->is_aborted = true;
return;
}
- zIn = (u8 *) sql_value_text(argv[0]);
+ zIn = (u8 *) mem_as_ustr(argv[0]);
if (zIn == 0)
zIn = (u8 *) "";
for (i = 0; zIn[i] && !sqlIsalpha(zIn[i]); i++) {
@@ -1836,7 +1841,7 @@ groupConcatStep(sql_context * context, int argc, sql_value ** argv)
pAccum->mxAlloc = db->aLimit[SQL_LIMIT_LENGTH];
if (!firstTerm) {
if (argc == 2) {
- zSep = (char *)sql_value_text(argv[1]);
+ zSep = mem_as_str0(argv[1]);
nSep = sql_value_bytes(argv[1]);
} else {
zSep = ",";
@@ -1845,7 +1850,7 @@ groupConcatStep(sql_context * context, int argc, sql_value ** argv)
if (zSep)
sqlStrAccumAppend(pAccum, zSep, nSep);
}
- zVal = (char *)sql_value_text(argv[0]);
+ zVal = mem_as_str0(argv[0]);
nVal = sql_value_bytes(argv[0]);
if (zVal)
sqlStrAccumAppend(pAccum, zVal, nVal);
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 6f787f7cc..8c2d09168 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1323,6 +1323,15 @@ mem_get_bool(const struct Mem *mem, bool *b)
return -1;
}
+int
+mem_get_str0(const struct Mem *mem, const char **s)
+{
+ if ((mem->flags & MEM_Str) == 0 || (mem->flags & MEM_Term) == 0)
+ return -1;
+ *s = mem->z;
+ return 0;
+}
+
int
mem_copy(struct Mem *to, const struct Mem *from)
{
@@ -2001,45 +2010,6 @@ mem_has_msgpack_subtype(struct Mem *mem)
mem->subtype == SQL_SUBTYPE_MSGPACK;
}
-/*
- * The pVal argument is known to be a value other than NULL.
- * Convert it into a string with encoding enc and return a pointer
- * to a zero-terminated version of that string.
- */
-static SQL_NOINLINE const void *
-valueToText(sql_value * pVal)
-{
- assert(pVal != 0);
- assert((pVal->flags & (MEM_Null)) == 0);
- if ((pVal->flags & (MEM_Blob | MEM_Str)) &&
- !mem_has_msgpack_subtype(pVal)) {
- if (ExpandBlob(pVal))
- return 0;
- pVal->flags |= MEM_Str;
- sqlVdbeMemNulTerminate(pVal); /* IMP: R-31275-44060 */
- } else {
- mem_to_str(pVal);
- assert(0 == (1 & SQL_PTR_TO_INT(pVal->z)));
- }
- return pVal->z;
-}
-
-/*
- * It is already known that pMem contains an unterminated string.
- * Add the zero terminator.
- */
-static SQL_NOINLINE int
-vdbeMemAddTerminator(Mem * pMem)
-{
- if (sqlVdbeMemGrow(pMem, pMem->n + 2, 1)) {
- return -1;
- }
- pMem->z[pMem->n] = 0;
- pMem->z[pMem->n + 1] = 0;
- pMem->flags |= MEM_Term;
- return 0;
-}
-
/*
* Both *pMem1 and *pMem2 contain string values. Compare the two values
* using the collation sequence pColl. As usual, return a negative , zero
@@ -2141,7 +2111,9 @@ sql_value_type(sql_value *pVal)
static SQL_NOINLINE int
valueBytes(sql_value * pVal)
{
- return valueToText(pVal) != 0 ? pVal->n : 0;
+ if (mem_to_str(pVal) != 0)
+ return 0;
+ return pVal->n;
}
int
@@ -2330,21 +2302,6 @@ registerTrace(int iReg, Mem *p) {
}
#endif
-/*
- * Make sure the given Mem is \u0000 terminated.
- */
-int
-sqlVdbeMemNulTerminate(Mem * pMem)
-{
- testcase((pMem->flags & (MEM_Term | MEM_Str)) == (MEM_Term | MEM_Str));
- testcase((pMem->flags & (MEM_Term | MEM_Str)) == 0);
- if ((pMem->flags & (MEM_Term | MEM_Str)) != MEM_Str) {
- return 0; /* Nothing to do */
- } else {
- return vdbeMemAddTerminator(pMem);
- }
-}
-
/*
* If the given Mem* has a zero-filled tail, turn it into an ordinary
* blob stored in dynamically allocated space.
@@ -2503,7 +2460,9 @@ sql_value_blob(sql_value * pVal)
p->flags |= MEM_Blob;
return p->n ? p->z : 0;
} else {
- return sql_value_text(pVal);
+ if (mem_to_str(pVal) != 0)
+ return NULL;
+ return pVal->z;
}
}
@@ -2513,36 +2472,6 @@ sql_value_bytes(sql_value * pVal)
return sqlValueBytes(pVal);
}
-const unsigned char *
-sql_value_text(sql_value * pVal)
-{
- return (const unsigned char *)sqlValueText(pVal);
-}
-
-/* This function is only available internally, it is not part of the
- * external API. It works in a similar way to sql_value_text(),
- * except the data returned is in the encoding specified by the second
- * parameter, which must be one of SQL_UTF16BE, SQL_UTF16LE or
- * SQL_UTF8.
- *
- * (2006-02-16:) The enc value can be or-ed with SQL_UTF16_ALIGNED.
- * If that is the case, then the result must be aligned on an even byte
- * boundary.
- */
-const void *
-sqlValueText(sql_value * pVal)
-{
- if (!pVal)
- return 0;
- if ((pVal->flags & (MEM_Str | MEM_Term)) == (MEM_Str | MEM_Term)) {
- return pVal->z;
- }
- if (pVal->flags & MEM_Null) {
- return 0;
- }
- return valueToText(pVal);
-}
-
/*
* Return a pointer to static memory containing an SQL NULL value.
*/
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index e48e8788c..5848ae729 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -568,6 +568,28 @@ mem_get_double(const struct Mem *mem, double *d);
int
mem_get_bool(const struct Mem *mem, bool *b);
+/**
+ * Return value for MEM of STRING type if MEM contains a NULL-terminated string.
+ * Otherwise convert value of the MEM to NULL-terminated string if possible and
+ * return converted value. Original MEM is not changed.
+ */
+int
+mem_get_str0(const struct Mem *mem, const char **s);
+
+/**
+ * Return value for MEM of STRING type if MEM contains NULL-terminated string.
+ * Otherwise convert MEM to MEM of string type that contains NULL-terminated
+ * string and return its value. Return NULL if conversion is impossible.
+ */
+static inline const char *
+mem_as_str0(struct Mem *mem)
+{
+ const char *str;
+ if (mem_to_str0(mem) != 0 || mem_get_str0(mem, &str) != 0)
+ return NULL;
+ return str;
+}
+
/**
* Simple type to str convertor. It is used to simplify
* error reporting.
@@ -601,7 +623,6 @@ registerTrace(int iReg, Mem *p);
#define memIsValid(M) !mem_is_invalid(M)
#endif
-int sqlVdbeMemNulTerminate(struct Mem *);
int sqlVdbeMemExpandBlob(struct Mem *);
#define ExpandBlob(P) (mem_is_zerobin(P)? sqlVdbeMemExpandBlob(P) : 0)
@@ -626,11 +647,6 @@ sql_value_blob(struct Mem *);
int
sql_value_bytes(struct Mem *);
-const unsigned char *
-sql_value_text(struct Mem *);
-
-const void *sqlValueText(struct Mem *);
-
#define VdbeFrameMem(p) ((Mem *)&((u8 *)p)[ROUND8(sizeof(VdbeFrame))])
const Mem *
diff --git a/src/box/sql/printf.c b/src/box/sql/printf.c
index 605478820..c3e5bb2e1 100644
--- a/src/box/sql/printf.c
+++ b/src/box/sql/printf.c
@@ -165,7 +165,8 @@ getTextArg(PrintfArguments * p)
{
if (p->nArg <= p->nUsed)
return 0;
- return (char *)sql_value_text(p->apArg[p->nUsed++]);
+ struct Mem *mem = p->apArg[p->nUsed++];
+ return (char *)mem_as_str0(mem);
}
/*
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index df9469941..51613aea3 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -442,10 +442,6 @@ sql_column_bytes(sql_stmt *, int iCol);
int
sql_column_bytes16(sql_stmt *, int iCol);
-const unsigned char *
-sql_column_text(sql_stmt *,
- int iCol);
-
char *
sql_result_to_msgpack(struct sql_stmt *stmt, uint32_t *tuple_size,
struct region *region);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 1d3c23a70..c229160b6 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -484,12 +484,6 @@ sql_column_bytes(sql_stmt * pStmt, int i)
return sql_value_bytes(columnMem(pStmt, i));
}
-const unsigned char *
-sql_column_text(sql_stmt * pStmt, int i)
-{
- return sql_value_text(columnMem(pStmt, i));
-}
-
char *
sql_result_to_msgpack(struct sql_stmt *stmt, uint32_t *tuple_size,
struct region *region)
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 93de722cb..fe7329ea8 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -312,8 +312,10 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix,
pVal =
sqlVdbeGetBoundValue(pReprepare, iCol,
FIELD_TYPE_SCALAR);
- if (pVal != NULL && mem_is_str(pVal))
- z = (char *)sql_value_text(pVal);
+ if (pVal != NULL && mem_is_str(pVal)) {
+ if (mem_as_str0(pVal) == NULL)
+ return -1;
+ }
assert(pRight->op == TK_VARIABLE || pRight->op == TK_REGISTER);
} else if (op == TK_STRING) {
z = pRight->u.zToken;
More information about the Tarantool-patches
mailing list