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 C1C7F2BC71 for ; Tue, 9 Oct 2018 12:02:34 -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 Zx5GNbRF8ubq for ; Tue, 9 Oct 2018 12:02:34 -0400 (EDT) Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 80E9F2BBB2 for ; Tue, 9 Oct 2018 12:02:32 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH] sql: change of PRAGMA INDEX_INFO syntax From: "n.pettik" In-Reply-To: Date: Tue, 9 Oct 2018 19:02:29 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: 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: tarantool-patches@freelists.org Cc: Alexander Turenko 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. >=20 > Cleaned up pragma column names array (pragma.h::pragCName). >=20 > Fixes #3194 > =E2=80=94 Could you rebase on the latest 2.0? At current state I can=E2=80=99t = 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 ::=3D PRAGMA nm(X) EQ minus_num(Y). { > cmd ::=3D PRAGMA nm(X) LP minus_num(Y) RP. { > sqlite3Pragma(pParse,&X,&Y,0,1); > } > -cmd ::=3D PRAGMA nm(X) EQ nm(Z) DOT nm(Y). { > +cmd ::=3D PRAGMA nm(X) LP nm(Z) DOT nm(Y) RP. { > sqlite3Pragma(pParse,&X,&Y,&Z,0); > } > cmd ::=3D 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) > } >=20 > /** > - * 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 =3D space_index(space, iid); > assert(idx !=3D NULL); > - /* PRAGMA index_xinfo (more informative version). */ > - if (pragma->iArg > 0) { > - parse->nMem =3D 6; > - } else { > - /* PRAGMA index_info ... */ > - parse->nMem =3D 3; > - } > + parse->nMem =3D 7; > struct Vdbe *v =3D sqlite3GetVdbe(parse); > assert(v !=3D NULL); > uint32_t part_count =3D idx->def->key_def->part_count; > assert(parse->nMem <=3D pragma->nPragCName); > struct key_part *part =3D idx->def->key_def->parts; > for (uint32_t i =3D 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 =3D part->coll_id; > - struct coll *coll =3D part->coll; > - if (coll !=3D NULL) > - c_n =3D coll_by_id(id)->name; > - else > - c_n =3D "BINARY"; > - sqlite3VdbeMultiLoad(v, 4, "isi", = part->sort_order, > - c_n, i < part_count); > - } > + const char *c_n; > + uint32_t id =3D part->coll_id; > + struct coll *coll =3D part->coll; > + if (coll !=3D NULL) > + c_n =3D coll_by_id(id)->name; > + else > + c_n =3D "BINARY"; > + uint32_t fieldno =3D part->fieldno; > + enum field_type type =3D = 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=E2=80=99t 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. + */