From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 5B8E026C35 for ; Tue, 20 Aug 2019 15:04:37 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nILV1BsxDWPG for ; Tue, 20 Aug 2019 15:04:37 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id AE4DA26C0B for ; Tue, 20 Aug 2019 15:04:36 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: [tarantool-patches] Re: [PATCH v3 7/9] sql: get rid of FuncDef function hash From: "n.pettik" In-Reply-To: <52e011b5-fe1c-6731-2726-c7d2d3f65e05@tarantool.org> Date: Tue, 20 Aug 2019 22:04:33 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20190816140902.GC18643@atlas> <52e011b5-fe1c-6731-2726-c7d2d3f65e05@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov , Konstantin Osipov > diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h > index 5a3e8f1c1..91a640816 100644 > --- a/src/box/sql/sqlInt.h > +++ b/src/box/sql/sqlInt.h >=20 > @@ -4487,9 +4330,64 @@ Expr *sqlExprForVectorField(Parse *, Expr *, = int); > */ > extern int sqlSubProgramsRemaining; >=20 > -/** Register built-in functions to work with ANALYZE data. */ > -void > -sql_register_analyze_builtins(void); > +struct func_sql_builtin { > + /** Function object base class. */ > + struct func base; > + /** A bitmask of SQL flags. */ > + uint16_t flags; > + /** User data to pass in call method. */ > + void *user_data; =E2=80=98user_data' is not used anymore, please remove. > + /** > + * A VDBE-memory-compatible call method for a function. > + * SQL Builting functions doesn't use base class call > + * method to gain the best possible performance for SQL > + * requests. As builtin functions are predefined and > + * non of them modifie schema, access checks are > + * redundant, functions have the same execution > + * privileges as SQL. > + */ > + void (*call)(sql_context *ctx, int argc, sql_value **argv); > + /** > + * Check whether do the function support a specified > + * number of input arguments. -> Check whether function supports given number of arguments. For instance, trim() function can accept one, two or three arguments. > + */ > + int (*check_param_count)(struct func_sql_builtin *func, int = argc); >=20 >=20 > diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h > index f77c019fb..d0c15d1fe 100644 > --- a/src/box/sql/vdbeInt.h > +++ b/src/box/sql/vdbeInt.h > @@ -46,6 +46,8 @@ > */ > typedef struct VdbeOp Op; >=20 > +struct func; > + > /* > * Boolean values > */ >=20 > @@ -516,7 +523,17 @@ int sqlVdbeMemNumerify(Mem *); > int sqlVdbeMemCast(Mem *, enum field_type type); > int sqlVdbeMemFromBtree(BtCursor *, u32, u32, Mem *); > void sqlVdbeMemRelease(Mem * p); > -int sqlVdbeMemFinalize(Mem *, FuncDef *); > + > +/** > + * Memory cell pMem contains the context of an aggregate function. Again: pMem -> mem. > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > index 07c019db9..b583b60c9 100644 > --- a/src/box/sql/func.c > +++ b/src/box/sql/func.c >=20 > @@ -498,6 +501,13 @@ substrFunc(sql_context * context, int argc, = sql_value ** argv) > } > } >=20 > +static int > +sql_builtin_substr_check_param_count(struct func_sql_builtin *func, = int argc) > +{ > + (void) func; > + return argc !=3D 2 && argc !=3D 3 ? -1 : 0; > +} > + > /* > * Implementation of the round() function > */ > @@ -538,6 +548,13 @@ roundFunc(sql_context * context, int argc, = sql_value ** argv) > sql_result_double(context, r); > } >=20 > +static int > +sql_builtin_round_check_param_count(struct func_sql_builtin *func, = int argc) > +{ > + (void) func; > + return argc !=3D 1 && argc !=3D 2 ? -1 : 0; > +} > + > /* > * Allocate nByte bytes of space using sqlMalloc(). If the > * allocation fails, return NULL. If nByte is larger than the > @@ -968,6 +985,13 @@ likeFunc(sql_context *context, int argc, = sql_value **argv) > sql_result_bool(context, res =3D=3D MATCH); > } >=20 > +static int > +sql_builtin_like_check_param_count(struct func_sql_builtin *func, int = argc) > +{ > + (void) func; > + return argc !=3D 2 && argc !=3D 3 ? -1 : 0; > +} > + > /* > * Implementation of the NULLIF(x,y) function. The result is the = first > * argument if the arguments are different. The result is NULL if the > @@ -1514,6 +1538,13 @@ trim_func(struct sql_context *context, int = argc, sql_value **argv) > } > } >=20 > +static int > +sql_builtin_trim_check_param_count(struct func_sql_builtin *func, int = argc) > +{ > + (void) func; > + return argc !=3D 1 && argc !=3D 2 && argc !=3D 3 ? -1 : 0; > +} > + > /* > * Compute the soundex encoding of a word. > * > @@ -1703,6 +1734,13 @@ countFinalize(sql_context * context) > sql_result_uint(context, p ? p->n : 0); > } >=20 > +static int > +sql_builtin_count_check_param_count(struct func_sql_builtin *func, = int argc) > +{ > + (void) func; > + return argc !=3D 0 && argc !=3D 1 ? -1 : 0; > +} > + To be honest, all these stubs and idea with count_check function at all now seems to be not as good as it used to be. I=E2=80=99ve removed this check and instead added several runtime ones and IMHO it looks way much better: np/sql-builtins diff --git a/src/box/errcode.h b/src/box/errcode.h index 817275b97..d4aca2caa 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -253,6 +253,7 @@ struct errcode_record { /*198 */_(ER_FUNC_INDEX_FUNC, "Failed to build a key = for functional index '%s' of space '%s': %s") \ /*199 */_(ER_FUNC_INDEX_FORMAT, "Key format doesn't = match one defined in functional index '%s' of space '%s': %s") \ /*200 */_(ER_FUNC_INDEX_PARTS, "Wrong functional index = definition: %s") \ + /*200 */_(ER_FUNC_WRONG_ARG_COUNT, "Wrong number of = arguments is passed to %s: expected %s, got %d") =20 /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 74f434346..9f5dffd73 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -691,7 +691,12 @@ substrFunc(sql_context * context, int argc, = sql_value ** argv) i64 p1, p2; int negP2 =3D 0; =20 - assert(argc =3D=3D 3 || argc =3D=3D 2); + if (argc !=3D 2 && argc !=3D 3) { + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "SUBSTR", + "1 or 2", argc); + context->is_aborted =3D true; + return; + } if (sql_value_is_null(argv[1]) || (argc =3D=3D 3 && sql_value_is_null(argv[2])) ) { @@ -779,13 +784,6 @@ substrFunc(sql_context * context, int argc, = sql_value ** argv) } } =20 -static int -sql_builtin_substr_check_param_count(struct func_sql_builtin *func, int = argc) -{ - (void) func; - return argc !=3D 2 && argc !=3D 3 ? -1 : 0; -} - /* * Implementation of the round() function */ @@ -794,7 +792,12 @@ roundFunc(sql_context * context, int argc, = sql_value ** argv) { int n =3D 0; double r; - assert(argc =3D=3D 1 || argc =3D=3D 2); + if (argc !=3D 1 && argc !=3D 2) { + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "ROUND", + "1 or 2", argc); + context->is_aborted =3D true; + return; + } if (argc =3D=3D 2) { if (sql_value_is_null(argv[1])) return; @@ -826,13 +829,6 @@ roundFunc(sql_context * context, int argc, = sql_value ** argv) sql_result_double(context, r); } =20 -static int -sql_builtin_round_check_param_count(struct func_sql_builtin *func, int = argc) -{ - (void) func; - return argc !=3D 1 && argc !=3D 2 ? -1 : 0; -} - /* * Allocate nByte bytes of space using sqlMalloc(). If the * allocation fails, return NULL. If nByte is larger than the @@ -1193,6 +1189,12 @@ likeFunc(sql_context *context, int argc, = sql_value **argv) { u32 escape =3D SQL_END_OF_STRING; int nPat; + if (argc !=3D 2 && argc !=3D 3) { + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, + "LIKE", "2 or 3", argc); + context->is_aborted =3D true; + return; + } sql *db =3D sql_context_db_handle(context); int rhs_type =3D sql_value_type(argv[0]); int lhs_type =3D sql_value_type(argv[1]); @@ -1263,13 +1265,6 @@ likeFunc(sql_context *context, int argc, = sql_value **argv) sql_result_bool(context, res =3D=3D MATCH); } =20 -static int -sql_builtin_like_check_param_count(struct func_sql_builtin *func, int = argc) -{ - (void) func; - return argc !=3D 2 && argc !=3D 3 ? -1 : 0; -} - /* * Implementation of the NULLIF(x,y) function. The result is the first * argument if the arguments are different. The result is NULL if the @@ -1812,17 +1807,12 @@ trim_func(struct sql_context *context, int argc, = sql_value **argv) trim_func_three_args(context, argv[0], argv[1], = argv[2]); break; default: - unreachable(); + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "TRIM", + "1 or 2 or 3", argc); + context->is_aborted =3D true; } } =20 -static int -sql_builtin_trim_check_param_count(struct func_sql_builtin *func, int = argc) -{ - (void) func; - return argc !=3D 1 && argc !=3D 2 && argc !=3D 3 ? -1 : 0; -} - /* * Compute the soundex encoding of a word. * @@ -1998,6 +1988,12 @@ static void countStep(sql_context * context, int argc, sql_value ** argv) { CountCtx *p; + if (argc !=3D 0 && argc !=3D 1) { + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, + "COUNT", "0 or 1", argc); + context->is_aborted =3D true; + return; + } p =3D sql_aggregate_context(context, sizeof(*p)); if ((argc =3D=3D 0 || ! sql_value_is_null(argv[0])) && p) { p->n++; @@ -2012,13 +2008,6 @@ countFinalize(sql_context * context) sql_result_uint(context, p ? p->n : 0); } =20 -static int -sql_builtin_count_check_param_count(struct func_sql_builtin *func, int = argc) -{ - (void) func; - return argc !=3D 0 && argc !=3D 1 ? -1 : 0; -} - /* * Routines to implement min() and max() aggregate functions. */ @@ -2083,7 +2072,12 @@ groupConcatStep(sql_context * context, int argc, = sql_value ** argv) StrAccum *pAccum; const char *zSep; int nVal, nSep; - assert(argc =3D=3D 1 || argc =3D=3D 2); + if (argc !=3D 1 && argc !=3D 2) { + diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, + "GROUP_CONCAT", "1 or 2", argc); + context->is_aborted =3D true; + return; + } if (sql_value_is_null(argv[0])) return; pAccum =3D @@ -2131,14 +2125,6 @@ groupConcatFinalize(sql_context * context) } } =20 -static int -sql_builtin_group_concat_check_param_count(struct func_sql_builtin = *func, - int argc) -{ - (void) func; - return argc !=3D 1 && argc !=3D 2 ? -1 : 0; -} - int sql_is_like_func(struct Expr *expr) { @@ -2158,16 +2144,9 @@ sql_func_by_signature(const char *name, int argc) struct func *base =3D func_by_name(name, strlen(name)); if (base =3D=3D NULL || !base->def->exports.sql) return NULL; - if (base->def->language =3D=3D FUNC_LANGUAGE_SQL_BUILTIN) { - struct func_sql_builtin *func =3D - (struct func_sql_builtin *) base; - if (func->check_param_count(func, argc) !=3D 0) - return NULL; - } else { - if (base->def->param_count !=3D -1 && - base->def->param_count !=3D argc) - return NULL; - } + + if (base->def->param_count !=3D -1 && base->def->param_count !=3D = argc) + return NULL; return base; } =20 @@ -2190,34 +2169,25 @@ sql_builtin_stub(sql_context *ctx, int argc, = sql_value **argv) ctx->is_aborted =3D true; } =20 -static int -sql_builtin_default_check_param_count(struct func_sql_builtin *func, = int argc) -{ - return (func->base.def->param_count !=3D -1 && - argc !=3D func->base.def->param_count) ? -1 : 0; -} - -static int -sql_builtin_coalesce_check_param_count(struct func_sql_builtin *func, = int argc) -{ - (void) func; - return argc =3D=3D 0 || argc =3D=3D 1 ? -1 : 0; -} - /** * A sequence of SQL builtins definitions in * lexicographic order. */ static struct { + /** + * Name is used to find corresponding entry in array + * sql_builtins applying binary search. + */ const char *name; - int param_count; - enum field_type returns; - enum func_aggregate aggregate; - bool is_deterministic; + /** Members below are related to struct func_sql_builtin. */ uint16_t flags; void (*call)(sql_context *ctx, int argc, sql_value **argv); - int (*check_param_count)(struct func_sql_builtin *func, int = argc); void (*finalize)(sql_context *ctx); + /** Members below are related to struct func_def. */ + bool is_deterministic; + int param_count; + enum field_type returns; + enum func_aggregate aggregate; } sql_builtins[] =3D { {.name =3D "ABS", .param_count =3D 1, @@ -2226,7 +2196,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D absFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "AVG", .param_count =3D 1, @@ -2235,7 +2204,6 @@ static struct { .aggregate =3D FUNC_AGGREGATE_GROUP, .flags =3D 0, .call =3D sum_step, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D avgFinalize}, {.name =3D "CHAR", .param_count =3D -1, @@ -2244,7 +2212,6 @@ static struct { .aggregate =3D FUNC_AGGREGATE_NONE, .flags =3D 0, .call =3D charFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "CHARACTER_LENGTH", .param_count =3D 1, @@ -2253,7 +2220,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D lengthFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "CHAR_LENGTH", .param_count =3D 1, @@ -2262,7 +2228,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D lengthFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "COALESCE", .param_count =3D -1, @@ -2271,7 +2236,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_COALESCE, .call =3D sql_builtin_stub, - .check_param_count =3D sql_builtin_coalesce_check_param_count, .finalize =3D NULL}, {.name =3D "COUNT", .param_count =3D -1, @@ -2280,7 +2244,6 @@ static struct { .is_deterministic =3D false, .flags =3D 0, .call =3D countStep, - .check_param_count =3D sql_builtin_count_check_param_count, .finalize =3D countFinalize}, {.name =3D "GREATEST", .param_count =3D -1, @@ -2289,7 +2252,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_NEEDCOLL | SQL_FUNC_MAX, .call =3D minmaxFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "GROUP_CONCAT", .param_count =3D -1, @@ -2298,7 +2260,6 @@ static struct { .is_deterministic =3D false, .flags =3D 0, .call =3D groupConcatStep, - .check_param_count =3D = sql_builtin_group_concat_check_param_count, .finalize =3D groupConcatFinalize}, {.name =3D "HEX", .param_count =3D 1, @@ -2307,7 +2268,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D hexFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "IFNULL", .param_count =3D 2, @@ -2316,7 +2276,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_COALESCE, .call =3D sql_builtin_stub, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "LEAST", .param_count =3D -1, @@ -2325,7 +2284,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_NEEDCOLL | SQL_FUNC_MIN, .call =3D minmaxFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "LENGTH", .param_count =3D 1, @@ -2334,7 +2292,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_LENGTH, .call =3D lengthFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "LIKE", .param_count =3D -1, @@ -2343,7 +2300,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_NEEDCOLL | SQL_FUNC_LIKE, .call =3D likeFunc, - .check_param_count =3D sql_builtin_like_check_param_count, .finalize =3D NULL}, {.name =3D "LIKELIHOOD", .param_count =3D 2, @@ -2352,7 +2308,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_UNLIKELY, .call =3D sql_builtin_stub, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "LIKELY", .param_count =3D 1, @@ -2361,7 +2316,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_UNLIKELY, .call =3D sql_builtin_stub, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "LOWER", .param_count =3D 1, @@ -2370,7 +2324,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_DERIVEDCOLL | SQL_FUNC_NEEDCOLL, .call =3D LowerICUFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "MAX", .param_count =3D 1, @@ -2379,7 +2332,6 @@ static struct { .is_deterministic =3D false, .flags =3D SQL_FUNC_NEEDCOLL | SQL_FUNC_MAX, .call =3D minmaxStep, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D minMaxFinalize}, {.name =3D "MIN", .param_count =3D 1, @@ -2388,7 +2340,6 @@ static struct { .is_deterministic =3D false, .flags =3D SQL_FUNC_NEEDCOLL | SQL_FUNC_MIN, .call =3D minmaxStep, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D minMaxFinalize}, {.name =3D "NULLIF", .param_count =3D 2, @@ -2397,7 +2348,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_NEEDCOLL, .call =3D nullifFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "POSITION", .param_count =3D 2, @@ -2406,7 +2356,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_NEEDCOLL, .call =3D position_func, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "PRINTF", .param_count =3D -1, @@ -2415,7 +2364,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D printfFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "QUOTE", .param_count =3D 1, @@ -2424,7 +2372,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D quoteFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "RANDOM", .param_count =3D 0, @@ -2433,7 +2380,6 @@ static struct { .is_deterministic =3D false, .flags =3D 0, .call =3D randomFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "RANDOMBLOB", .param_count =3D 1, @@ -2442,7 +2388,6 @@ static struct { .is_deterministic =3D false, .flags =3D 0, .call =3D randomBlob, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "REPLACE", .param_count =3D 3, @@ -2451,7 +2396,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_DERIVEDCOLL, .call =3D replaceFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "ROUND", .param_count =3D -1, @@ -2460,7 +2404,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D roundFunc, - .check_param_count =3D sql_builtin_round_check_param_count, .finalize =3D NULL}, {.name =3D "ROW_COUNT", .param_count =3D 0, @@ -2469,7 +2412,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D sql_row_count, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "SOUNDEX", .param_count =3D 1, @@ -2478,7 +2420,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D soundexFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "SUBSTR", .param_count =3D -1, @@ -2487,7 +2428,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_DERIVEDCOLL, .call =3D substrFunc, - .check_param_count =3D sql_builtin_substr_check_param_count, .finalize =3D NULL}, {.name =3D "SUM", .param_count =3D 1, @@ -2496,7 +2436,6 @@ static struct { .is_deterministic =3D false, .flags =3D 0, .call =3D sum_step, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D sumFinalize}, {.name =3D "TOTAL", .param_count =3D 1, @@ -2505,7 +2444,6 @@ static struct { .is_deterministic =3D false, .flags =3D 0, .call =3D sum_step, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D totalFinalize}, {.name =3D "TRIM", .param_count =3D -1, @@ -2514,7 +2452,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_DERIVEDCOLL, .call =3D trim_func, - .check_param_count =3D sql_builtin_trim_check_param_count, .finalize =3D NULL}, {.name =3D "TYPEOF", .param_count =3D 1, @@ -2523,7 +2460,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_TYPEOF, .call =3D typeofFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "UNICODE", .param_count =3D 1, @@ -2532,7 +2468,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D unicodeFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "UNLIKELY", .param_count =3D 1, @@ -2541,7 +2476,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_UNLIKELY, .call =3D sql_builtin_stub, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "UPPER", .param_count =3D 1, @@ -2550,7 +2484,6 @@ static struct { .is_deterministic =3D true, .flags =3D SQL_FUNC_DERIVEDCOLL | SQL_FUNC_NEEDCOLL, .call =3D UpperICUFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "VERSION", .param_count =3D 0, @@ -2559,7 +2492,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D sql_func_version, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, {.name =3D "ZEROBLOB", .param_count =3D 1, @@ -2568,7 +2500,6 @@ static struct { .is_deterministic =3D true, .flags =3D 0, .call =3D zeroblobFunc, - .check_param_count =3D sql_builtin_default_check_param_count, .finalize =3D NULL}, }; =20 @@ -2607,8 +2538,8 @@ func_sql_builtin_new(struct func_def *def) goto end; func->flags =3D sql_builtins[idx].flags; func->call =3D sql_builtins[idx].call; - func->check_param_count =3D sql_builtins[idx].check_param_count; func->finalize =3D sql_builtins[idx].finalize; + def->param_count =3D sql_builtins[idx].param_count; def->is_deterministic =3D sql_builtins[idx].is_deterministic; def->returns =3D sql_builtins[idx].returns; diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 2c037a5a4..cd4a303dd 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -38,8 +38,6 @@ #include "sqlInt.h" #include #include -#include "box/func.h" -#include "box/func_def.h" #include "box/schema.h" =20 /* @@ -678,18 +676,10 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) pParse->is_aborted =3D true; pNC->nErr++; } else if (wrong_num_args) { - const char *err; - if (func->def->param_count >=3D 0) { - err =3D "invalid number of = arguments is " - "passed to %.*s(): = expected %d, " - "got %d"; - } else { - err =3D "invalid number of = arguments is " - "passed to %.*s()"; - } - diag_set(ClientError, = ER_SQL_PARSER_GENERIC, - tt_sprintf(err, nId, zId, - = func->def->param_count, n)); + + diag_set(ClientError, = ER_FUNC_WRONG_ARG_COUNT, + tt_sprintf("%d", = func->def->param_count), + nId); pParse->is_aborted =3D true; pNC->nErr++; } diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 91a640816..c27eb1022 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -4335,8 +4335,6 @@ struct func_sql_builtin { struct func base; /** A bitmask of SQL flags. */ uint16_t flags; - /** User data to pass in call method. */ - void *user_data; /** * A VDBE-memory-compatible call method for a function. * SQL Builting functions doesn't use base class call @@ -4347,11 +4345,6 @@ struct func_sql_builtin { * privileges as SQL. */ void (*call)(sql_context *ctx, int argc, sql_value **argv); - /** - * Check whether do the function support a specified - * number of input arguments. - */ - int (*check_param_count)(struct func_sql_builtin *func, int = argc); /** * A VDBE-memory-compatible finalize method * (is valid only for aggregate function). > /* > * Routines to implement min() and max() aggregate functions. > */ > @@ -1713,6 +1751,8 @@ minmaxStep(sql_context * context, int NotUsed, = sql_value ** argv) > Mem *pBest; > UNUSED_PARAMETER(NotUsed); >=20 > + struct func_sql_builtin *func =3D > + (struct func_sql_builtin *)context->func; > pBest =3D (Mem *) sql_aggregate_context(context, = sizeof(*pBest)); > if (!pBest) > return; > @@ -1721,20 +1761,17 @@ minmaxStep(sql_context * context, int NotUsed, = sql_value ** argv) > if (pBest->flags) > sqlSkipAccumulatorLoad(context); > } else if (pBest->flags) { > - int max; > int cmp; > struct coll *pColl =3D sqlGetFuncCollSeq(context); > - /* This step function is used for both the min() and = max() aggregates, > - * the only difference between the two being that the = sense of the > - * comparison is inverted. For the max() aggregate, the > - * sql_user_data() function returns (void *)-1. For = min() it > - * returns (void *)db, where db is the sql* database = pointer. > - * Therefore the next statement sets variable 'max' to 1 = for the max() > - * aggregate, or 0 for min(). > + /* > + * This step function is used for both the min() > + * and max() aggregates, the only difference > + * between the two being that the sense of the > + * comparison is inverted. > */ > - max =3D sql_user_data(context) !=3D 0; > + bool is_max =3D (func->flags & SQL_FUNC_MAX) !=3D 0; I=E2=80=99d rather move introduction of SQL_FUNC_MAX flag to a separate = patch. > cmp =3D sqlMemCompare(pBest, pArg, pColl); > - if ((max && cmp < 0) || (!max && cmp > 0)) { > + if ((is_max && cmp < 0) || (!is_max && cmp > 0)) { > sqlVdbeMemCopy(pBest, pArg); > } else { > sqlSkipAccumulatorLoad(context); > @@ -1816,128 +1853,503 @@ groupConcatFinalize(sql_context * context) > } > } >=20 > +static int > +sql_builtin_group_concat_check_param_count(struct func_sql_builtin = *func, > + int argc) > +{ > + (void) func; > + return argc !=3D 1 && argc !=3D 2 ? -1 : 0; > +} >=20 > -/* > - * All of the FuncDef structures in the aBuiltinFunc[] array above > - * to the global function hash table. This occurs at start-time (as > - * a consequence of calling sql_initialize()). > - * > - * After this routine runs > +struct func * > +sql_func_by_signature(const char *name, int argc) > +{ > + struct func *base =3D func_by_name(name, strlen(name)); > + if (base =3D=3D NULL || !base->def->exports.sql) > + return NULL; > + if (base->def->language =3D=3D FUNC_LANGUAGE_SQL_BUILTIN) { > + struct func_sql_builtin *func =3D > + (struct func_sql_builtin *) base; > + if (func->check_param_count(func, argc) !=3D 0) > + return NULL; > + } else { > + if (base->def->param_count !=3D -1 && > + base->def->param_count !=3D argc) > + return NULL; > + } > + return base; > +} > + > + Nit: one extra empty line. > +static int > +sql_builtin_call_stub(struct func *func, struct port *args, struct = port *ret) I=E2=80=99d rename it to func_sql_builtin_call_stub() > +{ > + (void) func; (void) args; (void) ret; > + diag_set(ClientError, ER_UNSUPPORTED, > + "sql builtin function", "Lua frontend"); > + return -1; > +} > + > +static void > +sql_builtin_stub(sql_context *ctx, int argc, sql_value **argv) > +{ > + (void) argc; (void) argv; > + diag_set(ClientError, ER_SQL_EXECUTE, > + tt_sprintf("function '%s' is not implemented", > + ctx->func->def->name)); > + ctx->is_aborted =3D true; > +} > + > +static int > +sql_builtin_default_check_param_count(struct func_sql_builtin *func, = int argc) > +{ > + return (func->base.def->param_count !=3D -1 && > + argc !=3D func->base.def->param_count) ? -1 : 0; > +} > + > +static int > +sql_builtin_coalesce_check_param_count(struct func_sql_builtin *func, = int argc) > +{ > + (void) func; > + return argc =3D=3D 0 || argc =3D=3D 1 ? -1 : 0; > +} > + > +/** > + * A sequence of SQL builtins definitions in > + * lexicographic order. > */ > -void > -sqlRegisterBuiltinFunctions(void) > +static struct { > + const char *name; > + int param_count; > + enum field_type returns; > + enum func_aggregate aggregate; > + bool is_deterministic; > + uint16_t flags; > + void (*call)(sql_context *ctx, int argc, sql_value **argv); > + int (*check_param_count)(struct func_sql_builtin *func, int = argc); > + void (*finalize)(sql_context *ctx); Let=E2=80=99s reorganise a bit this struct: @@ -2209,17 +2209,24 @@ sql_builtin_coalesce_check_param_count(struct = func_sql_builtin *func, int argc) * lexicographic order. */ static struct { + /** + * Name is used to find corresponding entry in array + * sql_builtins applying binary search. + */ const char *name; - int param_count; - enum field_type returns; - enum func_aggregate aggregate; - bool is_deterministic; + /** Members below are related to struct func_sql_builtin. */ uint16_t flags; void (*call)(sql_context *ctx, int argc, sql_value **argv); int (*check_param_count)(struct func_sql_builtin *func, int = argc); void (*finalize)(sql_context *ctx); + /** Members below are related to struct func_def. */ + bool is_deterministic; + int param_count; + enum field_type returns; + enum func_aggregate aggregate; Now you can use memcpy() to copy corresponding parts to struct func_def and struct func_sql_builtin (it is up to you). > +} sql_builtins[] =3D { > + {.name =3D "ABS", > + .param_count =3D 1, > + .returns =3D FIELD_TYPE_NUMBER, > + .aggregate =3D FUNC_AGGREGATE_NONE, > + .is_deterministic =3D true, > + .flags =3D 0, > + .call =3D absFunc, > + .check_param_count =3D sql_builtin_default_check_param_count, > + .finalize =3D NULL}, > + {.name =3D "AVG=E2=80=9D, Please, leave one empty line between structs, so that one can visually separate them: - {.name =3D "ABS", + { + .name =3D "ABS", .param_count =3D 1, .returns =3D FIELD_TYPE_NUMBER, .aggregate =3D FUNC_AGGREGATE_NONE, @@ -2227,8 +2234,9 @@ static struct { .flags =3D 0, .call =3D absFunc, .check_param_count =3D sql_builtin_default_check_param_count, - .finalize =3D NULL}, - {.name =3D "AVG", + .finalize =3D NULL + }, { + .name =3D "AVG", .param_count =3D 1, .returns =3D FIELD_TYPE_NUMBER, .is_deterministic =3D false, @@ -2236,8 +2244,9 @@ static struct { .flags =3D 0, .call =3D sum_step, .check_param_count =3D sql_builtin_default_check_param_count, - .finalize =3D avgFinalize}, - {.name =3D "CHAR", + .finalize =3D avgFinalize + }, { + .name =3D "CHAR", etc > diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c > index 207f57ba6..682bb9c8e 100644 > --- a/src/box/sql/resolve.c > +++ b/src/box/sql/resolve.c > @@ -38,6 +38,9 @@ > #include "sqlInt.h" > #include > #include > +#include "box/func.h" > +#include "box/func_def.h=E2=80=9D func.h and func_def.h seem to be redundant - I=E2=80=99ve removed them and project has compiled. > +#include "box/schema.h" >=20 > /* > * Walk the expression tree pExpr and increase the aggregate function > @@ -596,27 +599,28 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) > int is_agg =3D 0; /* True if is an = aggregate function */ > int nId; /* Number of characters in = function name */ > const char *zId; /* The function name. */ > - FuncDef *pDef; /* Information about the = function */ >=20 > assert(!ExprHasProperty(pExpr, EP_xIsSelect)); > zId =3D pExpr->u.zToken; > nId =3D sqlStrlen30(zId); > - pDef =3D sqlFindFunction(pParse->db, zId, n, 0); > - if (pDef =3D=3D 0) { > - pDef =3D > - sqlFindFunction(pParse->db, zId, = -2,0); > - if (pDef =3D=3D 0) { > + struct func *func =3D sql_func_by_signature(zId, = n); > + if (func =3D=3D NULL) { > + func =3D func_by_name(zId, nId); This makes you include box/schema.h > + if (func =3D=3D NULL || = !func->def->exports.sql) > no_such_func =3D 1; "No such function=E2=80=9D and =E2=80=9Cnot available in SQL=E2=80=9D = are different errors. Please, handle both cases with proper error messages and add test cases. > - } else { > + else > wrong_num_args =3D 1; > - } > } else { > - is_agg =3D pDef->xFinalize !=3D 0; > - pExpr->type =3D pDef->ret_type; > + is_agg =3D func->def->language =3D=3D > + FUNC_LANGUAGE_SQL_BUILTIN && > + func->def->aggregate =3D=3D > + FUNC_AGGREGATE_GROUP; =3D=3D FUNC_AGGREGATE =E2=80=94 check on language is redundant. Instead, add an assertion. > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index ddb5de87e..b2ce868d6 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -4381,7 +4381,9 @@ is_simple_count(struct Select *select, struct = AggInfo *agg_info) > return NULL; > if (NEVER(agg_info->nFunc =3D=3D 0)) > return NULL; > - if ((agg_info->aFunc->pFunc->funcFlags & SQL_FUNC_COUNT) =3D=3D = 0 || > + assert(agg_info->aFunc->func->def->language =3D=3D > + FUNC_LANGUAGE_SQL_BUILTIN); > + if (!sql_func_flag_is_set(agg_info->aFunc->func, SQL_FUNC_COUNT) = =3D=3D 0 || Nit: sql_func_flag_is_set() returns boolean, no need to check it on equality to zero.=20 > diff --git a/src/box/alter.cc b/src/box/alter.cc > index 4f2e34bf0..22d3040ef 100644 > --- a/src/box/alter.cc > +++ b/src/box/alter.cc > @@ -2926,6 +2926,12 @@ on_replace_dd_func(struct trigger * /* trigger = */, void *event) > (unsigned) old_func->def->uid, > "function has references"); > } > + /* Can't' drop a builtin function. */ > + if (old_func->def->language =3D=3D = FUNC_LANGUAGE_SQL_BUILTIN) { > + tnt_raise(ClientError, ER_DROP_FUNCTION, > + (unsigned) old_func->def->uid, > + "function is SQL builtin=E2=80=9D); Nit: =E2=80=A6SQL built-in.