Tarantool development patches archive
 help / color / mirror / Atom feed
From: Imeev Mergen <imeevma@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
	tarantool-patches@freelists.org
Cc: korablev@tarantool.org
Subject: [tarantool-patches] Re: [PATCH v5 5/6] sql: get results of PRAGMA statement in YAML format
Date: Thu, 31 Jan 2019 17:56:55 +0300	[thread overview]
Message-ID: <9c3b19c5-a1ba-5816-d525-9f6dacc04144@tarantool.org> (raw)
In-Reply-To: <64adcf21-d8dc-6069-8382-79fc1a467e15@tarantool.org>

Hi Thank you for review! Answer, fixes and patch below.


On 1/30/19 4:56 PM, Vladislav Shpilevoy wrote:
>
>
> On 29/01/2019 17:29, imeevma@tarantool.org wrote:
>> Currently box.sql.execute ('PRAGMA') returns nothing, but prints
>> list of pragmas and their statuses to stdout. Such strategy is
>> considered to be wrong since output of this command would be
>> unavailable for users who redirect stdout, use net box connection
>> etc. This patch makes the command to return result as the rest of
>> SQL commands. The result contains only FLAG-type pragmas and their
>> statuses in YAML format.
>
> You do not return them in YAML format. You return them as a
> result set. Using C or python connector, I can call PRAGMA,
> and I will not get yaml, but a result set.
>
> In YAML you only print them, using console. No console - no yaml.
>
Fixed.


Diff:

commit 5fa2ea99453fb3df58ad0584d269df90939a4fd4
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Jan 31 16:50:23 2019 +0300

     Temporary: Review fix

diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index 4edb29f..e002817 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -22,7 +22,7 @@ box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
  ---
  ...
  --
--- Make PRAGMA command return the result in YAML format.
+-- Make PRAGMA command return the result as a result set.
  --
  box.sql.execute('PRAGMA')
  ---
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index d023ecb..e429c38 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -12,6 +12,6 @@ box.sql.execute('PRAGMA parser_trace')
  box.sql.execute('PRAGMA parser_trace = '.. result[1][1])

  --
--- Make PRAGMA command return the result in YAML format.
+-- Make PRAGMA command return the result as a result set.
  --
  box.sql.execute('PRAGMA')


Patch:

commit 3c930d8c81aed9ff25f2d582e8bd7743ff57d861
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Dec 13 21:07:31 2018 +0300

     sql: get results of PRAGMA statement as result set

     Currently box.sql.execute ('PRAGMA') returns nothing, but prints
     list of pragmas and their statuses to stdout. Such strategy is
     considered to be wrong since output of this command would be
     unavailable for users who redirect stdout, use net box connection
     etc. This patch makes the command to return result as the rest of
     SQL commands. The result contains only FLAG-type pragmas and their
     statuses.

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index a610345..de81c28 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -168,48 +168,28 @@ pragmaLocate(const char *zName)
      return lwr > upr ? 0 : &aPragmaName[mid];
  }

-#ifdef PRINT_PRAGMA
-#undef PRINT_PRAGMA
-#endif
-#define PRINT_PRAGMA(pragma_name, pragma_flag) do {        \
-    int nCoolSpaces = 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 = 30 - strlen(pragma_name); \
-    printf("%s %*c --  '%s' \n", pragma_name, nCoolSpaces, ' ', 
str_value);\
-} while (0)
-
  static void
-printActivePragmas(struct session *user_session)
+vdbe_emit_pragma_status(struct Parse *parse)
  {
-    int i;
-    for (i = 0; i < ArraySize(aPragmaName); ++i) {
-        switch (aPragmaName[i].ePragTyp) {
-            case PragTyp_FLAG:
-                PRINT_PRAGMA(aPragmaName[i].zName, aPragmaName[i].iArg);
-                break;
-            case PragTyp_DEFAULT_ENGINE: {
-                const char *engine_name =
-                    sql_storage_engine_strs[
-                        current_session()->
-                            sql_default_engine];
-                PRINT_STR_PRAGMA(aPragmaName[i].zName,
-                         engine_name);
-                break;
-            }
-        }
-    }
+    struct Vdbe *v = sqlite3GetVdbe(parse);
+    struct session *user_session = 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, "INTEGER", 
SQLITE_STATIC);

-    printf("Other available pragmas: \n");
-    for (i = 0; i < ArraySize(aPragmaName); ++i) {
+    parse->nMem = 2;
+    for (int i = 0; i < ArraySize(aPragmaName); ++i) {
          if (aPragmaName[i].ePragTyp != PragTyp_FLAG)
-            printf("-- %s \n", aPragmaName[i].zName);
+            continue;
+        sqlite3VdbeAddOp4(v, OP_String8, 0, 1, 0, aPragmaName[i].zName,
+                  0);
+        int val = (user_session->sql_flags & aPragmaName[i].iArg) != 0;
+        sqlite3VdbeAddOp2(v, OP_Integer, val, 2);
+        sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 2);
      }
  }

@@ -438,7 +418,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First 
part of [schema.]id field */

      zLeft = sqlite3NameFromToken(db, pId);
      if (!zLeft) {
-        printActivePragmas(user_session);
+        vdbe_emit_pragma_status(pParse);
          return;
      }

diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index 9388578..e002817 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -21,3 +21,27 @@ box.sql.execute('PRAGMA parser_trace')
  box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
  ---
  ...
+--
+-- Make PRAGMA command return the result as a result set.
+--
+box.sql.execute('PRAGMA')
+---
+- - ['case_sensitive_like', 0]
+  - ['count_changes', 0]
+  - ['defer_foreign_keys', 0]
+  - ['full_column_names', 0]
+  - ['parser_trace', 0]
+  - ['query_only', 0]
+  - ['read_uncommitted', 0]
+  - ['recursive_triggers', 1]
+  - ['reverse_unordered_selects', 0]
+  - ['select_trace', 0]
+  - ['short_column_names', 1]
+  - ['sql_trace', 0]
+  - ['vdbe_addoptrace', 0]
+  - ['vdbe_debug', 0]
+  - ['vdbe_eqp', 0]
+  - ['vdbe_listing', 0]
+  - ['vdbe_trace', 0]
+  - ['where_trace', 0]
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index 721ef19..e429c38 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -10,3 +10,8 @@ result = box.sql.execute('PRAGMA parser_trace')
  box.sql.execute('PRAGMA parser_trace = 1')
  box.sql.execute('PRAGMA parser_trace')
  box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
+
+--
+-- Make PRAGMA command return the result as a result set.
+--
+box.sql.execute('PRAGMA')

  reply	other threads:[~2019-01-31 14:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-29 14:29 [tarantool-patches] [PATCH v5 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2019-01-29 14:29 ` [tarantool-patches] [PATCH v5 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
2019-01-30 13:57   ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-31 14:56     ` Imeev Mergen
2019-01-29 14:29 ` [tarantool-patches] [PATCH v5 2/6] sql: fix "PRAGMA parser_trace" result imeevma
2019-01-30 13:57   ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-31 14:56     ` Imeev Mergen
2019-02-04 13:06       ` Vladislav Shpilevoy
2019-02-09 10:08         ` Mergen Imeev
2019-01-29 14:29 ` [tarantool-patches] [PATCH v5 3/6] sql: Show currently set sql_default_engine imeevma
2019-01-30 13:57   ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-31 14:56     ` Imeev Mergen
2019-01-29 14:29 ` [tarantool-patches] [PATCH v5 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
2019-01-30 13:56   ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-31 14:56     ` Imeev Mergen
2019-01-29 14:29 ` [tarantool-patches] [PATCH v5 5/6] sql: get results of PRAGMA statement in YAML format imeevma
2019-01-30 13:56   ` [tarantool-patches] " Vladislav Shpilevoy
2019-01-31 14:56     ` Imeev Mergen [this message]
2019-02-04 13:08       ` Vladislav Shpilevoy
2019-02-09 10:11         ` Mergen Imeev
2019-01-29 14:29 ` [tarantool-patches] [PATCH v5 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2019-01-30 13:59 ` [tarantool-patches] Re: [PATCH v5 0/6] " Vladislav Shpilevoy
2019-01-31 14:56   ` Imeev Mergen
2019-02-15 20:44 ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9c3b19c5-a1ba-5816-d525-9f6dacc04144@tarantool.org \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH v5 5/6] sql: get results of PRAGMA statement in YAML format' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox