[tarantool-patches] Re: [PATCH v1 18/28] sql: remove SQL_CONSTRAINT errcode
Mergen Imeev
imeevma at tarantool.org
Sat Jun 15 13:00:11 MSK 2019
On Fri, Jun 14, 2019 at 12:24:55AM +0200, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> I've found 'sql_CONSTRAINT_UNIQUE' in sql-tap/unique.test.lua
> on line 229. These tests are disabled by 'if 0 > 0', but another
> comment says, that the issue #2495 fix should have enabled them
> back. We need to either resurrect these tests, or drop them.
>
> Consider my review fixes below and on the branch
> in a separate commit.
>
Thank you! New patch:
m f7b4f7871f6d63e75b4951a2141b218a2d86c336 Mon Sep 17 00:00:00 2001
Date: Wed, 22 May 2019 17:42:11 +0300
Subject: [PATCH] sql: remove SQL_CONSTRAINT errcode
Removing this error code is part of getting rid of the SQL error
system.
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 08e7f62..6cb41f8 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -44,7 +44,7 @@
*
* Foreign keys in sql come in two flavours: deferred and immediate.
* If an immediate foreign key constraint is violated,
- * SQL_CONSTRAINT_FOREIGNKEY is returned and the current
+ * -1 is returned and the current
* statement transaction rolled back. If a
* deferred foreign key constraint is violated, no action is taken
* immediately. However if the application attempts to commit the
@@ -110,7 +110,7 @@
* is that the counter used is stored as part of each individual statement
* object (struct Vdbe). If, after the statement has run, its immediate
* constraint counter is greater than zero,
- * it returns SQL_CONSTRAINT_FOREIGNKEY
+ * it returns -1.
* and the statement transaction is rolled back. An exception is an INSERT
* statement that inserts a single row only (no triggers). In this case,
* instead of using a counter, an exception is thrown immediately if the
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 4af65b0..f9f19bf 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -332,7 +332,6 @@ enum sql_ret_code {
/** Some kind of disk I/O error occurred. */
SQL_IOERR,
/** Abort due to constraint violation. */
- SQL_CONSTRAINT,
SQL_TARANTOOL_ERROR,
/** sql_step() has another row ready. */
SQL_ROW,
@@ -554,9 +553,6 @@ sql_exec(sql *, /* An open database */
#define SQL_IOERR_CLOSE (SQL_IOERR | (16<<8))
#define SQL_IOERR_DELETE_NOENT (SQL_IOERR | (23<<8))
#define SQL_IOERR_GETTEMPPATH (SQL_IOERR | (25<<8))
-#define SQL_CONSTRAINT_FOREIGNKEY (SQL_CONSTRAINT | (3<<8))
-#define SQL_CONSTRAINT_NOTNULL (SQL_CONSTRAINT | (5<<8))
-#define SQL_CONSTRAINT_TRIGGER (SQL_CONSTRAINT | (7<<8))
/**
* Subtype of a main type. Allows to do some subtype specific
@@ -3520,17 +3516,16 @@ sql_generate_index_key(struct Parse *parse, struct index *index, int cursor,
* ---------- ---------- --------------------------------------
* any ROLLBACK The current transaction is rolled
* back and VDBE stops immediately
- * with return code of sql_CONSTRAINT.
+ * with an error.
*
* any ABORT Back out changes from the current
* command only (do not do a complete
* rollback) then cause VDBE to return
- * immediately with sql_CONSTRAINT.
+ * immediately with an error.
*
- * any FAIL VDBE returns immediately with a
- * return code of sql_CONSTRAINT. The
- * transaction is not rolled back and any
- * changes to prior rows are retained.
+ * any FAIL VDBE returns immediately with an error.
+ * The transaction is not rolled back and
+ * any changes to prior rows are retained.
*
* any IGNORE The attempt in insert or update the
* current row is skipped, without
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 918e73c..878724b 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1065,7 +1065,6 @@ case OP_Halt: {
}
rc = sqlVdbeHalt(p);
assert(rc == 0 || rc == SQL_ERROR);
- assert(rc == 0 || (p->rc & 0xff) == SQL_CONSTRAINT);
rc = p->rc ? SQL_TARANTOOL_ERROR : SQL_DONE;
goto vdbe_return;
}
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 2a9e39b..53e9463 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2013,8 +2013,7 @@ sqlVdbeCloseStatement(Vdbe * p, int eOp)
* violations, return SQL_ERROR. Otherwise, 0.
*
* If there are outstanding FK violations and this function returns
- * SQL_ERROR, set the result of the VM to SQL_CONSTRAINT_FOREIGNKEY
- * and write an error message to it. Then return SQL_ERROR.
+ * SQL_TARANTOOL_ERROR and set an error.
*/
int
sqlVdbeCheckFk(Vdbe * p, int deferred)
@@ -2177,7 +2176,6 @@ sqlVdbeHalt(Vdbe * p)
closeCursorsAndFree(p);
return SQL_ERROR;
}
- rc = SQL_CONSTRAINT_FOREIGNKEY;
} else {
/* The auto-commit flag is true, the vdbe program was successful
* or hit an 'OR FAIL' constraint and there are no deferred foreign
@@ -2220,17 +2218,15 @@ sqlVdbeHalt(Vdbe * p)
/* If eStatementOp is non-zero, then a statement transaction needs to
* be committed or rolled back. Call sqlVdbeCloseStatement() to
* do so. If this operation returns an error, and the current statement
- * error code is 0 or SQL_CONSTRAINT, then promote the
+ * error code is 0 or -1, then promote the
* current statement error code.
*/
if (eStatementOp) {
rc = sqlVdbeCloseStatement(p, eStatementOp);
if (rc) {
box_txn_rollback();
- if (p->rc == 0
- || (p->rc & 0xff) == SQL_CONSTRAINT) {
+ if (p->rc == 0)
p->rc = rc;
- }
closeCursorsAndFree(p);
sqlRollbackAll(p);
sqlCloseSavepoints(p);
diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
index 38d7c55..bc02d62 100755
--- a/test/sql-tap/trigger1.test.lua
+++ b/test/sql-tap/trigger1.test.lua
@@ -511,7 +511,7 @@ test:do_catchsql_test(
-- </trigger1-6.3>
})
--- verify_ex_errcode trigger1-6.3b sql_CONSTRAINT_TRIGGER
+-- Verify the previous test has not deleted anything.
test:do_execsql_test(
"trigger1-6.4",
[[
diff --git a/test/sql-tap/unique.test.lua b/test/sql-tap/unique.test.lua
index fbd73a6..358a06f 100755
--- a/test/sql-tap/unique.test.lua
+++ b/test/sql-tap/unique.test.lua
@@ -74,7 +74,7 @@ test:do_catchsql_test(
-- </unique-1.3>
})
--- verify_ex_errcode unique-1.3b sql_CONSTRAINT_PRIMARYKEY
+-- Verify the previous test has not inserted anything.
test:do_execsql_test(
"unique-1.4",
[[
@@ -95,7 +95,7 @@ test:do_catchsql_test(
-- </unique-1.5>
})
--- verify_ex_errcode unique-1.5b sql_CONSTRAINT_UNIQUE
+-- Verify the previous test has not inserted anything.
test:do_execsql_test(
"unique-1.6",
[[
@@ -171,7 +171,7 @@ test:do_catchsql_test(
-- </unique-2.3>
})
--- verify_ex_errcode unique-2.3b sql_CONSTRAINT_UNIQUE
+-- Verify the previous test has not inserted anything.
test:do_catchsql_test(
"unique-2.4",
[[
@@ -291,7 +291,6 @@ test:do_catchsql_test(
-- </unique-3.4>
})
--- verify_ex_errcode unique-3.4b sql_CONSTRAINT_UNIQUE
--integrity_check unique-3.5
-- Make sure NULLs are distinct as far as the UNIQUE tests are
-- concerned.
@@ -405,7 +404,6 @@ test:do_catchsql_test(
})
end
--- verify_ex_errcode unique-4.10b sql_CONSTRAINT_UNIQUE
--integrity_check unique-4.99
-- Test the error message generation logic. In particular, make sure we
-- do not overflow the static buffer used to generate the error message.
@@ -448,7 +446,4 @@ test:do_catchsql_test(
-- </unique-5.2>
})
--- verify_ex_errcode unique-5.2b sql_CONSTRAINT_UNIQUE
-
-
test:finish_test()
More information about the Tarantool-patches
mailing list