From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounces@dev.tarantool.org>
Received: from [87.239.111.99] (localhost [127.0.0.1])
	by dev.tarantool.org (Postfix) with ESMTP id 93AA27F627;
	Fri,  6 Aug 2021 22:41:04 +0300 (MSK)
DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 93AA27F627
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev;
	t=1628278864; bh=Lzil/zHSPHXugeg/7ONT/2qNw+Nq9uKijAYiY254Cwc=;
	h=Date:To:Cc:References:In-Reply-To:Subject:List-Id:
	 List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:
	 From:Reply-To:From;
	b=sAJ82KTphSEYpQRXFBG2+tpcPzPpx+sJdDBF82rVuvi9z3b59Be00de7MsoM8ABiJ
	 /Z05hyb996x0Ln2xXc2RmjdWGZH8sItc5AKCdkWS7VDw4b5jle66otwydjkpqmja6a
	 v2/8UV+mDQsy+bbrS5QV9zGcVfLaxEqdm0/aOHmQ=
Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104])
 (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 D7ED67F626
 for <tarantool-patches@dev.tarantool.org>;
 Fri,  6 Aug 2021 22:41:02 +0300 (MSK)
DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D7ED67F626
Received: by smtp44.i.mail.ru with esmtpa (envelope-from
 <imeevma@tarantool.org>)
 id 1mC5iI-0007h2-5J; Fri, 06 Aug 2021 22:41:02 +0300
Date: Fri, 6 Aug 2021 22:41:00 +0300
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Message-ID: <20210806194100.GA11107@tarantool.org>
References: <cover.1628081224.git.imeevma@gmail.com>
 <3f946be6c0aff87c70ac318ae7d5781350fe20f5.1628081224.git.imeevma@gmail.com>
 <6bcdc3da-9391-1b75-8a59-e1f7d5fffedf@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <6bcdc3da-9391-1b75-8a59-e1f7d5fffedf@tarantool.org>
X-4EC0790: 10
X-7564579A: B8F34718100C35BD
X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD94BB7C0677F3ED9D05EB08EAFC9298EA6182A05F538085040300C8ACC3B4E8A8F8ABB6EDE01832476BAF0D2C48EB1251E4EADC763D1100D38
X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7CD7ADFD33CE41673EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637D0C488966F20D4908638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C9C9877127872AEE24AF8A15D157783A117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EEFAD5A440E159F97D7B089FF177BE8049D8FC6C240DEA7642DBF02ECDB25306B2B78CF848AE20165D0A6AB1C7CE11FEE36633242DC0339950C0837EA9F3D19764C4224003CC836476EA7A3FFF5B025636E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637AD0424077D726551EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A
X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B50544FB5DDB7328F89A209A9E470C6C5278
X-C1DE0DAB: 0D63561A33F958A57CBD640B88881EF49D8689AB3EAAC2C8E4953D820BCF3891D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75AF0B556A5A327A45410CA545F18667F91A7EA1CDA0B5A7A0
X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34728AF701C68E453997B70A00EA291369E3CE8F0B7422BA9512E8855AC3A00186E981EE8A33B009061D7E09C32AA3244CFFC3AD1500BEBE44D480A0FAAD458806B4DF56057A86259F729B2BEF169E0186
X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojFhlvmGwdUwR0VlkOfnSuaA==
X-Mailru-Sender: 3A338A78718AEC5A7171F149183FF3A7E2A497B55B2988A4285CB65FB1A98946BAEDF1B2B720B08EA3E7B4BFDCAD2EFE027D9DD7AE851095A2E8D17B49942DB0CBEE3F9BE14373499437F6177E88F7363CDA0F3B3F5B9367
X-Mras: Ok
Subject: Re: [Tarantool-patches] [PATCH v2 1/6] sql: introduce
 sql_func_flags()
X-BeenThere: tarantool-patches@dev.tarantool.org
X-Mailman-Version: 2.1.34
Precedence: list
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>
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
Reply-To: Mergen Imeev <imeevma@tarantool.org>
Errors-To: tarantool-patches-bounces@dev.tarantool.org
Sender: "Tarantool-patches" <tarantool-patches-bounces@dev.tarantool.org>

Hi! Thank you for the review! My answer, diff and new patch below.

On Fri, Aug 06, 2021 at 12:14:45AM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> > index 115c52f96..97b7a2401 100644
> > --- a/src/box/sql/sqlInt.h
> > +++ b/src/box/sql/sqlInt.h
> > @@ -1186,6 +1186,8 @@ struct type_def {
> >   *     SQL_FUNC_LENGTH    ==  OPFLAG_LENGTHARG
> >   *     SQL_FUNC_TYPEOF    ==  OPFLAG_TYPEOFARG
> >   */
> > +/** Function is one of aggregate functions. */
> > +#define SQL_FUNC_AGG		0x0001
> 
> The value is misaligned with the values below. They are using
> whitespaces, so this new line probably also should use them.
> 
Fixed.

> >  #define SQL_FUNC_LIKE     0x0004	/* Candidate for the LIKE optimization */
> >  #define SQL_FUNC_NEEDCOLL 0x0020	/* sqlGetFuncCollSeq() might be called.


Diff:

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 97b7a2401..a92de0a2f 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1187,7 +1187,7 @@ struct type_def {
  *     SQL_FUNC_TYPEOF    ==  OPFLAG_TYPEOFARG
  */
 /** Function is one of aggregate functions. */
-#define SQL_FUNC_AGG		0x0001
+#define SQL_FUNC_AGG      0x0001
 #define SQL_FUNC_LIKE     0x0004	/* Candidate for the LIKE optimization */
 #define SQL_FUNC_NEEDCOLL 0x0020	/* sqlGetFuncCollSeq() might be called.
 					 * The flag is set when the collation



New patch:


commit 9f04ffb91f7347af0fe7ce83f80e6e62d7d4a64f
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Aug 3 11:35:33 2021 +0300

    sql: introduce sql_func_flags()
    
    This function returns a set of parameters for the function with the
    given name. This function is used when we do not need to call a
    function, but we need its parameters.
    
    In addition, this function will allow us to split the parameters
    between those that are the same for all implementations, and the
    parameters, the value of which is implementation-dependent.
    
    Needed for #6105
    Part of #6106

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index d2624516c..80f2d349a 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -328,11 +328,8 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
 		if (op == TK_FUNCTION) {
 			uint32_t arg_count = p->x.pList == NULL ? 0 :
 					     p->x.pList->nExpr;
-			struct func *func =
-				sql_func_by_signature(p->u.zToken, arg_count);
-			if (func == NULL)
-				break;
-			if (sql_func_flag_is_set(func, SQL_FUNC_DERIVEDCOLL) &&
+			uint32_t flags = sql_func_flags(p->u.zToken);
+			if (((flags & SQL_FUNC_DERIVEDCOLL) != 0) &&
 			    arg_count > 0) {
 				/*
 				 * Now we use quite straightforward
@@ -342,7 +339,7 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
 				 * built-in functions: trim, upper,
 				 * lower, replace, substr.
 				 */
-				assert(func->def->returns == FIELD_TYPE_STRING);
+				assert(p->type == FIELD_TYPE_STRING);
 				p = p->x.pList->a->pExpr;
 				continue;
 			}
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 6ca852dec..28d383293 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2655,6 +2655,18 @@ static struct {
 	},
 };
 
+uint32_t
+sql_func_flags(const char *name)
+{
+	struct func *func = func_by_name(name, strlen(name));
+	if (func == NULL || func->def->language != FUNC_LANGUAGE_SQL_BUILTIN)
+		return 0;
+	uint32_t flags = ((struct func_sql_builtin *)func)->flags;
+	if (func->def->aggregate == FUNC_AGGREGATE_GROUP)
+		flags |= SQL_FUNC_AGG;
+	return flags;
+}
+
 static struct func_vtab func_sql_builtin_vtab;
 
 struct func *
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 115c52f96..a92de0a2f 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1186,6 +1186,8 @@ struct type_def {
  *     SQL_FUNC_LENGTH    ==  OPFLAG_LENGTHARG
  *     SQL_FUNC_TYPEOF    ==  OPFLAG_TYPEOFARG
  */
+/** Function is one of aggregate functions. */
+#define SQL_FUNC_AGG      0x0001
 #define SQL_FUNC_LIKE     0x0004	/* Candidate for the LIKE optimization */
 #define SQL_FUNC_NEEDCOLL 0x0020	/* sqlGetFuncCollSeq() might be called.
 					 * The flag is set when the collation
@@ -4372,6 +4374,14 @@ sql_func_flag_is_set(struct func *func, uint16_t flag)
 struct func *
 sql_func_by_signature(const char *name, int argc);
 
+/**
+ * Return the parameters of the function with the given name. If the function
+ * with the given name does not exist, or the function is not a built-in SQL
+ * function, 0 is returned, which means no parameters have been set.
+ */
+uint32_t
+sql_func_flags(const char *name);
+
 /**
  * Generate VDBE code to halt execution with correct error if
  * the object with specified key is already present (or doesn't