[patches] [PATCH] sql: remove unused fields from SQL index

[v.shpilevoy at tarantool.org]

> 7 марта 2018 г., в 19:37, n.pettik <korablev at tarantool.org> написал(а):
> 
> 
>> On 7 Mar 2018, at 18:27, v.shpilevoy at tarantool.org wrote:
>> 
>> 
>> 
>>> 7 марта 2018 г., в 17:33, n.pettik <korablev at tarantool.org> написал(а):
>>> 
>>> 
>>>> On 7 Mar 2018, at 13:19, v.shpilevoy at tarantool.org wrote:
>>>> 
>>>> See below 6 comments.
>>>> 
>>>>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>>>>> index 9f45c6224..f2011163c 100644
>>>>> --- a/src/box/sql/build.c
>>>>> +++ b/src/box/sql/build.c
>>>>> @@ -3375,14 +3367,53 @@ sqlite3DefaultRowEst(Index * pIdx)
>>>>> 	 * 6 and each subsequent value (if any) is 5.
>>>>> 	 */
>>>>> 	memcpy(&a[1], aVal, nCopy * sizeof(LogEst));
>>>>> -	for (i = nCopy + 1; i <= pIdx->nKeyCol; i++) {
>>>>> +	for (i = nCopy + 1; i <= pIdx->nColumn; i++) {
>>>> 
>>>> 1. Why do you use here pIdx->nColumn, but in other places index_column_count()?
>>> 
>>> As far as I could remember, my initial intend lies in the fact that this function is called
>>> only inside build routine (i.e. before space is actually created), so there is no
>>> reason at attempting to fetch column number from Tarantool.
>> 
>> If it must be called only inside build, then why it is called in sqlite3DeleteFrom, sqlite3GenerateRowIndexDelete, sqlite3Pragma, xferCompatibleIndex and other non-build functions?
> 
> You misunderstood me. I mean sqlite3DefaultRowEst() is called before space construction, not index_column_count().
> And before space construction there is no reason to call index_column_count(), since
> it won’t find space in space cache and will return column count from SQL index.
> (But now I grep it again and it turns out that sqlite3DefaultRowEst() is also used in analyze routine, so
> basically it may be called after space creation. Anyway, I have already fixed it.)

Ok, now I understand.

> 
>>> 
>>>> 
>>>>> diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
>>>>> index 088d84e27..c7917c595 100644
>>>>> --- a/src/box/sql/fkey.c
>>>>> +++ b/src/box/sql/fkey.c
>>>>> @@ -668,7 +669,7 @@ fkScanChildren(Parse * pParse,	/* Parse context */
>>>>> 		Expr *pEq, *pAll = 0;
>>>>> 		Index *pPk = sqlite3PrimaryKeyIndex(pTab);
>>>>> 		assert(pIdx != 0);
>>>>> -		for (i = 0; i < pPk->nKeyCol; i++) {
>>>>> +		for (i = 0; i < (int)index_column_count(pPk); i++) {
>>>> 
>>>> 5. If you write a function in for condition, is it called on each iteration with -O3 compiler flag? I do not know and on just in case I would save a result of index_column_count() in a variable and use it in the for condition, like it is done below.
>>> 
>>> I guess, compiler is smart enough for such optimisation. However, fixed anyway to avoid any suspicions:
>> 
>> To help compiler make index_column_count parameter be const struct Index * instead of struct Index *, please.
> 
> Done.
> 
> Should I send second version of patch, or these changes are considered to be minor?

Now, thanks. Now the patch seems to be ok.

>