Hi! Thanks for the patch! See 3 comments below,
review fixes at the end of the email and on the
branch.
On 11/12/2018 23:41, imeevma@xxxxxxxxxxxxx wrote:
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
---
https://github.com/tarantool/tarantool/issues/3832
https://github.com/tarantool/tarantool/tree/imeevma/gh-3832-no-column-types
src/box/execute.c | 5 +-
src/box/sql/pragma.c | 29 +++----
src/box/sql/pragma.h | 214 +++++++++++++++++++++++++++++++++--------------
src/box/sql/prepare.c | 52 ++++++++----
test/sql/iproto.result | 69 +++++++++++++++
test/sql/iproto.test.lua | 18 +++-
6 files changed, 295 insertions(+), 92 deletions(-)
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 5c35017..e1598d9 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -112,25 +112,26 @@ sqlite3GetBoolean(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.
+ *
+ * @param v the query under construction.
+ * @param pPragma the pragma.
*/
static void
-setPragmaResultColumnNames(Vdbe * v, /* The query under construction */
- const PragmaName * pPragma /* The pragma */
- )
+setPragmaResultColumnNames(Vdbe *v, const PragmaName *pPragma)
{
u8 n = pPragma->nPragCName;
- sqlite3VdbeSetNumCols(v, n == 0 ? 1 : n);
- if (n == 0) {
- sqlite3VdbeSetColName(v, 0, COLNAME_NAME, pPragma->zName,
+ assert(n > 0);
+ sqlite3VdbeSetNumCols(v, n);
+ int i, j;
+ for (i = 0, j = pPragma->iPragCName; i < n; i++, j++) {
+ /* Value of j is index of column name in array */
+ sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j++],
+ SQLITE_STATIC);
+ /* Value of j is index of column type in array */
+ sqlite3VdbeSetColName(v, i, COLNAME_DECLTYPE, pragCName[j],
SQLITE_STATIC);
- } else {
- int i, j;
- for (i = 0, j = pPragma->iPragCName; i < n; i++, j++) {
- sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j],
- SQLITE_STATIC);
- }
}
}
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index e608016..c6c3e63 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -27,50 +27,142 @@
#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",
- /* 19 */ "origin",
- /* 20 */ "partial",
+ /* 32 */ "seq",
+ /* 33 */ "INTEGER",
+ /* 34 */ "name",
+ /* 35 */ "TEXT",
+ /* 36 */ "unique",
+ /* 37 */ "INTEGER",
+ /* 38 */ "origin",
+ /* 39 */ "TEXT",
+ /* 40 */ "partial",
+ /* 41 */ "INTEGER",
/* Used by: collation_list */
- /* 21 */ "seq",
- /* 22 */ "name",
+ /* 42 */ "seq",
+ /* 43 */ "INTEGER",
+ /* 44 */ "name",
+ /* 45 */ "TEXT",
/* Used by: foreign_key_list */
- /* 23 */ "id",
- /* 24 */ "seq",
- /* 25 */ "table",
- /* 26 */ "from",
- /* 27 */ "to",
- /* 28 */ "on_update",
- /* 29 */ "on_delete",
- /* 30 */ "match",
+ /* 46 */ "id",
+ /* 47 */ "INTEGER",
+ /* 48 */ "seq",
+ /* 49 */ "INTEGER",
+ /* 50 */ "table",
+ /* 51 */ "TEXT",
+ /* 52 */ "from",
+ /* 53 */ "TEXT",
+ /* 54 */ "to",
+ /* 55 */ "TEXT",
+ /* 56 */ "on_update",
+ /* 57 */ "TEXT",
+ /* 58 */ "on_delete",
+ /* 59 */ "TEXT",
+ /* 60 */ "match",
+ /* 61 */ "TEXT",
/* Used by: busy_timeout */
- /* 31 */ "timeout",
+ /* 62 */ "timeout",
+ /* 63 */ "INTEGER",
+ /* Used by: case_sensitive_like */
+ /* 64 */ "case_sensitive_like",
+ /* 65 */ "INTEGER",
+ /* Used by: count_changes */
+ /* 66 */ "count_changes",
+ /* 67 */ "INTEGER",
+ /* Used by: defer_foreign_keys */
+ /* 68 */ "defer_foreign_keys",
+ /* 69 */ "INTEGER",
+ /* Used by: full_column_names */
+ /* 70 */ "full_column_names",
+ /* 71 */ "INTEGER",
+ /* Used by: parser_trace */
+ /* 72 */ "parser_trace",
+ /* 73 */ "INTEGER",
+ /* Used by: query_only */
+ /* 74 */ "query_only",
+ /* 75 */ "INTEGER",
+ /* Used by: read_uncommitted */
+ /* 76 */ "read_uncommitted",
+ /* 77 */ "INTEGER",
+ /* Used by: recursive_triggers */
+ /* 78 */ "recursive_triggers",
+ /* 79 */ "INTEGER",
+ /* Used by: reverse_unordered_selects */
+ /* 80 */ "reverse_unordered_selects",
+ /* 81 */ "INTEGER",
+ /* Used by: select_trace */
+ /* 82 */ "select_trace",
+ /* 83 */ "INTEGER",
+ /* Used by: short_column_names */
+ /* 84 */ "short_column_names",
+ /* 85 */ "INTEGER",
+ /* Used by: sql_compound_select_limit */
+ /* 86 */ "sql_compound_select_limit",
+ /* 87 */ "INTEGER",
+ /* Used by: sql_default_engine */
+ /* 88 */ "sql_default_engine",
+ /* 89 */ "INTEGER",
+ /* Used by: sql_trace */
+ /* 90 */ "sql_trace",
+ /* 91 */ "INTEGER",
+ /* Used by: vdbe_addoptrace */
+ /* 92 */ "vdbe_addoptrace",
+ /* 93 */ "INTEGER",
+ /* Used by: vdbe_debug */
+ /* 94 */ "vdbe_debug",
+ /* 95 */ "INTEGER",
+ /* Used by: vdbe_eqp */
+ /* 96 */ "vdbe_eqp",
+ /* 97 */ "INTEGER",
+ /* Used by: vdbe_listing */
+ /* 98 */ "vdbe_listing",
+ /* 99 */ "INTEGER",
+ /* Used by: vdbe_trace */
+ /* 100 */ "vdbe_trace",
+ /* 101 */ "INTEGER",
+ /* Used by: where_trace */
+ /* 102 */ "where_trace",
+ /* 103 */ "INTEGER",
};
