[tarantool-patches] Re: [PATCH v2 3/4] sql: fix error message for improperly created index

["n.pettik" <korablev@tarantool.org>]

> On 28 Mar 2019, at 18:11, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Thanks for the patch! See my review fix at the bottom
> of the email, and on the branch. In my fixes I've accounted
> Kostja's comment about a proper error code.

I guess Kostja meant to remove this check and use means of
on_replace_dd_index() trigger. In other words, generate creation
of secondary index without HALT and index id == 1 to delegate
verification of index correctness to on replace trigger.

Here’s what I did:
(I put this version on separate branch:
https://github.com/tarantool/tarantool/tree/np/gh-3914-fix-create-index-v3)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 78cd3cee6..ef19ac926 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1924,9 +1924,14 @@ sql_drop_foreign_key(struct Parse *parse_context)
  * following:
  *
  * 1 Seek for space id in _index, goto l1 if seeks fails.
- * 2 Goto l2.
- * 3 l1: Halt.
- * 4 l2: Fetch index id from _index record.
+ * 2 Fetch index id from _index record.
+ * 3 Goto l2
+ * 4 l1: Generate iid == 1..
+ * 6 l2: Continue index creation.
+ *
+ * Note that we generate iid == 1 in case of index search on
+ * purpose: it allows on_replace_dd_index() raise correct
+ * error - "can not add a secondary key before primary".
  *
  * @return Register holding a new index id.
  */
@@ -1941,24 +1946,20 @@ vdbe_emit_new_sec_index_id(struct Parse *parse, uint32_t space_id,
        int seek_adr = sqlVdbeAddOp4Int(v, OP_SeekLE, _index_cursor, 0,
                                        key_reg, 1);
        sqlVdbeAddOp4Int(v, OP_IdxLT, _index_cursor, 0, key_reg, 1);
-       /* Jump over Halt block. */
+       int iid_reg = ++parse->nMem;
+       /* Fetch iid from the row and increment it. */
+       sqlVdbeAddOp3(v, OP_Column, _index_cursor, BOX_INDEX_FIELD_ID, iid_reg);
+       sqlVdbeAddOp2(v, OP_AddImm, iid_reg, 1);
        int goto_succ_addr = sqlVdbeAddOp0(v, OP_Goto);
+       sqlVdbeJumpHere(v, seek_adr);
+       sqlVdbeJumpHere(v, seek_adr + 1);
        /*
         * Absence of any records in _index for that space is
-        * handled here.
+        * handled here: to indicate that secondary index can't
+        * be created before primary.
         */
-       sqlVdbeJumpHere(v, seek_adr);
-       sqlVdbeJumpHere(v, seek_adr + 1);
-       sqlVdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, ON_CONFLICT_ACTION_FAIL,
-                     0, "can not add a secondary key before primary",
-                     P4_STATIC);
-       sqlVdbeChangeP5(v, ER_ALTER_SPACE);
-
+       sqlVdbeAddOp2(v, OP_Integer, 1, iid_reg);
        sqlVdbeJumpHere(v, goto_succ_addr);
-       /* Fetch iid from the row and increment it. */
-       int iid_reg = ++parse->nMem;
-       sqlVdbeAddOp3(v, OP_Column, _index_cursor, BOX_INDEX_FIELD_ID, iid_reg);
-       sqlVdbeAddOp2(v, OP_AddImm, iid_reg, 1);
        return iid_reg;
 }
 
diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua
index 5f6706a40..63c98e206 100755
--- a/test/sql-tap/index1.test.lua
+++ b/test/sql-tap/index1.test.lua
@@ -1031,7 +1031,7 @@ test:do_catchsql_test(
     [[
         CREATE UNIQUE INDEX pk ON t("id");
     ]], {
-        1, "can not add a secondary key before primary"
+        1, "Can't modify space 'T': can not add a secondary key before primary"
     })