Re: [Tarantool-patches] [PATCH v3 1/4] sql: rename TK_COLUMN to TK_COLUMN_NAME

[Roman Khabibov <roman.habibov@tarantool.org>]

Hi! Thanks for the review.

> On Aug 20, 2020, at 01:20, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Thanks for the patch!
> 
>> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
>> index adf90d824..beb83ce95 100644
>> --- a/src/box/sql/sqlInt.h
>> +++ b/src/box/sql/sqlInt.h
>> @@ -1587,20 +1587,20 @@ struct Expr {
>> #if SQL_MAX_EXPR_DEPTH>0
>> 	int nHeight;		/* Height of the tree headed by this node */
>> #endif
>> -	int iTable;		/* TK_COLUMN: cursor number of table holding column
>> +	int iTable;		/* TK_COLUMN_NAME: cursor number of table holding column
>> 				 * TK_REGISTER: register number
>> 				 * TK_TRIGGER: 1 -> new, 0 -> old
>> 				 * EP_Unlikely:  134217728 times likelihood
>> 				 * TK_SELECT: 1st register of result vector
>> 				 */
>> -	ynVar iColumn;		/* TK_COLUMN: column index.
>> +	ynVar iColumn;		/* TK_COLUMN_NAME: column index.
> 
> Does not this look wrong to you? - 'COLUMN_NAME: column index'. In
> some other places TK_COLUMN_NAME also designates presense of a column
> index, not name. Probably a better name would be TK_COLUMN_REF.
> 
Ok. Done.

>> 				 * TK_VARIABLE: variable number (always >= 1).
>> 				 * TK_SELECT_COLUMN: column of the result vector
>> 				 */
>> 	i16 iAgg;		/* Which entry in pAggInfo->aCol[] or ->aFunc[] */
>> 	i16 iRightJoinTable;	/* If EP_FromJoin, the right table of the join */
>> 	u8 op2;			/* TK_REGISTER: original value of Expr.op
>> -				 * TK_COLUMN: the value of p5 for OP_Column
>> +				 * TK_COLUMN_NAME: the value of p5 for OP_Column
>> 				 * TK_AGG_FUNCTION: nesting depth
>> 				 */
>> 	AggInfo *pAggInfo;	/* Used by TK_AGG_COLUMN and TK_AGG_FUNCTION */