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 6B9942D27C for ; Tue, 23 Oct 2018 19:28:31 -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 AFRGHDHyAHER for ; Tue, 23 Oct 2018 19:28:31 -0400 (EDT) Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (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 8CCD12D239 for ; Tue, 23 Oct 2018 19:28:30 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH 5/6] sql: return result-set type via IProto From: n.pettik In-Reply-To: Date: Wed, 24 Oct 2018 02:28:21 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <3C315E98-880F-4218-85C7-42631453A818@tarantool.org> References: <4c4651e2-ef29-6d82-a9de-25e67ba54ce5@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 >>> 4. Why is the type 'UNKNOWN' if the value is integer? >> It is how bindings work. >=20 > No, bindings as an API have nothing to do with types. > It is Vdbe's issue, into which the values are inserted, > and it should detect their type. It is not affinity > issue, but Vdbe changing after its compilation. Anyway, > a user placed a number as a parameter and got UNKNOWN - > it is extra weird. >=20 > By the way, '? as ' works ok - the result Yep, but we can=E2=80=99t substitute =E2=80=98?' with real column name, = only with constant alias (at least I didn=E2=80=99t manage to find the way how to do it). I want to do this: tarantool> cn:execute('select ?, ?, 3 from t1', {1, =E2=80=98id'}) =20 --- - metadata: - name: '?' type: UNKNOWN - name: '?' type: UNKNOWN - name: '3' type: INTEGER rows: - [1, 'id', 3] =E2=80=A6 But I meant not string literal =E2=80=9Cid=E2=80=9D, but column id. I don=E2=80=99t see such test case in sql/iproto.test.lua as well. Also, I can ask similar question: why in the example above name of bound literals 1 and =E2=80=98abc' is =E2=80=98?=E2=80=99 but = for non-binded literal 3 is =E2=80=983=E2=80=99? I believe it was made on purpose, and = I don=E2=80=99t mind to display correct type of bound values but it would look a little bit confusing (I mean real name is not substituted, but real type is revealed). > set contains real name instead of '?'. I think, it should > work for types too. You are talking about the case when name (or alias) is known at compilation stage: cn:execute(=E2=80=99select ? as param1=E2=80=99, {1}) It is quite different case - we don=E2=80=99t know type of parameter at compilation stage. Nevertheless, I agree with you that we should display correct type instead of UNKNOWN. > If you want to do it as a separate ticket, I do not mind, > but please consult Kirill to be sure. I think this issue rather relates to current patch, so I=E2=80=99ve fixed it right here: *Extended commit message* sql: return result-set type via IProto =20 Lets evaluate an expression type during processing of expression's = AST and code generation. It allows to calculate resulting columns data = types and export them as IProto meta alongside with columns' names. Also, correct types are also returned for binding parameters as = well. =20 Note that NULL literal has type "BOOLEAN". It was made on purpose - different DBs interpret NULL's type in different ways: some of them use INT; others - VARCHAR; still others - UNKNOWN. We've decided = that NULL is rather of type "BOOLEAN", since NULL is kind if subset of "BOOLEAN" values: any comparison with NULL results in neither TRUE = nor FALSE, but in NULL. =20 Part of #2620 diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 3dcce078a..16b1044d6 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -36,6 +36,7 @@ #include "coll.h" #include "sqliteInt.h" #include "tarantoolInt.h" +#include "vdbeInt.h" #include "box/box.h" #include "box/coll_id_cache.h" #include "box/schema.h" @@ -1687,11 +1688,19 @@ generateColumnNames(Parse * pParse, /* = Parser context */ if (pParse->colNamesSet || db->mallocFailed) return; assert(v !=3D 0); + size_t var_pos_sz =3D pParse->nVar * sizeof(uint32_t); + uint32_t *var_pos =3D (uint32_t *) region_alloc(&pParse->region, + var_pos_sz); + if (var_pos =3D=3D NULL) { + diag_set(OutOfMemory, var_pos_sz, "region", "var_pos"); + return; + } assert(pTabList !=3D 0); pParse->colNamesSet =3D 1; fullNames =3D (user_session->sql_flags & SQLITE_FullColNames) !=3D= 0; shortNames =3D (user_session->sql_flags & SQLITE_ShortColNames) = !=3D 0; sqlite3VdbeSetNumCols(v, pEList->nExpr); + uint32_t var_count =3D 0; for (i =3D 0; i < pEList->nExpr; i++) { Expr *p; p =3D pEList->a[i].pExpr; @@ -1715,8 +1724,21 @@ generateColumnNames(Parse * pParse, /* = Parser context */ sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "BLOB", SQLITE_TRANSIENT); break; - default: - sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "UNKNOWN", + default: ; + char *type; + /* + * For variables we set BOOLEAN type since + * unassigned bindings will be replaced + * with NULL automatically, i.e. without + * explicit call of sql_bind_value(). + */ + if (p->op =3D=3D TK_VARIABLE) { + var_pos[var_count++] =3D i; + type =3D "BOOLEAN"; + } else { n:tarantool n.pettik$ git diff diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 3dcce078a..16b1044d6 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -36,6 +36,7 @@ #include "coll.h" #include "sqliteInt.h" #include "tarantoolInt.h" +#include "vdbeInt.h" #include "box/box.h" #include "box/coll_id_cache.h" #include "box/schema.h" @@ -1687,11 +1688,19 @@ generateColumnNames(Parse * pParse, /* = Parser context */ if (pParse->colNamesSet || db->mallocFailed) return; assert(v !=3D 0); + size_t var_pos_sz =3D pParse->nVar * sizeof(uint32_t); + uint32_t *var_pos =3D (uint32_t *) region_alloc(&pParse->region, + var_pos_sz); + if (var_pos =3D=3D NULL) { + diag_set(OutOfMemory, var_pos_sz, "region", "var_pos"); + return; + } assert(pTabList !=3D 0); pParse->colNamesSet =3D 1; fullNames =3D (user_session->sql_flags & SQLITE_FullColNames) !=3D= 0; shortNames =3D (user_session->sql_flags & SQLITE_ShortColNames) = !=3D 0; sqlite3VdbeSetNumCols(v, pEList->nExpr); + uint32_t var_count =3D 0; for (i =3D 0; i < pEList->nExpr; i++) { Expr *p; p =3D pEList->a[i].pExpr; @@ -1715,8 +1724,21 @@ generateColumnNames(Parse * pParse, /* = Parser context */ sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "BLOB", SQLITE_TRANSIENT); break; - default: - sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = "UNKNOWN", + default: ; + char *type; + /* + * For variables we set BOOLEAN type since + * unassigned bindings will be replaced + * with NULL automatically, i.e. without + * explicit call of sql_bind_value(). + */ + if (p->op =3D=3D TK_VARIABLE) { + var_pos[var_count++] =3D i; + type =3D "BOOLEAN"; + } else { + type =3D "UNKNOWN"; + } + sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, = type, SQLITE_TRANSIENT); } if (pEList->a[i].zName) { @@ -1760,6 +1782,16 @@ generateColumnNames(Parse * pParse, /* = Parser context */ SQLITE_DYNAMIC); } } + if (var_count =3D=3D 0) + return; + v->var_pos =3D (uint32_t *) malloc(var_count * = sizeof(uint32_t)); + if (v->var_pos =3D=3D NULL) { + diag_set(OutOfMemory, var_count * sizeof(uint32_t), + "malloc", "v->var_pos"); + return; + } + memcpy(v->var_pos, var_pos, var_count * sizeof(uint32_t)); + v->res_var_count =3D var_count; } =20 /* diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index 174f5980f..5ad1e0a80 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -380,6 +380,19 @@ struct Vdbe { char *zErrMsg; /* Error message written here */ VdbeCursor **apCsr; /* One element of this array for each = open cursor */ Mem *aVar; /* Values for the OP_Variable opcode. */ + /** + * Array which contains positions of variables to be + * bound in resulting set of SELECT. + **/ + uint32_t *var_pos; + /** + * Number of variables to be bound in result set. + * In other words - size of @var_pos array. + * For example: + * SELECT ?, ? WHERE id =3D ?; + * Result set consists of two binding variables. + */ + uint32_t res_var_count; VList *pVList; /* Name of variables */ #ifndef SQLITE_OMIT_TRACE i64 startTime; /* Time when query started - used for = profiling */ diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index b16e916e9..9e57af051 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -1231,6 +1231,50 @@ vdbeUnbind(Vdbe * p, int i) return SQLITE_OK; } =20 +/** + * This function sets type for bound variable. + * We should bind types only for variables which occur in + * result set of SELECT query. For example: + * + * SELECT id, ?, ?, a WHERE id =3D ?; + * + * In this case we should set types only for two variables. + * That one which is situated under WHERE condition - is out + * of our interest. + * + * For named binding parameters we should propagate type + * for all occurrences of this parameter - since binding + * routine takes place only once for each DISTINCT parameter + * from list. + * + * @param v Current VDBE. + * @param position Ordinal position of binding parameter. + * @param type String literal representing type of binding param. + * @retval 0 on success. + */ +static int +sql_bind_type(struct Vdbe *v, uint32_t position, const char *type) +{ + if (v->res_var_count < position) + return 0; + int rc =3D sqlite3VdbeSetColName(v, v->var_pos[position - 1], + COLNAME_DECLTYPE, type, + SQLITE_TRANSIENT); + const char *bind_name =3D v->aColName[position - 1].z; + if (strcmp(bind_name, "?") =3D=3D 0) + return rc; + for (uint32_t i =3D position; i < v->res_var_count; ++i) { + if (strcmp(bind_name, v->aColName[i].z) =3D=3D 0) { + rc =3D sqlite3VdbeSetColName(v, v->var_pos[i], + COLNAME_DECLTYPE, = type, + SQLITE_TRANSIENT); + if (rc !=3D 0) + return rc; + } + } + return 0; +} + /* * Bind a text or BLOB value. */ @@ -1251,6 +1295,8 @@ bindText(sqlite3_stmt * pStmt, /* The statement = to bind against */ if (zData !=3D 0) { pVar =3D &p->aVar[i - 1]; rc =3D sqlite3VdbeMemSetStr(pVar, zData, nData, = 1, xDel); + if (rc =3D=3D SQLITE_OK) + rc =3D sql_bind_type(p, i, "TEXT"); sqlite3Error(p->db, rc); rc =3D sqlite3ApiExit(p->db, rc); } @@ -1297,6 +1343,7 @@ sqlite3_bind_double(sqlite3_stmt * pStmt, int i, = double rValue) Vdbe *p =3D (Vdbe *) pStmt; rc =3D vdbeUnbind(p, i); if (rc =3D=3D SQLITE_OK) { + rc =3D sql_bind_type(p, i, "NUMERIC"); sqlite3VdbeMemSetDouble(&p->aVar[i - 1], rValue); } return rc; @@ -1315,6 +1362,7 @@ sqlite3_bind_int64(sqlite3_stmt * pStmt, int i, = sqlite_int64 iValue) Vdbe *p =3D (Vdbe *) pStmt; rc =3D vdbeUnbind(p, i); if (rc =3D=3D SQLITE_OK) { + rc =3D sql_bind_type(p, i, "INTEGER"); sqlite3VdbeMemSetInt64(&p->aVar[i - 1], iValue); } return rc; @@ -1326,6 +1374,8 @@ sqlite3_bind_null(sqlite3_stmt * pStmt, int i) int rc; Vdbe *p =3D (Vdbe *) pStmt; rc =3D vdbeUnbind(p, i); + if (rc =3D=3D SQLITE_OK) + rc =3D sql_bind_type(p, i, "BOOLEAN"); return rc; } =20 diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 4bc81bd03..088ac3b3e 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -2780,6 +2780,7 @@ sqlite3VdbeDelete(Vdbe * p) } p->magic =3D VDBE_MAGIC_DEAD; p->db =3D 0; + free(p->var_pos); sqlite3DbFree(db, p); } =20 diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 16ffd0991..e8b53bf11 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -161,11 +161,11 @@ cn:execute('select ?, ?, ?', {1, 2, 3}) --- - metadata: - name: '?' - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: INTEGER rows: - [1, 2, 3] ... @@ -191,11 +191,11 @@ cn:execute('select ?, :value1, @value2', = parameters) --- - metadata: - name: '?' - type: UNKNOWN + type: INTEGER - name: :value1 - type: UNKNOWN + type: INTEGER - name: '@value2' - type: UNKNOWN + type: INTEGER rows: - [10, 11, 12] ... @@ -233,21 +233,21 @@ cn:execute('select :value3, ?, :value1, ?, ?, = @value2, ?, :value3', parameters) --- - metadata: - name: :value3 - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: INTEGER - name: :value1 - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: INTEGER - name: '@value2' - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: BOOLEAN - name: :value3 - type: UNKNOWN + type: INTEGER rows: - [1, 2, 3, 4, 5, 6, null, 1] ... @@ -259,15 +259,15 @@ cn:execute('select ?, ?, ?, ?, ?', {'abc', = -123.456, msgpack.NULL, true, false}) --- - metadata: - name: '?' - type: UNKNOWN + type: TEXT - name: '?' - type: UNKNOWN + type: NUMERIC - name: '?' - type: UNKNOWN + type: BOOLEAN - name: '?' - type: UNKNOWN + type: INTEGER - name: '?' - type: UNKNOWN + type: INTEGER rows: - ['abc', -123.456, null, 1, 0] ... @@ -276,9 +276,9 @@ cn:execute('select ? as kek, ? as kek2', {1, 2}) --- - metadata: - name: KEK - type: UNKNOWN + type: INTEGER - name: KEK2 - type: UNKNOWN + type: INTEGER rows: - [1, 2] ... @@ -529,11 +529,11 @@ cn:execute('select $2, $1, $3', parameters) --- - metadata: - name: $2 - type: UNKNOWN + type: INTEGER - name: $1 - type: UNKNOWN + type: INTEGER - name: $3 - type: UNKNOWN + type: INTEGER rows: - [22, 11, 33] ...