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 7FD0F221F0 for ; Mon, 24 Dec 2018 09:02:12 -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 qWYKgdmR4xfo for ; Mon, 24 Dec 2018 09:02:12 -0500 (EST) Received: from smtp1.mail.ru (smtp1.mail.ru [94.100.179.111]) (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 390EF221D4 for ; Mon, 24 Dec 2018 09:02:12 -0500 (EST) 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 v2 5/6] sql: 'PRAGMA' result in Tarantool format From: "n.pettik" In-Reply-To: <78a21763e93af6b3e6977ff4d211371779954d0f.1544871594.git.imeevma@gmail.com> Date: Mon, 24 Dec 2018 16:02:10 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <53F2238B-909C-4ACB-A80D-9D271876B7A6@tarantool.org> References: <78a21763e93af6b3e6977ff4d211371779954d0f.1544871594.git.imeevma@gmail.com> 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: Imeev Mergen There is no =E2=80=99Tarantool format=E2=80=99... > On 15 Dec 2018, at 14:05, imeevma@tarantool.org wrote: >=20 > Currently, box.sql.execute('PRAGMA') returns nothing but it prints > some information on stdout. This isn't right. Why do you consider it as wrong? AFAIR I made it exclusively to extend debug facilities. Things like - ['index_info', null] - ['index_list', null] look quite strange IMO. What is more, it may be misleading: index_list with null arg looks like there is no indexes at all. Btw I don=E2=80=99t understand all these efforts to make pragma better. As I already said we are going to remove it anyway. > This patch makes > the command to return result in Tarantool format. >=20 > Part of #3832 > --- > src/box/sql/pragma.c | 83 = +++++++++++++++++++++++++++++--------------------- > test/sql/misc.result | 12 ++++++++ > test/sql/misc.test.lua | 5 +++ > 3 files changed, 65 insertions(+), 35 deletions(-) >=20 > diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c > index 9390f6f..fc87976 100644 > --- a/src/box/sql/pragma.c > +++ b/src/box/sql/pragma.c > @@ -168,48 +168,61 @@ pragmaLocate(const char *zName) > return lwr > upr ? 0 : &aPragmaName[mid]; > } >=20 > -#ifdef PRINT_PRAGMA > -#undef PRINT_PRAGMA > -#endif > -#define PRINT_PRAGMA(pragma_name, pragma_flag) do { = \ > - int nCoolSpaces =3D 30 - strlen(pragma_name); = \ > - if (user_session->sql_flags & (pragma_flag)) { = \ > - printf("%s %*c -- [true] \n", pragma_name, nCoolSpaces, = ' '); \ > - } else { = \ > - printf("%s %*c -- [false] \n", pragma_name, = nCoolSpaces, ' ');\ > - } = \ > -} while (0) > - > -#define PRINT_STR_PRAGMA(pragma_name, str_value) do { = \ > - int nCoolSpaces =3D 30 - strlen(pragma_name); = \ > - printf("%s %*c -- '%s' \n", pragma_name, nCoolSpaces, ' ', = str_value);\ > -} while (0) > - > static void > -printActivePragmas(struct session *user_session) > +sql_pragma_active_pragmas(struct Parse *parse) Name seems to be a mess, lets rename it to =E2=80=99sql_pragma_status=E2=80= =99 or =E2=80=98vdbe_emit_pragma_status=E2=80=99. > { > - int i; > - for (i =3D 0; i < ArraySize(aPragmaName); ++i) { > + struct Vdbe *v =3D sqlite3GetVdbe(parse); > + struct session *user_session =3D current_session(); > + > + sqlite3VdbeSetNumCols(v, 2); > + sqlite3VdbeSetColName(v, 0, COLNAME_NAME, "pragma_name", = SQLITE_STATIC); > + sqlite3VdbeSetColName(v, 0, COLNAME_DECLTYPE, "TEXT", = SQLITE_STATIC); > + sqlite3VdbeSetColName(v, 1, COLNAME_NAME, "pragma_value", > + SQLITE_STATIC); > + sqlite3VdbeSetColName(v, 1, COLNAME_DECLTYPE, "TEXT", = SQLITE_STATIC); > + > + parse->nMem =3D 2; > + for (int i =3D 0; i < ArraySize(aPragmaName); ++i) { > + sqlite3VdbeAddOp4(v, OP_String8, 0, 1, 0, = aPragmaName[i].zName, > + 0); > switch (aPragmaName[i].ePragTyp) { > - case PragTyp_FLAG: > - PRINT_PRAGMA(aPragmaName[i].zName, = aPragmaName[i].iArg); > + case PragTyp_PARSER_TRACE: > + case PragTyp_CASE_SENSITIVE_LIKE: > + case PragTyp_FLAG: { > + const char *value; > + if ((user_session->sql_flags & > + aPragmaName[i].iArg) !=3D 0) > + value =3D "true"; > + else > + value =3D "false"; > + sqlite3VdbeAddOp4(v, OP_String8, 0, 2, = 0, value, > + 0); > break; > + } > case PragTyp_DEFAULT_ENGINE: { > - const char *engine_name =3D > - sql_storage_engine_strs[ > - current_session()-> > - = sql_default_engine]; > - PRINT_STR_PRAGMA(aPragmaName[i].zName, > - engine_name); > + const char *value =3D = sql_storage_engine_strs[ > + = user_session->sql_default_engine]; > + sqlite3VdbeAddOp4(v, OP_String8, 0, 2, = 0, value, > + 0); > break; > } > + case PragTyp_BUSY_TIMEOUT: { > + int value =3D parse->db->busyTimeout; > + sqlite3VdbeAddOp2(v, OP_Integer, value, = 2); > + sqlite3VdbeAddOp2(v, OP_Cast, 2, = AFFINITY_TEXT); > + break; > + } > + case PragTyp_COMPOUND_SELECT_LIMIT: { > + int value =3D sqlite3_limit(parse->db, > + SQL_LIMIT_COMPOUND_SELECT, -1); > + sqlite3VdbeAddOp2(v, OP_Integer, value, = 2); > + sqlite3VdbeAddOp2(v, OP_Cast, 2, = AFFINITY_TEXT); > + break; > + } > + default: > + sqlite3VdbeAddOp2(v, OP_Null, 0, 2); > } > - } > - > - printf("Other available pragmas: \n"); > - for (i =3D 0; i < ArraySize(aPragmaName); ++i) { > - if (aPragmaName[i].ePragTyp !=3D PragTyp_FLAG) > - printf("-- %s \n", aPragmaName[i].zName); > + sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 2); > } > } >=20 > @@ -440,7 +453,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, = /* First part of [schema.]id field */ >=20 > zLeft =3D sqlite3NameFromToken(db, pId); > if (!zLeft) { > - printActivePragmas(user_session); > + sql_pragma_active_pragmas(pParse); > return; > } >=20 > diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua > index 5952035..1c0dd07 100644 > --- a/test/sql/misc.test.lua > +++ b/test/sql/misc.test.lua > @@ -24,3 +24,8 @@ result[1][1] =3D=3D 0 or result[1][1] =3D=3D 1 > -- Check that "PRAGMA case_sensitive_like" returns nothing if > -- called with parameter. > box.sql.execute('PRAGMA case_sensitive_like =3D '.. result[1][1]) > + > +-- Make command "PRAGMA" return result in Tarantool format. > +res =3D box.sql.execute('PRAGMA') > +#res > 0 > +res[1][1] This test look ridiculous: number of elements in res doesn=E2=80=99t mean anything; first element with =E2=80=98busy_timeout=E2= =80=99 value as well. I guess you are willing to avoid fixing this test each time number/content of pragmes are changed. And it this acceptable solution. On the other hand, it barely tests anything. So I propose to display full content of pragma.