[tarantool-patches] Re: [PATCH v1 18/28] sql: remove SQL_CONSTRAINT errcode

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

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.

======================================================

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index acfeea588..128f5747d 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3719,17 +3719,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/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
index 38d7c5525..bc02d6236 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 fbd73a650..358a06f40 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()