[tarantool-patches] Re: [PATCH v1 18/28] sql: remove SQL_CONSTRAINT errcode
Mergen Imeev
imeevma at tarantool.org
Wed Jun 19 11:02:49 MSK 2019
Thank you for review! My answer, diff and new patch below.
On Tue, Jun 18, 2019 at 10:40:04PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> On 15/06/2019 12:00, Mergen Imeev wrote:
> > 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
>
> It is still here. Please, fix.
>
Sorry, missed this. Fixed.
Diff:
diff --git a/test/sql-tap/unique.test.lua b/test/sql-tap/unique.test.lua
index 358a06f..9818f90 100755
--- a/test/sql-tap/unique.test.lua
+++ b/test/sql-tap/unique.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(32)
+test:plan(35)
--!./tcltestrunner.lua
-- 2001 September 27
@@ -20,12 +20,7 @@ test:plan(32)
-- $Id: unique.test,v 1.9 2009/05/02 15:46:47 drh Exp $
-- ["set","testdir",[["file","dirname",["argv0"]]]]
-- ["source",[["testdir"],"\/tester.tcl"]]
--- MUST_WORK_TEST
-if (0 > 0)
- then
- -- Try to create a table with two primary keys.
- -- (This is allowed in sql even that it is not valid SQL)
-end
+
test:do_catchsql_test(
"unique-1.1",
[[
@@ -213,20 +208,16 @@ test:do_catchsql_test(
-- </unique-2.7>
})
--- MUST_WORK_TEST i2 is checking not only "a" column #2495
-if 0 > 0 then
test:do_catchsql_test(
"unique-2.8",
[[
- select a from t2;
- --CREATE UNIQUE INDEX i2 ON t2(a);
+ CREATE UNIQUE INDEX i2 ON t2(a);
]], {
-- <unique-2.8>
- 1, "UNIQUE constraint failed: t2.a"
+ 1, "Duplicate key exists in unique index 'I2' in space 'T2'"
-- </unique-2.8>
})
--- verify_ex_errcode unique-2.8b sql_CONSTRAINT_UNIQUE
test:do_catchsql_test(
"unique-2.9",
[[
@@ -236,7 +227,6 @@ test:do_catchsql_test(
0
-- </unique-2.9>
})
-end
--integrity_check unique-2.10
-- Test the UNIQUE keyword as used on two or more fields.
@@ -391,18 +381,15 @@ test:do_catchsql_test(
-- </unique-4.9>
})
--- MUST_WORK_TEST i4c is checking not only "b" column #2495
-if 0 > 0 then
test:do_catchsql_test(
"unique-4.10",
[[
CREATE UNIQUE INDEX i4c ON t4(b)
]], {
-- <unique-4.10>
- 1, "UNIQUE constraint failed: t4.b"
+ 1, "Duplicate key exists in unique index 'I4C' in space 'T4'"
-- </unique-4.10>
})
-end
--integrity_check unique-4.99
-- Test the error message generation logic. In particular, make sure we
diff --git a/test/sql-tap/index4.test.lua b/test/sql-tap/index4.test.lua
index d8c6bab..37c0569 100755
--- a/test/sql-tap/index4.test.lua
+++ b/test/sql-tap/index4.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(6)
+test:plan(7)
--!./tcltestrunner.lua
-- 2011 July 9
@@ -150,20 +150,15 @@ test:do_execsql_test(
COMMIT;
]])
--- MUST_WORK_TEST possible reason of failing #2495
-if (0 > 0)
- then
- test:do_catchsql_test(
- 2.2,
- [[
- CREATE UNIQUE INDEX i3 ON t2(x);
- ]], {
- -- <2.2>
- 1, "UNIQUE constraint failed: t2.x"
- -- </2.2>
- })
-
-end
+test:do_catchsql_test(
+ 2.2,
+ [[
+ CREATE UNIQUE INDEX i3 ON t2(x);
+ ]], {
+ -- <2.2>
+ 1, "Duplicate key exists in unique index 'I3' in space 'T2'"
+ -- </2.2>
+ })
test:finish_test()
New patch:
>From 8394c34eab7f945ce3d18bb80027a2f5c195328f 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/index4.test.lua b/test/sql-tap/index4.test.lua
index d8c6bab..37c0569 100755
--- a/test/sql-tap/index4.test.lua
+++ b/test/sql-tap/index4.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(6)
+test:plan(7)
--!./tcltestrunner.lua
-- 2011 July 9
@@ -150,20 +150,15 @@ test:do_execsql_test(
COMMIT;
]])
--- MUST_WORK_TEST possible reason of failing #2495
-if (0 > 0)
- then
- test:do_catchsql_test(
- 2.2,
- [[
- CREATE UNIQUE INDEX i3 ON t2(x);
- ]], {
- -- <2.2>
- 1, "UNIQUE constraint failed: t2.x"
- -- </2.2>
- })
-
-end
+test:do_catchsql_test(
+ 2.2,
+ [[
+ CREATE UNIQUE INDEX i3 ON t2(x);
+ ]], {
+ -- <2.2>
+ 1, "Duplicate key exists in unique index 'I3' in space 'T2'"
+ -- </2.2>
+ })
test:finish_test()
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..9818f90 100755
--- a/test/sql-tap/unique.test.lua
+++ b/test/sql-tap/unique.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(32)
+test:plan(35)
--!./tcltestrunner.lua
-- 2001 September 27
@@ -20,12 +20,7 @@ test:plan(32)
-- $Id: unique.test,v 1.9 2009/05/02 15:46:47 drh Exp $
-- ["set","testdir",[["file","dirname",["argv0"]]]]
-- ["source",[["testdir"],"\/tester.tcl"]]
--- MUST_WORK_TEST
-if (0 > 0)
- then
- -- Try to create a table with two primary keys.
- -- (This is allowed in sql even that it is not valid SQL)
-end
+
test:do_catchsql_test(
"unique-1.1",
[[
@@ -74,7 +69,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 +90,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 +166,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",
[[
@@ -213,20 +208,16 @@ test:do_catchsql_test(
-- </unique-2.7>
})
--- MUST_WORK_TEST i2 is checking not only "a" column #2495
-if 0 > 0 then
test:do_catchsql_test(
"unique-2.8",
[[
- select a from t2;
- --CREATE UNIQUE INDEX i2 ON t2(a);
+ CREATE UNIQUE INDEX i2 ON t2(a);
]], {
-- <unique-2.8>
- 1, "UNIQUE constraint failed: t2.a"
+ 1, "Duplicate key exists in unique index 'I2' in space 'T2'"
-- </unique-2.8>
})
--- verify_ex_errcode unique-2.8b sql_CONSTRAINT_UNIQUE
test:do_catchsql_test(
"unique-2.9",
[[
@@ -236,7 +227,6 @@ test:do_catchsql_test(
0
-- </unique-2.9>
})
-end
--integrity_check unique-2.10
-- Test the UNIQUE keyword as used on two or more fields.
@@ -291,7 +281,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.
@@ -392,20 +381,16 @@ test:do_catchsql_test(
-- </unique-4.9>
})
--- MUST_WORK_TEST i4c is checking not only "b" column #2495
-if 0 > 0 then
test:do_catchsql_test(
"unique-4.10",
[[
CREATE UNIQUE INDEX i4c ON t4(b)
]], {
-- <unique-4.10>
- 1, "UNIQUE constraint failed: t4.b"
+ 1, "Duplicate key exists in unique index 'I4C' in space 'T4'"
-- </unique-4.10>
})
-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 +433,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