[tarantool-patches] Re: [PATCH v1 11/21] sql: remove SQL_SCHEMA errcode

[Mergen Imeev <imeevma@tarantool.org>]

On Sat, May 25, 2019 at 07:18:45PM +0300, n.pettik wrote:
> 
> > diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
> > index 2083d27..61081fb 100644
> > --- a/src/box/sql/prepare.c
> > +++ b/src/box/sql/prepare.c
> > @@ -190,56 +190,15 @@ sqlLockAndPrepare(sql * db,		/* Database handle. */
> > 		      sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
> > 		      const char **pzTail)	/* OUT: End of parsed string */
> > {
> > -	int rc;
> 
> Why not completely replace with sqlPrepare?
> 
Done.

> > -
> > 	*ppStmt = 0;
> > 	assert(zSql != NULL && db != NULL);
> > -	rc = sqlPrepare(db, zSql, nBytes, saveSqlFlag, pOld, ppStmt,
> > -			    pzTail);
> > -	if (rc == SQL_SCHEMA) {
> > -		sql_finalize(*ppStmt);
> > -		rc = sqlPrepare(db, zSql, nBytes, saveSqlFlag, pOld, ppStmt,
> > -				    pzTail);
> > -	}
> > -	assert(rc == 0 || *ppStmt == 0);
> > -	return rc;
> > -}
> > -
> > -/*
> > - * Rerun the compilation of a statement after a schema change.
> > - */
> > -int
> > -sqlReprepare(Vdbe * p)
> > -{
> > -	int rc;
> > -	sql_stmt *pNew;
> > -	const char *zSql;
> > -	sql *db;
> > -
> > -	zSql = sql_sql((sql_stmt *) p);
> > -	assert(zSql != 0);	/* Reprepare only called for prepare_v2() statements */
> > -	db = sqlVdbeDb(p);
> > -	rc = sqlLockAndPrepare(db, zSql, -1, 0, p, &pNew, 0);
> > -	if (rc) {
> > -		if (rc == SQL_NOMEM) {
> > -			sqlOomFault(db);
> > -		}
> > -		assert(pNew == 0);
> > -		return rc;
> > -	} else {
> > -		assert(pNew != 0);
> > -	}
> > -	sqlVdbeSwap((Vdbe *) pNew, p);
> > -	sqlTransferBindings(pNew, (sql_stmt *) p);
> > -	sqlVdbeResetStepResult((Vdbe *) pNew);
> > -	sqlVdbeFinalize((Vdbe *) pNew);
> > -	return 0;
> > +	return sqlPrepare(db, zSql, nBytes, saveSqlFlag, pOld, ppStmt, pzTail);
> > }
> 
> This routine may turn out to be useful when we add
> working prepared statements.
> 
Done.

Diff:

diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 61081fb..33f6783 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -56,7 +56,7 @@ sqlPrepare(sql * db,	/* Database handle. */
 	Parse sParse;		/* Parsing context */
 	sql_parser_create(&sParse, db);
 	sParse.pReprepare = pReprepare;
-	assert(ppStmt && *ppStmt == 0);
+	*ppStmt = NULL;
 	/* assert( !db->mallocFailed ); // not true with SQL_USE_ALLOCA */
 
 	/* Check to verify that it is possible to get a read lock on all
@@ -181,24 +181,35 @@ sqlPrepare(sql * db,	/* Database handle. */
 	return rc;
 }
 
-static int
-sqlLockAndPrepare(sql * db,		/* Database handle. */
-		      const char *zSql,		/* UTF-8 encoded SQL statement. */
-		      int nBytes,		/* Length of zSql in bytes. */
-		      int saveSqlFlag,		/* True to copy SQL text into the sql_stmt */
-		      Vdbe * pOld,		/* VM being reprepared */
-		      sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
-		      const char **pzTail)	/* OUT: End of parsed string */
+/*
+ * Rerun the compilation of a statement after a schema change.
+ */
+int
+sqlReprepare(Vdbe * p)
 {
-	*ppStmt = 0;
-	assert(zSql != NULL && db != NULL);
-	return sqlPrepare(db, zSql, nBytes, saveSqlFlag, pOld, ppStmt, pzTail);
+	sql_stmt *pNew;
+	const char *zSql;
+	sql *db;
+
+	zSql = sql_sql((sql_stmt *) p);
+	assert(zSql != 0);	/* Reprepare only called for prepare_v2() statements */
+	db = sqlVdbeDb(p);
+	if (sqlPrepare(db, zSql, -1, 0, p, &pNew, 0) != 0) {
+		assert(pNew == 0);
+		return -1;
+	}
+	assert(pNew != 0);
+	sqlVdbeSwap((Vdbe *) pNew, p);
+	sqlTransferBindings(pNew, (sql_stmt *) p);
+	sqlVdbeResetStepResult((Vdbe *) pNew);
+	sqlVdbeFinalize((Vdbe *) pNew);
+	return 0;
 }
 
 /*
  * Two versions of the official API.  Legacy and new use.  In the legacy
  * version, the original SQL text is not saved in the prepared statement
- * and so if a schema change occurs, error is returned by
+ * and so if a schema change occurs, SQL_SCHEMA is returned by
  * sql_step().  In the new version, the original SQL text is retained
  * and the statement is automatically recompiled if an schema change
  * occurs.
@@ -211,7 +222,7 @@ sql_prepare(sql * db,		/* Database handle. */
 		const char **pzTail)	/* OUT: End of parsed string */
 {
 	int rc;
-	rc = sqlLockAndPrepare(db, zSql, nBytes, 0, 0, ppStmt, pzTail);
+	rc = sqlPrepare(db, zSql, nBytes, 0, 0, ppStmt, pzTail);
 	assert(rc == 0 || ppStmt == 0 || *ppStmt == 0);	/* VERIFY: F13021 */
 	return rc;
 }
@@ -225,7 +236,7 @@ sql_prepare_v2(sql * db,	/* Database handle. */
     )
 {
 	int rc;
-	rc = sqlLockAndPrepare(db, zSql, nBytes, 1, 0, ppStmt, pzTail);
+	rc = sqlPrepare(db, zSql, nBytes, 1, 0, ppStmt, pzTail);
 	assert(rc == 0 || ppStmt == 0 || *ppStmt == 0);	/* VERIFY: F13021 */
 	return rc;
 }
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 0419933..f0d0fed 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4685,6 +4685,7 @@ void sqlInvalidFunction(sql_context *, int, sql_value **);
 sql_int64 sqlStmtCurrentTime(sql_context *);
 int sqlVdbeParameterIndex(Vdbe *, const char *, int);
 int sqlTransferBindings(sql_stmt *, sql_stmt *);
+int sqlReprepare(Vdbe *);
 
 /**
  * This function verifies that two collations (to be more precise