[tarantool-patches] [PATCH] sql: remove pragma schema_version

  • From: Kirill Yukhin <kyukhin@xxxxxxxxxxxxx>
  • To: v.shpilevoy@xxxxxxxxxxxxx
  • Date: Tue, 31 Jul 2018 13:54:08 +0300

This pragma is dead and produces nothing else but segfault.
Along w/ this pragma, remove now dead opcodes which set/read
schema_version and all related routines.
Also, improve opcode generation script.

Part of #3541
---
Branch: 
https://github.com/tarantool/tarantool/commits/kyukhin/gh-3541-remove-schema-version-pragma
Issue: https://github.com/tarantool/tarantool/issues/3541

 extra/mkopcodeh.sh    |  33 ++++++++++++--
 src/box/sql/build.c   |  29 ------------
 src/box/sql/pragma.c  | 122 +++++++-------------------------------------------
 src/box/sql/pragma.h  |  12 -----
 src/box/sql/trigger.c |   2 -
 src/box/sql/vdbe.c    |  45 +------------------
 6 files changed, 47 insertions(+), 196 deletions(-)

diff --git a/extra/mkopcodeh.sh b/extra/mkopcodeh.sh
index 63ad0d5..9e97a50 100755
--- a/extra/mkopcodeh.sh
+++ b/extra/mkopcodeh.sh
@@ -35,6 +35,7 @@ set -f   # disable pathname expansion
 
 currentOp=""
 nOp=0
+mxTk=-1
 newline="$(printf '\n')"
 IFS="$newline"
 while read line; do
@@ -106,6 +107,9 @@ while read line; do
                         eval "ARRAY_used_$val=1"
                         eval "ARRAY_sameas_$val=$sym"
                         eval "ARRAY_def_$val=$name"
+                       if [ $val -gt $mxTk ] ; then
+                            mxTk=$val
+                       fi
                     fi
                 ;;
                 jump) eval "ARRAY_jump_$name=1" ;;
@@ -220,8 +224,12 @@ while [ "$i" -lt "$nOp" ]; do
     i=$((i + 1))
 done
 max="$cnt"
+echo "//*************** $max $nOp $mxTk"
+if [ $mxTk -lt $nOp ] ; then
+    mxTk=$nOp
+fi
 i=0
-while [ "$i" -lt "$nOp" ]; do
+while [ "$i" -le "$mxTk" ]; do
     eval "used=\${ARRAY_used_$i:-}"
     if [ -z "$used" ]; then
         eval "ARRAY_def_$i=OP_NotUsed_$i"
@@ -251,9 +259,21 @@ done
 # Generate the bitvectors:
 ARRAY_bv_0=0
 i=0
-while [ "$i" -le "$max" ]; do
+while [ "$i" -le "$mxTk" ]; do
+    eval "is_exists=\${ARRAY_def_$i:-}"
+    if [ ! -n "$is_exists" ] ; then
+    echo "//SKIP $i"
+        i=$((i + 1))
+       continue
+    fi
     eval "name=\$ARRAY_def_$i"
     x=0
+    eval "is_exists=\${ARRAY_jump_$name:-}"
+    if [ ! -n "$is_exists" ] ; then
+    echo "//SKIP2 $i"
+        i=$((i + 1))
+       continue
+    fi
     eval "jump=\$ARRAY_jump_$name"
     eval "in1=\$ARRAY_in1_$name"
     eval "in2=\$ARRAY_in2_$name"
@@ -283,11 +303,16 @@ printf '%s\n' "#define OPFLG_OUT2        0x10  /* out2:  
P2 is an output */"
 printf '%s\n' "#define OPFLG_OUT3        0x20  /* out3:  P3 is an output */"
 printf '%s\n' "#define OPFLG_INITIALIZER {\\"
 i=0
-while [ "$i" -le "$max" ]; do
+while [ "$i" -le "$mxTk" ]; do
     if [ "$((i % 8))" -eq 0 ]; then
         printf '/* %3d */' "$i"
     fi
-    eval "bv=\$ARRAY_bv_$i"
+    eval "is_exists=\${ARRAY_bv_$i:-}"
+    if [ ! -n "$is_exists" ] ; then
+        bv=0
+    else
+        eval "bv=\$ARRAY_bv_$i"
+    fi
     printf ' 0x%02x,' "$bv"
     if [ "$((i % 8))" -eq 7 ]; then
         printf '%s\n' "\\"
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 5772ee5..dcbeeb0 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1086,33 +1086,6 @@ vdbe_emit_open_cursor(struct Parse *parse_context, int 
cursor, int index_id,
        return sqlite3VdbeAddOp4(parse_context->pVdbe, OP_OpenWrite, cursor,
                                 index_id, 0, (void *) space, P4_SPACEPTR);
 }
-/*
- * Generate code that will increment the schema cookie.
- *
- * The schema cookie is used to determine when the schema for the
- * database changes.  After each schema change, the cookie value
- * changes.  When a process first reads the schema it records the
- * cookie.  Thereafter, whenever it goes to access the database,
- * it checks the cookie to make sure the schema has not changed
- * since it was last read.
- *
- * This plan is not completely bullet-proof.  It is possible for
- * the schema to change multiple times and for the cookie to be
- * set back to prior value.  But schema changes are infrequent
- * and the probability of hitting the same cookie value is only
- * 1 chance in 2^32.  So we're safe enough.
- *
- * IMPLEMENTATION-OF: R-34230-56049 SQLite automatically increments
- * the schema-version whenever the schema changes.
- */
-void
-sqlite3ChangeCookie(Parse * pParse)
-{
-       sqlite3 *db = pParse->db;
-       Vdbe *v = pParse->pVdbe;
-       sqlite3VdbeAddOp3(v, OP_SetCookie, 0, 0,
-                         db->pSchema->schema_cookie + 1);
-}
 
 /*
  * Measure the number of characters needed to output the given
@@ -1495,7 +1468,6 @@ parseTableSchemaRecord(Parse * pParse, int iSpaceId, char 
*zStmt)
                makeIndexSchemaRecord(pParse, pIdx, iSpaceId, ++i, NULL);
        }
 
-       sqlite3ChangeCookie(pParse);
        sqlite3VdbeAddParseSchema2Op(v, iTop, pParse->nMem - iTop + 1);
 }
 
@@ -2891,7 +2863,6 @@ sql_create_index(struct Parse *parse, struct Token *token,
                 * Reparse the schema. Code an OP_Expire
                 * to invalidate all pre-compiled statements.
                 */
-               sqlite3ChangeCookie(parse);
                sqlite3VdbeAddParseSchema2Op(vdbe, first_schema_col, 4);
                sqlite3VdbeAddOp0(vdbe, OP_Expire);
        }
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index d427f78..0d317ee 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -813,112 +813,25 @@ sqlite3Pragma(Parse * pParse, Token * pId,       /* 
First part of [schema.]id field */
                        break;
                }
 
-#ifndef SQLITE_INTEGRITY_CHECK_ERROR_MAX
-#define SQLITE_INTEGRITY_CHECK_ERROR_MAX 100
-#endif
-
-#ifndef SQLITE_OMIT_SCHEMA_VERSION_PRAGMAS
-               /* *   PRAGMA [schema.]schema_version *   PRAGMA
-                * [schema.]schema_version = <integer> *
-                *
-                * PRAGMA [schema.]user_version *   PRAGMA
-                * [schema.]user_version = <integer> *
-                *
-                * PRAGMA [schema.]freelist_count *
-                *
-                * PRAGMA [schema.]data_version *
-                *
-                * PRAGMA [schema.]application_id *   PRAGMA
-                * [schema.]application_id = <integer> *
-                *
-                * The pragma's schema_version and user_version are used
-                * to set or get * the value of the schema-version and
-                * user-version, respectively. Both * the
-                * schema-version and the user-version are 32-bit
-                * signed integers * stored in the database header. *
-                *
-                * The schema-cookie is usually only manipulated
-                * internally by SQLite. It * is incremented by SQLite
-                * whenever the database schema is modified (by *
-                * creating or dropping a table or index). The schema
-                * version is used by * SQLite each time a query is
-                * executed to ensure that the internal cache * of the
-                * schema used when compiling the SQL query matches the
-                * schema of * the database against which the compiled
-                * query is actually executed. * Subverting this
-                * mechanism by using "PRAGMA schema_version" to modify *
-                * the schema-version is potentially dangerous and may
-                * lead to program * crashes or database corruption.
-                * Use with caution! *
-                *
-                * The user-version is not used internally by SQLite. It
-                * may be used by * applications for any purpose.
-                */
-       case PragTyp_HEADER_VALUE:{
-                       int iCookie = pPragma->iArg;    /* Which cookie to read
-                                                        * or write
-                                                        */
-                       if (zRight
-                           && (pPragma->mPragFlg & PragFlg_ReadOnly) == 0) {
-                               /* Write the specified cookie value */
-                               static const VdbeOpList setCookie[] = {
-                                       {OP_SetCookie, 0, 0, 0},        /* 1 */
-                               };
-                               VdbeOp *aOp;
-                               sqlite3VdbeVerifyNoMallocRequired(v,
-                                                                 ArraySize
-                                                                 (setCookie));
-                               aOp =
-                                   sqlite3VdbeAddOpList(v,
-                                                        ArraySize(setCookie),
-                                                        setCookie, 0);
-                               if (ONLY_IF_REALLOC_STRESS(aOp == 0))
-                                       break;
-                               aOp[0].p1 = 0;
-                               aOp[0].p2 = iCookie;
-                               aOp[0].p3 = sqlite3Atoi(zRight);
-                       } else {
-                               /* Read the specified cookie value */
-                               static const VdbeOpList readCookie[] = {
-                                       {OP_ReadCookie, 0, 1, 0},       /* 1 */
-                                       {OP_ResultRow, 1, 1, 0}
-                               };
-                               VdbeOp *aOp;
-                               sqlite3VdbeVerifyNoMallocRequired(v,
-                                                                 ArraySize
-                                                                 (readCookie));
-                               aOp =
-                                   sqlite3VdbeAddOpList(v,
-                                                        ArraySize(readCookie),
-                                                        readCookie, 0);
-                               if (ONLY_IF_REALLOC_STRESS(aOp == 0))
-                                       break;
-                               aOp[0].p1 = 0;
-                               aOp[1].p1 = 0;
-                               aOp[1].p3 = iCookie;
-                               sqlite3VdbeReusable(v);
-                       }
-                       break;
-               }
-               case PragTyp_DEFAULT_ENGINE: {
-                       if (sql_default_engine_set(zRight) != 0) {
-                               pParse->rc = SQL_TARANTOOL_ERROR;
-                               pParse->nErr++;
-                               goto pragma_out;
-                       }
-                       sqlite3VdbeAddOp0(v, OP_Expire);
-                       break;
+       case PragTyp_DEFAULT_ENGINE: {
+               if (sql_default_engine_set(zRight) != 0) {
+                       pParse->rc = SQL_TARANTOOL_ERROR;
+                       pParse->nErr++;
+                       goto pragma_out;
                }
+               sqlite3VdbeAddOp0(v, OP_Expire);
+               break;
+       }
 
-               /* *   PRAGMA busy_timeout *   PRAGMA busy_timeout = N *
-                *
-                * Call sqlite3_busy_timeout(db, N).  Return the current
-                * timeout value * if one is set.  If no busy handler
-                * or a different busy handler is set * then 0 is
-                * returned.  Setting the busy_timeout to 0 or negative *
-                * disables the timeout.
-                */
-               /* case PragTyp_BUSY_TIMEOUT */
+       /* *   PRAGMA busy_timeout *   PRAGMA busy_timeout = N *
+        *
+        * Call sqlite3_busy_timeout(db, N).  Return the current
+        * timeout value * if one is set.  If no busy handler
+        * or a different busy handler is set * then 0 is
+        * returned.  Setting the busy_timeout to 0 or negative *
+        * disables the timeout.
+        */
+       /* case PragTyp_BUSY_TIMEOUT */
        default:{
                        assert(pPragma->ePragTyp == PragTyp_BUSY_TIMEOUT);
                        if (zRight) {
@@ -945,5 +858,4 @@ sqlite3Pragma(Parse * pParse, Token * pId,  /* First part 
of [schema.]id field */
        sqlite3DbFree(db, zTable);
 }
 
-#endif                         /* SQLITE_OMIT_PRAGMA */
 #endif
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 795c98c..6771749 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -5,7 +5,6 @@
  */
 
 /* The various pragma types */
-#define PragTyp_HEADER_VALUE                   0
 #define PragTyp_BUSY_TIMEOUT                   1
 #define PragTyp_CASE_SENSITIVE_LIKE            2
 #define PragTyp_COLLATION_LIST                 3
@@ -23,7 +22,6 @@
 #define PragFlg_NeedSchema 0x01        /* Force schema load before running */
 #define PragFlg_NoColumns  0x02        /* OP_ResultRow called with zero 
columns */
 #define PragFlg_NoColumns1 0x04        /* zero columns if RHS argument is 
present */
-#define PragFlg_ReadOnly   0x08        /* Read-only HEADER_VALUE */
 #define PragFlg_Result0    0x10        /* Acts as query when no argument */
 #define PragFlg_Result1    0x20        /* Acts as query when has one argument 
*/
 #define PragFlg_SchemaOpt  0x40        /* Schema restricts name search if 
present */
@@ -224,16 +222,6 @@ static const PragmaName aPragmaName[] = {
         /* ColNames:  */ 0, 0,
         /* iArg:      */ SQLITE_ReverseOrder},
 #endif
-#if !defined(SQLITE_OMIT_SCHEMA_VERSION_PRAGMAS)
-       { /* zName:     */ "schema_version",
-        /* ePragTyp:  */ PragTyp_HEADER_VALUE,
-        /* ePragFlg:  */ PragFlg_NoColumns1 | PragFlg_Result0,
-        /* ColNames:  */ 0, 0,
-         /* Tarantool: need to take schema version from
-          * backend.
-          */
-        /* iArg:      */ 0},
-#endif
 #if !defined(SQLITE_OMIT_FLAG_PRAGMAS) && defined(SQLITE_ENABLE_SELECTTRACE)
        { /* zName:     */ "select_trace",
        /* ePragTyp:  */ PragTyp_FLAG,
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index ec0bc98..46f7c31 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -260,7 +260,6 @@ sql_trigger_finish(struct Parse *parse, struct TriggerStep 
*step_list,
                sqlite3VdbeAddOp1(v, OP_Close, cursor);
 
                sql_set_multi_write(parse, false);
-               sqlite3ChangeCookie(parse);
        } else {
                parse->parsed_ast.trigger = trigger;
                parse->parsed_ast_type = AST_TYPE_TRIGGER;
@@ -450,7 +449,6 @@ vdbe_code_drop_trigger(struct Parse *parser, const char 
*trigger_name,
                          record_to_delete);
        if (account_changes)
                sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
-       sqlite3ChangeCookie(parser);
 }
 
 void
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 2144d95..d8ddb3a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3076,53 +3076,10 @@ case OP_TTransaction: {
        break;
 }
 
-/* Opcode: ReadCookie P1 P2 P3 * *
- *
- * Read cookie number P3 from database P1 and write it into register P2.
- * P3==1 is the schema version.  P3==2 is the database format.
- * P3==3 is the recommended pager cache size, and so forth.  P1==0 is
- * the main database file and P1==1 is the database file used to store
- * temporary tables.
- *
- * There must be a read-lock on the database (either a transaction
- * must be started or there must be an open cursor) before
- * executing this instruction.
- */
-case OP_ReadCookie: {               /* out2 */
-       pOut = out2Prerelease(p, pOp);
-       pOut->u.i = 0;
-       break;
-}
-
-/* Opcode: SetCookie P1 P2 P3 * *
- *
- * Write the integer value P3 into the schema version.
- * P2==3 is the recommended pager cache
- * size, and so forth.  P1==0 is the main database file and P1==1 is the
- * database file used to store temporary tables.
- *
- * A transaction must be started before executing this opcode.
- */
-case OP_SetCookie: {
-       assert(pOp->p1==0);
-       /* See note about index shifting on OP_ReadCookie */
-       /* When the schema cookie changes, record the new cookie internally */
-       user_session->sql_flags |= SQLITE_InternChanges;
-       if (pOp->p1==1) {
-               /* Invalidate all prepared statements whenever the TEMP database
-                * schema is changed.  Ticket #1644
-                */
-               sqlite3ExpirePreparedStatements(db);
-               p->expired = 0;
-       }
-       if (rc) goto abort_due_to_error;
-       break;
-}
-
 /* Opcode: OpenRead P1 P2 P3 P4 P5
  * Synopsis: index id = P2, space ptr = P4
  *
- * Open a cursor for a space specified by pointer in P4 and index
+\ * Open a cursor for a space specified by pointer in P4 and index
  * id in P2. Give the new cursor an identifier of P1. The P1
  * values need not be contiguous but all P1 values should be
  * small integers. It is an error for P1 to be negative.
-- 
2.16.2


Other related posts: