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

[v.shpilevoy at tarantool.org]

And your branch fails on Travis. Please, fix a build error.

> 7 марта 2018 г., в 13:19, v.shpilevoy at tarantool.org написал(а):
> 
> 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()?
> 
>> 		a[i] = 23;
>> 		assert(23 == sqlite3LogEst(5));
>> 	}
>> 
>> 	assert(0 == sqlite3LogEst(1));
>> 	if (IsUniqueIndex(pIdx))
>> -		a[pIdx->nKeyCol] = 0;
>> +		a[pIdx->nColumn] = 0;
>> +}
>> +
>> +/**
>> + * Return number of columns in given index.
>> + * If space is ephemeral or represents view, use internal
>> + * SQL structure to fetch the value.
>> + */
>> +inline uint32_t
>> +index_column_count(Index *idx)
>> +{
>> +	assert(idx != NULL);
>> +	uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(idx->tnum);
>> +	struct space *space = space_by_id(space_id);
>> +	/* It is impossible to find an ephemeral space by id. */
>> +	if (space == NULL)
>> +		return idx->nColumn;
>> +
>> +	uint32_t index_id = SQLITE_PAGENO_TO_INDEXID(idx->tnum);
>> +	struct index *index = space_index(space, index_id);
>> +	/* Views don't feature any functional parts. */
>> +	if (index == NULL || index->def->key_def->part_count == 0)
>> +		return idx->nColumn;
> 
> 2. View space can not be found using space_by_id, so this code is unreachable, it is not?
> 
>> +	return index->def->key_def->part_count;
>> +}
>> +
>> +/* Return true if given index is unique and not nullable. */
> 
> 3. For function description at the beginning use /** instead of /*, please.
> 
>> +inline 	bool
> 
> 4. At first, this function is declared in a header as non-static and non-inline, so it is not inlined. At second, please, write one whitespace between function return type and attributes like inline, static.
> 
>> 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.
>> 
>> diff --git a/src/box/sql/where.c b/src/box/sql/where.c
>> index 2f1c627e5..8d1e3a393 100644
>> --- a/src/box/sql/where.c
>> +++ b/src/box/sql/where.c
>> @@ -557,7 +557,7 @@ isDistinctRedundant(Parse * pParse,		/* Parsing context */
>> 	for (pIdx = pTab->pIndex; pIdx; pIdx = pIdx->pNext) {
>> 		if (!IsUniqueIndex(pIdx))
>> 			continue;
>> -		for (i = 0; i < pIdx->nKeyCol; i++) {
>> +		for (i = 0; i < (int)index_column_count(pIdx); i++) {
> 
> 6. Same as 5.
>