[tarantool-patches] Re: [PATCH 1/3] sql: fetch primary index for affinity only

[Vladislav Shpilevoy]

Hello. Thanks for the fixes! See one minor comment below.

And I do not see the new commit on the branch. Branch still has the
old one + 2 my review fixes commits.


> --
> Regards, Kirill Yukhin
> 
> commit 6ef9616b71c6e253336c012442b90c4d2ebb2c55
> Author: Kirill Yukhin <kyukhin at tarantool.org>
> Date:   Wed May 23 15:31:37 2018 +0300
> 
>      sql: fetch primary index for affinity only
>      
>      This small patch removes usages of primary index throughout
>      code sql_table_delete_from, limiting use to fetching of
>      affinities only. We cannot use space_def here, since primary
>      index might contain calculated columns.
>      
>      Part of #3235
> 
> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index ddad54b..a4a8da6 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -325,8 +338,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
>   					  (void *)space, P4_SPACEPTR);
>   			sqlite3VdbeAddOp3(v, OP_OpenWrite, tab_cursor,
>   					  table->tnum, space_ptr_reg);
> -			sql_vdbe_set_p4_key_def(parse, pk);
> -			VdbeComment((v, "%s", pk->zName));
> +			struct key_def *def = key_def_dup(pk_def);
> +			if (def == NULL) {
> +				sqlite3OomFault(parse->db);
> +				goto delete_from_cleanup;
> +			}
> +			sqlite3VdbeAppendP4(v, def, P4_KEYDEF);

Why could not you at first dup key_def, and then just use AddOp4? Instead of
AddOp3 + dup + AppendP4.

> +
> +			VdbeComment((v, "%s", space->index[0]->def->name));
>   
>   			if (one_pass == ONEPASS_MULTI)
>   				sqlite3VdbeJumpHere(v, iAddrOnce);