[tarantool-patches] Re: [PATCH 7/7] sql: finish DD integration

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch! See my fixes on the branch and below:

commit 7976184bfe14a6a829c4be432fd206e750224c6f
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Aug 28 21:53:54 2018 -0300

     Review fixes

diff --git a/src/box/sql.c b/src/box/sql.c
index 85d1602da..191925b43 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -77,10 +77,6 @@ sql_init()
  void
  sql_load_schema()
  {
-	struct session *user_session = current_session();
-	int commit_internal = !(user_session->sql_flags
-				& SQLITE_InternChanges);
-
  	assert(db->init.busy == 0);
  	/*
  	 * This function is called before version upgrade.
@@ -97,8 +93,6 @@ sql_load_schema()
  	if (db->errCode != SQLITE_OK)
  		panic("failed to initialize SQL subsystem");
  	db->init.busy = 0;
-	if (commit_internal)
-		sqlite3CommitInternalChanges();
  }
  
  void
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index da1c42248..5f889740f 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -135,15 +135,6 @@ sql_space_index_by_name(struct space *space, const char *name)
  	return NULL;
  }
  
-/*
- * This routine is called when a commit occurs.
- */
-void
-sqlite3CommitInternalChanges()
-{
-	current_session()->sql_flags &= ~SQLITE_InternChanges;
-}
-
  bool
  sql_space_column_is_in_pk(struct space *space, uint32_t column)
  {
diff --git a/src/box/sql/cursor.c b/src/box/sql/cursor.c
index b0966910c..f984f8c32 100644
--- a/src/box/sql/cursor.c
+++ b/src/box/sql/cursor.c
@@ -108,8 +108,7 @@ sqlite3CursorPayload(BtCursor *pCur, u32 offset, u32 amt, void *pBuf)
  	const void *pPayload;
  	u32 sz;
  	pPayload = tarantoolSqlite3PayloadFetch(pCur, &sz);
-	if ((uptr) (offset + amt) > sz)
-		return SQLITE_CORRUPT_BKPT;
+	assert((uptr) (offset + amt) <= sz);
  	memcpy(pBuf, pPayload + offset, amt);
  	return SQLITE_OK;
  }
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 996f13c25..782f99452 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -850,9 +850,6 @@ sqlite3ErrName(int rc)
  		case SQLITE_IOERR_CONVPATH:
  			zName = "SQLITE_IOERR_CONVPATH";
  			break;
-		case SQLITE_CORRUPT:
-			zName = "SQLITE_CORRUPT";
-			break;
  		case SQLITE_NOTFOUND:
  			zName = "SQLITE_NOTFOUND";
  			break;
@@ -942,7 +939,6 @@ sqlite3ErrStr(int rc)
  		/* SQLITE_NOMEM       */ "out of memory",
  		/* SQLITE_INTERRUPT   */ "interrupted",
  		/* SQLITE_IOERR       */ "disk I/O error",
-		/* SQLITE_CORRUPT     */ "database disk image is malformed",
  		/* SQLITE_NOTFOUND    */ "unknown operation",
  		/* SQLITE_FULL        */ "database or disk is full",
  		/* SQLITE_CANTOPEN    */ "unable to open database file",
@@ -1949,7 +1945,7 @@ opendb_out:
  }
  
  /*
- * The following routines are substitutes for constants SQLITE_CORRUPT,
+ * The following routines are substitutes for constants
   * SQLITE_MISUSE, SQLITE_CANTOPEN, SQLITE_NOMEM and possibly other error
   * constants.  They serve two purposes:
   *
@@ -1967,13 +1963,6 @@ reportError(int iErr, int lineno, const char *zType)
  	return iErr;
  }
  
-int
-sqlite3CorruptError(int lineno)
-{
-	testcase(sqlite3GlobalConfig.xLog != 0);
-	return reportError(SQLITE_CORRUPT, lineno, "database corruption");
-}
-
  int
  sqlite3MisuseError(int lineno)
  {
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 3047d41c3..dc919f0c0 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -399,8 +399,6 @@ enum sql_ret_code {
  	SQLITE_INTERRUPT,
  	/** Some kind of disk I/O error occurred. */
  	SQLITE_IOERR,
-	/** The database disk image is malformed. */
-	SQLITE_CORRUPT,
  	/** Unknown opcode in sqlite3_file_control(). */
  	SQLITE_NOTFOUND,
  	/** Insertion failed because database is full. */
@@ -1593,7 +1591,6 @@ struct sqlite3 {
   * Possible values for the sqlite3.flags.
   */
  #define SQLITE_VdbeTrace      0x00000001	/* True to trace VDBE execution */
-#define SQLITE_InternChanges  0x00000002	/* Uncommitted Hash table changes */
  #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, */
@@ -3125,10 +3122,8 @@ struct TreeView {
   * using sqlite3_log().  The routines also provide a convenient place
   * to set a debugger breakpoint.
   */
-int sqlite3CorruptError(int);
  int sqlite3MisuseError(int);
  int sqlite3CantopenError(int);
-#define SQLITE_CORRUPT_BKPT sqlite3CorruptError(__LINE__)
  #define SQLITE_MISUSE_BKPT sqlite3MisuseError(__LINE__)
  #define SQLITE_CANTOPEN_BKPT sqlite3CantopenError(__LINE__)
  #ifdef SQLITE_DEBUG
@@ -3310,7 +3305,6 @@ u32 sqlite3ExprListFlags(const ExprList *);
  int sqlite3Init(sqlite3 *);
  
  void sqlite3Pragma(Parse *, Token *, Token *, Token *, int);
-void sqlite3CommitInternalChanges();
  void sqlite3DeleteColumnNames(sqlite3 *, Table *);
  
  /**
@@ -4799,8 +4793,6 @@ void sqlite3VectorErrorMsg(Parse *, Expr *);
   */
  extern int sqlSubProgramsRemaining;
  
-extern int sqlite3InitDatabase(sqlite3 * db);
-
  /**
   * Generate VDBE code to halt execution with correct error if
   * the object with specified key is already present (or doesn't
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 8973d68d8..813b0a041 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2903,12 +2903,8 @@ case OP_Savepoint: {
  				}
  				rc = p->rc;
  			} else {
-				if (p1==SAVEPOINT_ROLLBACK) {
+				if (p1==SAVEPOINT_ROLLBACK)
  					box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint);
-					if ((user_session->sql_flags &
-					     SQLITE_InternChanges) != 0)
-						sqlite3ExpirePreparedStatements(db);
-				}
  			}
  
  			/* Regardless of whether this is a RELEASE or ROLLBACK, destroy all
@@ -4584,7 +4580,6 @@ case OP_RenameTable: {
  			 * In this case, rename table back and
  			 * try again.
  			 */
-			sqlite3CommitInternalChanges();
  			goto abort_due_to_error;
  		}
  		trigger = next_trigger;
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 94696cef5..c843bc786 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2428,8 +2428,6 @@ sqlite3VdbeHalt(Vdbe * p)
  					closeCursorsAndFree(p);
  					sqlite3RollbackAll(p);
  					p->nChange = 0;
-				} else {
-					sqlite3CommitInternalChanges();
  				}
  			} else {
  				box_txn_rollback();