From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 01F862B095 for ; Thu, 18 Apr 2019 13:54:40 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 41UWvqvOMw6s for ; Thu, 18 Apr 2019 13:54:39 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 8922D2ABCA for ; Thu, 18 Apr 2019 13:54:39 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: [tarantool-patches] Re: [PATCH 3/9] sql: use msgpack types instead of custom ones From: "n.pettik" In-Reply-To: <374916ec-2e49-643f-eccb-42f9d66e340e@tarantool.org> Date: Thu, 18 Apr 2019 20:54:37 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <7d55169718d4bc193a41710da12cfd6a7d4edebc.1555252410.git.korablev@tarantool.org> <374916ec-2e49-643f-eccb-42f9d66e340e@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy >> 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 =3D type; >=20 > 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.=E2=80=99 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 #include =20 +#include "msgpuck.h" + struct sql_stmt; =20 /** @@ -55,8 +57,8 @@ struct sql_bind { =20 /** 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 =3D=3D 0) { >> return setDateTimeToCurrent(context, p); >> } >> - if ((eType =3D sql_value_type(argv[0])) =3D=3D SQL_FLOAT >> + if ((eType =3D sql_value_type(argv[0])) =3D=3D MP_DOUBLE >=20 > 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. >=20 > 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 =3D sqlGetFuncCollSeq(context); assert(mask =3D=3D -1 || mask =3D=3D 0); iBest =3D 0; - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) + if (sql_value_type(argv[0]) =3D=3D MP_NIL) return; for (i =3D 1; i < argc; i++) { - if (sql_value_mp_type(argv[i]) =3D=3D MP_NIL) + if (sql_value_type(argv[i]) =3D=3D MP_NIL) return; if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) = >=3D 0) { @@ -109,7 +109,7 @@ typeofFunc(sql_context * context, int NotUsed, = sql_value ** argv) { const char *z =3D 0; UNUSED_PARAMETER(NotUsed); - switch (sql_value_mp_type(argv[0])) { + switch (sql_value_type(argv[0])) { case MP_INT: z =3D "integer"; break; @@ -139,7 +139,7 @@ lengthFunc(sql_context * context, int argc, = sql_value ** argv) =20 assert(argc =3D=3D 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 =3D=3D 1); UNUSED_PARAMETER(argc); - switch (sql_value_mp_type(argv[0])) { + switch (sql_value_type(argv[0])) { case MP_INT:{ i64 iVal =3D 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 =3D argv[0]; struct Mem *haystack =3D argv[1]; - enum mp_type needle_type =3D sql_value_mp_type(needle); - enum mp_type haystack_type =3D sql_value_mp_type(haystack); + enum mp_type needle_type =3D sql_value_type(needle); + enum mp_type haystack_type =3D sql_value_type(haystack); =20 if (haystack_type =3D=3D MP_NIL || needle_type =3D=3D MP_NIL) return; @@ -398,12 +398,12 @@ substrFunc(sql_context * context, int argc, = sql_value ** argv) int negP2 =3D 0; =20 assert(argc =3D=3D 3 || argc =3D=3D 2); - if (sql_value_mp_type(argv[1]) =3D=3D MP_NIL - || (argc =3D=3D 3 && sql_value_mp_type(argv[2]) =3D=3D = MP_NIL) + if (sql_value_type(argv[1]) =3D=3D MP_NIL + || (argc =3D=3D 3 && sql_value_type(argv[2]) =3D=3D MP_NIL) ) { return; } - p0type =3D sql_value_mp_type(argv[0]); + p0type =3D sql_value_type(argv[0]); p1 =3D sql_value_int(argv[1]); if (p0type =3D=3D MP_BIN) { len =3D sql_value_bytes(argv[0]); @@ -507,13 +507,13 @@ roundFunc(sql_context * context, int argc, = sql_value ** argv) char *zBuf; assert(argc =3D=3D 1 || argc =3D=3D 2); if (argc =3D=3D 2) { - if (MP_NIL =3D=3D sql_value_mp_type(argv[1])) + if (MP_NIL =3D=3D sql_value_type(argv[1])) return; n =3D sql_value_int(argv[1]); if (n < 0) n =3D 0; } - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) + if (sql_value_type(argv[0]) =3D=3D MP_NIL) return; r =3D sql_value_double(argv[0]); /* If Y=3D=3D0 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 =3D sql_context_db_handle(context); int is_like_ci =3D SQL_PTR_TO_INT(sql_user_data(context)); - int rhs_type =3D sql_value_mp_type(argv[0]); - int lhs_type =3D sql_value_mp_type(argv[1]); + int rhs_type =3D sql_value_type(argv[0]); + int lhs_type =3D sql_value_type(argv[1]); =20 if (lhs_type !=3D MP_STR || rhs_type !=3D MP_STR) { if (lhs_type =3D=3D MP_NIL || rhs_type =3D=3D MP_NIL) @@ -1018,7 +1018,7 @@ quoteFunc(sql_context * context, int argc, = sql_value ** argv) { assert(argc =3D=3D 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]) =3D=3D = MP_NIL); + assert(sql_value_type(argv[0]) =3D=3D 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 =3D=3D sql_value_text(argv[0])); /* No = encoding change */ zPattern =3D sql_value_text(argv[1]); if (zPattern =3D=3D 0) { - assert(sql_value_mp_type(argv[1]) =3D=3D MP_NIL + assert(sql_value_type(argv[1]) =3D=3D MP_NIL || sql_context_db_handle(context)->mallocFailed); return; } nPattern =3D sql_value_bytes(argv[1]); if (nPattern =3D=3D 0) { - assert(sql_value_mp_type(argv[1]) !=3D MP_NIL); + assert(sql_value_type(argv[1]) !=3D 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 =3D 0; /* Individual characters in = zCharSet */ int nChar; /* Number of characters in zCharSet */ =20 - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) { + if (sql_value_type(argv[0]) =3D=3D MP_NIL) { return; } zIn =3D sql_value_text(argv[0]); @@ -1499,7 +1499,7 @@ sumStep(sql_context * context, int argc, sql_value = ** argv) UNUSED_PARAMETER(argc); p =3D sql_aggregate_context(context, sizeof(*p)); assert(p !=3D NULL); - enum mp_type type =3D sql_value_mp_type(argv[0]); + enum mp_type type =3D sql_value_type(argv[0]); if (type =3D=3D MP_NIL) return; if (type !=3D MP_DOUBLE && type !=3D MP_INT) { @@ -1510,7 +1510,7 @@ sumStep(sql_context * context, int argc, sql_value = ** argv) context->isError =3D SQL_TARANTOOL_ERROR; return; } - type =3D sql_value_mp_type(argv[0]); + type =3D sql_value_type(argv[0]); } p->cnt++; if (type =3D=3D MP_INT) { @@ -1578,7 +1578,7 @@ countStep(sql_context * context, int argc, = sql_value ** argv) { CountCtx *p; p =3D sql_aggregate_context(context, sizeof(*p)); - if ((argc =3D=3D 0 || MP_NIL !=3D sql_value_mp_type(argv[0])) && = p) { + if ((argc =3D=3D 0 || MP_NIL !=3D sql_value_type(argv[0])) && p) = { p->n++; } } @@ -1605,7 +1605,7 @@ minmaxStep(sql_context * context, int NotUsed, = sql_value ** argv) if (!pBest) return; =20 - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) { + if (sql_value_type(argv[0]) =3D=3D 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 =3D=3D 1 || argc =3D=3D 2); - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) + if (sql_value_type(argv[0]) =3D=3D MP_NIL) return; pAccum =3D (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 *); =20 enum mp_type -sql_value_mp_type(sql_value *); +sql_value_type(sql_value *); =20 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 =3D sql_value_mp_type(columnMem(pStmt, i)); + enum mp_type type =3D 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 =3D 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 =3D sqlVdbeGetBoundValue(pReprepare, iCol, FIELD_TYPE_SCALAR); - if (pVal && sql_value_mp_type(pVal) =3D=3D MP_STR) { + if (pVal && sql_value_type(pVal) =3D=3D MP_STR) { z =3D (char *)sql_value_text(pVal); } >> || eType =3D=3D 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 =3D sqlGetFuncCollSeq(context); >> assert(mask =3D=3D -1 || mask =3D=3D 0); >> iBest =3D 0; >> - if (sql_value_type(argv[0]) =3D=3D SQL_NULL) >> + if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) >=20 > 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=E2=80=99. 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 =3D sqlGetFuncCollSeq(context); assert(mask =3D=3D -1 || mask =3D=3D 0); iBest =3D 0; - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) + if (sql_value_is_null(argv[0])) return; for (i =3D 1; i < argc; i++) { - if (sql_value_mp_type(argv[i]) =3D=3D MP_NIL) + if (sql_value_is_null(argv[i])) return; if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) = >=3D 0) { @@ -398,12 +398,12 @@ substrFunc(sql_context * context, int argc, = sql_value ** argv) int negP2 =3D 0; =20 assert(argc =3D=3D 3 || argc =3D=3D 2); - if (sql_value_mp_type(argv[1]) =3D=3D MP_NIL - || (argc =3D=3D 3 && sql_value_mp_type(argv[2]) =3D=3D = MP_NIL) + if (sql_value_is_null(argv[1]) + || (argc =3D=3D 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 =3D=3D 1 || argc =3D=3D 2); if (argc =3D=3D 2) { - if (MP_NIL =3D=3D sql_value_mp_type(argv[1])) + if (sql_value_is_null(argv[1])) return; n =3D sql_value_int(argv[1]); if (n < 0) n =3D 0; } - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) + if (sql_value_is_null(argv[0])) return; r =3D sql_value_double(argv[0]); /* If Y=3D=3D0 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]) =3D=3D = 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 =3D=3D sql_value_text(argv[0])); /* No = encoding change */ zPattern =3D sql_value_text(argv[1]); if (zPattern =3D=3D 0) { - assert(sql_value_mp_type(argv[1]) =3D=3D MP_NIL + assert(sql_value_is_null(argv[1]) || sql_context_db_handle(context)->mallocFailed); return; } nPattern =3D sql_value_bytes(argv[1]); if (nPattern =3D=3D 0) { - assert(sql_value_mp_type(argv[1]) !=3D 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 =3D 0; /* Individual characters in = zCharSet */ int nChar; /* Number of characters in zCharSet */ =20 - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) { + if (sql_value_is_null(argv[0])) { return; } zIn =3D sql_value_text(argv[0]); @@ -1578,7 +1578,7 @@ countStep(sql_context * context, int argc, = sql_value ** argv) { CountCtx *p; p =3D sql_aggregate_context(context, sizeof(*p)); - if ((argc =3D=3D 0 || MP_NIL !=3D sql_value_mp_type(argv[0])) && = p) { + if ((argc =3D=3D 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; =20 - if (sql_value_mp_type(argv[0]) =3D=3D 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 =3D=3D 1 || argc =3D=3D 2); - if (sql_value_mp_type(argv[0]) =3D=3D MP_NIL) + if (sql_value_is_null(argv[0])) return; pAccum =3D (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 *); =20 +static inline bool +sql_value_is_null(sql_value *value) +{ + return sql_value_type(value) =3D=3D MP_NIL; +} >> return; >> for (i =3D 1; i < argc; i++) { >> - if (sql_value_type(argv[i]) =3D=3D SQL_NULL) >> + if (sql_value_mp_type(argv[i]) =3D=3D MP_NIL) >> return; >> if ((sqlMemCompare(argv[iBest], argv[i], pColl) ^ mask) = >=3D >> 0) { >> @@ -139,15 +139,15 @@ lengthFunc(sql_context * context, int argc, = sql_value ** argv) >>=20 >> assert(argc =3D=3D 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:{ >=20 > 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=E2=80=99t want to involve here non-trivial changes.