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

[Vladislav Shpilevoy]

Hello. Thanks for the patch! See 2 comments below.

On 01/06/2018 18:16, Kirill Yukhin wrote:
> 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.

1. What are calculated columns?

> 
> Part of #3235
> ---
>   src/box/sql/delete.c | 54 ++++++++++++++++++++++++++++++++++++----------------
>   1 file changed, 38 insertions(+), 16 deletions(-)
> 
> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index ddad54b..28713c8 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -183,6 +183,16 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
>   		struct NameContext nc;
>   		memset(&nc, 0, sizeof(nc));
>   		nc.pParse = parse;
> +		if (tab_list->a[0].pTab == NULL) {
> +			struct Table *t = malloc(sizeof(struct Table));
> +			if (t == NULL) {
> +				sqlite3OomFault(parse->db);
> +				goto delete_from_cleanup;
> +			}
> +			assert(space != NULL);
> +			t->def = space->def;
> +			tab_list->a[0].pTab = t;

2. Why can not you declare struct Table on the stack at the top of this
function, and here use its address? Can this pTab be used out of
sql_table_delete_from()? And I do not see where do you delete this
table.

> +		}
>   		nc.pSrcList = tab_list;
>   		if (sqlite3ResolveExprNames(&nc, where))
>   			goto delete_from_cleanup;