[tarantool-patches] [PATCH 4/7] sql: refactor ALTER RENAME code generation

[Nikita Pettik <korablev@tarantool.org>]

We are going to remove legacy cache of struct Table, so lets get rid of
using SQLite tables in alter routine.

Part of #3561
---
 src/box/sql/alter.c         | 114 ++++++++++++++------------------------------
 src/box/sql/parse.y         |   2 +-
 src/box/sql/sqliteInt.h     |  13 ++++-
 src/box/sql/vdbe.c          |   5 +-
 test/sql-tap/alter.test.lua |   4 +-
 5 files changed, 53 insertions(+), 85 deletions(-)

diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 349589be4..320dfdd05 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -34,91 +34,51 @@
  * that implements the ALTER TABLE command.
  */
 #include "sqliteInt.h"
-#include "src/box/session.h"
-#include "tarantoolInt.h"
+#include "box/box.h"
+#include "box/schema.h"
 
-/*
- * Generate code to drop and reload the internal representation of table
- * pTab from the database, including triggers.
- * Argument zName is the name of the table in the database schema at
- * the time the generated code is executed. This can be different from
- * pTab->zName if this function is being called to code part of an
- * "ALTER TABLE RENAME TO" statement.
- */
-static void
-reloadTableSchema(Parse * pParse, Table * pTab, const char *zName)
-{
-	Vdbe *v;
-	v = sqlite3GetVdbe(pParse);
-	if (NEVER(v == 0))
-		return;
-
-	char *zNewName = sqlite3MPrintf(pParse->db, "%s", zName);
-	sqlite3VdbeAddOp4(v, OP_RenameTable, pTab->def->id, 0, 0, zNewName,
-			  P4_DYNAMIC);
-}
-
-/*
- * Generate code to implement the "ALTER TABLE xxx RENAME TO yyy"
- * command.
- */
 void
-sqlite3AlterRenameTable(Parse * pParse,	/* Parser context. */
-			SrcList * pSrc,	/* The table to rename. */
-			Token * pName)	/* The new table name. */
+sql_alter_table_rename(struct Parse *parse, struct SrcList *src_tab,
+		       struct Token *new_name_tk)
 {
-	Table *pTab;		/* Table being renamed */
-	char *zName = 0;	/* NULL-terminated version of pName */
-	sqlite3 *db = pParse->db;	/* Database connection */
-	Vdbe *v;
-	uint32_t savedDbFlags;	/* Saved value of db->flags */
-	struct session *user_session = current_session();
-
-	savedDbFlags = user_session->sql_flags;
-
-	if (NEVER(db->mallocFailed))
-		goto exit_rename_table;
-	assert(pSrc->nSrc == 1);
-
-	pTab = sqlite3LocateTable(pParse, 0, pSrc->a[0].zName);
-	if (pTab == NULL)
-		goto exit_rename_table;
-
-	user_session->sql_flags |= SQLITE_PreferBuiltin;
-
-	/* Get a NULL terminated version of the new table name. */
-	zName = sqlite3NameFromToken(db, pName);
-	if (!zName)
-		goto exit_rename_table;
-
-	/* Check that a table named 'zName' does not already exist
-	 * in database. If so, this is an error.
-	 */
-	if (sqlite3HashFind(&db->pSchema->tblHash, zName) != NULL) {
-		sqlite3ErrorMsg(pParse,
-				"there is already another table or index with this name: %s",
-				zName);
+	assert(src_tab->nSrc == 1);
+	struct sqlite3 *db = parse->db;
+	char *new_name = sqlite3NameFromToken(db, new_name_tk);
+	if (new_name == NULL)
 		goto exit_rename_table;
+	/* Check that new name isn't occupied by another table. */
+	uint32_t space_id = box_space_id_by_name(new_name, strlen(new_name));
+	if (space_id != BOX_ID_NIL) {
+		diag_set(ClientError, ER_SQL_EXECUTE, tt_sprintf("there is "
+			"already another table with this name: %s", new_name));
+		goto tnt_error;
 	}
-	if (pTab->def->opts.is_view) {
-		sqlite3ErrorMsg(pParse, "view %s may not be altered",
-				pTab->def->name);
-		goto exit_rename_table;
+	const char *tbl_name = src_tab->a[0].zName;
+	space_id = box_space_id_by_name(tbl_name, strlen(tbl_name));
+	if (space_id == BOX_ID_NIL) {
+		diag_set(ClientError, ER_NO_SUCH_SPACE, tbl_name);
+		goto tnt_error;
 	}
-	/* Begin a transaction for database. */
-	v = sqlite3GetVdbe(pParse);
-	if (v == 0) {
-		goto exit_rename_table;
+	struct space *space = space_by_id(space_id);
+	assert(space != NULL);
+	if (space->def->opts.is_view) {
+		diag_set(ClientError, ER_SQL_EXECUTE,
+			 "view may not be altered");
+		goto tnt_error;
 	}
-	sql_set_multi_write(pParse, false);
-
+	sql_set_multi_write(parse, false);
 	/* Drop and reload the internal table schema. */
-	reloadTableSchema(pParse, pTab, zName);
-
- exit_rename_table:
-	sqlite3SrcListDelete(db, pSrc);
-	sqlite3DbFree(db, zName);
-	user_session->sql_flags = savedDbFlags;
+	struct Vdbe *v = sqlite3GetVdbe(parse);
+	sqlite3VdbeAddOp4(v, OP_RenameTable, space_id, 0, 0, new_name,
+			  P4_DYNAMIC);
+exit_rename_table:
+	sqlite3SrcListDelete(db, src_tab);
+	return;
+tnt_error:
+	sqlite3DbFree(db, new_name);
+	parse->rc = SQL_TARANTOOL_ERROR;
+	parse->nErr++;
+	goto exit_rename_table;
 }
 
 /*
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 60c0a8eed..92ce5a5bc 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1432,7 +1432,7 @@ cmd ::= ANALYZE nm(X).          {sqlite3Analyze(pParse, &X);}
 
 //////////////////////// ALTER TABLE table ... ////////////////////////////////
 cmd ::= ALTER TABLE fullname(X) RENAME TO nm(Z). {
-  sqlite3AlterRenameTable(pParse,X,&Z);
+  sql_alter_table_rename(pParse,X,&Z);
 }
 
 cmd ::= ALTER TABLE fullname(X) ADD CONSTRAINT nm(Z) FOREIGN KEY
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 96e2fff9f..3ae8db1bc 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4427,7 +4427,18 @@ extern int sqlite3PendingByte;
 #endif
 #endif
 void sqlite3Reindex(Parse *, Token *, Token *);
-void sqlite3AlterRenameTable(Parse *, SrcList *, Token *);
+
+/**
+ * Generate code to implement the "ALTER TABLE xxx RENAME TO yyy"
+ * command.
+ *
+ * @param parse Current parsing context.
+ * @param src_tab The table to rename.
+ * @param new_name_tk Token containing new name of the table.
+ */
+void
+sql_alter_table_rename(struct Parse *parse, struct SrcList *src_tab,
+		       struct Token *new_name_tk);
 
 /**
  * Return the length (in bytes) of the token that begins at z[0].
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index d54d169ee..ad251ca99 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4599,7 +4599,6 @@ case OP_RenameTable: {
 	struct space *space;
 	const char *zOldTableName;
 	const char *zNewTableName;
-	Table *pTab;
 	struct init_data init;
 	char *zSqlStmt;
 
@@ -4610,15 +4609,13 @@ case OP_RenameTable: {
 	struct sql_trigger *triggers = space->sql_triggers;
 	zOldTableName = space_name(space);
 	assert(zOldTableName);
-	pTab = sqlite3HashFind(&db->pSchema->tblHash, zOldTableName);
-	assert(pTab);
 	zNewTableName = pOp->p4.z;
 	zOldTableName = sqlite3DbStrNDup(db, zOldTableName,
 					 sqlite3Strlen30(zOldTableName));
 	rc = sql_rename_table(space_id, zNewTableName, &zSqlStmt);
 	if (rc) goto abort_due_to_error;
 
-	sqlite3UnlinkAndDeleteTable(db, pTab->def->name);
+	sqlite3UnlinkAndDeleteTable(db, space->def->name);
 
 	init.db = db;
 	init.pzErrMsg = &p->zErrMsg;
diff --git a/test/sql-tap/alter.test.lua b/test/sql-tap/alter.test.lua
index db87c7003..ffd0ba6dc 100755
--- a/test/sql-tap/alter.test.lua
+++ b/test/sql-tap/alter.test.lua
@@ -72,7 +72,7 @@ test:do_catchsql_test(
         ALTER TABLE none RENAME TO hi;
     ]], {
         -- <alter-2.1>
-        1, "no such table: NONE"
+        1, "Space 'NONE' does not exist"
         -- </alter-2.1>
     })
 
@@ -83,7 +83,7 @@ test:do_catchsql_test(
         ALTER TABLE t2 RENAME TO t3;
     ]], {
         -- <alter-2.2>
-        1, "there is already another table or index with this name: T3"
+        1, "Failed to execute SQL statement: there is already another table with this name: T3"
         -- </alter-2.2>
     })
 
-- 
2.15.1