[tarantool-patches] Re: [PATCH 3/9] sql: use msgpack types instead of custom ones
n.pettik
korablev at tarantool.org
Thu Apr 18 20:54:37 MSK 2019
>> diff --git a/src/box/bind.c b/src/box/bind.c
>> index 4b57e23c8..5aa79751a 100644
>> --- a/src/box/bind.c
>> +++ b/src/box/bind.c
>> @@ -138,6 +121,7 @@ sql_bind_decode(struct sql_bind *bind, int i, const char **packet)
>> default:
>> unreachable();
>> }
>> + bind->type = type;
>
> 1. struct sql_bind.type still is uint8_t and commented as
> 'SQL type of the value.'. Please, change its type to enum mp_type
> and say in the comment 'MessagePack type of the value.’
diff --git a/src/box/bind.h b/src/box/bind.h
index cacb4a2d9..a915381e4 100644
--- a/src/box/bind.h
+++ b/src/box/bind.h
@@ -39,6 +39,8 @@ extern "C" {
#include <stdint.h>
#include <stdlib.h>
+#include "msgpuck.h"
+
struct sql_stmt;
/**
@@ -55,8 +57,8 @@ struct sql_bind {
/** Byte length of the value. */
uint32_t bytes;
- /** SQL type of the value. */
- uint8_t type;
+ /** MessagePack type of the value. */
+ enum mp_type type;
/** Bind value. */
union {
double d;
>>> diff --git a/src/box/sql/date.c b/src/box/sql/date.c
>> index 5f5272ea3..ead0c01d0 100644
>> --- a/src/box/sql/date.c
>> +++ b/src/box/sql/date.c
>> @@ -937,7 +937,7 @@ isDate(sql_context * context, int argc, sql_value ** argv, DateTime * p)
>> if (argc == 0) {
>> return setDateTimeToCurrent(context, p);
>> }
>> - if ((eType = sql_value_type(argv[0])) == SQL_FLOAT
>> + if ((eType = sql_value_type(argv[0])) == MP_DOUBLE
>
> 2. Firstly, the code is commented out. Secondly, sql_value_type
> does not exist anymore. Thirdly, eType is of type int, not
> enum mp_type, and can not be compared with MP_DOUBLE.
>
> Please, rename sql_value_mp_type to sql_value_type, because we
> do not have 'not mp' type anymore. Then this hunk will be ok
> except the third objection.
Ok, renamed and fixed type of eType (nevertheless that chunk is
related to dead code).
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 3cdb119c8..667837528 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -87,10 +87,10 @@ minmaxFunc(sql_context * context, int argc, sql_value ** argv)
pColl = sqlGetFuncCollSeq(context);
assert(mask == -1 || mask == 0);
iBest = 0;
- if (sql_value_mp_type(argv[0]) == MP_NIL)
+ if (sql_value_type(argv[0]) == MP_NIL)
return;
for (i = 1; i < argc; i++) {
- if (sql_value_mp_type(argv[i]) == MP_NIL)
+ if (sql_value_type(argv[i]) == MP_NIL)
return;
if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) >=
0) {
@@ -109,7 +109,7 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
{
const char *z = 0;
UNUSED_PARAMETER(NotUsed);
- switch (sql_value_mp_type(argv[0])) {
+ switch (sql_value_type(argv[0])) {
case MP_INT:
z = "integer";
break;
@@ -139,7 +139,7 @@ lengthFunc(sql_context * context, int argc, sql_value ** argv)
assert(argc == 1);
UNUSED_PARAMETER(argc);
- switch (sql_value_mp_type(argv[0])) {
+ switch (sql_value_type(argv[0])) {
case MP_BIN:
case MP_INT:
case MP_DOUBLE:{
@@ -173,7 +173,7 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
{
assert(argc == 1);
UNUSED_PARAMETER(argc);
- switch (sql_value_mp_type(argv[0])) {
+ switch (sql_value_type(argv[0])) {
case MP_INT:{
i64 iVal = sql_value_int64(argv[0]);
if (iVal < 0) {
@@ -229,8 +229,8 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
UNUSED_PARAMETER(argc);
struct Mem *needle = argv[0];
struct Mem *haystack = argv[1];
- enum mp_type needle_type = sql_value_mp_type(needle);
- enum mp_type haystack_type = sql_value_mp_type(haystack);
+ enum mp_type needle_type = sql_value_type(needle);
+ enum mp_type haystack_type = sql_value_type(haystack);
if (haystack_type == MP_NIL || needle_type == MP_NIL)
return;
@@ -398,12 +398,12 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
int negP2 = 0;
assert(argc == 3 || argc == 2);
- if (sql_value_mp_type(argv[1]) == MP_NIL
- || (argc == 3 && sql_value_mp_type(argv[2]) == MP_NIL)
+ if (sql_value_type(argv[1]) == MP_NIL
+ || (argc == 3 && sql_value_type(argv[2]) == MP_NIL)
) {
return;
}
- p0type = sql_value_mp_type(argv[0]);
+ p0type = sql_value_type(argv[0]);
p1 = sql_value_int(argv[1]);
if (p0type == MP_BIN) {
len = sql_value_bytes(argv[0]);
@@ -507,13 +507,13 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
char *zBuf;
assert(argc == 1 || argc == 2);
if (argc == 2) {
- if (MP_NIL == sql_value_mp_type(argv[1]))
+ if (MP_NIL == sql_value_type(argv[1]))
return;
n = sql_value_int(argv[1]);
if (n < 0)
n = 0;
}
- if (sql_value_mp_type(argv[0]) == MP_NIL)
+ if (sql_value_type(argv[0]) == MP_NIL)
return;
r = sql_value_double(argv[0]);
/* If Y==0 and X will fit in a 64-bit int,
@@ -900,8 +900,8 @@ likeFunc(sql_context *context, int argc, sql_value **argv)
int nPat;
sql *db = sql_context_db_handle(context);
int is_like_ci = SQL_PTR_TO_INT(sql_user_data(context));
- int rhs_type = sql_value_mp_type(argv[0]);
- int lhs_type = sql_value_mp_type(argv[1]);
+ int rhs_type = sql_value_type(argv[0]);
+ int lhs_type = sql_value_type(argv[1]);
if (lhs_type != MP_STR || rhs_type != MP_STR) {
if (lhs_type == MP_NIL || rhs_type == MP_NIL)
@@ -1018,7 +1018,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
{
assert(argc == 1);
UNUSED_PARAMETER(argc);
- switch (sql_value_mp_type(argv[0])) {
+ switch (sql_value_type(argv[0])) {
case MP_DOUBLE:{
double r1, r2;
char zBuf[50];
@@ -1092,7 +1092,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
break;
}
default:{
- assert(sql_value_mp_type(argv[0]) == MP_NIL);
+ assert(sql_value_type(argv[0]) == MP_NIL);
sql_result_text(context, "NULL", 4, SQL_STATIC);
break;
}
@@ -1228,13 +1228,13 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
assert(zStr == sql_value_text(argv[0])); /* No encoding change */
zPattern = sql_value_text(argv[1]);
if (zPattern == 0) {
- assert(sql_value_mp_type(argv[1]) == MP_NIL
+ assert(sql_value_type(argv[1]) == MP_NIL
|| sql_context_db_handle(context)->mallocFailed);
return;
}
nPattern = sql_value_bytes(argv[1]);
if (nPattern == 0) {
- assert(sql_value_mp_type(argv[1]) != MP_NIL);
+ assert(sql_value_type(argv[1]) != MP_NIL);
sql_result_value(context, argv[0]);
return;
}
@@ -1302,7 +1302,7 @@ trimFunc(sql_context * context, int argc, sql_value ** argv)
unsigned char **azChar = 0; /* Individual characters in zCharSet */
int nChar; /* Number of characters in zCharSet */
- if (sql_value_mp_type(argv[0]) == MP_NIL) {
+ if (sql_value_type(argv[0]) == MP_NIL) {
return;
}
zIn = sql_value_text(argv[0]);
@@ -1499,7 +1499,7 @@ sumStep(sql_context * context, int argc, sql_value ** argv)
UNUSED_PARAMETER(argc);
p = sql_aggregate_context(context, sizeof(*p));
assert(p != NULL);
- enum mp_type type = sql_value_mp_type(argv[0]);
+ enum mp_type type = sql_value_type(argv[0]);
if (type == MP_NIL)
return;
if (type != MP_DOUBLE && type != MP_INT) {
@@ -1510,7 +1510,7 @@ sumStep(sql_context * context, int argc, sql_value ** argv)
context->isError = SQL_TARANTOOL_ERROR;
return;
}
- type = sql_value_mp_type(argv[0]);
+ type = sql_value_type(argv[0]);
}
p->cnt++;
if (type == MP_INT) {
@@ -1578,7 +1578,7 @@ countStep(sql_context * context, int argc, sql_value ** argv)
{
CountCtx *p;
p = sql_aggregate_context(context, sizeof(*p));
- if ((argc == 0 || MP_NIL != sql_value_mp_type(argv[0])) && p) {
+ if ((argc == 0 || MP_NIL != sql_value_type(argv[0])) && p) {
p->n++;
}
}
@@ -1605,7 +1605,7 @@ minmaxStep(sql_context * context, int NotUsed, sql_value ** argv)
if (!pBest)
return;
- if (sql_value_mp_type(argv[0]) == MP_NIL) {
+ if (sql_value_type(argv[0]) == MP_NIL) {
if (pBest->flags)
sqlSkipAccumulatorLoad(context);
} else if (pBest->flags) {
@@ -1657,7 +1657,7 @@ groupConcatStep(sql_context * context, int argc, sql_value ** argv)
const char *zSep;
int nVal, nSep;
assert(argc == 1 || argc == 2);
- if (sql_value_mp_type(argv[0]) == MP_NIL)
+ if (sql_value_type(argv[0]) == MP_NIL)
return;
pAccum =
(StrAccum *) sql_aggregate_context(context, sizeof(*pAccum));
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 9134f767d..3b15d9775 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -456,7 +456,7 @@ const unsigned char *
sql_value_text(sql_value *);
enum mp_type
-sql_value_mp_type(sql_value *);
+sql_value_type(sql_value *);
sql *
sql_context_db_handle(sql_context *);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 6e867ca84..149f3d047 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -244,7 +244,7 @@ sql_value_text(sql_value * pVal)
* point number string BLOB NULL
*/
enum mp_type
-sql_value_mp_type(sql_value *pVal)
+sql_value_type(sql_value *pVal)
{
switch (pVal->flags & MEM_PURE_TYPE_MASK) {
case MEM_Int: return MP_INT;
@@ -1013,7 +1013,7 @@ sql_column_value(sql_stmt * pStmt, int i)
enum mp_type
sql_column_type(sql_stmt * pStmt, int i)
{
- enum mp_type type = sql_value_mp_type(columnMem(pStmt, i));
+ enum mp_type type = sql_value_type(columnMem(pStmt, i));
columnMallocFailure(pStmt);
return type;
}
@@ -1388,7 +1388,7 @@ int
sql_bind_value(sql_stmt * pStmt, int i, const sql_value * pValue)
{
int rc;
- switch (sql_value_mp_type((sql_value *) pValue)) {
+ switch (sql_value_type((sql_value *) pValue)) {
case MP_INT:{
rc = sql_bind_int64(pStmt, i, pValue->u.i);
break;
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index a80c7a0ab..30017b080 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -290,7 +290,7 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix,
pVal =
sqlVdbeGetBoundValue(pReprepare, iCol,
FIELD_TYPE_SCALAR);
- if (pVal && sql_value_mp_type(pVal) == MP_STR) {
+ if (pVal && sql_value_type(pVal) == MP_STR) {
z = (char *)sql_value_text(pVal);
}
>> || eType == SQL_INTEGER) {
>> setRawDateNumber(p, sql_value_double(argv[0]));
>> } else {
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index 9adfeec67..3cdb119c8 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -87,10 +87,10 @@ minmaxFunc(sql_context * context, int argc, sql_value ** argv)
>> pColl = sqlGetFuncCollSeq(context);
>> assert(mask == -1 || mask == 0);
>> iBest = 0;
>> - if (sql_value_type(argv[0]) == SQL_NULL)
>> + if (sql_value_mp_type(argv[0]) == MP_NIL)
>
> 3. Please, move that check into a separate static inline
> function in a header file. It appeared to be used often.
> A possible name 'sql_value_is_null’.
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 3cdb119c8..66f601346 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -87,10 +87,10 @@ minmaxFunc(sql_context * context, int argc, sql_value ** argv)
pColl = sqlGetFuncCollSeq(context);
assert(mask == -1 || mask == 0);
iBest = 0;
- if (sql_value_mp_type(argv[0]) == MP_NIL)
+ if (sql_value_is_null(argv[0]))
return;
for (i = 1; i < argc; i++) {
- if (sql_value_mp_type(argv[i]) == MP_NIL)
+ if (sql_value_is_null(argv[i]))
return;
if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) >=
0) {
@@ -398,12 +398,12 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
int negP2 = 0;
assert(argc == 3 || argc == 2);
- if (sql_value_mp_type(argv[1]) == MP_NIL
- || (argc == 3 && sql_value_mp_type(argv[2]) == MP_NIL)
+ if (sql_value_is_null(argv[1])
+ || (argc == 3 && sql_value_is_null(argv[2]))
) {
return;
}
@@ -507,13 +507,13 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
char *zBuf;
assert(argc == 1 || argc == 2);
if (argc == 2) {
- if (MP_NIL == sql_value_mp_type(argv[1]))
+ if (sql_value_is_null(argv[1]))
return;
n = sql_value_int(argv[1]);
if (n < 0)
n = 0;
}
- if (sql_value_mp_type(argv[0]) == MP_NIL)
+ if (sql_value_is_null(argv[0]))
return;
r = sql_value_double(argv[0]);
/* If Y==0 and X will fit in a 64-bit int,
@@ -1092,7 +1092,7 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
break;
}
default:{
- assert(sql_value_mp_type(argv[0]) == MP_NIL);
+ assert(sql_value_is_null(argv[0]));
sql_result_text(context, "NULL", 4, SQL_STATIC);
break;
}
@@ -1228,13 +1228,13 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
assert(zStr == sql_value_text(argv[0])); /* No encoding change */
zPattern = sql_value_text(argv[1]);
if (zPattern == 0) {
- assert(sql_value_mp_type(argv[1]) == MP_NIL
+ assert(sql_value_is_null(argv[1])
|| sql_context_db_handle(context)->mallocFailed);
return;
}
nPattern = sql_value_bytes(argv[1]);
if (nPattern == 0) {
- assert(sql_value_mp_type(argv[1]) != MP_NIL);
+ assert(! sql_value_is_null(argv[1]));
sql_result_value(context, argv[0]);
return;
}
@@ -1302,7 +1302,7 @@ trimFunc(sql_context * context, int argc, sql_value ** argv)
unsigned char **azChar = 0; /* Individual characters in zCharSet */
int nChar; /* Number of characters in zCharSet */
- if (sql_value_mp_type(argv[0]) == MP_NIL) {
+ if (sql_value_is_null(argv[0])) {
return;
}
zIn = sql_value_text(argv[0]);
@@ -1578,7 +1578,7 @@ countStep(sql_context * context, int argc, sql_value ** argv)
{
CountCtx *p;
p = sql_aggregate_context(context, sizeof(*p));
- if ((argc == 0 || MP_NIL != sql_value_mp_type(argv[0])) && p) {
+ if ((argc == 0 || ! sql_value_is_null(argv[0])) && p) {
p->n++;
}
}
@@ -1605,7 +1605,7 @@ minmaxStep(sql_context * context, int NotUsed, sql_value ** argv)
if (!pBest)
return;
- if (sql_value_mp_type(argv[0]) == MP_NIL) {
+ if (sql_value_is_null(argv[0])) {
if (pBest->flags)
sqlSkipAccumulatorLoad(context);
} else if (pBest->flags) {
@@ -1657,7 +1657,7 @@ groupConcatStep(sql_context * context, int argc, sql_value ** argv)
const char *zSep;
int nVal, nSep;
assert(argc == 1 || argc == 2);
- if (sql_value_mp_type(argv[0]) == MP_NIL)
+ if (sql_value_is_null(argv[0]))
return;
pAccum =
(StrAccum *) sql_aggregate_context(context, sizeof(*pAccum));
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 9134f767d..2189d8c3e 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -456,7 +456,13 @@ const unsigned char *
sql_value_text(sql_value *);
+static inline bool
+sql_value_is_null(sql_value *value)
+{
+ return sql_value_type(value) == MP_NIL;
+}
>> return;
>> for (i = 1; i < argc; i++) {
>> - if (sql_value_type(argv[i]) == SQL_NULL)
>> + if (sql_value_mp_type(argv[i]) == MP_NIL)
>> return;
>> if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) >=
>> 0) {
>> @@ -139,15 +139,15 @@ lengthFunc(sql_context * context, int argc, sql_value ** argv)
>>
>> assert(argc == 1);
>> UNUSED_PARAMETER(argc);
>> - switch (sql_value_type(argv[0])) {
>> - case SQL_BLOB:
>> - case SQL_INTEGER:
>> - case SQL_FLOAT:{
>> + switch (sql_value_mp_type(argv[0])) {
>> + case MP_BIN:
>> + case MP_INT:
>> + case MP_DOUBLE:{
>
> 4. Probably you could fix part of this:
> https://github.com/tarantool/tarantool/issues/4159 in scope of
> this commit, alongside.
THis patch provides straightforward refactoring, I don’t want
to involve here non-trivial changes.
More information about the Tarantool-patches
mailing list