[tarantool-patches] [PATCH v1 11/12] sql: remove sqlError() and remove sqlErrorWithMsg()

[imeevma@tarantool.org]

These functions are part of the SQL error system, which is
replaced by the Tarantool error system. They are not needed now,
so they will be deleted.
---
 src/box/lua/lua_sql.c         |  2 +-
 src/box/sql/legacy.c          |  2 --
 src/box/sql/main.c            | 83 +++++++------------------------------------
 src/box/sql/malloc.c          |  1 -
 src/box/sql/prepare.c         |  6 ++--
 src/box/sql/sqlInt.h          |  6 ----
 src/box/sql/util.c            | 69 -----------------------------------
 src/box/sql/vdbeInt.h         |  1 -
 src/box/sql/vdbeapi.c         |  7 +---
 src/box/sql/vdbeaux.c         | 31 +++++-----------
 test/sql/func-recreate.result |  3 +-
 11 files changed, 27 insertions(+), 184 deletions(-)

diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 36b75ff..d28045a 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -196,6 +196,6 @@ lbox_sql_create_function(struct lua_State *L)
 					   func_info, lua_sql_call, NULL, NULL,
 					   lua_sql_destroy);
 	if (rc != 0)
-		return luaL_error(L, sqlErrStr(rc));
+		return luaT_error(L);
 	return 0;
 }
diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
index ee680ef..b6e8f6e 100644
--- a/src/box/sql/legacy.c
+++ b/src/box/sql/legacy.c
@@ -70,7 +70,6 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 	if (zSql == 0)
 		zSql = "";
 
-	sqlError(db, SQL_OK);
 	while (rc == SQL_OK && zSql[0]) {
 		int nCol;
 		char **azVals = 0;
@@ -145,7 +144,6 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 					rc = SQL_ABORT;
 					sqlVdbeFinalize((Vdbe *) pStmt);
 					pStmt = 0;
-					sqlError(db, SQL_ABORT);
 					goto exec_out;
 				}
 			}
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 155cbcc..f35c84f 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -284,61 +284,6 @@ functionDestroy(sql * db, FuncDef * p)
 }
 
 /*
- * Return TRUE if database connection db has unfinalized prepared
- * statement.
- */
-static int
-connectionIsBusy(sql * db)
-{
-	if (db->pVdbe)
-		return 1;
-	return 0;
-}
-
-/*
- * Close an existing sql database
- */
-static int
-sqlClose(sql * db, int forceZombie)
-{
-	assert(db);
-	if (!sqlSafetyCheckSickOrOk(db)) {
-		return SQL_MISUSE;
-	}
-	if (db->mTrace & SQL_TRACE_CLOSE) {
-		db->xTrace(SQL_TRACE_CLOSE, db->pTraceArg, db, 0);
-	}
-
-	/* Legacy behavior (sql_close() behavior) is to return
-	 * SQL_BUSY if the connection can not be closed immediately.
-	 */
-	if (!forceZombie && connectionIsBusy(db)) {
-		sqlErrorWithMsg(db, SQL_BUSY,
-				    "unable to close due to unfinalized "
-				    "statements");
-		return SQL_BUSY;
-	}
-
-	/* Convert the connection into a zombie and then close it.
-	 */
-	db->magic = SQL_MAGIC_ZOMBIE;
-
-	return SQL_OK;
-}
-
-/*
- * Two variations on the public interface for closing a database
- * connection. The sql_close() version returns SQL_BUSY and
- * leaves the connection option if there are unfinalized prepared
- * statements.
- */
-int
-sql_close(sql * db)
-{
-	return sqlClose(db, 0);
-}
-
-/*
  * Rollback all database files.  If tripCode is not SQL_OK, then
  * any write cursors are invalidated ("tripped" - as in "tripping a circuit
  * breaker") and made to return tripCode if there are any further
@@ -417,7 +362,9 @@ sqlCreateFunc(sql * db,
 	    (!xSFunc && (!xFinal && xStep)) ||
 	    (nArg < -1 || nArg > SQL_MAX_FUNCTION_ARG) ||
 	    (255 < (sqlStrlen30(zFunctionName)))) {
-		return SQL_MISUSE;
+		diag_set(ClientError, ER_CREATE_FUNCTION, zFunctionName,
+			 "wrong function definition");
+		return SQL_TARANTOOL_ERROR;
 	}
 
 	assert(SQL_FUNC_CONSTANT == SQL_DETERMINISTIC);
@@ -432,10 +379,10 @@ sqlCreateFunc(sql * db,
 	p = sqlFindFunction(db, zFunctionName, nArg, 0);
 	if (p && p->nArg == nArg) {
 		if (db->nVdbeActive) {
-			sqlErrorWithMsg(db, SQL_BUSY,
-					    "unable to delete/modify user-function due to active statements");
-			assert(!db->mallocFailed);
-			return SQL_BUSY;
+			diag_set(ClientError, ER_CREATE_FUNCTION, zFunctionName,
+				 "unable to create function due to active "\
+				 "statements");
+			return SQL_TARANTOOL_ERROR;
 		} else {
 			sqlExpirePreparedStatements(db);
 		}
@@ -443,9 +390,8 @@ sqlCreateFunc(sql * db,
 
 	p = sqlFindFunction(db, zFunctionName, nArg, 1);
 	assert(p || db->mallocFailed);
-	if (!p) {
-		return SQL_NOMEM;
-	}
+	if (p == NULL)
+		return SQL_TARANTOOL_ERROR;
 
 	/* If an older version of the function with a configured destructor is
 	 * being replaced invoke the destructor function here.
@@ -716,22 +662,17 @@ sql_init_db(sql **out_db)
 	 * database schema yet. This is delayed until the first time the database
 	 * is accessed.
 	 */
-	sqlError(db, SQL_OK);
 	sqlRegisterPerConnectionBuiltinFunctions(db);
 
-	if (rc)
-		sqlError(db, rc);
-
 	/* Enable the lookaside-malloc subsystem */
 	setupLookaside(db, 0, sqlGlobalConfig.szLookaside,
 		       sqlGlobalConfig.nLookaside);
 
 opendb_out:
 	assert(db != 0 || rc == SQL_NOMEM);
-	if (rc == SQL_NOMEM) {
-		sql_close(db);
-		db = 0;
-	} else if (rc != SQL_OK)
+	if (rc == SQL_NOMEM)
+		db = NULL;
+	else if (rc != SQL_OK)
 		db->magic = SQL_MAGIC_SICK;
 
 	*out_db = db;
diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c
index dbc6846..a531a8c 100644
--- a/src/box/sql/malloc.c
+++ b/src/box/sql/malloc.c
@@ -926,7 +926,6 @@ static SQL_NOINLINE int
 apiOomError(sql * db)
 {
 	sqlOomClear(db);
-	sqlError(db, SQL_NOMEM);
 	return SQL_NOMEM;
 }
 
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 3f1e1e0..5e0faa1 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -81,9 +81,9 @@ sqlPrepare(sql * db,	/* Database handle. */
 		testcase(nBytes == mxLen);
 		testcase(nBytes == mxLen + 1);
 		if (nBytes > mxLen) {
-			sqlErrorWithMsg(db, SQL_TOOBIG,
-					    "statement too long");
-			rc = sqlApiExit(db, SQL_TOOBIG);
+			diag_set(ClientError, ER_SQL_PARSER_LIMIT,
+				 "SQL command length", nBytes, mxLen);
+			rc = SQL_TARANTOOL_ERROR;
 			goto end_prepare;
 		}
 		zSqlCopy = sqlDbStrNDup(db, zSql, nBytes);
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index e6de71c..c757fb5 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -922,9 +922,6 @@ sql_stmt_busy(sql_stmt *);
 int
 sql_init_db(sql **db);
 
-int
-sql_close(sql *);
-
 
 /**
  * Get number of the named parameter in the prepared sql
@@ -1429,7 +1426,6 @@ struct sql {
 	struct coll *pDfltColl;	/* The default collating sequence (BINARY) */
 	i64 szMmap;		/* Default mmap_size setting */
 	int errMask;		/* & result codes with this before returning */
-	int iSysErrno;		/* Errno value from last system error */
 	u16 dbOptFlags;		/* Flags to enable/disable optimizations */
 	u8 enc;			/* Text encoding */
 	u8 temp_store;		/* 1: file 2: memory 0: default */
@@ -4431,8 +4427,6 @@ sql_atoi64(const char *z, int64_t *val, int length);
 int
 sql_dec_or_hex_to_i64(const char *z, int64_t *val);
 
-void sqlErrorWithMsg(sql *, int, const char *, ...);
-void sqlError(sql *, int);
 void sqlSystemError(sql *, int);
 void *sqlHexToBlob(sql *, const char *z, int n);
 u8 sqlHexToInt(int h);
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index 819f7fa..d5f6447 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -131,75 +131,6 @@ sqlStrlen30(const char *z)
 }
 
 /*
- * Helper function for sqlError() - called rarely.  Broken out into
- * a separate routine to avoid unnecessary register saves on entry to
- * sqlError().
- */
-static SQL_NOINLINE void
-sqlErrorFinish(sql * db, int err_code)
-{
-	sqlSystemError(db, err_code);
-}
-
-/*
- * Set the current error code to err_code and clear any prior error message.
- * Also set iSysErrno (by calling sqlSystem) if the err_code indicates
- * that would be appropriate.
- */
-void
-sqlError(sql * db, int err_code)
-{
-	assert(db != 0);
-	if (err_code)
-		sqlErrorFinish(db, err_code);
-}
-
-/*
- * Load the sql.iSysErrno field if that is an appropriate thing
- * to do based on the sql error code in rc.
- */
-void
-sqlSystemError(sql * db, int rc)
-{
-	if (rc == SQL_IOERR_NOMEM)
-		return;
-	rc &= 0xff;
-	if (rc == SQL_CANTOPEN || rc == SQL_IOERR) {
-		db->iSysErrno = sqlOsGetLastError(db->pVfs);
-	}
-}
-
-/*
- * Set the most recent error code and error string for the sql
- * handle "db". The error code is set to "err_code".
- *
- * If it is not NULL, string zFormat specifies the format of the
- * error string in the style of the printf functions: The following
- * format characters are allowed:
- *
- *      %s      Insert a string
- *      %z      A string that should be freed after use
- *      %d      Insert an integer
- *      %T      Insert a token
- *      %S      Insert the first element of a SrcList
- *
- * zFormat and any string tokens that follow it are assumed to be
- * encoded in UTF-8.
- *
- * To clear the most recent error for sql handle "db", sqlError
- * should be called with err_code set to SQL_OK and zFormat set
- * to NULL.
- */
-void
-sqlErrorWithMsg(sql * db, int err_code, const char *zFormat, ...)
-{
-	assert(db != 0);
-	sqlSystemError(db, err_code);
-	if (zFormat == 0)
-		sqlError(db, err_code);
-}
-
-/*
  * Convert an SQL-style quoted string into a normal string by removing
  * the quote characters.  The conversion is done in-place.  If the
  * input does not begin with a quote character, then this routine
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index de9dbd6..6aadca2 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -519,7 +519,6 @@ int sqlVdbeMemClearAndResize(Mem * pMem, int n);
 int sqlVdbeCloseStatement(Vdbe *, int);
 void sqlVdbeFrameDelete(VdbeFrame *);
 int sqlVdbeFrameRestore(VdbeFrame *);
-int sqlVdbeTransferError(Vdbe * p);
 
 int sqlVdbeSorterInit(struct sql *db, struct VdbeCursor *cursor);
 void sqlVdbeSorterReset(sql *, VdbeSorter *);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 5a859fe..d9dd28f 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -573,7 +573,7 @@ sqlStep(Vdbe * p)
 		 * error has occurred, then return the error code in p->rc to the
 		 * caller. Set the error code in the database handle to the same value.
 		 */
-		rc = sqlVdbeTransferError(p);
+		rc = p->rc;
 	}
 	return (rc & db->errMask);
 }
@@ -877,7 +877,6 @@ columnMem(sql_stmt * pStmt, int i)
 	if (pVm->pResultSet != 0 && i < pVm->nResColumn && i >= 0) {
 		pOut = &pVm->pResultSet[i];
 	} else {
-		sqlError(pVm->db, SQL_RANGE);
 		pOut = (Mem *) columnNullValue();
 	}
 	return pOut;
@@ -1148,20 +1147,17 @@ vdbeUnbind(Vdbe * p, int i)
 		return SQL_MISUSE;
 	}
 	if (p->magic != VDBE_MAGIC_RUN || p->pc >= 0) {
-		sqlError(p->db, SQL_MISUSE);
 		sql_log(SQL_MISUSE,
 			    "bind on a busy prepared statement: [%s]", p->zSql);
 		return SQL_MISUSE;
 	}
 	if (i < 1 || i > p->nVar) {
-		sqlError(p->db, SQL_RANGE);
 		return SQL_RANGE;
 	}
 	i--;
 	pVar = &p->aVar[i];
 	sqlVdbeMemRelease(pVar);
 	pVar->flags = MEM_Null;
-	sqlError(p->db, SQL_OK);
 
 	/* If the bit corresponding to this variable in Vdbe.expmask is set, then
 	 * binding a new value to this variable invalidates the current query plan.
@@ -1247,7 +1243,6 @@ bindText(sql_stmt * pStmt,	/* The statement to bind against */
 			rc = sqlVdbeMemSetStr(pVar, zData, nData, 1, xDel);
 			if (rc == SQL_OK)
 				rc = sql_bind_type(p, i, "TEXT");
-			sqlError(p->db, rc);
 			rc = sqlApiExit(p->db, rc);
 		}
 	} else if (xDel != SQL_STATIC && xDel != SQL_TRANSIENT) {
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 300a44b..38cd195 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2373,23 +2373,6 @@ sqlVdbeResetStepResult(Vdbe * p)
 }
 
 /*
- * Copy the error code and error message belonging to the VDBE passed
- * as the first argument to its database handle (so that they will be
- * returned by calls to sql_errcode() and sql_errmsg()).
- *
- * This function does not clear the VDBE error code or message, just
- * copies them to the database handle.
- */
-int
-sqlVdbeTransferError(Vdbe * p)
-{
-	sql *db = p->db;
-	int rc = p->rc;
-	sqlError(db, rc);
-	return rc;
-}
-
-/*
  * Clean up a VDBE after execution but do not delete the VDBE just yet.
  * Return the result code.
  *
@@ -2418,15 +2401,17 @@ sqlVdbeReset(Vdbe * p)
 	 * instructions yet, leave the main database error information unchanged.
 	 */
 	if (p->pc >= 0) {
-		sqlVdbeTransferError(p);
 		if (p->runOnlyOnce)
 			p->expired = 1;
-	} else if (p->rc && p->expired) {
-		/* The expired flag was set on the VDBE before the first call
-		 * to sql_step(). For consistency (since sql_step() was
-		 * called), set the database error in this case as well.
+	} else {
+		/*
+		 * An error should be thrown here if the expired
+		 * flag is set on the VDBE flag with the first
+		 * call to sql_step(). However, the expired flag
+		 * is currently disabled, so this error has been
+		 * replaced with assert.
 		 */
-		sqlErrorWithMsg(db, p->rc, NULL);
+		assert(p->rc == 0 || p->expired == 0);
 	}
 
 	/* Reclaim all memory used by the VDBE
diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result
index d713292..73fb03c 100644
--- a/test/sql/func-recreate.result
+++ b/test/sql/func-recreate.result
@@ -26,7 +26,8 @@ fiber.sleep(0.1)
 ...
 box.internal.sql_create_function('WAITFOR', 'INT', function (n) require('fiber').sleep(n) return n end)
 ---
-- error: database is locked
+- error: 'Failed to create function ''WAITFOR'': unable to create function due to
+    active statements'
 ...
 ch:get()
 ---
-- 
2.7.4