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

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

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.

On 28/03/2019 15:07, Nikita Pettik wrote:
> Table can be created without any indexes (for instance, from Lua-land).
> On the other hand, bytecode generated for CREATE INDEX statement
> attempts at finding entry in _index space with given space id.
> In case it is not found error "wrong space id" is raised. On the other
> hand, it is obvious that such message is appeared when table doesn't
> have any created indexes yet. Hence, lets fix this message to indicate
> that primary key should be created before any secondary indexes.
> 
> Closes #3914
> ---
>  src/box/sql/build.c          |  3 +--
>  test/sql-tap/index1.test.lua | 19 ++++++++++++++++++-
>  2 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index f55f6d800..20cc346a0 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1946,8 +1946,7 @@ generate_index_id(struct Parse *parse, uint32_t space_id, int cursor)
>  	sqlVdbeJumpHere(v, seek_adr);
>  	sqlVdbeJumpHere(v, seek_adr + 1);
>  	sqlVdbeAddOp4(v, OP_Halt, SQL_ERROR, ON_CONFLICT_ACTION_FAIL, 0,
> -		      sqlMPrintf(parse->db, "Invalid space id: %d", space_id),
> -		      P4_DYNAMIC);
> +		      "can not add a secondary key before primary", P4_STATIC);
>  
>  	sqlVdbeJumpHere(v, goto_succ_addr);
>  	/* Fetch iid from the row and increment it. */

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

commit 50adf5f01374a4cd6741c862e23ba902ab651909
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Thu Mar 28 16:03:26 2019 +0300

    Review fix

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 076f62dde..78cd3cee6 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1943,11 +1943,16 @@ vdbe_emit_new_sec_index_id(struct Parse *parse, uint32_t space_id,
 	sqlVdbeAddOp4Int(v, OP_IdxLT, _index_cursor, 0, key_reg, 1);
 	/* Jump over Halt block. */
 	int goto_succ_addr = sqlVdbeAddOp0(v, OP_Goto);
-	/* Invalid space id handling block starts here. */
+	/*
+	 * Absence of any records in _index for that space is
+	 * handled here.
+	 */
 	sqlVdbeJumpHere(v, seek_adr);
 	sqlVdbeJumpHere(v, seek_adr + 1);
-	sqlVdbeAddOp4(v, OP_Halt, SQL_ERROR, ON_CONFLICT_ACTION_FAIL, 0,
-		      "can not add a secondary key before primary", P4_STATIC);
+	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);
 
 	sqlVdbeJumpHere(v, goto_succ_addr);
 	/* Fetch iid from the row and increment it. */