[tarantool-patches] Re: [PATCH v1 15/28] sql: remove SQL_SCHEMA errcode
Mergen Imeev
imeevma at tarantool.org
Sat Jun 15 12:55:49 MSK 2019
On Fri, Jun 14, 2019 at 12:24:57AM +0200, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> Consider my review fix below and on the branch
> in a separate commit.
>
> ======================================================
>
Thank you! New patch:
>From 3ee6578d0c3400b991d8cceabe572b52c7c8cd85 Mon Sep 17 00:00:00 2001
Date: Tue, 21 May 2019 17:34:13 +0300
Subject: [PATCH] sql: remove SQL_SCHEMA errcode
Removing this error code is part of getting rid of the SQL error
system.
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 3ed1269..d909c8b 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, current_session()->sql_flags);
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,37 +181,12 @@ 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 */
-{
- int rc;
-
- *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 == NULL);
- 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;
@@ -219,16 +194,11 @@ sqlReprepare(Vdbe * p)
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);
- }
+ if (sqlPrepare(db, zSql, -1, 0, p, &pNew, 0) != 0) {
assert(pNew == 0);
- return rc;
- } else {
- assert(pNew != 0);
+ return -1;
}
+ assert(pNew != 0);
sqlVdbeSwap((Vdbe *) pNew, p);
sqlTransferBindings(pNew, (sql_stmt *) p);
sqlVdbeResetStepResult((Vdbe *) pNew);
@@ -239,7 +209,7 @@ sqlReprepare(Vdbe * p)
/*
* 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, SQL_SCHEMA is returned by
+ * and so if a schema change occurs, an error 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.
@@ -251,8 +221,7 @@ sql_prepare(sql * db, /* Database handle. */
sql_stmt ** ppStmt, /* OUT: A pointer to the prepared statement */
const char **pzTail) /* OUT: End of parsed string */
{
- int rc;
- rc = sqlLockAndPrepare(db, zSql, nBytes, 0, 0, ppStmt, pzTail);
+ int rc = sqlPrepare(db, zSql, nBytes, 0, 0, ppStmt, pzTail);
assert(rc == 0 || ppStmt == NULL || *ppStmt == NULL); /* VERIFY: F13021 */
return rc;
}
@@ -265,8 +234,7 @@ sql_prepare_v2(sql * db, /* Database handle. */
const char **pzTail /* OUT: End of parsed string */
)
{
- int rc;
- rc = sqlLockAndPrepare(db, zSql, nBytes, 1, 0, ppStmt, pzTail);
+ int rc = sqlPrepare(db, zSql, nBytes, 1, 0, ppStmt, pzTail);
assert(rc == 0 || ppStmt == NULL || *ppStmt == NULL); /* VERIFY: F13021 */
return rc;
}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index beae49c..9dfecb7 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -333,8 +333,6 @@ enum sql_ret_code {
SQL_NOMEM,
/** Some kind of disk I/O error occurred. */
SQL_IOERR,
- /** The database schema changed. */
- SQL_SCHEMA,
/** String or BLOB exceeds size limit. */
SQL_TOOBIG,
/** Abort due to constraint violation. */
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 37e0892..bc13eb2 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -5089,10 +5089,7 @@ case OP_AggFinal: {
/* Opcode: Expire P1 * * * *
*
- * Cause precompiled statements to expire. When an expired statement
- * is executed using sql_step() it will either automatically
- * reprepare itself (if it was originally created using sql_prepare_v2())
- * or it will fail with SQL_SCHEMA.
+ * Cause precompiled statements to expire.
*
* If P1 is 0, then all SQL statements become expired. If P1 is non-zero,
* then only the currently executing statement is expired.
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 064eb46..2b53aaf 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -40,14 +40,6 @@
#define SQL_VDBEINT_H
/*
- * The maximum number of times that a statement will try to reparse
- * itself before giving up and returning SQL_SCHEMA.
- */
-#ifndef SQL_MAX_SCHEMA_RETRY
-#define SQL_MAX_SCHEMA_RETRY 50
-#endif
-
-/*
* SQL is translated into a sequence of instructions to be
* executed by a virtual machine. Each instruction is an instance
* of the following structure.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 2f69853..46ca499 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -424,9 +424,8 @@ sqlStep(Vdbe * p)
}
if (p->pc <= 0 && p->expired) {
- p->rc = SQL_SCHEMA;
- rc = SQL_ERROR;
- goto end_of_step;
+ p->rc = SQL_TARANTOOL_ERROR;
+ return SQL_TARANTOOL_ERROR;
}
if (p->pc < 0) {
@@ -455,7 +454,6 @@ sqlStep(Vdbe * p)
if (SQL_NOMEM == sqlApiExit(p->db, p->rc)) {
p->rc = SQL_NOMEM;
}
- end_of_step:
if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
/* If this statement was prepared using sql_prepare_v2(), and an
* error has occurred, then return the error code in p->rc to the
@@ -474,24 +472,9 @@ sqlStep(Vdbe * p)
int
sql_step(sql_stmt * pStmt)
{
- int rc; /* Result from sqlStep() */
Vdbe *v = (Vdbe *) pStmt; /* the prepared statement */
- int cnt = 0; /* Counter to prevent infinite loop of reprepares */
-
assert(v != NULL);
- v->doingRerun = 0;
- while ((rc = sqlStep(v)) == SQL_SCHEMA
- && cnt++ < SQL_MAX_SCHEMA_RETRY) {
- int savedPc = v->pc;
- rc = sqlReprepare(v);
- if (rc != 0)
- break;
- sql_reset(pStmt);
- if (savedPc >= 0)
- v->doingRerun = 1;
- assert(v->expired == 0);
- }
- return rc;
+ return sqlStep(v);
}
/*
More information about the Tarantool-patches
mailing list