From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
 bits)) (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 4D7A94696C3
 for <tarantool-patches@dev.tarantool.org>;
 Sat, 25 Apr 2020 01:55:54 +0300 (MSK)
References: <20200403152752.8923-1-roman.habibov@tarantool.org>
 <20200403152752.8923-2-roman.habibov@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <4241abff-452b-ba0c-841f-5adc8c3d910f@tarantool.org>
Date: Sat, 25 Apr 2020 00:55:52 +0200
MIME-Version: 1.0
In-Reply-To: <20200403152752.8923-2-roman.habibov@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v2 1/2] sql: rename TK_COLUMN to
 TK_COLUMN_NAME
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Roman Khabibov <roman.habibov@tarantool.org>, tarantool-patches@dev.tarantool.org

Hi! Thanks for the patch!

> @@ -5081,7 +5081,7 @@ sqlExprCompare(Expr * pA, Expr * pB, int iTab)
>  		}
>  		return 2;
>  	}
> -	if (pA->op != TK_COLUMN && pA->op != TK_AGG_COLUMN && pA->u.zToken) {
> +	if (pA->op != TK_COLUMN_NAME && pA->op != TK_AGG_COLUMN && pA->u.zToken) {

1. This is out of 80 symbols. Please, wrap the line.

>  		if (pA->op == TK_FUNCTION) {
>  			if (sqlStrICmp(pA->u.zToken, pB->u.zToken) != 0)
>  				return 2;
> @@ -5161,7 +5161,7 @@ sqlExprListCompare(ExprList * pA, ExprList * pB, int iTab)
>   *     pE1: x IS NULL  pE2: x IS NOT NULL    Result: false
>   *     pE1: x IS ?2    pE2: x IS NOT NULL    Reuslt: false
>   *
> - * When comparing TK_COLUMN nodes between pE1 and pE2, if pE2 has
> + * When comparing TK_COLUMN_NAME nodes between pE1 and pE2, if pE2 has
>   * Expr.iTable<0 then assume a table number given by iTab.
>   *
>   * When in doubt, return false.  Returning true might give a performance
> @@ -5209,11 +5209,11 @@ exprSrcCount(Walker * pWalker, Expr * pExpr)
>  {
>  	/* The NEVER() on the second term is because sqlFunctionUsesThisSrc()
>  	 * is always called before sqlExprAnalyzeAggregates() and so the
> -	 * TK_COLUMNs have not yet been converted into TK_AGG_COLUMN.  If
> +	 * TK_COLUMN_NAMEs have not yet been converted into TK_AGG_COLUMN.  If

2. While you are here, please, remove the multiplied whitespace.

>  	 * sqlFunctionUsesThisSrc() is used differently in the future, the
>  	 * NEVER() will need to be removed.
>  	 */
> -	if (pExpr->op == TK_COLUMN || NEVER(pExpr->op == TK_AGG_COLUMN)) {
> +	if (pExpr->op == TK_COLUMN_NAME || NEVER(pExpr->op == TK_AGG_COLUMN)) {
>  		int i;
>  		struct SrcCount *p = pWalker->u.pSrcCount;
>  		SrcList *pSrc = p->pSrc;