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 86FCB27A6E for ; Thu, 21 Feb 2019 08:00:57 -0500 (EST) 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 VD13Uc2zrqGt for ; Thu, 21 Feb 2019 08:00:57 -0500 (EST) Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (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 1CB8427AE2 for ; Thu, 21 Feb 2019 08:00:57 -0500 (EST) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v6 6/7] sql: set column types for EXPLAIN and PRAGMA Date: Thu, 21 Feb 2019 16:00:54 +0300 Message-Id: <9aaa43869fb3875e71bc66c639d50f784fca221d.1550753723.git.imeevma@gmail.com> In-Reply-To: References: 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: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org Currently, EXPLAIN and PRAGMA do not set the column types for the result. This is incorrect, since any returned row must have a column type. This patch defines the types for these columns. Closes #3832 --- src/box/execute.c | 5 +- src/box/sql/pragma.c | 37 ++++----- src/box/sql/pragma.h | 193 +++++++++++++++++++++++++++++++++-------------- src/box/sql/prepare.c | 53 +++++++++---- test/sql/iproto.result | 69 +++++++++++++++++ test/sql/iproto.test.lua | 18 ++++- 6 files changed, 279 insertions(+), 96 deletions(-) diff --git a/src/box/execute.c b/src/box/execute.c index 7f1a40d..7c77df2 100644 --- a/src/box/execute.c +++ b/src/box/execute.c @@ -482,11 +482,12 @@ sql_get_description(struct sql_stmt *stmt, struct obuf *out, const char *name = sql_column_name(stmt, i); const char *type = sql_column_datatype(stmt, i); /* - * Can not fail, since all column names are - * preallocated during prepare phase and the + * Can not fail, since all column names and types + * are preallocated during prepare phase and the * column_name simply returns them. */ assert(name != NULL); + assert(type != NULL); size += mp_sizeof_str(strlen(name)); size += mp_sizeof_str(strlen(type)); char *pos = (char *) obuf_alloc(out, size); diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index e166197..5b0e9f2 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -112,25 +112,18 @@ sqlGetBoolean(const char *z, u8 dflt) * the rest of the file if PRAGMAs are omitted from the build. */ -/* - * Set result column names for a pragma. - */ +/** Set result column names and types for a pragma. */ static void -setPragmaResultColumnNames(Vdbe * v, /* The query under construction */ - const PragmaName * pPragma /* The pragma */ - ) +vdbe_set_pragma_result_columns(struct Vdbe *v, const struct PragmaName *pragma) { - u8 n = pPragma->nPragCName; - sqlVdbeSetNumCols(v, n == 0 ? 1 : n); - if (n == 0) { - sqlVdbeSetColName(v, 0, COLNAME_NAME, pPragma->zName, - SQL_STATIC); - } else { - int i, j; - for (i = 0, j = pPragma->iPragCName; i < n; i++, j++) { - sqlVdbeSetColName(v, i, COLNAME_NAME, pragCName[j], - SQL_STATIC); - } + int n = pragma->nPragCName; + assert(n > 0); + sqlVdbeSetNumCols(v, n); + for (int i = 0, j = pragma->iPragCName; i < n; ++i) { + sqlVdbeSetColName(v, i, COLNAME_NAME, pragCName[j++], + SQL_STATIC); + sqlVdbeSetColName(v, i, COLNAME_DECLTYPE, pragCName[j++], + SQL_STATIC); } } @@ -451,17 +444,15 @@ sqlPragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */ goto pragma_out; } /* Register the result column names for pragmas that return results */ - if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 - && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0) - ) { - setPragmaResultColumnNames(v, pPragma); - } + if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 && + ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == NULL)) + vdbe_set_pragma_result_columns(v, pPragma); /* Jump to the appropriate pragma handler */ switch (pPragma->ePragTyp) { case PragTyp_FLAG:{ if (zRight == NULL) { - setPragmaResultColumnNames(v, pPragma); + vdbe_set_pragma_result_columns(v, pPragma); returnSingleInt(v, (user_session->sql_flags & pPragma->iArg) != 0); } else { diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h index 31c83b5..aa7e7cd 100644 --- a/src/box/sql/pragma.h +++ b/src/box/sql/pragma.h @@ -24,46 +24,129 @@ #define PragFlg_SchemaOpt 0x40 /* Schema restricts name search if present */ #define PragFlg_SchemaReq 0x80 /* Schema required - "main" is default */ -/* Names of columns for pragmas that return multi-column result - * or that return single-column results where the name of the - * result column is different from the name of the pragma +/** + * Column names and types for pragmas. The type of the column is + * the following value after its name. */ static const char *const pragCName[] = { /* Used by: table_info */ /* 0 */ "cid", - /* 1 */ "name", - /* 2 */ "type", - /* 3 */ "notnull", - /* 4 */ "dflt_value", - /* 5 */ "pk", + /* 1 */ "INTEGER", + /* 2 */ "name", + /* 3 */ "TEXT", + /* 4 */ "type", + /* 3 */ "TEXT", + /* 6 */ "notnull", + /* 1 */ "INTEGER", + /* 8 */ "dflt_value", + /* 9 */ "TEXT", + /* 10 */ "pk", + /* 11 */ "INTEGER", /* Used by: stats */ - /* 6 */ "table", - /* 7 */ "index", - /* 8 */ "width", - /* 9 */ "height", + /* 12 */ "table", + /* 13 */ "TEXT", + /* 14 */ "index", + /* 15 */ "TEXT", + /* 16 */ "width", + /* 17 */ "INTEGER", + /* 18 */ "height", + /* 19 */ "INTEGER", /* Used by: index_info */ - /* 10 */ "seqno", - /* 11 */ "cid", - /* 12 */ "name", - /* 13 */ "desc", - /* 14 */ "coll", - /* 15 */ "type", + /* 20 */ "seqno", + /* 21 */ "INTEGER", + /* 22 */ "cid", + /* 23 */ "INTEGER", + /* 24 */ "name", + /* 25 */ "TEXT", + /* 26 */ "desc", + /* 27 */ "INTEGER", + /* 28 */ "coll", + /* 29 */ "TEXT", + /* 30 */ "type", + /* 31 */ "TEXT", /* Used by: index_list */ - /* 16 */ "seq", - /* 17 */ "name", - /* 18 */ "unique", + /* 32 */ "seq", + /* 33 */ "INTEGER", + /* 34 */ "name", + /* 35 */ "TEXT", + /* 36 */ "unique", + /* 37 */ "INTEGER", /* Used by: collation_list */ - /* 19 */ "seq", - /* 20 */ "name", + /* 38 */ "seq", + /* 39 */ "INTEGER", + /* 40 */ "name", + /* 41 */ "TEXT", /* Used by: foreign_key_list */ - /* 21 */ "id", - /* 22 */ "seq", - /* 23 */ "table", - /* 24 */ "from", - /* 25 */ "to", - /* 26 */ "on_update", - /* 27 */ "on_delete", - /* 28 */ "match", + /* 42 */ "id", + /* 43 */ "INTEGER", + /* 44 */ "seq", + /* 45 */ "INTEGER", + /* 46 */ "table", + /* 47 */ "TEXT", + /* 48 */ "from", + /* 49 */ "TEXT", + /* 50 */ "to", + /* 51 */ "TEXT", + /* 52 */ "on_update", + /* 53 */ "TEXT", + /* 54 */ "on_delete", + /* 55 */ "TEXT", + /* 56 */ "match", + /* 57 */ "TEXT", + /* Used by: case_sensitive_like */ + /* 58 */ "case_sensitive_like", + /* 59 */ "INTEGER", + /* Used by: count_changes */ + /* 60 */ "count_changes", + /* 61 */ "INTEGER", + /* Used by: defer_foreign_keys */ + /* 62 */ "defer_foreign_keys", + /* 63 */ "INTEGER", + /* Used by: full_column_names */ + /* 64 */ "full_column_names", + /* 65 */ "INTEGER", + /* Used by: parser_trace */ + /* 66 */ "parser_trace", + /* 67 */ "INTEGER", + /* Used by: recursive_triggers */ + /* 68 */ "recursive_triggers", + /* 69 */ "INTEGER", + /* Used by: reverse_unordered_selects */ + /* 70 */ "reverse_unordered_selects", + /* 71 */ "INTEGER", + /* Used by: select_trace */ + /* 72 */ "select_trace", + /* 73 */ "INTEGER", + /* Used by: short_column_names */ + /* 74 */ "short_column_names", + /* 75 */ "INTEGER", + /* Used by: sql_compound_select_limit */ + /* 76 */ "sql_compound_select_limit", + /* 77 */ "INTEGER", + /* Used by: sql_default_engine */ + /* 78 */ "sql_default_engine", + /* 79 */ "TEXT", + /* Used by: sql_trace */ + /* 80 */ "sql_trace", + /* 81 */ "INTEGER", + /* Used by: vdbe_addoptrace */ + /* 82 */ "vdbe_addoptrace", + /* 83 */ "INTEGER", + /* Used by: vdbe_debug */ + /* 84 */ "vdbe_debug", + /* 85 */ "INTEGER", + /* Used by: vdbe_eqp */ + /* 86 */ "vdbe_eqp", + /* 87 */ "INTEGER", + /* Used by: vdbe_listing */ + /* 88 */ "vdbe_listing", + /* 89 */ "INTEGER", + /* Used by: vdbe_trace */ + /* 90 */ "vdbe_trace", + /* 91 */ "INTEGER", + /* Used by: where_trace */ + /* 92 */ "where_trace", + /* 93 */ "INTEGER", }; /* Definitions of all built-in pragmas */ @@ -72,7 +155,7 @@ typedef struct PragmaName { u8 ePragTyp; /* PragTyp_XXX value */ u8 mPragFlg; /* Zero or more PragFlg_XXX values */ u8 iPragCName; /* Start of column names in pragCName[] */ - u8 nPragCName; /* Num of col names. 0 means use pragma name */ + u8 nPragCName; /* Num of col names. */ u32 iArg; /* Extra argument */ } PragmaName; /** @@ -83,97 +166,97 @@ static const PragmaName aPragmaName[] = { { /* zName: */ "case_sensitive_like", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 58, 1, /* iArg: */ LIKE_CASE_SENS_FLAG}, { /* zName: */ "collation_list", /* ePragTyp: */ PragTyp_COLLATION_LIST, /* ePragFlg: */ PragFlg_Result0, - /* ColNames: */ 19, 2, + /* ColNames: */ 38, 2, /* iArg: */ 0}, { /* zName: */ "count_changes", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 60, 1, /* iArg: */ SQL_CountRows}, { /* zName: */ "defer_foreign_keys", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 62, 1, /* iArg: */ SQL_DeferFKs}, { /* zName: */ "foreign_key_list", /* ePragTyp: */ PragTyp_FOREIGN_KEY_LIST, /* ePragFlg: */ PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt, - /* ColNames: */ 21, 8, + /* ColNames: */ 42, 8, /* iArg: */ 0}, { /* zName: */ "full_column_names", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 64, 1, /* iArg: */ SQL_FullColNames}, { /* zName: */ "index_info", /* ePragTyp: */ PragTyp_INDEX_INFO, /* ePragFlg: */ PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt, - /* ColNames: */ 10, 6, + /* ColNames: */ 20, 6, /* iArg: */ 1}, { /* zName: */ "index_list", /* ePragTyp: */ PragTyp_INDEX_LIST, /* ePragFlg: */ PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt, - /* ColNames: */ 16, 3, + /* ColNames: */ 32, 3, /* iArg: */ 0}, #if defined(SQL_DEBUG) { /* zName: */ "parser_trace", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 66, 1, /* iArg: */ PARSER_TRACE_FLAG}, #endif { /* zName: */ "recursive_triggers", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 68, 1, /* iArg: */ SQL_RecTriggers}, { /* zName: */ "reverse_unordered_selects", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 70, 1, /* iArg: */ SQL_ReverseOrder}, #if defined(SQL_DEBUG) { /* zName: */ "select_trace", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 72, 1, /* iArg: */ SQL_SelectTrace}, #endif { /* zName: */ "short_column_names", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 73, 1, /* iArg: */ SQL_ShortColNames}, { /* zName: */ "sql_compound_select_limit", /* ePragTyp: */ PragTyp_COMPOUND_SELECT_LIMIT, /* ePragFlg: */ PragFlg_Result0, - /* ColNames: */ 0, 0, + /* ColNames: */ 76, 1, /* iArg: */ 0}, { /* zName: */ "sql_default_engine", /* ePragTyp: */ PragTyp_DEFAULT_ENGINE, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 78, 1, /* iArg: */ 0}, #if defined(SQL_DEBUG) { /* zName: */ "sql_trace", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 80, 1, /* iArg: */ SQL_SqlTrace}, #endif { /* zName: */ "stats", /* ePragTyp: */ PragTyp_STATS, /* ePragFlg: */ PragFlg_NeedSchema | PragFlg_Result0 | PragFlg_SchemaReq, - /* ColNames: */ 6, 4, + /* ColNames: */ 12, 4, /* iArg: */ 0}, { /* zName: */ "table_info", /* ePragTyp: */ PragTyp_TABLE_INFO, @@ -185,33 +268,33 @@ static const PragmaName aPragmaName[] = { { /* zName: */ "vdbe_addoptrace", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 82, 1, /* iArg: */ SQL_VdbeAddopTrace}, { /* zName: */ "vdbe_debug", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 84, 1, /* iArg: */ SQL_SqlTrace | SQL_VdbeListing | SQL_VdbeTrace}, { /* zName: */ "vdbe_eqp", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 86, 1, /* iArg: */ SQL_VdbeEQP}, { /* zName: */ "vdbe_listing", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 88, 1, /* iArg: */ SQL_VdbeListing}, { /* zName: */ "vdbe_trace", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 90, 1, /* iArg: */ SQL_VdbeTrace}, { /* zName: */ "where_trace", /* ePragTyp: */ PragTyp_FLAG, /* ePragFlg: */ PragFlg_Result0 | PragFlg_NoColumns1, - /* ColNames: */ 0, 0, + /* ColNames: */ 92, 1, /* iArg: */ SQL_WhereTrace}, #endif }; diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c index f6c3429..dd8c5b0 100644 --- a/src/box/sql/prepare.c +++ b/src/box/sql/prepare.c @@ -54,7 +54,6 @@ sqlPrepare(sql * db, /* Database handle. */ { char *zErrMsg = 0; /* Error message */ int rc = SQL_OK; /* Result code */ - int i; /* Loop counter */ Parse sParse; /* Parsing context */ sql_parser_create(&sParse, db); sParse.pReprepare = pReprepare; @@ -113,24 +112,48 @@ sqlPrepare(sql * db, /* Database handle. */ if (rc == SQL_OK && sParse.pVdbe && sParse.explain) { static const char *const azColName[] = { - "addr", "opcode", "p1", "p2", "p3", "p4", "p5", - "comment", - "selectid", "order", "from", "detail" + /* 0 */ "addr", + /* 1 */ "INTEGER", + /* 2 */ "opcode", + /* 3 */ "TEXT", + /* 4 */ "p1", + /* 5 */ "INTEGER", + /* 6 */ "p2", + /* 7 */ "INTEGER", + /* 8 */ "p3", + /* 9 */ "INTEGER", + /* 10 */ "p4", + /* 11 */ "TEXT", + /* 12 */ "p5", + /* 13 */ "TEXT", + /* 14 */ "comment", + /* 15 */ "TEXT", + /* 16 */ "selectid", + /* 17 */ "INTEGER", + /* 18 */ "order", + /* 19 */ "INTEGER", + /* 20 */ "from", + /* 21 */ "INTEGER", + /* 22 */ "detail", + /* 23 */ "TEXT", }; - int iFirst, mx; + + int name_first, name_count; if (sParse.explain == 2) { - sqlVdbeSetNumCols(sParse.pVdbe, 4); - iFirst = 8; - mx = 12; + name_first = 16; + name_count = 4; } else { - sqlVdbeSetNumCols(sParse.pVdbe, 8); - iFirst = 0; - mx = 8; + name_first = 0; + name_count = 8; } - for (i = iFirst; i < mx; i++) { - sqlVdbeSetColName(sParse.pVdbe, i - iFirst, - COLNAME_NAME, azColName[i], - SQL_STATIC); + sqlVdbeSetNumCols(sParse.pVdbe, name_count); + for (int i = 0; i < name_count; i++) { + int name_index = 2 * i + name_first; + sqlVdbeSetColName(sParse.pVdbe, i, COLNAME_NAME, + azColName[name_index], SQL_STATIC); + sqlVdbeSetColName(sParse.pVdbe, i, COLNAME_DECLTYPE, + azColName[name_index + 1], + SQL_STATIC); } } diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 562e068..da7b40f 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -879,6 +879,75 @@ box.sql.execute('DROP TABLE t1') cn:close() --- ... +-- gh-3832: Some statements do not return column type +box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)') +--- +... +cn = remote.connect(box.cfg.listen) +--- +... +-- PRAGMA: +res = cn:execute("PRAGMA table_info(t1)") +--- +... +res.metadata +--- +- - name: cid + type: INTEGER + - name: name + type: TEXT + - name: type + type: TEXT + - name: notnull + type: INTEGER + - name: dflt_value + type: TEXT + - name: pk + type: INTEGER +... +-- EXPLAIN +res = cn:execute("EXPLAIN SELECT 1") +--- +... +res.metadata +--- +- - name: addr + type: INTEGER + - name: opcode + type: TEXT + - name: p1 + type: INTEGER + - name: p2 + type: INTEGER + - name: p3 + type: INTEGER + - name: p4 + type: TEXT + - name: p5 + type: TEXT + - name: comment + type: TEXT +... +res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1") +--- +... +res.metadata +--- +- - name: selectid + type: INTEGER + - name: order + type: INTEGER + - name: from + type: INTEGER + - name: detail + type: TEXT +... +cn:close() +--- +... +box.sql.execute('DROP TABLE t1') +--- +... box.schema.user.revoke('guest', 'read,write,execute', 'universe') --- ... diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua index 5829239..fbdc5a2 100644 --- a/test/sql/iproto.test.lua +++ b/test/sql/iproto.test.lua @@ -275,10 +275,26 @@ box.sql.execute('DROP TABLE t1') cn:close() +-- gh-3832: Some statements do not return column type +box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)') +cn = remote.connect(box.cfg.listen) + +-- PRAGMA: +res = cn:execute("PRAGMA table_info(t1)") +res.metadata + +-- EXPLAIN +res = cn:execute("EXPLAIN SELECT 1") +res.metadata +res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1") +res.metadata + +cn:close() +box.sql.execute('DROP TABLE t1') + box.schema.user.revoke('guest', 'read,write,execute', 'universe') box.schema.user.revoke('guest', 'create', 'space') space = nil -- Cleanup xlog box.snapshot() - -- 2.7.4