Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA
@ 2019-01-19 12:37 imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

This patch-set defines the types for the result columns of EXPLAIN
and PRAGMA commands. In addition, it fixes some problems of the
PRAGMA commands that have something to do with their result.

https://github.com/tarantool/tarantool/issues/3832
https://github.com/tarantool/tarantool/tree/imeevma/gh-3832-no-column-types

Changes in fourth version:
  - Removed two macros.
  - Removed unnecessary comments.
  - Fixed varible name.
  - Fixes commit-message.
  - Changed 'PRAGMA' command.
  - Refactoring.

v1: 
https://www.freelists.org/post/tarantool-patches/PATCH-v1-11-sql-set-column-types-for-EXPLAIN-and-PRAGMA
v2: 
https://www.freelists.org/post/tarantool-patches/PATCH-v2-06-sql-set-column-types-for-EXPLAIN-and-PRAGMA
v2: 
https://www.freelists.org/post/tarantool-patches/PATCH-v3-06-sql-set-column-types-for-EXPLAIN-and-PRAGMA

Mergen Imeev (6):
  sql: remove unused macros from pragma.c and pragma.h
  sql: fix "PRAGMA parser_trace" result
  sql: Show currently set sql_default_engine
  sql: fix "PRAGMA case_sensitive_like" result
  sql: 'PRAGMA' result in YAML format
  sql: set column types for EXPLAIN and PRAGMA

 src/box/execute.c                    |   5 +-
 src/box/sql/CMakeLists.txt           |   2 -
 src/box/sql/parse.y                  |   4 +-
 src/box/sql/pragma.c                 | 195 ++++++++++----------------
 src/box/sql/pragma.h                 | 259 ++++++++++++++++++++++-------------
 src/box/sql/prepare.c                |  52 +++++--
 src/box/sql/select.c                 |  22 +--
 src/box/sql/sqliteInt.h              |  22 +--
 src/box/sql/where.c                  |  46 +++----
 src/box/sql/whereInt.h               |   4 +-
 test/sql-tap/gh-2367-pragma.test.lua |  35 +++--
 test/sql/iproto.result               |  69 ++++++++++
 test/sql/iproto.test.lua             |  18 ++-
 test/sql/misc.result                 |  17 +++
 test/sql/misc.test.lua               |  10 ++
 test/sql/sql-debug.result            |  53 +++++++
 test/sql/sql-debug.test.lua          |  19 +++
 test/sql/suite.ini                   |   2 +-
 18 files changed, 530 insertions(+), 304 deletions(-)
 create mode 100644 test/sql/sql-debug.result
 create mode 100644 test/sql/sql-debug.test.lua

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h
  2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
@ 2019-01-19 12:37 ` imeevma
  2019-01-20  0:16   ` [tarantool-patches] " n.pettik
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 2/6] sql: fix "PRAGMA parser_trace" result imeevma
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

Hi! Thank you for review. My answers, diff between versions and
new version below.


On 1/16/19 6:34 PM, n.pettik wrote:
>
>
>> On 26 Dec 2018, at 20:17, imeevma@tarantool.org wrote:
>>
>> Hi! Thank you for review! My answers and new version below. I
>> didn't include diff, because all that changed was the commit
>> message.
>>
>>
>> On 12/24/18 5:01 PM, n.pettik wrote:
>>> Nit: at the end of commit subject redundant ‘*’ and dot.
>> Fixed.
>>
>>> They weren’t deleted, they are simply not used.
>>> I guess they still can be enabled, if add appropriate commands
>>> to cmake lists. On the other hand, I doubt that someday we
>>> really may need them.
>> Made changes in in commit-message.
>>
>>> In fact, this commit has nothing in common with mentioned issue.
>> Removed from commit-message.
>>
>>> Why didn’t you remove ENABLE_SELECTTRACE as well?
>> I didn't remove it because it can be set through cmake.
>
> So? As I already said, I also was able to set OMIT_FLAG_PRAGMAS 
> Look:
>
> diff --git a/src/box/sql/CMakeLists.txt b/src/box/sql/CMakeLists.txt
> index 7f7b60e22..4d4cbb21f 100644
> --- a/src/box/sql/CMakeLists.txt
> +++ b/src/box/sql/CMakeLists.txt
> @@ -1,6 +1,7 @@
>  if(CMAKE_BUILD_TYPE STREQUAL "Debug")
>    add_definitions(-DSQLITE_DEBUG=1)
>    add_definitions(-DSQLITE_ENABLE_SELECTTRACE)
> +  add_definitions(-DSQLITE_OMIT_FLAG_PRAGMAS=1)
>    add_definitions(-DSQLITE_ENABLE_WHERETRACE)
>  endif()
>
> Make sure that build is debug but flag-pragmas are disabled:
>
> tarantool> \set language sql
> ---
> - true
> ...
>
> tarantool> pragma vdbe_debug=1
> ---
> - error: 'no such pragma: VDBE_DEBUG'
> ...
>
> tarantool> pragma parser_trace=1
> parser: Shift 'cmd', go to state 419
> parser: Shift 'SEMI'
> parser: Return. Stack=[explain cmd SEMI]
> parser: Input '$'
> parser: Reduce [ecmd ::= explain cmdx SEMI], go to state 0.
> parser: Shift 'ecmd', go to state 420
> parser: Reduce [input ::= ecmd], go to state 0.
> parser: Accept!
> parser: Return. Stack=]
> ---
> - []
> ...
>
>
> Lets remove it and replace with simple ifdef debug.
> The same if for ENABLE_WHERETRACE
>
>
Fixed. Also, removed two more definitions that become unnecessary.


Diff between versions:

commit e454c0de481dd62dbb5962fabcc81638472d3d45
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Jan 17 11:42:57 2019 +0300

    Temporary: Review fixes

diff --git a/src/box/sql/CMakeLists.txt b/src/box/sql/CMakeLists.txt
index 7f7b60e..68e2929 100644
--- a/src/box/sql/CMakeLists.txt
+++ b/src/box/sql/CMakeLists.txt
@@ -1,7 +1,5 @@
 if(CMAKE_BUILD_TYPE STREQUAL "Debug")
   add_definitions(-DSQLITE_DEBUG=1)
-  add_definitions(-DSQLITE_ENABLE_SELECTTRACE)
-  add_definitions(-DSQLITE_ENABLE_WHERETRACE)
 endif()
 
 set(EXT_SRC_DIR ${CMAKE_SOURCE_DIR}/extra)
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 50bb2ba..f18f597 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -449,11 +449,11 @@ multiselect_op(A) ::= EXCEPT|INTERSECT(OP).  {A = @OP; /*A-overwrites-OP*/}
 %endif SQLITE_OMIT_COMPOUND_SELECT
 oneselect(A) ::= SELECT(S) distinct(D) selcollist(W) from(X) where_opt(Y)
                  groupby_opt(P) having_opt(Q) orderby_opt(Z) limit_opt(L). {
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
   Token s = S; /*A-overwrites-S*/
 #endif
   A = sqlite3SelectNew(pParse,W,X,Y,P,Q,Z,D,L.pLimit,L.pOffset);
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
   /* Populate the Select.zSelName[] string that is used to help with
   ** query planner debugging, to differentiate between multiple Select
   ** objects in a complex query.
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 84ab478..fd76b49 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -162,7 +162,7 @@ static const PragmaName aPragmaName[] = {
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_ReverseOrder},
-#if defined(SQLITE_ENABLE_SELECTTRACE)
+#if defined(SQLITE_DEBUG)
  { /* zName:     */ "select_trace",
  /* ePragTyp:  */ PragTyp_FLAG,
  /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
@@ -230,9 +230,6 @@ static const PragmaName aPragmaName[] = {
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_VdbeTrace},
-#endif
-#if defined(SQLITE_ENABLE_WHERETRACE)
-
  { /* zName:     */ "where_trace",
  /* ePragTyp:  */ PragTyp_FLAG,
  /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 02ee225..c91f42f 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -45,7 +45,7 @@
 /*
  * Trace output macros
  */
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /***/ int sqlite3SelectTrace = 0;
 #define SELECTTRACE(K,P,S,X)  \
   if(sqlite3SelectTrace&(K))   \
@@ -174,7 +174,7 @@ sqlite3SelectNew(Parse * pParse,  /* Parsing context */
  pNew->selFlags = selFlags;
  pNew->iLimit = 0;
  pNew->iOffset = 0;
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  pNew->zSelName[0] = 0;
  if (user_session->sql_flags & SQLITE_SelectTrace)
    sqlite3SelectTrace = 0xfff;
@@ -208,7 +208,7 @@ sqlite3SelectNew(Parse * pParse,  /* Parsing context */
  return pNew;
 }
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /*
  * Set the name of a Select object
  */
@@ -4214,7 +4214,7 @@ flattenSubquery(Parse * pParse,   /* Parsing context */
   */
  sql_select_delete(db, pSub1);
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3SelectTrace & 0x100) {
    SELECTTRACE(0x100, pParse, p, ("After flattening:\n"));
    sqlite3TreeViewSelect(0, p, 0);
@@ -5460,7 +5460,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
    return 1;
  }
  memset(&sAggInfo, 0, sizeof(sAggInfo));
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  pParse->nSelectIndent++;
  SELECTTRACE(1, pParse, p, ("begin processing:\n"));
  if (sqlite3SelectTrace & 0x100) {
@@ -5496,7 +5496,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
  }
  assert(p->pEList != 0);
  isAgg = (p->selFlags & SF_Aggregate) != 0;
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3SelectTrace & 0x100) {
    SELECTTRACE(0x100, pParse, p, ("after name resolution:\n"));
    sqlite3TreeViewSelect(0, p, 0);
@@ -5563,7 +5563,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
    }
    sqlite3VdbeResolveLabel(v, end);
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
    SELECTTRACE(1, pParse, p, ("end compound-select processing\n"));
    pParse->nSelectIndent--;
 #endif
@@ -5611,7 +5611,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
        && pushDownWhereTerms(pParse, pSub, p->pWhere,
            pItem->iCursor)
        ) {
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
      if (sqlite3SelectTrace & 0x100) {
        SELECTTRACE(0x100, pParse, p,
              ("After WHERE-clause push-down:\n"));
@@ -5712,7 +5712,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
  pHaving = p->pHaving;
  sDistinct.isTnct = (p->selFlags & SF_Distinct) != 0;
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3SelectTrace & 0x400) {
    SELECTTRACE(0x400, pParse, p,
          ("After all FROM-clause analysis:\n"));
@@ -5745,7 +5745,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
     */
    assert(sDistinct.isTnct);
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
    if (sqlite3SelectTrace & 0x400) {
      SELECTTRACE(0x400, pParse, p,
            ("Transform DISTINCT into GROUP BY:\n"));
@@ -6394,7 +6394,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
 
  sqlite3DbFree(db, sAggInfo.aCol);
  sqlite3DbFree(db, sAggInfo.aFunc);
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  SELECTTRACE(1, pParse, p, ("end processing\n"));
  pParse->nSelectIndent--;
 #endif
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 7e16edc..970a422 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1295,22 +1295,6 @@ extern const int sqlite3one;
 #endif
 
 /*
- * SELECTTRACE_ENABLED will be either 1 or 0 depending on whether or not
- * the Select query generator tracing logic is turned on.
- */
-#if defined(SQLITE_DEBUG) || defined(SQLITE_ENABLE_SELECTTRACE)
-#define SELECTTRACE_ENABLED
-#else
-#undef SELECTTRACE_ENABLED
-#endif
-
-#if defined(SQLITE_DEBUG) || defined(SQLITE_ENABLE_WHERETRACE)
-#define WHERETRACE_ENABLED
-#else
-#undef WHERETRACE_ENABLED
-#endif
-
-/*
  * An instance of the following structure is used to store the busy-handler
  * callback for a given sqlite handle.
  *
@@ -3898,7 +3882,7 @@ Expr *sqlite3ExprDup(sqlite3 *, Expr *, int);
 SrcList *sqlite3SrcListDup(sqlite3 *, SrcList *, int);
 IdList *sqlite3IdListDup(sqlite3 *, IdList *);
 Select *sqlite3SelectDup(sqlite3 *, Select *, int);
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
 void sqlite3SelectSetName(Select *, const char *);
 #else
 #define sqlite3SelectSetName(A,B)
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 571b5af..aca0dc1 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -50,7 +50,7 @@
 static int whereLoopResize(sqlite3 *, WhereLoop *, int);
 
 /* Test variable that can be set to enable WHERE tracing */
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /***/ int sqlite3WhereTrace = 0; /* -1; */
 #endif
 
@@ -145,7 +145,7 @@ sqlite3WhereOkOnePass(WhereInfo * pWInfo, int *aiCur)
  if (pWInfo->eOnePass == ONEPASS_MULTI) {
    pWInfo->eOnePass = ONEPASS_OFF;
  }
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3WhereTrace && pWInfo->eOnePass != ONEPASS_OFF) {
    sqlite3DebugPrintf("%s cursors: %d %d\n",
           pWInfo->eOnePass ==
@@ -1501,7 +1501,7 @@ whereRangeScanEst(Parse * pParse, /* Parsing & code generating context */
    nNew = 10;
  if (nNew < nOut)
    nOut = nNew;
-#if defined(WHERETRACE_ENABLED)
+#if defined(SQLITE_DEBUG)
  if (pLoop->nOut > nOut) {
    WHERETRACE(0x10, ("Range scan lowers nOut from %d to %d\n",
          pLoop->nOut, nOut));
@@ -1615,7 +1615,7 @@ whereInScanEst(Parse * pParse,  /* Parsing & code generating context */
  return rc;
 }
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /*
  * Print the content of a WhereTerm object
  */
@@ -1659,9 +1659,7 @@ whereTermPrint(WhereTerm * pTerm, int iTerm)
    sqlite3TreeViewExpr(0, pTerm->pExpr, 0);
  }
 }
-#endif
 
-#ifdef WHERETRACE_ENABLED
 /*
  * Show the complete content of a WhereClause
  */
@@ -1673,9 +1671,7 @@ sqlite3WhereClausePrint(WhereClause * pWC)
    whereTermPrint(&pWC->a[i], i);
  }
 }
-#endif
 
-#ifdef WHERETRACE_ENABLED
 /*
  * Print a WhereLoop object for debugging purposes
  */
@@ -1687,12 +1683,10 @@ whereLoopPrint(WhereLoop * p, WhereClause * pWC)
  struct SrcList_item *pItem = pWInfo->pTabList->a + p->iTab;
  Table *pTab = pItem->pTab;
  Bitmask mAll = (((Bitmask) 1) << (nb * 4)) - 1;
-#ifdef SQLITE_DEBUG
  sqlite3DebugPrintf("%c%2d.%0*llx.%0*llx", p->cId,
         p->iTab, nb, p->maskSelf, nb, p->prereq & mAll);
  sqlite3DebugPrintf(" %12s",
         pItem->zAlias ? pItem->zAlias : pTab->def->name);
-#endif
  const char *zName;
  if (p->index_def != NULL && (zName = p->index_def->name) != NULL) {
    if (strncmp(zName, "sql_autoindex_", 17) == 0) {
@@ -2053,14 +2047,14 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
   */
  if (pBuilder->pOrSet != 0) {
    if (pTemplate->nLTerm) {
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
      u16 n = pBuilder->pOrSet->n;
      int x =
 #endif
          whereOrInsert(pBuilder->pOrSet, pTemplate->prereq,
            pTemplate->rRun,
            pTemplate->nOut);
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
      if (sqlite3WhereTrace & 0x8) {
        sqlite3DebugPrintf(x ? "   or-%d:  " :
               "   or-X:  ", n);
@@ -2080,7 +2074,7 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
    /* There already exists a WhereLoop on the list that is better
     * than pTemplate, so just ignore pTemplate
     */
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
    if (sqlite3WhereTrace & 0x8) {
      sqlite3DebugPrintf("   skip: ");
      whereLoopPrint(pTemplate, pBuilder->pWC);
@@ -2095,7 +2089,7 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
   * with pTemplate[] if p[] exists, or if p==NULL then allocate a new
   * WhereLoop and insert it.
   */
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
  if (sqlite3WhereTrace & 0x8) {
    if (p != 0) {
      sqlite3DebugPrintf("replace: ");
@@ -2127,7 +2121,7 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
      if (pToDel == 0)
        break;
      *ppTail = pToDel->pNextLoop;
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
      if (sqlite3WhereTrace & 0x8) {
        sqlite3DebugPrintf(" delete: ");
        whereLoopPrint(pToDel, pBuilder->pWC);
@@ -3015,7 +3009,7 @@ whereLoopAddOr(WhereLoopBuilder * pBuilder, Bitmask mPrereq, Bitmask mUnusable)
          continue;
        }
        sCur.n = 0;
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
        WHERETRACE(0x200,
             ("OR-term %d of %p has %d subterms:\n",
              (int)(pOrTerm - pOrWC->a), pTerm,
@@ -3513,7 +3507,7 @@ sqlite3WhereIsSorted(WhereInfo * pWInfo)
  return pWInfo->sorted;
 }
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /* For debugging use only: */
 static const char *
 wherePathName(WherePath * pPath, int nLoop, WhereLoop * pLast)
@@ -3781,7 +3775,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
             * paths currently in the best-so-far buffer.  So discard
             * this candidate as not viable.
             */
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
            if (sqlite3WhereTrace & 0x4) {
              sqlite3DebugPrintf("Skip   %s cost=%-3d,%3d order=%c\n",
                     wherePathName(pFrom, iLoop,
@@ -3804,7 +3798,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
            jj = mxI;
          }
          pTo = &aTo[jj];
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
          if (sqlite3WhereTrace & 0x4) {
            sqlite3DebugPrintf
                ("New    %s cost=%-3d,%3d order=%c\n",
@@ -3824,7 +3818,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
          if (pTo->rCost < rCost
              || (pTo->rCost == rCost
            && pTo->nRow <= nOut)) {
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
            if (sqlite3WhereTrace & 0x4) {
              sqlite3DebugPrintf("Skip   %s cost=%-3d,%3d order=%c",
                     wherePathName(pFrom, iLoop,
@@ -3849,7 +3843,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
          /* Control reaches here if the candidate path is better than the
           * pTo path.  Replace pTo with the candidate.
           */
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
          if (sqlite3WhereTrace & 0x4) {
            sqlite3DebugPrintf("Update %s cost=%-3d,%3d order=%c",
                   wherePathName(pFrom, iLoop,
@@ -3898,7 +3892,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
      }
    }
 
-#ifdef WHERETRACE_ENABLED  /* >=2 */
+#ifdef SQLITE_DEBUG  /* >=2 */
    if (sqlite3WhereTrace & 0x02) {
      sqlite3DebugPrintf("---- after round %d ----\n", iLoop);
      for (ii = 0, pTo = aTo; ii < nTo; ii++, pTo++) {
@@ -4258,7 +4252,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
  u8 bFordelete = 0;  /* OPFLAG_FORDELETE or zero, as appropriate */
  struct session *user_session = current_session();
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (user_session->sql_flags & SQLITE_WhereTrace)
    sqlite3WhereTrace = 0xfff;
  else
@@ -4421,7 +4415,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
  }
 
  /* Construct the WhereLoop objects */
-#if defined(WHERETRACE_ENABLED)
+#if defined(SQLITE_DEBUG)
  if (sqlite3WhereTrace & 0xffff) {
    sqlite3DebugPrintf("*** Optimizer Start *** (wctrlFlags: 0x%x",
           wctrlFlags);
@@ -4440,7 +4434,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
    if (rc)
      goto whereBeginError;
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
    if (sqlite3WhereTrace) {  /* Display all of the WhereLoop objects */
      WhereLoop *p;
      int i;
@@ -4471,7 +4465,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
  if (pParse->nErr || NEVER(db->mallocFailed)) {
    goto whereBeginError;
  }
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3WhereTrace) {
    sqlite3DebugPrintf("---- Solution nRow=%d", pWInfo->nRowOut);
    if (pWInfo->nOBSat > 0) {
diff --git a/src/box/sql/whereInt.h b/src/box/sql/whereInt.h
index 4657055..35891dd 100644
--- a/src/box/sql/whereInt.h
+++ b/src/box/sql/whereInt.h
@@ -39,7 +39,7 @@
 /*
  * Trace output macros
  */
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /***/ extern int sqlite3WhereTrace;
 #define WHERETRACE(K,X)  if(sqlite3WhereTrace&(K)) sqlite3DebugPrintf X
 #else
@@ -443,7 +443,7 @@ struct WhereInfo {
  * where.c:
  */
 Bitmask sqlite3WhereGetMask(WhereMaskSet *, int);
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 void sqlite3WhereClausePrint(WhereClause * pWC);
 #endif
 WhereTerm *sqlite3WhereFindTerm(WhereClause * pWC, /* The WHERE clause to be searched */


New version:

commit 28ff08bf9df3a3948fdc399f4801f01d170c52a5
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Dec 12 21:58:54 2018 +0300

    sql: remove unused macros from pragma.c and pragma.h
    
    Some macros in pragma.c and pragma.h are obsolete because the
    values they are checking are no longer used. Let's remove them.

diff --git a/src/box/sql/CMakeLists.txt b/src/box/sql/CMakeLists.txt
index 7f7b60e..68e2929 100644
--- a/src/box/sql/CMakeLists.txt
+++ b/src/box/sql/CMakeLists.txt
@@ -1,7 +1,5 @@
 if(CMAKE_BUILD_TYPE STREQUAL "Debug")
   add_definitions(-DSQLITE_DEBUG=1)
-  add_definitions(-DSQLITE_ENABLE_SELECTTRACE)
-  add_definitions(-DSQLITE_ENABLE_WHERETRACE)
 endif()
 
 set(EXT_SRC_DIR ${CMAKE_SOURCE_DIR}/extra)
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 50bb2ba..f18f597 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -449,11 +449,11 @@ multiselect_op(A) ::= EXCEPT|INTERSECT(OP).  {A = @OP; /*A-overwrites-OP*/}
 %endif SQLITE_OMIT_COMPOUND_SELECT
 oneselect(A) ::= SELECT(S) distinct(D) selcollist(W) from(X) where_opt(Y)
                  groupby_opt(P) having_opt(Q) orderby_opt(Z) limit_opt(L). {
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
   Token s = S; /*A-overwrites-S*/
 #endif
   A = sqlite3SelectNew(pParse,W,X,Y,P,Q,Z,D,L.pLimit,L.pOffset);
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
   /* Populate the Select.zSelName[] string that is used to help with
   ** query planner debugging, to differentiate between multiple Select
   ** objects in a complex query.
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index eef1ed9..5729fe6 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -465,7 +465,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,  /* First part of [schema.]id field */
  /* Jump to the appropriate pragma handler */
  switch (pPragma->ePragTyp) {
 
-#ifndef SQLITE_OMIT_FLAG_PRAGMAS
  case PragTyp_FLAG:{
      if (zRight == 0) {
        setPragmaResultColumnNames(v, pPragma);
@@ -494,9 +493,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,  /* First part of [schema.]id field */
      }
      break;
    }
-#endif       /* SQLITE_OMIT_FLAG_PRAGMAS */
 
-#ifndef SQLITE_OMIT_SCHEMA_PRAGMAS
  case PragTyp_TABLE_INFO:
    sql_pragma_table_info(pParse, zRight);
    break;
@@ -538,7 +535,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,  /* First part of [schema.]id field */
    box_iterator_free(iter);
    break;
  }
-#endif       /* SQLITE_OMIT_SCHEMA_PRAGMAS */
 
  case PragTyp_FOREIGN_KEY_LIST:{
    if (zRight == NULL)
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index e608016..fd76b49 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -97,41 +97,32 @@ static const PragmaName aPragmaName[] = {
   /* ePragFlg:  */ PragFlg_NoColumns,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ 0},
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
  { /* zName:     */ "collation_list",
   /* ePragTyp:  */ PragTyp_COLLATION_LIST,
   /* ePragFlg:  */ PragFlg_Result0,
   /* ColNames:  */ 21, 2,
   /* iArg:      */ 0},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
  { /* zName:     */ "count_changes",
   /* ePragTyp:  */ PragTyp_FLAG,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_CountRows},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
  { /* zName:     */ "defer_foreign_keys",
   /* ePragTyp:  */ PragTyp_FLAG,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_DeferFKs},
-#endif
  { /* zName:     */ "foreign_key_list",
   /* ePragTyp:  */ PragTyp_FOREIGN_KEY_LIST,
   /* ePragFlg:  */
   PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
   /* ColNames:  */ 23, 8,
   /* iArg:      */ 0},
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
  { /* zName:     */ "full_column_names",
   /* ePragTyp:  */ PragTyp_FLAG,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_FullColNames},
-#endif
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
  { /* zName:     */ "index_info",
   /* ePragTyp:  */ PragTyp_INDEX_INFO,
   /* ePragFlg:  */
@@ -144,15 +135,13 @@ static const PragmaName aPragmaName[] = {
   PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
   /* ColNames:  */ 16, 5,
   /* iArg:      */ 0},
-#endif
-#if defined(SQLITE_DEBUG) && !defined(SQLITE_OMIT_PARSER_TRACE)
+#if defined(SQLITE_DEBUG)
  { /* zName:     */ "parser_trace",
   /* ePragTyp:  */ PragTyp_PARSER_TRACE,
   /* ePragFlg:  */ 0,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ 0},
 #endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
  { /* zName:     */ "query_only",
   /* ePragTyp:  */ PragTyp_FLAG,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
@@ -168,28 +157,23 @@ static const PragmaName aPragmaName[] = {
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_RecTriggers},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
  { /* zName:     */ "reverse_unordered_selects",
   /* ePragTyp:  */ PragTyp_FLAG,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_ReverseOrder},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS) && defined(SQLITE_ENABLE_SELECTTRACE)
+#if defined(SQLITE_DEBUG)
  { /* zName:     */ "select_trace",
  /* ePragTyp:  */ PragTyp_FLAG,
  /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
  /* ColNames:  */ 0, 0,
  /* iArg:      */ SQLITE_SelectTrace},
 #endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
  { /* zName:     */ "short_column_names",
   /* ePragTyp:  */ PragTyp_FLAG,
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_ShortColNames},
-#endif
  { /* zName:     */ "sql_compound_select_limit",
  /* ePragTyp:  */ PragTyp_COMPOUND_SELECT_LIMIT,
  /* ePragFlg:  */ PragFlg_Result0,
@@ -200,7 +184,6 @@ static const PragmaName aPragmaName[] = {
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ 0},
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
 #if defined(SQLITE_DEBUG)
  { /* zName:     */ "sql_trace",
   /* ePragTyp:  */ PragTyp_FLAG,
@@ -208,24 +191,18 @@ static const PragmaName aPragmaName[] = {
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_SqlTrace},
 #endif
-#endif
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
  { /* zName:     */ "stats",
   /* ePragTyp:  */ PragTyp_STATS,
   /* ePragFlg:  */
   PragFlg_NeedSchema | PragFlg_Result0 | PragFlg_SchemaReq,
   /* ColNames:  */ 6, 4,
   /* iArg:      */ 0},
-#endif
-#if !defined(SQLITE_OMIT_SCHEMA_PRAGMAS)
  { /* zName:     */ "table_info",
   /* ePragTyp:  */ PragTyp_TABLE_INFO,
   /* ePragFlg:  */
   PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
   /* ColNames:  */ 0, 6,
   /* iArg:      */ 0},
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
 #if defined(SQLITE_DEBUG)
  { /* zName:     */ "vdbe_addoptrace",
   /* ePragTyp:  */ PragTyp_FLAG,
@@ -253,10 +230,6 @@ static const PragmaName aPragmaName[] = {
   /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
   /* ColNames:  */ 0, 0,
   /* iArg:      */ SQLITE_VdbeTrace},
-#endif
-#endif
-#if !defined(SQLITE_OMIT_FLAG_PRAGMAS) && defined(SQLITE_ENABLE_WHERETRACE)
-
  { /* zName:     */ "where_trace",
  /* ePragTyp:  */ PragTyp_FLAG,
  /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 02ee225..c91f42f 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -45,7 +45,7 @@
 /*
  * Trace output macros
  */
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /***/ int sqlite3SelectTrace = 0;
 #define SELECTTRACE(K,P,S,X)  \
   if(sqlite3SelectTrace&(K))   \
@@ -174,7 +174,7 @@ sqlite3SelectNew(Parse * pParse,  /* Parsing context */
  pNew->selFlags = selFlags;
  pNew->iLimit = 0;
  pNew->iOffset = 0;
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  pNew->zSelName[0] = 0;
  if (user_session->sql_flags & SQLITE_SelectTrace)
    sqlite3SelectTrace = 0xfff;
@@ -208,7 +208,7 @@ sqlite3SelectNew(Parse * pParse,  /* Parsing context */
  return pNew;
 }
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /*
  * Set the name of a Select object
  */
@@ -4214,7 +4214,7 @@ flattenSubquery(Parse * pParse,   /* Parsing context */
   */
  sql_select_delete(db, pSub1);
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3SelectTrace & 0x100) {
    SELECTTRACE(0x100, pParse, p, ("After flattening:\n"));
    sqlite3TreeViewSelect(0, p, 0);
@@ -5460,7 +5460,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
    return 1;
  }
  memset(&sAggInfo, 0, sizeof(sAggInfo));
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  pParse->nSelectIndent++;
  SELECTTRACE(1, pParse, p, ("begin processing:\n"));
  if (sqlite3SelectTrace & 0x100) {
@@ -5496,7 +5496,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
  }
  assert(p->pEList != 0);
  isAgg = (p->selFlags & SF_Aggregate) != 0;
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3SelectTrace & 0x100) {
    SELECTTRACE(0x100, pParse, p, ("after name resolution:\n"));
    sqlite3TreeViewSelect(0, p, 0);
@@ -5563,7 +5563,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
    }
    sqlite3VdbeResolveLabel(v, end);
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
    SELECTTRACE(1, pParse, p, ("end compound-select processing\n"));
    pParse->nSelectIndent--;
 #endif
@@ -5611,7 +5611,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
        && pushDownWhereTerms(pParse, pSub, p->pWhere,
            pItem->iCursor)
        ) {
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
      if (sqlite3SelectTrace & 0x100) {
        SELECTTRACE(0x100, pParse, p,
              ("After WHERE-clause push-down:\n"));
@@ -5712,7 +5712,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
  pHaving = p->pHaving;
  sDistinct.isTnct = (p->selFlags & SF_Distinct) != 0;
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3SelectTrace & 0x400) {
    SELECTTRACE(0x400, pParse, p,
          ("After all FROM-clause analysis:\n"));
@@ -5745,7 +5745,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
     */
    assert(sDistinct.isTnct);
 
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
    if (sqlite3SelectTrace & 0x400) {
      SELECTTRACE(0x400, pParse, p,
            ("Transform DISTINCT into GROUP BY:\n"));
@@ -6394,7 +6394,7 @@ sqlite3Select(Parse * pParse,   /* The parser context */
 
  sqlite3DbFree(db, sAggInfo.aCol);
  sqlite3DbFree(db, sAggInfo.aFunc);
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
  SELECTTRACE(1, pParse, p, ("end processing\n"));
  pParse->nSelectIndent--;
 #endif
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 7e16edc..970a422 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1295,22 +1295,6 @@ extern const int sqlite3one;
 #endif
 
 /*
- * SELECTTRACE_ENABLED will be either 1 or 0 depending on whether or not
- * the Select query generator tracing logic is turned on.
- */
-#if defined(SQLITE_DEBUG) || defined(SQLITE_ENABLE_SELECTTRACE)
-#define SELECTTRACE_ENABLED
-#else
-#undef SELECTTRACE_ENABLED
-#endif
-
-#if defined(SQLITE_DEBUG) || defined(SQLITE_ENABLE_WHERETRACE)
-#define WHERETRACE_ENABLED
-#else
-#undef WHERETRACE_ENABLED
-#endif
-
-/*
  * An instance of the following structure is used to store the busy-handler
  * callback for a given sqlite handle.
  *
@@ -3898,7 +3882,7 @@ Expr *sqlite3ExprDup(sqlite3 *, Expr *, int);
 SrcList *sqlite3SrcListDup(sqlite3 *, SrcList *, int);
 IdList *sqlite3IdListDup(sqlite3 *, IdList *);
 Select *sqlite3SelectDup(sqlite3 *, Select *, int);
-#ifdef SELECTTRACE_ENABLED
+#ifdef SQLITE_DEBUG
 void sqlite3SelectSetName(Select *, const char *);
 #else
 #define sqlite3SelectSetName(A,B)
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 571b5af..aca0dc1 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -50,7 +50,7 @@
 static int whereLoopResize(sqlite3 *, WhereLoop *, int);
 
 /* Test variable that can be set to enable WHERE tracing */
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /***/ int sqlite3WhereTrace = 0; /* -1; */
 #endif
 
@@ -145,7 +145,7 @@ sqlite3WhereOkOnePass(WhereInfo * pWInfo, int *aiCur)
  if (pWInfo->eOnePass == ONEPASS_MULTI) {
    pWInfo->eOnePass = ONEPASS_OFF;
  }
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3WhereTrace && pWInfo->eOnePass != ONEPASS_OFF) {
    sqlite3DebugPrintf("%s cursors: %d %d\n",
           pWInfo->eOnePass ==
@@ -1501,7 +1501,7 @@ whereRangeScanEst(Parse * pParse, /* Parsing & code generating context */
    nNew = 10;
  if (nNew < nOut)
    nOut = nNew;
-#if defined(WHERETRACE_ENABLED)
+#if defined(SQLITE_DEBUG)
  if (pLoop->nOut > nOut) {
    WHERETRACE(0x10, ("Range scan lowers nOut from %d to %d\n",
          pLoop->nOut, nOut));
@@ -1615,7 +1615,7 @@ whereInScanEst(Parse * pParse,  /* Parsing & code generating context */
  return rc;
 }
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /*
  * Print the content of a WhereTerm object
  */
@@ -1659,9 +1659,7 @@ whereTermPrint(WhereTerm * pTerm, int iTerm)
    sqlite3TreeViewExpr(0, pTerm->pExpr, 0);
  }
 }
-#endif
 
-#ifdef WHERETRACE_ENABLED
 /*
  * Show the complete content of a WhereClause
  */
@@ -1673,9 +1671,7 @@ sqlite3WhereClausePrint(WhereClause * pWC)
    whereTermPrint(&pWC->a[i], i);
  }
 }
-#endif
 
-#ifdef WHERETRACE_ENABLED
 /*
  * Print a WhereLoop object for debugging purposes
  */
@@ -1687,12 +1683,10 @@ whereLoopPrint(WhereLoop * p, WhereClause * pWC)
  struct SrcList_item *pItem = pWInfo->pTabList->a + p->iTab;
  Table *pTab = pItem->pTab;
  Bitmask mAll = (((Bitmask) 1) << (nb * 4)) - 1;
-#ifdef SQLITE_DEBUG
  sqlite3DebugPrintf("%c%2d.%0*llx.%0*llx", p->cId,
         p->iTab, nb, p->maskSelf, nb, p->prereq & mAll);
  sqlite3DebugPrintf(" %12s",
         pItem->zAlias ? pItem->zAlias : pTab->def->name);
-#endif
  const char *zName;
  if (p->index_def != NULL && (zName = p->index_def->name) != NULL) {
    if (strncmp(zName, "sql_autoindex_", 17) == 0) {
@@ -2053,14 +2047,14 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
   */
  if (pBuilder->pOrSet != 0) {
    if (pTemplate->nLTerm) {
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
      u16 n = pBuilder->pOrSet->n;
      int x =
 #endif
          whereOrInsert(pBuilder->pOrSet, pTemplate->prereq,
            pTemplate->rRun,
            pTemplate->nOut);
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
      if (sqlite3WhereTrace & 0x8) {
        sqlite3DebugPrintf(x ? "   or-%d:  " :
               "   or-X:  ", n);
@@ -2080,7 +2074,7 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
    /* There already exists a WhereLoop on the list that is better
     * than pTemplate, so just ignore pTemplate
     */
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
    if (sqlite3WhereTrace & 0x8) {
      sqlite3DebugPrintf("   skip: ");
      whereLoopPrint(pTemplate, pBuilder->pWC);
@@ -2095,7 +2089,7 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
   * with pTemplate[] if p[] exists, or if p==NULL then allocate a new
   * WhereLoop and insert it.
   */
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
  if (sqlite3WhereTrace & 0x8) {
    if (p != 0) {
      sqlite3DebugPrintf("replace: ");
@@ -2127,7 +2121,7 @@ whereLoopInsert(WhereLoopBuilder * pBuilder, WhereLoop * pTemplate)
      if (pToDel == 0)
        break;
      *ppTail = pToDel->pNextLoop;
-#ifdef WHERETRACE_ENABLED    /* 0x8 */
+#ifdef SQLITE_DEBUG    /* 0x8 */
      if (sqlite3WhereTrace & 0x8) {
        sqlite3DebugPrintf(" delete: ");
        whereLoopPrint(pToDel, pBuilder->pWC);
@@ -3015,7 +3009,7 @@ whereLoopAddOr(WhereLoopBuilder * pBuilder, Bitmask mPrereq, Bitmask mUnusable)
          continue;
        }
        sCur.n = 0;
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
        WHERETRACE(0x200,
             ("OR-term %d of %p has %d subterms:\n",
              (int)(pOrTerm - pOrWC->a), pTerm,
@@ -3513,7 +3507,7 @@ sqlite3WhereIsSorted(WhereInfo * pWInfo)
  return pWInfo->sorted;
 }
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /* For debugging use only: */
 static const char *
 wherePathName(WherePath * pPath, int nLoop, WhereLoop * pLast)
@@ -3781,7 +3775,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
             * paths currently in the best-so-far buffer.  So discard
             * this candidate as not viable.
             */
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
            if (sqlite3WhereTrace & 0x4) {
              sqlite3DebugPrintf("Skip   %s cost=%-3d,%3d order=%c\n",
                     wherePathName(pFrom, iLoop,
@@ -3804,7 +3798,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
            jj = mxI;
          }
          pTo = &aTo[jj];
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
          if (sqlite3WhereTrace & 0x4) {
            sqlite3DebugPrintf
                ("New    %s cost=%-3d,%3d order=%c\n",
@@ -3824,7 +3818,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
          if (pTo->rCost < rCost
              || (pTo->rCost == rCost
            && pTo->nRow <= nOut)) {
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
            if (sqlite3WhereTrace & 0x4) {
              sqlite3DebugPrintf("Skip   %s cost=%-3d,%3d order=%c",
                     wherePathName(pFrom, iLoop,
@@ -3849,7 +3843,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
          /* Control reaches here if the candidate path is better than the
           * pTo path.  Replace pTo with the candidate.
           */
-#ifdef WHERETRACE_ENABLED  /* 0x4 */
+#ifdef SQLITE_DEBUG  /* 0x4 */
          if (sqlite3WhereTrace & 0x4) {
            sqlite3DebugPrintf("Update %s cost=%-3d,%3d order=%c",
                   wherePathName(pFrom, iLoop,
@@ -3898,7 +3892,7 @@ wherePathSolver(WhereInfo * pWInfo, LogEst nRowEst)
      }
    }
 
-#ifdef WHERETRACE_ENABLED  /* >=2 */
+#ifdef SQLITE_DEBUG  /* >=2 */
    if (sqlite3WhereTrace & 0x02) {
      sqlite3DebugPrintf("---- after round %d ----\n", iLoop);
      for (ii = 0, pTo = aTo; ii < nTo; ii++, pTo++) {
@@ -4258,7 +4252,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
  u8 bFordelete = 0;  /* OPFLAG_FORDELETE or zero, as appropriate */
  struct session *user_session = current_session();
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (user_session->sql_flags & SQLITE_WhereTrace)
    sqlite3WhereTrace = 0xfff;
  else
@@ -4421,7 +4415,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
  }
 
  /* Construct the WhereLoop objects */
-#if defined(WHERETRACE_ENABLED)
+#if defined(SQLITE_DEBUG)
  if (sqlite3WhereTrace & 0xffff) {
    sqlite3DebugPrintf("*** Optimizer Start *** (wctrlFlags: 0x%x",
           wctrlFlags);
@@ -4440,7 +4434,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
    if (rc)
      goto whereBeginError;
 
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
    if (sqlite3WhereTrace) {  /* Display all of the WhereLoop objects */
      WhereLoop *p;
      int i;
@@ -4471,7 +4465,7 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
  if (pParse->nErr || NEVER(db->mallocFailed)) {
    goto whereBeginError;
  }
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
  if (sqlite3WhereTrace) {
    sqlite3DebugPrintf("---- Solution nRow=%d", pWInfo->nRowOut);
    if (pWInfo->nOBSat > 0) {
diff --git a/src/box/sql/whereInt.h b/src/box/sql/whereInt.h
index 4657055..35891dd 100644
--- a/src/box/sql/whereInt.h
+++ b/src/box/sql/whereInt.h
@@ -39,7 +39,7 @@
 /*
  * Trace output macros
  */
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 /***/ extern int sqlite3WhereTrace;
 #define WHERETRACE(K,X)  if(sqlite3WhereTrace&(K)) sqlite3DebugPrintf X
 #else
@@ -443,7 +443,7 @@ struct WhereInfo {
  * where.c:
  */
 Bitmask sqlite3WhereGetMask(WhereMaskSet *, int);
-#ifdef WHERETRACE_ENABLED
+#ifdef SQLITE_DEBUG
 void sqlite3WhereClausePrint(WhereClause * pWC);
 #endif
 WhereTerm *sqlite3WhereFindTerm(WhereClause * pWC, /* The WHERE clause to be searched */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [PATCH v4 2/6] sql: fix "PRAGMA parser_trace" result
  2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
@ 2019-01-19 12:37 ` imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 3/6] sql: Show currently set sql_default_engine imeevma
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

Hi! Thank you for review. My answers, diff between versions and
new version below.


On 1/16/19 6:35 PM, n.pettik wrote:
>
>> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
>> index b8edc76..6122986 100644
>> --- a/src/box/sql/pragma.c
>> +++ b/src/box/sql/pragma.c
>> @@ -466,33 +466,31 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
>> 	switch (pPragma->ePragTyp) {
>>
>> 	case PragTyp_FLAG:{
>> -			if (zRight == 0) {
>> -				setPragmaResultColumnNames(v, pPragma);
>> -				returnSingleInt(v,
>> -						(user_session->
>> -						 sql_flags & pPragma->iArg) !=
>> -						0);
>> -			} else {
>> -				int mask = pPragma->iArg;	/* Mask of bits to set
>> -								 * or clear.
>> -								 */
>> -
>> -				if (sqlite3GetBoolean(zRight, 0)) {
>> -					user_session->sql_flags |= mask;
>> -				} else {
>> -					user_session->sql_flags &= ~mask;
>> -				}
>> -
>> -				/* Many of the flag-pragmas modify the code
>> -				 * generated by the SQL * compiler (eg.
>> -				 * count_changes). So add an opcode to expire
>> -				 * all * compiled SQL statements after
>> -				 * modifying a pragma value.
>> -				 */
>> -				sqlite3VdbeAddOp0(v, OP_Expire);
>> +		if (zRight == 0) {
>
> Since you have already aligned code, I propose to fix it
> according to our codestyle:
>
> zRight == NULL
>
Fixed.

>> +			setPragmaResultColumnNames(v, pPragma);
>> +			returnSingleInt(v, (user_session->sql_flags &
>> +					    pPragma->iArg) != 0);
>> +		} else {
>> +			int mask = pPragma->iArg;	/* Mask of bits to set
>> +							 * or clear.
>> +							 */
>
> Smth wrong with comment.
>
Fixed.

>> +			bool is_value_true = sqlite3GetBoolean(zRight, 0);
>
> Bad name imho. I would call it “is_set” or “is_pragma_set” or “pragma_status”.
> Explanation: variable itself is of type bool and “value” says nothing.
>
Fixed, new name is "is_pragma_set".

>> diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
>> new file mode 100644
>> index 0000000..66f47b3
>> --- /dev/null
>> +++ b/test/sql/sql-debug.test.lua
>> @@ -0,0 +1,17 @@
>> +remote = require('net.box')
>> +test_run = require('test_run').new()
>> +engine = test_run:get_cfg('engine')
>> +box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
>> +
>> +--
>> +-- gh-3832: Some statements do not return column type
>> +
>> +-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
>> +-- without parameter.
>> +result = box.sql.execute('PRAGMA parser_trace')
>> +-- Should be nothing.
>> +box.sql.execute('PRAGMA parser_trace = 1')
>> +-- Should be 1.
>> +box.sql.execute('PRAGMA parser_trace')
>> +-- Should be nothing.
>
> These three comments say nothing.
> Results and mismatches can be seen from .result and
> .reject files.
>
>
Fixed.


Diff between versions:

commit 4508278fa4b9100e0e1794f185d27a221bf7f50c
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Jan 17 11:48:52 2019 +0300

    Temporary: Review fixes

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 6122986..476771d 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -466,23 +466,22 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 	switch (pPragma->ePragTyp) {
 
 	case PragTyp_FLAG:{
-		if (zRight == 0) {
+		if (zRight == NULL) {
 			setPragmaResultColumnNames(v, pPragma);
 			returnSingleInt(v, (user_session->sql_flags &
 					    pPragma->iArg) != 0);
 		} else {
-			int mask = pPragma->iArg;	/* Mask of bits to set
-							 * or clear.
-							 */
-			bool is_value_true = sqlite3GetBoolean(zRight, 0);
+			/* Mask of bits to set or clear. */
+			int mask = pPragma->iArg;
+			bool is_pragma_set = sqlite3GetBoolean(zRight, 0);
 
-			if (is_value_true)
+			if (is_pragma_set)
 				user_session->sql_flags |= mask;
 			else
 				user_session->sql_flags &= ~mask;
-#ifndef NDEBUG
+#if defined(SQLITE_DEBUG)
 			if (mask == PARSER_TRACE_FLAG) {
-				if (is_value_true)
+				if (is_pragma_set)
 					sqlite3ParserTrace(stdout, "parser: ");
 				else
 					sqlite3ParserTrace(0, 0);
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index 692fa82..0c9ac97 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -17,16 +17,13 @@ box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
 result = box.sql.execute('PRAGMA parser_trace')
 ---
 ...
--- Should be nothing.
 box.sql.execute('PRAGMA parser_trace = 1')
 ---
 ...
--- Should be 1.
 box.sql.execute('PRAGMA parser_trace')
 ---
 - - [1]
 ...
--- Should be nothing.
 box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
 ---
 ...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
index 66f47b3..f946306 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -9,9 +9,6 @@ box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
 -- Check that "PRAGMA parser_trace" returns 0 or 1 if called
 -- without parameter.
 result = box.sql.execute('PRAGMA parser_trace')
--- Should be nothing.
 box.sql.execute('PRAGMA parser_trace = 1')
--- Should be 1.
 box.sql.execute('PRAGMA parser_trace')
--- Should be nothing.
 box.sql.execute('PRAGMA parser_trace = '.. result[1][1])


New version:

commit 61bfea175fbd91b86e45874ae123c411754d165d
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Dec 12 22:16:33 2018 +0300

    sql: fix "PRAGMA parser_trace" result
    
    Currently PRAGMA parser_trace returns an empty table. This seems
    wrong, since other similar pragmas return their status. Fixed in
    the current patch.

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 5729fe6..476771d 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -466,33 +466,30 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 	switch (pPragma->ePragTyp) {
 
 	case PragTyp_FLAG:{
-			if (zRight == 0) {
-				setPragmaResultColumnNames(v, pPragma);
-				returnSingleInt(v,
-						(user_session->
-						 sql_flags & pPragma->iArg) !=
-						0);
-			} else {
-				int mask = pPragma->iArg;	/* Mask of bits to set
-								 * or clear.
-								 */
-
-				if (sqlite3GetBoolean(zRight, 0)) {
-					user_session->sql_flags |= mask;
-				} else {
-					user_session->sql_flags &= ~mask;
-				}
-
-				/* Many of the flag-pragmas modify the code
-				 * generated by the SQL * compiler (eg.
-				 * count_changes). So add an opcode to expire
-				 * all * compiled SQL statements after
-				 * modifying a pragma value.
-				 */
-				sqlite3VdbeAddOp0(v, OP_Expire);
+		if (zRight == NULL) {
+			setPragmaResultColumnNames(v, pPragma);
+			returnSingleInt(v, (user_session->sql_flags &
+					    pPragma->iArg) != 0);
+		} else {
+			/* Mask of bits to set or clear. */
+			int mask = pPragma->iArg;
+			bool is_pragma_set = sqlite3GetBoolean(zRight, 0);
+
+			if (is_pragma_set)
+				user_session->sql_flags |= mask;
+			else
+				user_session->sql_flags &= ~mask;
+#if defined(SQLITE_DEBUG)
+			if (mask == PARSER_TRACE_FLAG) {
+				if (is_pragma_set)
+					sqlite3ParserTrace(stdout, "parser: ");
+				else
+					sqlite3ParserTrace(0, 0);
 			}
-			break;
+#endif
 		}
+		break;
+	}
 
 	case PragTyp_TABLE_INFO:
 		sql_pragma_table_info(pParse, zRight);
@@ -569,18 +566,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		}
 		break;
 	}
-#ifndef NDEBUG
-	case PragTyp_PARSER_TRACE:{
-			if (zRight) {
-				if (sqlite3GetBoolean(zRight, 0)) {
-					sqlite3ParserTrace(stdout, "parser: ");
-				} else {
-					sqlite3ParserTrace(0, 0);
-				}
-			}
-			break;
-		}
-#endif
 
 		/*
 		 * Reinstall the LIKE and functions. The variant
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index fd76b49..4837923 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -14,7 +14,6 @@
 #define PragTyp_INDEX_LIST                    11
 #define PragTyp_STATS                         15
 #define PragTyp_TABLE_INFO                    17
-#define PragTyp_PARSER_TRACE                  24
 #define PragTyp_DEFAULT_ENGINE                25
 #define PragTyp_COMPOUND_SELECT_LIMIT         26
 
@@ -137,10 +136,10 @@ static const PragmaName aPragmaName[] = {
 	 /* iArg:      */ 0},
 #if defined(SQLITE_DEBUG)
 	{ /* zName:     */ "parser_trace",
-	 /* ePragTyp:  */ PragTyp_PARSER_TRACE,
-	 /* ePragFlg:  */ 0,
+	 /* ePragTyp:  */ PragTyp_FLAG,
+	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
 	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ 0},
+	 /* iArg:      */ PARSER_TRACE_FLAG},
 #endif
 	{ /* zName:     */ "query_only",
 	 /* ePragTyp:  */ PragTyp_FLAG,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 970a422..740db9f 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1547,6 +1547,8 @@ struct sqlite3 {
  * Possible values for the sqlite3.flags.
  */
 #define SQLITE_VdbeTrace      0x00000001	/* True to trace VDBE execution */
+/* Debug print info about SQL query as it parsed */
+#define PARSER_TRACE_FLAG     0x00000002
 #define SQLITE_FullColNames   0x00000004	/* Show full column names on SELECT */
 #define SQLITE_ShortColNames  0x00000040	/* Show short columns names */
 #define SQLITE_CountRows      0x00000080	/* Count rows changed by INSERT, */
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
new file mode 100644
index 0000000..0c9ac97
--- /dev/null
+++ b/test/sql/sql-debug.result
@@ -0,0 +1,29 @@
+remote = require('net.box')
+---
+...
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+--
+-- gh-3832: Some statements do not return column type
+-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
+-- without parameter.
+result = box.sql.execute('PRAGMA parser_trace')
+---
+...
+box.sql.execute('PRAGMA parser_trace = 1')
+---
+...
+box.sql.execute('PRAGMA parser_trace')
+---
+- - [1]
+...
+box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
+---
+...
diff --git a/test/sql/sql-debug.test.lua b/test/sql/sql-debug.test.lua
new file mode 100644
index 0000000..f946306
--- /dev/null
+++ b/test/sql/sql-debug.test.lua
@@ -0,0 +1,14 @@
+remote = require('net.box')
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+--
+-- gh-3832: Some statements do not return column type
+
+-- Check that "PRAGMA parser_trace" returns 0 or 1 if called
+-- without parameter.
+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])
diff --git a/test/sql/suite.ini b/test/sql/suite.ini
index 4504731..ce6ccb7 100644
--- a/test/sql/suite.ini
+++ b/test/sql/suite.ini
@@ -6,4 +6,4 @@ use_unix_sockets = True
 config = engine.cfg
 is_parallel = True
 lua_libs = lua/sql_tokenizer.lua
-release_disabled = errinj.test.lua view_delayed_wal.test.lua
+release_disabled = errinj.test.lua view_delayed_wal.test.lua sql-debug.test.lua
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [PATCH v4 3/6] sql: Show currently set sql_default_engine
  2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 2/6] sql: fix "PRAGMA parser_trace" result imeevma
@ 2019-01-19 12:37 ` imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

After this patch, "PRAGMA sql_default_engine" called without
arguments will return currently set sql_default_engine.
---
 src/box/sql/pragma.c                 | 18 +++++++++++++-----
 test/sql-tap/gh-2367-pragma.test.lua | 35 +++++++++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 476771d..5e276b4 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -582,12 +582,20 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		}
 
 	case PragTyp_DEFAULT_ENGINE: {
-		if (sql_default_engine_set(zRight) != 0) {
-			pParse->rc = SQL_TARANTOOL_ERROR;
-			pParse->nErr++;
-			goto pragma_out;
+		if (zRight == NULL) {
+			const char *engine_name =
+				sql_storage_engine_strs[current_session()->
+							sql_default_engine];
+			sqlite3VdbeLoadString(v, 1, engine_name);
+			sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 1);
+		} else {
+			if (sql_default_engine_set(zRight) != 0) {
+				pParse->rc = SQL_TARANTOOL_ERROR;
+				pParse->nErr++;
+				goto pragma_out;
+			}
+			sqlite3VdbeAddOp0(v, OP_Expire);
 		}
-		sqlite3VdbeAddOp0(v, OP_Expire);
 		break;
 	}
 
diff --git a/test/sql-tap/gh-2367-pragma.test.lua b/test/sql-tap/gh-2367-pragma.test.lua
index c0792c9..90ecd56 100755
--- a/test/sql-tap/gh-2367-pragma.test.lua
+++ b/test/sql-tap/gh-2367-pragma.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
 
-test:plan(7)
+test:plan(8)
 
 test:do_catchsql_test(
 	"pragma-1.3",
@@ -41,25 +41,44 @@ test:do_catchsql_test(
 test:do_catchsql_test(
 	"pragma-2.4",
 	[[
-		pragma sql_default_engine;
+		pragma sql_default_engine 'memtx';
 	]], {
-	1, 'Illegal parameters, \'sql_default_engine\' was not specified'
+	1, 'near \"\'memtx\'\": syntax error'
 })
 
 test:do_catchsql_test(
 	"pragma-2.5",
 	[[
-		pragma sql_default_engine 'memtx';
+		pragma sql_default_engine 1;
 	]], {
-	1, 'near \"\'memtx\'\": syntax error'
+	1, 'near \"1\": syntax error'
 })
 
+--
+-- gh-3832: Some statements do not return column type
+--
+-- Check that "PRAGMA sql_default_engine" called without arguments
+-- returns currently set sql_default_engine.
 test:do_catchsql_test(
-	"pragma-2.5",
+	"pragma-3.1",
 	[[
-		pragma sql_default_engine 1;
+		pragma sql_default_engine='vinyl';
+		pragma sql_default_engine;
 	]], {
-	1, 'near \"1\": syntax error'
+	-- <pragma-3.1>
+	0, {'vinyl'}
+	-- <pragma-3.1>
+})
+
+test:do_catchsql_test(
+	"pragma-3.2",
+	[[
+		pragma sql_default_engine='memtx';
+		pragma sql_default_engine;
+	]], {
+	-- <pragma-3.2>
+	0, {'memtx'}
+	-- <pragma-3.2>
 })
 
 test:finish_test()
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [PATCH v4 4/6] sql: fix "PRAGMA case_sensitive_like" result
  2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
                   ` (2 preceding siblings ...)
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 3/6] sql: Show currently set sql_default_engine imeevma
@ 2019-01-19 12:37 ` imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format imeevma
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  5 siblings, 0 replies; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

Hi! Thank you for review. My answers, diff between versions and
new version below.


On 1/16/19 6:35 PM, n.pettik wrote:
>
>> diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
>> index 994e64f..cc31a5d 100644
>> --- a/test/sql/misc.test.lua
>> +++ b/test/sql/misc.test.lua
>> @@ -11,3 +11,16 @@ box.sql.execute(';')
>> box.sql.execute('')
>> box.sql.execute('     ;')
>> box.sql.execute('\n\n\n\t\t\t   ')
>> +
>> +--
>> +-- gh-3832: Some statements do not return column type
>> +
>> +-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
>> +-- called without parameter.
>> +result = box.sql.execute('PRAGMA case_sensitive_like')
>> +-- Should be nothing.
>> +box.sql.execute('PRAGMA case_sensitive_like = 1')
>> +-- Should be 1.
>> +box.sql.execute('PRAGMA case_sensitive_like')
>> +-- Should be nothing.
>
> Again: useless comments.
> The rest is OK.
>
Fixed.


Diff between versions:

commit 6623963e948b8367534446769dfc8160d5c39228
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Jan 17 11:56:21 2019 +0300

    Temporary: Review fixes

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index efee229..a610345 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -494,7 +494,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 			 */
 			if (mask == LIKE_CASE_SENS_FLAG) {
 				sqlite3RegisterLikeFunctions(db,
-							     !is_value_true);
+							     !is_pragma_set);
 			}
 		}
 		break;
diff --git a/test/sql/misc.result b/test/sql/misc.result
index 66f5a7b..3496fab 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -47,16 +47,13 @@ box.sql.execute('\n\n\n\t\t\t   ')
 result = box.sql.execute('PRAGMA case_sensitive_like')
 ---
 ...
--- Should be nothing.
 box.sql.execute('PRAGMA case_sensitive_like = 1')
 ---
 ...
--- Should be 1.
 box.sql.execute('PRAGMA case_sensitive_like')
 ---
 - - [1]
 ...
--- Should be nothing.
 box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
 ---
 ...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index cc31a5d..9d4abf5 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -18,9 +18,6 @@ box.sql.execute('\n\n\n\t\t\t   ')
 -- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
 -- called without parameter.
 result = box.sql.execute('PRAGMA case_sensitive_like')
--- Should be nothing.
 box.sql.execute('PRAGMA case_sensitive_like = 1')
--- Should be 1.
 box.sql.execute('PRAGMA case_sensitive_like')
--- Should be nothing.
 box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])


New version:

commit 0de150401e2a202e515ed7a5c4daa2bad593244b
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Dec 13 15:18:54 2018 +0300

    sql: fix "PRAGMA case_sensitive_like" result
    
    Currently PRAGMA case_sensitive_like returns nothing. This seems
    wrong, since other similar pragmas return their status. Fixed in
    the current patch.

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 5e276b4..a610345 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -487,6 +487,15 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 					sqlite3ParserTrace(0, 0);
 			}
 #endif
+			/*
+			 * Reinstall the LIKE and functions. The
+			 * variant of LIKE * used will be case
+			 * sensitive or not depending on the RHS.
+			 */
+			if (mask == LIKE_CASE_SENS_FLAG) {
+				sqlite3RegisterLikeFunctions(db,
+							     !is_pragma_set);
+			}
 		}
 		break;
 	}
@@ -567,20 +576,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		break;
 	}
 
-		/*
-		 * Reinstall the LIKE and functions. The variant
-		 * of LIKE * used will be case sensitive or not
-		 * depending on the RHS.
-		 */
-	case PragTyp_CASE_SENSITIVE_LIKE:{
-			if (zRight) {
-				int is_like_ci =
-					!(sqlite3GetBoolean(zRight, 0));
-				sqlite3RegisterLikeFunctions(db, is_like_ci);
-			}
-			break;
-		}
-
 	case PragTyp_DEFAULT_ENGINE: {
 		if (zRight == NULL) {
 			const char *engine_name =
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 4837923..6ccf720 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -6,7 +6,6 @@
 
 /* The various pragma types */
 #define PragTyp_BUSY_TIMEOUT                   1
-#define PragTyp_CASE_SENSITIVE_LIKE            2
 #define PragTyp_COLLATION_LIST                 3
 #define PragTyp_FLAG                           5
 #define PragTyp_FOREIGN_KEY_LIST               9
@@ -92,10 +91,10 @@ static const PragmaName aPragmaName[] = {
 	 /* ColNames:  */ 31, 1,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
-	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
-	 /* ePragFlg:  */ PragFlg_NoColumns,
+	 /* ePragTyp:  */ PragTyp_FLAG,
+	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
 	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ 0},
+	 /* iArg:      */ LIKE_CASE_SENS_FLAG},
 	{ /* zName:     */ "collation_list",
 	 /* ePragTyp:  */ PragTyp_COLLATION_LIST,
 	 /* ePragFlg:  */ PragFlg_Result0,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 740db9f..026fada 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1550,6 +1550,8 @@ struct sqlite3 {
 /* Debug print info about SQL query as it parsed */
 #define PARSER_TRACE_FLAG     0x00000002
 #define SQLITE_FullColNames   0x00000004	/* Show full column names on SELECT */
+/* True if LIKE is case sensitive. */
+#define LIKE_CASE_SENS_FLAG   0x00000008
 #define SQLITE_ShortColNames  0x00000040	/* Show short columns names */
 #define SQLITE_CountRows      0x00000080	/* Count rows changed by INSERT, */
 					  /*   DELETE, or UPDATE and return */
diff --git a/test/sql/misc.result b/test/sql/misc.result
index ef104c1..3496fab 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -40,3 +40,20 @@ box.sql.execute('\n\n\n\t\t\t   ')
 ---
 - error: 'syntax error: empty request'
 ...
+--
+-- gh-3832: Some statements do not return column type
+-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
+-- called without parameter.
+result = box.sql.execute('PRAGMA case_sensitive_like')
+---
+...
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+---
+...
+box.sql.execute('PRAGMA case_sensitive_like')
+---
+- - [1]
+...
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
+---
+...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index 994e64f..9d4abf5 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -11,3 +11,13 @@ box.sql.execute(';')
 box.sql.execute('')
 box.sql.execute('     ;')
 box.sql.execute('\n\n\n\t\t\t   ')
+
+--
+-- gh-3832: Some statements do not return column type
+
+-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
+-- called without parameter.
+result = box.sql.execute('PRAGMA case_sensitive_like')
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+box.sql.execute('PRAGMA case_sensitive_like')
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format
  2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
                   ` (3 preceding siblings ...)
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
@ 2019-01-19 12:37 ` imeevma
  2019-01-20  0:19   ` [tarantool-patches] " n.pettik
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
  5 siblings, 1 reply; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

Hi! Thank you for review. My answers, diff between versions and
new version below.


On 1/16/19 6:35 PM, n.pettik wrote:
>
>>> There is no ’Tarantool format’...
>> Changed to "more appropriate”.
>
> Cmon, it is called YAML...
>
Thanks, fixed.

>>
>>> 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.
>> Changed rows with values == NULL. Now instean of NULL they shows
>> "No value”.
>
> Now it is even worse.
> I vote for displaying only flag pragmas and make
> second field of resulting filed to be of type bool. 
> You can ask other members for their opinion.
>
I did as you suggested since there were no suggestions from dev
chat.

>>> Btw I don’t understand all these efforts to make pragma
>>> better. As I already said we are going to remove it anyway.
>> I thought it would be correct, since it can be used by the user.
>
> It can be used by users until we remove it. That’s it.
>
>> commit 1b0d86990ecf8c9b310cdfdc58ec9006c77c038a
>> Author: Mergen Imeev <imeevma@gmail.com>
>> Date:   Thu Dec 13 21:07:31 2018 +0300
>>
>>    sql: 'PRAGMA' result in the appropriate format
>>
>>    Currently, box.sql.execute('PRAGMA') returns nothing but it prints
>>    some information on stdout.
>
> Nit: be specific: not ’some information’ but list of pragmas
> and their statuses.
>
Fixed.

> Nit: print to stdout.
>
Fixed.

>> This looks wrong.
>
> "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."
>
Thanks, fixed.

>> This patch makes the
>>    command to return result in more appropriate format.
>
> "In yaml format as the rest of SQL commands.”
>
Fixed.

> Sorry for being too meticulous.
>
It would be more appropriate for me to say thank you for this, I
think.

>>
>> diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
>> index 692fa82..9c5c9b3 100644
>> --- a/test/sql/sql-debug.result
>> +++ b/test/sql/sql-debug.result
>
> Why did you move test to this file?
> PRAGMA itself is not debug-only command.
>
>
I did this because the result of the command in the debug build is
different from the result in the non-debug build.


Diff between versions:

commit 6a94e1371e4691d19a96ed8ac1aa76915239c86f
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Sat Jan 19 12:08:10 2019 +0300

    Temporary: Review fix

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 7851a57..de81c28 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -179,50 +179,16 @@ vdbe_emit_pragma_status(struct Parse *parse)
 	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);
+	sqlite3VdbeSetColName(v, 1, COLNAME_DECLTYPE, "INTEGER", SQLITE_STATIC);
 
 	parse->nMem = 2;
 	for (int i = 0; i < ArraySize(aPragmaName); ++i) {
+		if (aPragmaName[i].ePragTyp != PragTyp_FLAG)
+			continue;
 		sqlite3VdbeAddOp4(v, OP_String8, 0, 1, 0, aPragmaName[i].zName,
 				  0);
-		switch (aPragmaName[i].ePragTyp) {
-			case PragTyp_FLAG: {
-				const char *value;
-				if ((user_session->sql_flags &
-				     aPragmaName[i].iArg) != 0)
-					value = "true";
-				else
-					value = "false";
-				sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
-						  0);
-				break;
-			}
-			case PragTyp_DEFAULT_ENGINE: {
-				const char *value = 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 = parse->db->busyTimeout;
-				sqlite3VdbeAddOp2(v, OP_Integer, value, 2);
-				sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
-				break;
-			}
-			case PragTyp_COMPOUND_SELECT_LIMIT: {
-				int value = sqlite3_limit(parse->db,
-					SQL_LIMIT_COMPOUND_SELECT, -1);
-				sqlite3VdbeAddOp2(v, OP_Integer, value, 2);
-				sqlite3VdbeAddOp2(v, OP_Cast, 2, AFFINITY_TEXT);
-				break;
-			}
-			default: {
-				const char *value = "No value";
-				sqlite3VdbeAddOp4(v, OP_String8, 0, 2, 0, value,
-						  0);
-			}
-		}
+		int val = (user_session->sql_flags & aPragmaName[i].iArg) != 0;
+		sqlite3VdbeAddOp2(v, OP_Integer, val, 2);
 		sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 2);
 	}
 }
diff --git a/test/sql/sql-debug.result b/test/sql/sql-debug.result
index fdf4241..6f1d889 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -28,42 +28,26 @@ box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
 ---
 ...
 --
--- Make PRAGMA command return the result in a more appropriate
--- format.
+-- Make PRAGMA command return the result in YAML format.
 --
-result = box.sql.execute('PRAGMA')
----
-...
-for _,v in pairs(result) do v[2] = nil end
----
-...
-result
----
-- - ['busy_timeout']
-  - ['case_sensitive_like']
-  - ['collation_list']
-  - ['count_changes']
-  - ['defer_foreign_keys']
-  - ['foreign_key_list']
-  - ['full_column_names']
-  - ['index_info']
-  - ['index_list']
-  - ['parser_trace']
-  - ['query_only']
-  - ['read_uncommitted']
-  - ['recursive_triggers']
-  - ['reverse_unordered_selects']
-  - ['select_trace']
-  - ['short_column_names']
-  - ['sql_compound_select_limit']
-  - ['sql_default_engine']
-  - ['sql_trace']
-  - ['stats']
-  - ['table_info']
-  - ['vdbe_addoptrace']
-  - ['vdbe_debug']
-  - ['vdbe_eqp']
-  - ['vdbe_listing']
-  - ['vdbe_trace']
-  - ['where_trace']
+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 6173b3a..0cbc747 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -14,9 +14,6 @@ box.sql.execute('PRAGMA parser_trace')
 box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
 
 --
--- Make PRAGMA command return the result in a more appropriate
--- format.
+-- Make PRAGMA command return the result in YAML format.
 --
-result = box.sql.execute('PRAGMA')
-for _,v in pairs(result) do v[2] = nil end
-result
+box.sql.execute('PRAGMA')


New version:

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

    sql: 'PRAGMA' result in YAML format
    
    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.

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 0c9ac97..6f1d889 100644
--- a/test/sql/sql-debug.result
+++ b/test/sql/sql-debug.result
@@ -27,3 +27,27 @@ box.sql.execute('PRAGMA parser_trace')
 box.sql.execute('PRAGMA parser_trace = '.. result[1][1])
 ---
 ...
+--
+-- Make PRAGMA command return the result in YAML format.
+--
+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 f946306..0cbc747 100644
--- a/test/sql/sql-debug.test.lua
+++ b/test/sql/sql-debug.test.lua
@@ -12,3 +12,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 in YAML format.
+--
+box.sql.execute('PRAGMA')
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] [PATCH v4 6/6] sql: set column types for EXPLAIN and PRAGMA
  2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
                   ` (4 preceding siblings ...)
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format imeevma
@ 2019-01-19 12:37 ` imeevma
  5 siblings, 0 replies; 11+ messages in thread
From: imeevma @ 2019-01-19 12:37 UTC (permalink / raw)
  To: tarantool-patches, korablev

Hi! Thank you for review. My answers, diff between versions and
new version below.


On 1/16/19 6:35 PM, n.pettik wrote:
>
>> @@ -471,17 +464,15 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
>> 		goto pragma_out;
>> 	}
>> 	/* Register the result column names for pragmas that return results */
>> -	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
>> -	    && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0)
>> -	    ) {
>> -		setPragmaResultColumnNames(v, pPragma);
>> -	}
>> +	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 &&
>> +	    ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
>> +		vdbe_set_pragma_result_columns(v, pPragma);
>
> zRight != NULL
>
> The rest seems to be OK.
Fixed.


Diff between versions:

commit 5591e89ff67d07aceb59789ff5e443428b2c4403
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Jan 17 12:44:49 2019 +0300

    Temporary: Review fix

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 6248dac..03ba2a4 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -431,7 +431,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 	}
 	/* Register the result column names for pragmas that return results */
 	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 &&
-	    ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0))
+	    ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == NULL))
 		vdbe_set_pragma_result_columns(v, pPragma);
 	/* Jump to the appropriate pragma handler */
 	switch (pPragma->ePragTyp) {


New version:

commit 6f429ea3482f8310d0fc89cecff29613711e6d1a
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Dec 5 14:30:49 2018 +0300

    sql: set column types for EXPLAIN and PRAGMA
    
    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

diff --git a/src/box/execute.c b/src/box/execute.c
index 7fff5fd..201d9c1 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -482,11 +482,12 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
 		const char *name = sqlite3_column_name(stmt, i);
 		const char *type = sqlite3_column_datatype(stmt, i);
 		/*
-		 * Can not fail, since all column names are
-		 * preallocated during prepare phase and the
+		 * Can not fail, since all column names and types
+		 * are preallocated during prepare phase and the
 		 * column_name simply returns them.
 		 */
 		assert(name != NULL);
+		assert(type != NULL);
 		size += mp_sizeof_str(strlen(name));
 		size += mp_sizeof_str(strlen(type));
 		char *pos = (char *) obuf_alloc(out, size);
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index de81c28..03ba2a4 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -112,25 +112,18 @@ 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. */
 static void
-setPragmaResultColumnNames(Vdbe * v,	/* The query under construction */
-			   const PragmaName * pPragma	/* The pragma */
-    )
+vdbe_set_pragma_result_columns(struct Vdbe *v, const struct PragmaName *pragma)
 {
-	u8 n = pPragma->nPragCName;
-	sqlite3VdbeSetNumCols(v, n == 0 ? 1 : n);
-	if (n == 0) {
-		sqlite3VdbeSetColName(v, 0, COLNAME_NAME, pPragma->zName,
+	int n = pragma->nPragCName;
+	assert(n > 0);
+	sqlite3VdbeSetNumCols(v, n);
+	for (int i = 0, j = pragma->iPragCName; i < n; ++i) {
+		sqlite3VdbeSetColName(v, i, COLNAME_NAME, pragCName[j++],
+				      SQLITE_STATIC);
+		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);
-		}
 	}
 }
 
@@ -437,17 +430,15 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		goto pragma_out;
 	}
 	/* Register the result column names for pragmas that return results */
-	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0
-	    && ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == 0)
-	    ) {
-		setPragmaResultColumnNames(v, pPragma);
-	}
+	if ((pPragma->mPragFlg & PragFlg_NoColumns) == 0 &&
+	    ((pPragma->mPragFlg & PragFlg_NoColumns1) == 0 || zRight == NULL))
+		vdbe_set_pragma_result_columns(v, pPragma);
 	/* Jump to the appropriate pragma handler */
 	switch (pPragma->ePragTyp) {
 
 	case PragTyp_FLAG:{
 		if (zRight == NULL) {
-			setPragmaResultColumnNames(v, pPragma);
+			vdbe_set_pragma_result_columns(v, pPragma);
 			returnSingleInt(v, (user_session->sql_flags &
 					    pPragma->iArg) != 0);
 		} else {
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 6ccf720..b7b2110 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -25,50 +25,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 */ "TEXT",
+	/* 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",
 };
 
 /* Definitions of all built-in pragmas */
@@ -77,7 +169,7 @@ typedef struct PragmaName {
 	u8 ePragTyp;		/* PragTyp_XXX value */
 	u8 mPragFlg;		/* Zero or more PragFlg_XXX values */
 	u8 iPragCName;		/* Start of column names in pragCName[] */
-	u8 nPragCName;		/* Num of col names. 0 means use pragma name */
+	u8 nPragCName;		/* Num of col names. */
 	u32 iArg;		/* Extra argument */
 } PragmaName;
 /**
@@ -88,112 +180,112 @@ static const PragmaName aPragmaName[] = {
 	{ /* zName:     */ "busy_timeout",
 	 /* ePragTyp:  */ PragTyp_BUSY_TIMEOUT,
 	 /* ePragFlg:  */ PragFlg_Result0,
-	 /* ColNames:  */ 31, 1,
+	 /* ColNames:  */ 62, 1,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 64, 1,
 	 /* iArg:      */ LIKE_CASE_SENS_FLAG},
 	{ /* zName:     */ "collation_list",
 	 /* ePragTyp:  */ PragTyp_COLLATION_LIST,
 	 /* ePragFlg:  */ PragFlg_Result0,
-	 /* ColNames:  */ 21, 2,
+	 /* ColNames:  */ 42, 2,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "count_changes",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 66, 1,
 	 /* iArg:      */ SQLITE_CountRows},
 	{ /* zName:     */ "defer_foreign_keys",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 68, 1,
 	 /* iArg:      */ SQLITE_DeferFKs},
 	{ /* zName:     */ "foreign_key_list",
 	 /* ePragTyp:  */ PragTyp_FOREIGN_KEY_LIST,
 	 /* ePragFlg:  */
 	 PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
-	 /* ColNames:  */ 23, 8,
+	 /* ColNames:  */ 46, 8,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "full_column_names",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 70, 1,
 	 /* iArg:      */ SQLITE_FullColNames},
 	{ /* zName:     */ "index_info",
 	 /* ePragTyp:  */ PragTyp_INDEX_INFO,
 	 /* ePragFlg:  */
 	 PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
-	 /* ColNames:  */ 10, 6,
+	 /* ColNames:  */ 20, 6,
 	 /* iArg:      */ 1},
 	{ /* zName:     */ "index_list",
 	 /* ePragTyp:  */ PragTyp_INDEX_LIST,
 	 /* ePragFlg:  */
 	 PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
-	 /* ColNames:  */ 16, 5,
+	 /* ColNames:  */ 32, 5,
 	 /* iArg:      */ 0},
 #if defined(SQLITE_DEBUG)
 	{ /* zName:     */ "parser_trace",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 72, 1,
 	 /* iArg:      */ PARSER_TRACE_FLAG},
 #endif
 	{ /* zName:     */ "query_only",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 74, 1,
 	 /* iArg:      */ SQLITE_QueryOnly},
 	{ /* zName:     */ "read_uncommitted",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 76, 1,
 	 /* iArg:      */ SQLITE_ReadUncommitted},
 	{ /* zName:     */ "recursive_triggers",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 78, 1,
 	 /* iArg:      */ SQLITE_RecTriggers},
 	{ /* zName:     */ "reverse_unordered_selects",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 80, 1,
 	 /* iArg:      */ SQLITE_ReverseOrder},
 #if defined(SQLITE_DEBUG)
 	{ /* zName:     */ "select_trace",
 	/* ePragTyp:  */ PragTyp_FLAG,
 	/* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	/* ColNames:  */ 0, 0,
+	/* ColNames:  */ 82, 1,
 	/* iArg:      */ SQLITE_SelectTrace},
 #endif
 	{ /* zName:     */ "short_column_names",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 84, 1,
 	 /* iArg:      */ SQLITE_ShortColNames},
 	{ /* zName:     */ "sql_compound_select_limit",
 	/* ePragTyp:  */ PragTyp_COMPOUND_SELECT_LIMIT,
 	/* ePragFlg:  */ PragFlg_Result0,
-	/* ColNames:  */ 0, 0,
+	/* ColNames:  */ 86, 1,
 	/* iArg:      */ 0},
 	{ /* zName:     */ "sql_default_engine",
 	 /* ePragTyp:  */ PragTyp_DEFAULT_ENGINE,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 88, 1,
 	 /* iArg:      */ 0},
 #if defined(SQLITE_DEBUG)
 	{ /* zName:     */ "sql_trace",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 90, 1,
 	 /* iArg:      */ SQLITE_SqlTrace},
 #endif
 	{ /* zName:     */ "stats",
 	 /* ePragTyp:  */ PragTyp_STATS,
 	 /* ePragFlg:  */
 	 PragFlg_NeedSchema | PragFlg_Result0 | PragFlg_SchemaReq,
-	 /* ColNames:  */ 6, 4,
+	 /* ColNames:  */ 12, 4,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "table_info",
 	 /* ePragTyp:  */ PragTyp_TABLE_INFO,
@@ -205,33 +297,33 @@ static const PragmaName aPragmaName[] = {
 	{ /* zName:     */ "vdbe_addoptrace",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 92, 1,
 	 /* iArg:      */ SQLITE_VdbeAddopTrace},
 	{ /* zName:     */ "vdbe_debug",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 94, 1,
 	 /* iArg:      */
 	 SQLITE_SqlTrace | SQLITE_VdbeListing | SQLITE_VdbeTrace},
 	{ /* zName:     */ "vdbe_eqp",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 96, 1,
 	 /* iArg:      */ SQLITE_VdbeEQP},
 	{ /* zName:     */ "vdbe_listing",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 98, 1,
 	 /* iArg:      */ SQLITE_VdbeListing},
 	{ /* zName:     */ "vdbe_trace",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
+	 /* ColNames:  */ 100, 1,
 	 /* iArg:      */ SQLITE_VdbeTrace},
 	{ /* zName:     */ "where_trace",
 	/* ePragTyp:  */ PragTyp_FLAG,
 	/* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	/* ColNames:  */ 0, 0,
+	/* ColNames:  */ 102, 1,
 	/* iArg:      */ SQLITE_WhereTrace},
 #endif
 };
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 824578e..87326da 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -54,7 +54,6 @@ sqlite3Prepare(sqlite3 * db,	/* Database handle. */
 {
 	char *zErrMsg = 0;	/* Error message */
 	int rc = SQLITE_OK;	/* Result code */
-	int i;			/* Loop counter */
 	Parse sParse;		/* Parsing context */
 	sql_parser_create(&sParse, db);
 	sParse.pReprepare = pReprepare;
@@ -113,23 +112,48 @@ sqlite3Prepare(sqlite3 * db,	/* Database handle. */
 
 	if (rc == SQLITE_OK && sParse.pVdbe && sParse.explain) {
 		static const char *const azColName[] = {
-			"addr", "opcode", "p1", "p2", "p3", "p4", "p5",
-			    "comment",
-			"selectid", "order", "from", "detail"
+			/*  0 */ "addr",
+			/*  1 */ "INTEGER",
+			/*  2 */ "opcode",
+			/*  3 */ "TEXT",
+			/*  4 */ "p1",
+			/*  5 */ "INTEGER",
+			/*  6 */ "p2",
+			/*  7 */ "INTEGER",
+			/*  8 */ "p3",
+			/*  9 */ "INTEGER",
+			/* 10 */ "p4",
+			/* 11 */ "TEXT",
+			/* 12 */ "p5",
+			/* 13 */ "TEXT",
+			/* 14 */ "comment",
+			/* 15 */ "TEXT",
+			/* 16 */ "selectid",
+			/* 17 */ "INTEGER",
+			/* 18 */ "order",
+			/* 19 */ "INTEGER",
+			/* 20 */ "from",
+			/* 21 */ "INTEGER",
+			/* 22 */ "detail",
+			/* 23 */ "TEXT",
 		};
-		int iFirst, mx;
+
+		int name_first, name_count;
 		if (sParse.explain == 2) {
-			sqlite3VdbeSetNumCols(sParse.pVdbe, 4);
-			iFirst = 8;
-			mx = 12;
+			name_first = 16;
+			name_count = 4;
 		} else {
-			sqlite3VdbeSetNumCols(sParse.pVdbe, 8);
-			iFirst = 0;
-			mx = 8;
+			name_first = 0;
+			name_count = 8;
 		}
-		for (i = iFirst; i < mx; i++) {
-			sqlite3VdbeSetColName(sParse.pVdbe, i - iFirst,
-					      COLNAME_NAME, azColName[i],
+		sqlite3VdbeSetNumCols(sParse.pVdbe, name_count);
+		for (int i = 0; i < name_count; i++) {
+			int name_index = 2 * i + name_first;
+			sqlite3VdbeSetColName(sParse.pVdbe, i, COLNAME_NAME,
+					      azColName[name_index],
+					      SQLITE_STATIC);
+			sqlite3VdbeSetColName(sParse.pVdbe, i, COLNAME_DECLTYPE,
+					      azColName[name_index + 1],
 					      SQLITE_STATIC);
 		}
 	}
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 9ace282..0a17e4a 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -821,6 +821,75 @@ box.sql.execute('DROP TABLE t1')
 cn:close()
 ---
 ...
+-- gh-3832: Some statements do not return column type
+box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)')
+---
+...
+cn = remote.connect(box.cfg.listen)
+---
+...
+-- PRAGMA:
+res = cn:execute("PRAGMA table_info(t1)")
+---
+...
+res.metadata
+---
+- - name: cid
+    type: INTEGER
+  - name: name
+    type: TEXT
+  - name: type
+    type: TEXT
+  - name: notnull
+    type: INTEGER
+  - name: dflt_value
+    type: TEXT
+  - name: pk
+    type: INTEGER
+...
+-- EXPLAIN
+res = cn:execute("EXPLAIN SELECT 1")
+---
+...
+res.metadata
+---
+- - name: addr
+    type: INTEGER
+  - name: opcode
+    type: TEXT
+  - name: p1
+    type: INTEGER
+  - name: p2
+    type: INTEGER
+  - name: p3
+    type: INTEGER
+  - name: p4
+    type: TEXT
+  - name: p5
+    type: TEXT
+  - name: comment
+    type: TEXT
+...
+res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
+---
+...
+res.metadata
+---
+- - name: selectid
+    type: INTEGER
+  - name: order
+    type: INTEGER
+  - name: from
+    type: INTEGER
+  - name: detail
+    type: TEXT
+...
+cn:close()
+---
+...
+box.sql.execute('DROP TABLE t1')
+---
+...
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 ---
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 6640903..6fa5d95 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -263,10 +263,26 @@ box.sql.execute('DROP TABLE t1')
 
 cn:close()
 
+-- gh-3832: Some statements do not return column type
+box.sql.execute('CREATE TABLE t1(id INTEGER PRIMARY KEY)')
+cn = remote.connect(box.cfg.listen)
+
+-- PRAGMA:
+res = cn:execute("PRAGMA table_info(t1)")
+res.metadata
+
+-- EXPLAIN
+res = cn:execute("EXPLAIN SELECT 1")
+res.metadata
+res = cn:execute("EXPLAIN QUERY PLAN SELECT COUNT(*) FROM t1")
+res.metadata
+
+cn:close()
+box.sql.execute('DROP TABLE t1')
+
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 box.schema.user.revoke('guest', 'create', 'space')
 space = nil
 
 -- Cleanup xlog
 box.snapshot()
-
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
@ 2019-01-20  0:16   ` n.pettik
  2019-01-24 14:52     ` Imeev Mergen
  0 siblings, 1 reply; 11+ messages in thread
From: n.pettik @ 2019-01-20  0:16 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen

> 
> commit 28ff08bf9df3a3948fdc399f4801f01d170c52a5
> Author: Mergen Imeev <imeevma@gmail.com>
> Date:   Wed Dec 12 21:58:54 2018 +0300
> 
>    sql: remove unused macros from pragma.c and pragma.h
> 
>    Some macros in pragma.c and pragma.h are obsolete because the
>    values they are checking are no longer used. Let's remove them.

In fact they are used, but their usage makes no sense.
Then LGTM.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format
  2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format imeevma
@ 2019-01-20  0:19   ` n.pettik
  2019-01-24 14:53     ` Imeev Mergen
  0 siblings, 1 reply; 11+ messages in thread
From: n.pettik @ 2019-01-20  0:19 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Imeev Mergen


> New version:
> 
> commit a3e6e457680e9aef5af78ca7a92f1572fcfbdef4
> Author: Mergen Imeev <imeevma@gmail.com>
> Date:   Thu Dec 13 21:07:31 2018 +0300
> 
>    sql: 'PRAGMA' result in YAML format

Commit's subject would look better with verb.

For instance:

sql: get results of PRAGMA statement in YAML format

Or sort of.

The rest is OK in this patch-set, so LGTM.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h
  2019-01-20  0:16   ` [tarantool-patches] " n.pettik
@ 2019-01-24 14:52     ` Imeev Mergen
  0 siblings, 0 replies; 11+ messages in thread
From: Imeev Mergen @ 2019-01-24 14:52 UTC (permalink / raw)
  To: n.pettik, tarantool-patches

Hi! Thank you for review.

On 1/20/19 3:16 AM, n.pettik wrote:
>> commit 28ff08bf9df3a3948fdc399f4801f01d170c52a5
>> Author: Mergen Imeev <imeevma@gmail.com>
>> Date:   Wed Dec 12 21:58:54 2018 +0300
>>
>>     sql: remove unused macros from pragma.c and pragma.h
>>
>>     Some macros in pragma.c and pragma.h are obsolete because the
>>     values they are checking are no longer used. Let's remove them.
> In fact they are used, but their usage makes no sense.
> Then LGTM.

Fixed. New commit-message:

commit 1f667d56f6684a5cb7030097a66c4ac8b46d36cc
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Dec 12 21:58:54 2018 +0300

     sql: remove unused macros from pragma.c and pragma.h

     Some macros in pragma.c and pragma.h are obsolete because the
     values they are checking are no longer used or their usage makes
     no sense. Let's remove them.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tarantool-patches] Re: [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format
  2019-01-20  0:19   ` [tarantool-patches] " n.pettik
@ 2019-01-24 14:53     ` Imeev Mergen
  0 siblings, 0 replies; 11+ messages in thread
From: Imeev Mergen @ 2019-01-24 14:53 UTC (permalink / raw)
  To: n.pettik, tarantool-patches

Hi! Thank you for review. Changed commit-message.

On 1/20/19 3:19 AM, n.pettik wrote:
>> New version:
>>
>> commit a3e6e457680e9aef5af78ca7a92f1572fcfbdef4
>> Author: Mergen Imeev <imeevma@gmail.com>
>> Date:   Thu Dec 13 21:07:31 2018 +0300
>>
>>     sql: 'PRAGMA' result in YAML format
> Commit's subject would look better with verb.
>
> For instance:
>
> sql: get results of PRAGMA statement in YAML format
>
> Or sort of.
>
> The rest is OK in this patch-set, so LGTM.
>
Fixed. New commit-message:

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

     sql: get results of PRAGMA statement in YAML format

     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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-24 14:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-19 12:37 [tarantool-patches] [PATCH v4 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
2019-01-20  0:16   ` [tarantool-patches] " n.pettik
2019-01-24 14:52     ` Imeev Mergen
2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 2/6] sql: fix "PRAGMA parser_trace" result imeevma
2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 3/6] sql: Show currently set sql_default_engine imeevma
2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 4/6] sql: fix "PRAGMA case_sensitive_like" result imeevma
2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 5/6] sql: 'PRAGMA' result in YAML format imeevma
2019-01-20  0:19   ` [tarantool-patches] " n.pettik
2019-01-24 14:53     ` Imeev Mergen
2019-01-19 12:37 ` [tarantool-patches] [PATCH v4 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma

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