From: Vladimir Davydov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: imeevma@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 2/5] sql: introduce SQL built-in functions to parser
Date: Thu, 19 Aug 2021 11:35:14 +0300 [thread overview]
Message-ID: <20210819083514.i4hlig526gy6domj@esperanza> (raw)
In-Reply-To: <6f4f310f2d43267c4264844cd966641ce70e804b.1629297142.git.imeevma@gmail.com>
On Wed, Aug 18, 2021 at 05:34:59PM +0300, imeevma@tarantool.org wrote:
> diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
> index 0d998506c..369d9e1dd 100644
> --- a/extra/mkkeywordhash.c
> +++ b/extra/mkkeywordhash.c
> @@ -184,7 +184,6 @@ static Keyword aKeywordTable[] = {
> { "BLOB", "TK_STANDARD", true },
> { "BINARY", "TK_ID", true },
> { "CALL", "TK_STANDARD", true },
> - { "CHAR", "TK_CHAR", true },
> { "CHARACTER", "TK_ID", true },
> { "CONDITION", "TK_STANDARD", true },
> { "CONNECT", "TK_STANDARD", true },
> @@ -251,6 +250,42 @@ static Keyword aKeywordTable[] = {
> { "LEADING", "TK_LEADING", true },
> { "TRAILING", "TK_TRAILING", true },
> { "BOTH", "TK_BOTH", true },
> + { "ABS", "TK_ABS", true },
> + { "AVG", "TK_AVG", true },
> + { "CHAR", "TK_CHAR", true },
> + { "CHAR_LENGTH", "TK_CHAR_LEN", true },
> + { "CHARACTER_LENGTH", "TK_CHAR_LEN", true },
> + { "COALESCE", "TK_COALESCE", true },
> + { "COUNT", "TK_COUNT", true },
> + { "GREATEST", "TK_GREATEST", true },
> + { "GROUP_CONCAT", "TK_GROUP_CONCAT",true },
> + { "HEX", "TK_HEX", true },
> + { "IFNULL", "TK_IFNULL", true },
> + { "LEAST", "TK_LEAST", true },
> + { "LENGTH", "TK_LENGTH", true },
> + { "LIKELIHOOD", "TK_LIKELIHOOD", true },
> + { "LIKELY", "TK_LIKELY", true },
> + { "LOWER", "TK_LOWER", true },
> + { "MAX", "TK_MAX", true },
> + { "MIN", "TK_MIN", true },
> + { "NULLIF", "TK_NULLIF", true },
> + { "POSITION", "TK_POSITION", true },
> + { "PRINTF", "TK_PRINTF", true },
> + { "QUOTE", "TK_QUOTE", true },
> + { "RANDOM", "TK_RANDOM", true },
> + { "RANDOMBLOB", "TK_RANDOMBLOB", true },
> + { "ROUND", "TK_ROUND", true },
> + { "ROW_COUNT", "TK_ROW_COUNT", true },
> + { "SOUNDEX", "TK_SOUNDEX", true },
> + { "SUBSTR", "TK_SUBSTR", true },
> + { "SUM", "TK_SUM", true },
> + { "TOTAL", "TK_TOTAL", true },
> + { "TYPEOF", "TK_TYPEOF", true },
> + { "UNICODE", "TK_UNICODE", true },
> + { "UNLIKELY", "TK_UNLIKELY", true },
> + { "UPPER", "TK_UPPER", true },
> + { "VERSION", "TK_VERSION", true },
> + { "ZEROBLOB", "TK_ZEROBLOB", true },
Should be sorted?
> };
>
> /* Number of keywords */
> diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
> index bd041e862..cb2e627db 100644
> --- a/src/box/sql/parse.y
> +++ b/src/box/sql/parse.y
> @@ -1172,27 +1172,506 @@ trim_specification(A) ::= LEADING. { A = TRIM_LEADING; }
> trim_specification(A) ::= TRAILING. { A = TRIM_TRAILING; }
> trim_specification(A) ::= BOTH. { A = TRIM_BOTH; }
>
> -expr(A) ::= id(X) LP distinct(D) exprlist(Y) RP(E). {
> - if( Y && Y->nExpr>pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG] ){
> - const char *err =
> - tt_sprintf("Number of arguments to function %.*s", X.n, X.z);
> - diag_set(ClientError, ER_SQL_PARSER_LIMIT, err, Y->nExpr,
> - pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]);
> +expr(A) ::= ABS(X) LP distinct(D) exprlist(Y) RP(E). {
> + if (Y == NULL || Y->nExpr != 1) {
> + int n = Y == NULL ? 0 : Y->nExpr;
> + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "ABS", "1", n);
> pParse->is_aborted = true;
> + return;
> }
> A.pExpr = sqlExprFunction(pParse, Y, &X);
> - spanSet(&A,&X,&E);
> - if( D==SF_Distinct && A.pExpr ){
> + spanSet(&A, &X, &E);
> + if(D == SF_Distinct && A.pExpr)
> A.pExpr->flags |= EP_Distinct;
> +}
> +
> +expr(A) ::= AVG(X) LP distinct(D) exprlist(Y) RP(E). {
> + if (Y == NULL || Y->nExpr != 1) {
> + int n = Y == NULL ? 0 : Y->nExpr;
> + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "AVG", "1", n);
> + pParse->is_aborted = true;
> + return;
> }
> + A.pExpr = sqlExprFunction(pParse, Y, &X);
> + spanSet(&A, &X, &E);
> + if(D == SF_Distinct && A.pExpr)
> + A.pExpr->flags |= EP_Distinct;
> }
>
> -/*
> - * type_func(A) ::= DATE(A) .
> - * type_func(A) ::= DATETIME(A) .
> - */
> -type_func(A) ::= CHAR(A) .
> -expr(A) ::= type_func(X) LP distinct(D) exprlist(Y) RP(E). {
> +expr(A) ::= CHAR(X) LP distinct(D) exprlist(Y) RP(E). {
> + if (Y != NULL && Y->nExpr > pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]) {
> + const char *str = tt_sprintf("from %d to %d", 0,
> + pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]);
> + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "CHAR", str, Y->nExpr);
> + pParse->is_aborted = true;
> + return;
> + }
> + A.pExpr = sqlExprFunction(pParse, Y, &X);
> + spanSet(&A, &X, &E);
> + if(D == SF_Distinct && A.pExpr)
> + A.pExpr->flags |= EP_Distinct;
> +}
> +
> +expr(A) ::= CHAR_LEN(X) LP distinct(D) exprlist(Y) RP(E). {
> + if (Y == NULL || Y->nExpr != 1) {
> + int n = Y == NULL ? 0 : Y->nExpr;
> + const char *name = X.n == strlen("CHAR_LENGTH") ? "CHAR_LENGTH" :
> + "CHARACTER_LENGTH";
> + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, name, "1", n);
> + pParse->is_aborted = true;
> + return;
> + }
> + A.pExpr = sqlExprFunction(pParse, Y, &X);
> + spanSet(&A, &X, &E);
> + if(D == SF_Distinct && A.pExpr)
> + A.pExpr->flags |= EP_Distinct;
> +}
> +
> +expr(A) ::= COALESCE(X) LP distinct(D) exprlist(Y) RP(E). {
> + if (Y == NULL || Y->nExpr < 2 ||
> + Y->nExpr > pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]) {
> + int n = Y == NULL ? 0 : Y->nExpr;
> + const char *str = tt_sprintf("from %d to %d", 2,
> + pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG]);
> + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "COALESCE", str, n);
> + pParse->is_aborted = true;
> + return;
> + }
> + A.pExpr = sqlExprFunction(pParse, Y, &X);
> + spanSet(&A, &X, &E);
> + if(D == SF_Distinct && A.pExpr)
> + A.pExpr->flags |= EP_Distinct;
> +}
Can you the code to a helper function to avoid copy-paste?
> diff --git a/test/box/tx_man.result b/test/box/tx_man.result
> index 786d7fc30..b99fbc2ca 100644
> --- a/test/box/tx_man.result
> +++ b/test/box/tx_man.result
> @@ -2129,11 +2129,11 @@ tx1:rollback()
>
> -- gh-6095: SQL query may crash in MVCC mode if it involves ephemeral spaces.
> --
> -box.execute([[ CREATE TABLE test (id INT NOT NULL PRIMARY KEY, count INT NOT NULL)]])
> +box.execute([[ CREATE TABLE test (id INT NOT NULL PRIMARY KEY, "COUNT" INT NOT NULL)]])
> | ---
> | - row_count: 1
> | ...
> -box.execute([[ UPDATE test SET count = count + 1 WHERE id = 0 ]])
> +box.execute([[ UPDATE test SET "COUNT" = "COUNT" + 1 WHERE id = 0 ]])
This looks bad. MySQL and PostgreSQL allow that.
> | ---
> | - row_count: 0
> | ...
> diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
> index e7b35c9d9..7dd85025a 100755
> --- a/test/sql-tap/func.test.lua
> +++ b/test/sql-tap/func.test.lua
> @@ -68,7 +68,7 @@ test:do_catchsql_test(
> SELECT length(*) FROM tbl1 ORDER BY t1
> ]], {
> -- <func-1.1>
> - 1, "Wrong number of arguments is passed to LENGTH(): expected 1, got 0"
> + 1, "Syntax error at line 1 near '*'"
This is probably okay.
> -- </func-1.1>
> })
>
> @@ -2483,7 +2483,7 @@ test:do_catchsql_test(
> SELECT coalesce()
> ]], {
> -- <func-27.1>
> - 1, "Wrong number of arguments is passed to COALESCE(): expected at least two, got 0"
> + 1, "Wrong number of arguments is passed to COALESCE(): expected from 2 to 127, got 0"
And this too.
next prev parent reply other threads:[~2021-08-19 8:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-18 14:34 [Tarantool-patches] [PATCH v2 0/5] Prepare for static arguments type check Mergen Imeev via Tarantool-patches
2021-08-18 14:34 ` [Tarantool-patches] [PATCH v2 1/5] sql: modify arithmetic aggregate functions Mergen Imeev via Tarantool-patches
2021-08-18 14:34 ` [Tarantool-patches] [PATCH v2 2/5] sql: introduce SQL built-in functions to parser Mergen Imeev via Tarantool-patches
2021-08-19 8:35 ` Vladimir Davydov via Tarantool-patches [this message]
2021-08-21 0:27 ` Safin Timur via Tarantool-patches
2021-08-18 14:35 ` [Tarantool-patches] [PATCH v2 3/5] sql: separate functions in parser Mergen Imeev via Tarantool-patches
2021-08-18 14:35 ` [Tarantool-patches] [PATCH v2 4/5] sql: separate function flags from functions Mergen Imeev via Tarantool-patches
2021-08-18 14:35 ` [Tarantool-patches] [PATCH v2 5/5] sql: encapsulate SQL built-in functions opcodes Mergen Imeev via Tarantool-patches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210819083514.i4hlig526gy6domj@esperanza \
--to=tarantool-patches@dev.tarantool.org \
--cc=imeevma@tarantool.org \
--cc=vdavydov@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v2 2/5] sql: introduce SQL built-in functions to parser' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox