[tarantool-patches] Re: [PATCH] sql: change of PRAGMA INDEX_INFO syntax
n.pettik
korablev at tarantool.org
Tue Oct 9 19:02:29 MSK 2018
In general LGTM, several minor nits.
> This change removes 'pragma index_xinfo' syntax. 'pragma index_info'
> now works as 'pragma index_xinfo' and also displays type of columns in
> index.
>
> Cleaned up pragma column names array (pragma.h::pragCName).
>
> Fixes #3194
> —
Could you rebase on the latest 2.0? At current state I can’t build it on MacOS:
[ 0%] Building C object src/lib/csv/CMakeFiles/csv.dir/csv.c.o
Scanning dependencies of target json_path
[ 1%] Built target mkkeywordhash
[ 1%] Building C object src/lib/salad/CMakeFiles/salad.dir/rope.c.o
[ 1%] Building C object src/lib/salad/CMakeFiles/salad.dir/rtree.c.o
[ 1%] Building C object src/lib/salad/CMakeFiles/salad.dir/guava.c.o
[ 1%] Building C object src/lib/salad/CMakeFiles/salad.dir/bloom.c.o
[ 1%] Building C object src/lib/json/CMakeFiles/json_path.dir/path.c.o
[ 2%] Built target lemon
[ 2%] Building C object CMakeFiles/misc.dir/third_party/base64.c.o
[ 2%] Building C object CMakeFiles/misc.dir/third_party/qsort_arg.c.o
[ 2%] Building C object CMakeFiles/ev.dir/third_party/tarantool_ev.c.o
[ 2%] Built target eio
[ 5%] Built target zstd
Scanning dependencies of target uri
Scanning dependencies of target crc32
Scanning dependencies of target small
Scanning dependencies of target bit
[ 5%] Building C object src/lib/msgpuck/CMakeFiles/msgpuck.dir/msgpuck.c.o
error: unknown warning option '-Wno-cast-function-type'; did you mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
[ 6%] Building C object src/lib/msgpuck/CMakeFiles/msgpuck.dir/hints.c.o
error: unknown warning option '-Wno-cast-function-type'; did you mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
make[2]: *** [src/lib/salad/CMakeFiles/salad.dir/rtree.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....
error: unknown warning option '-Wno-cast-function-type'; did you make[2]: *** [src/lib/salad/CMakeFiles/salad.dir/rope.c.o] Error 1
mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
error: unknown warning option '-Wno-cast-function-type'; did you mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
make[2]: *** [src/lib/json/CMakeFiles/json_path.dir/path.c.o] Error 1
error: unknown warning option '-Wno-cast-function-type'; did you mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
make[2]: *** [src/lib/msgpuck/CMakeFiles/msgpuck.dir/msgpuck.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....
error: unknown warning option '-Wno-cast-function-type'; did you mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
make[2]: *** [src/lib/msgpuck/CMakeFiles/msgpuck.dir/hints.c.o] Error 1
error: unknown warning option '-Wno-cast-function-type'; did you mean '-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option]
[ 6%] Building CXX object test/unit/CMakeFiles/reflection_cxx.test.dir/reflection_cxx.cc.o
[ 6%] Building C object test/unit/CMakeFiles/reflection_cxx.test.dir/unit.c.o
[ 6%] Building CXX object test/unit/CMakeFiles/int96.test.dir/int96.cc.o
[ 6%] Building C object test/unit/CMakeFiles/reflection_cxx.test.dir/__/__/src/reflection.c.o
[ 6%] Building C object test/unit/CMakeFiles/find_path.test.dir/__/__/src/find_path.c.o
[ 6%] Building C object test/unit/CMakeFiles/find_path.test.dir/find_path.c.o
errorerror: : unknownunknown warningwarning optionoption '-Wno-cast-function-type';'-Wno-cast-function-type'; diddid youyou meanmean '-Wno-bad-function-cast'?'-Wno-bad-function-cast'? [-Werror,-Wunknown-warning-option][-Werror,-Wunknown-warning-option]
...
Etc.
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index 473cf89d8..042823267 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -1286,7 +1286,7 @@ cmd ::= PRAGMA nm(X) EQ minus_num(Y). {
> cmd ::= PRAGMA nm(X) LP minus_num(Y) RP. {
> sqlite3Pragma(pParse,&X,&Y,0,1);
> }
> -cmd ::= PRAGMA nm(X) EQ nm(Z) DOT nm(Y). {
> +cmd ::= PRAGMA nm(X) LP nm(Z) DOT nm(Y) RP. {
> sqlite3Pragma(pParse,&X,&Y,&Z,0);
> }
> cmd ::= PRAGMA . {
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 4f64ab6f2..0eacef5c0 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -322,8 +322,7 @@ sql_pragma_table_stats(struct space *space, void *data)
> }
>
> /**
> - * This function handles PRAGMA INDEX_INFO and PRAGMA INDEX_XINFO
> - * statements.
> + * This function handles PRAGMA INDEX_INFO statement.
Nit: it would be great if you put here (explanation of) format of
displayed output. For example see sql_pragma_table_info().
> *
> * @param parse Current parsing content.
> * @param pragma Definition of index_info pragma.
> @@ -349,32 +348,26 @@ sql_pragma_index_info(struct Parse *parse, const PragmaName *pragma,
> return;
> struct index *idx = space_index(space, iid);
> assert(idx != NULL);
> - /* PRAGMA index_xinfo (more informative version). */
> - if (pragma->iArg > 0) {
> - parse->nMem = 6;
> - } else {
> - /* PRAGMA index_info ... */
> - parse->nMem = 3;
> - }
> + parse->nMem = 7;
> struct Vdbe *v = sqlite3GetVdbe(parse);
> assert(v != NULL);
> uint32_t part_count = idx->def->key_def->part_count;
> assert(parse->nMem <= pragma->nPragCName);
> struct key_part *part = idx->def->key_def->parts;
> for (uint32_t i = 0; i < part_count; i++, part++) {
> - sqlite3VdbeMultiLoad(v, 1, "iis", i, part->fieldno,
> - space->def->fields[part->fieldno].name);
> - if (pragma->iArg > 0) {
> - const char *c_n;
> - uint32_t id = part->coll_id;
> - struct coll *coll = part->coll;
> - if (coll != NULL)
> - c_n = coll_by_id(id)->name;
> - else
> - c_n = "BINARY";
> - sqlite3VdbeMultiLoad(v, 4, "isi", part->sort_order,
> - c_n, i < part_count);
> - }
> + const char *c_n;
> + uint32_t id = part->coll_id;
> + struct coll *coll = part->coll;
> + if (coll != NULL)
> + c_n = coll_by_id(id)->name;
> + else
> + c_n = "BINARY";
> + uint32_t fieldno = part->fieldno;
> + enum field_type type = space->def->fields[fieldno].type;
> + sqlite3VdbeMultiLoad(v, 1, "iisisis", i, fieldno,
> + space->def->fields[fieldno].name,
> + part->sort_order, c_n, i < part_count,
But i < part_count is always true, isn’t it?
> /* Definitions of all built-in pragmas */
> @@ -97,11 +82,13 @@ typedef struct PragmaName {
> u8 nPragCName; /* Num of col names. 0 means use pragma name */
> u32 iArg; /* Extra argument */
> } PragmaName;
> +/* The order of pragmas in this array is important: it has */
> +/* to be sorted. For more info see pragma_locate function. */
Nit:
-/* The order of pragmas in this array is important: it has */
-/* to be sorted. For more info see pragma_locate function. */
+
+/**
+ * The order of pragmas in this array is important: it has
+ * to be sorted. For more info see pragma_locate function.
+ */
More information about the Tarantool-patches
mailing list