From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v3 12/15] sql: refactor GROUP_CONCAT() function Date: Fri, 1 Oct 2021 10:43:15 +0300 [thread overview] Message-ID: <afd34df22db96b82cbdfed5a186d974b4abbdd20.1633073760.git.imeevma@gmail.com> (raw) In-Reply-To: <cover.1633073759.git.imeevma@gmail.com> This patch does some refactoring for the GROUP_CONCAT() function. He also adds one test to make sure the behavior hasn't changed after refactoring. Part of #4145 --- src/box/sql/func.c | 130 +++++++++++++++++-------------------- test/sql-tap/func.test.lua | 15 +++-- 2 files changed, 68 insertions(+), 77 deletions(-) diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 5c6d38d08..93181e89c 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -217,6 +217,61 @@ fin_minmax(struct sql_context *ctx) mem_copy(ctx->pOut, ctx->pMem); } +/** Implementation of the GROUP_CONCAT() function. */ +static void +step_group_concat(struct sql_context *ctx, int argc, struct Mem **argv) +{ + assert(argc == 1 || argc == 2); + (void)argc; + if (mem_is_null(argv[0])) + return; + assert(mem_is_str(argv[0]) || mem_is_bin(argv[0])); + if (mem_is_null(ctx->pMem)) { + if (mem_copy(ctx->pMem, argv[0]) != 0) + ctx->is_aborted = true; + return; + } + assert(!mem_is_zerobin(ctx->pMem)); + const char *sep = NULL; + int sep_len = 0; + if (argc == 1) { + sep = ","; + sep_len = 1; + } else if (mem_is_null(argv[1])) { + sep = ""; + sep_len = 0; + } else { + assert(mem_is_same_type(argv[0], argv[1])); + sep = argv[1]->z; + sep_len = argv[1]->n; + } + if (mem_append(ctx->pMem, sep, sep_len) != 0) { + ctx->is_aborted = true; + return; + } + uint32_t size; + char *str; + if (mem_is_zerobin(argv[0])) { + size = argv[0]->u.nZero; + str = sqlDbMallocRawNN(sql_get(), size); + memset(str, 0, size); + } else { + size = argv[0]->n; + str = argv[0]->z; + } + if (mem_append(ctx->pMem, str, size) != 0) + ctx->is_aborted = true; + if (mem_is_zerobin(argv[0])) + sqlDbFree(sql_get(), str); +} + +/** Finalizer for the GROUP_CONCAT() function. */ +static void +fin_group_concat(struct sql_context *ctx) +{ + mem_copy(ctx->pOut, ctx->pMem); +} + static const unsigned char * mem_as_ustr(struct Mem *mem) { @@ -1761,73 +1816,6 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv) } } -/* - * group_concat(EXPR, ?SEPARATOR?) - */ -static void -groupConcatStep(sql_context * context, int argc, sql_value ** argv) -{ - const char *zVal; - StrAccum *pAccum; - const char *zSep; - int nVal, nSep; - if (argc != 1 && argc != 2) { - diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, - "GROUP_CONCAT", "1 or 2", argc); - context->is_aborted = true; - return; - } - if (mem_is_null(argv[0])) - return; - pAccum = - (StrAccum *) sql_aggregate_context(context, sizeof(*pAccum)); - - if (pAccum) { - sql *db = sql_context_db_handle(context); - int firstTerm = pAccum->mxAlloc == 0; - pAccum->mxAlloc = db->aLimit[SQL_LIMIT_LENGTH]; - if (!firstTerm) { - if (argc == 2) { - zSep = mem_as_str0(argv[1]); - nSep = mem_len_unsafe(argv[1]); - } else { - zSep = ","; - nSep = 1; - } - if (zSep) - sqlStrAccumAppend(pAccum, zSep, nSep); - } - zVal = mem_as_str0(argv[0]); - nVal = mem_len_unsafe(argv[0]); - if (zVal) - sqlStrAccumAppend(pAccum, zVal, nVal); - } -} - -static void -groupConcatFinalize(sql_context * context) -{ - StrAccum *pAccum; - pAccum = sql_aggregate_context(context, 0); - if (pAccum) { - if (pAccum->accError == STRACCUM_TOOBIG) { - diag_set(ClientError, ER_SQL_EXECUTE, "string or binary"\ - "string is too big"); - context->is_aborted = true; - } else if (pAccum->accError == STRACCUM_NOMEM) { - context->is_aborted = true; - } else { - char *str = sqlStrAccumFinish(pAccum); - int len = pAccum->nChar; - assert(len >= 0); - if (context->func->def->returns == FIELD_TYPE_STRING) - mem_set_str_dynamic(context->pOut, str, len); - else - mem_set_bin_dynamic(context->pOut, str, len); - } - } -} - int sql_is_like_func(struct Expr *expr) { @@ -1994,13 +1982,13 @@ static struct sql_func_definition definitions[] = { NULL}, {"GROUP_CONCAT", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, - groupConcatStep, groupConcatFinalize}, + step_group_concat, fin_group_concat}, {"GROUP_CONCAT", 2, {FIELD_TYPE_STRING, FIELD_TYPE_STRING}, - FIELD_TYPE_STRING, groupConcatStep, groupConcatFinalize}, + FIELD_TYPE_STRING, step_group_concat, fin_group_concat}, {"GROUP_CONCAT", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_VARBINARY, - groupConcatStep, groupConcatFinalize}, + step_group_concat, fin_group_concat}, {"GROUP_CONCAT", 2, {FIELD_TYPE_VARBINARY, FIELD_TYPE_VARBINARY}, - FIELD_TYPE_VARBINARY, groupConcatStep, groupConcatFinalize}, + FIELD_TYPE_VARBINARY, step_group_concat, fin_group_concat}, {"HEX", 1, {FIELD_TYPE_VARBINARY}, FIELD_TYPE_STRING, hexFunc, NULL}, {"IFNULL", 2, {FIELD_TYPE_ANY, FIELD_TYPE_ANY}, FIELD_TYPE_SCALAR, diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua index 416f27d69..bd8a8fe78 100755 --- a/test/sql-tap/func.test.lua +++ b/test/sql-tap/func.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool local test = require("sqltester") -test:plan(14680) +test:plan(14681) --!./tcltestrunner.lua -- 2001 September 15 @@ -2142,11 +2142,14 @@ test:do_execsql_test( -- </func-24.2> }) --- do_test func-24.3 { --- execsql { --- SELECT group_concat(t1,' ' || rowid || ' ') FROM tbl1 --- } --- } {{this 2 program 3 is 4 free 5 software}} +test:do_execsql_test( + "func-24.3", + [[ + SELECT group_concat(zeroblob(10)); + ]], { + '\0\0\0\0\0\0\0\0\0\0' + }) + test:do_execsql_test( "func-24.4", [[ -- 2.25.1
next prev parent reply other threads:[~2021-10-01 7:50 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-01 7:42 [Tarantool-patches] [PATCH v3 00/15] sql: refactor aggregate functions Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 01/15] sql: fix possible undefined behavior during cast Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 02/15] sql: use register P1 for number of arguments Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 03/15] sql: remove AggStep0 and OP_BuiltinFunction0 Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 04/15] sql: move collation to struct sql_context Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 05/15] sql: introduce mem_append() Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 06/15] sql: remove sql_vdbemem_finalize() Mergen Imeev via Tarantool-patches 2021-10-01 7:42 ` [Tarantool-patches] [PATCH v3 07/15] sql: refactor SUM() function Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 08/15] sql: refactor TOTAL() function Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 09/15] sql: refactor AVG() function Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 10/15] sql: refactor COUNT() function Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 11/15] sql: refactor MIN() and MAX() functions Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` Mergen Imeev via Tarantool-patches [this message] 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 13/15] sql: remove copying of result in finalizers Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 14/15] sql: remove MEM_TYPE_AGG Mergen Imeev via Tarantool-patches 2021-10-01 7:43 ` [Tarantool-patches] [PATCH v3 15/15] sql: remove field argv from struct sql_context 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=afd34df22db96b82cbdfed5a186d974b4abbdd20.1633073760.git.imeevma@gmail.com \ --to=tarantool-patches@dev.tarantool.org \ --cc=imeevma@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v3 12/15] sql: refactor GROUP_CONCAT() function' \ /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