From dde13e6f887953e5d5d3273fc264da3532b62c25 Mon Sep 17 00:00:00 2001 From: larrybr Date: Wed, 29 Sep 2021 00:32:13 +0000 Subject: [PATCH] Get group_concat() to handle varying separator lengths when windowing FossilOrigin-Name: 98e0f2bf67cdee1da1edadeb54ff8564728b3f28fc821e46e8de201247c3fc87 --- manifest | 19 ++++---- manifest.uuid | 2 +- src/func.c | 116 +++++++++++++++++++++++++++++++++++----------- test/windowB.test | 8 ++++ 4 files changed, 108 insertions(+), 37 deletions(-) diff --git a/manifest b/manifest index fdeb8b705e..6203b9826a 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Load\srecent\sdbsqlfuzz\scases\sinto\stest/fuzzdata8.db. -D 2021-09-25T20:28:39.895 +C Get\sgroup_concat()\sto\shandle\svarying\sseparator\slengths\swhen\swindowing +D 2021-09-29T00:32:13.146 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -502,7 +502,7 @@ F src/delete.c 3ce6af6b64c8b476de51ccc32da0cb3142d42e65754e1d8118addf65b8bcba15 F src/expr.c 38597afb008db2e0a5f86a82827567acb98f502cab61663ef16bc688bc256803 F src/fault.c 460f3e55994363812d9d60844b2a6de88826e007 F src/fkey.c 1905af1821b88321e1bb9d6a69e704495b6844a9b6c29398d40117cc251e893c -F src/func.c 812ac5383067bed7150d8597e83c47b714d73db0e62af55811d1a145243e58e1 +F src/func.c c852d68d0a984263f969e3b84d602c6d821147a32905fecca65b1d86098367b4 F src/global.c 612ea60c9acbcb45754c2ed659b4a56936a06814718e969636fedc7e3b889808 F src/hash.c 8d7dda241d0ebdafb6ffdeda3149a412d7df75102cecfc1021c98d6219823b19 F src/hash.h 9d56a9079d523b648774c1784b74b89bd93fac7b365210157482e4319a468f38 @@ -1797,7 +1797,7 @@ F test/window8.tcl 5e02e41d9d9a80f597063aed1a381eb19d1d0ef677a4f0df352c5365cf23f F test/window8.test 4ab16817414af0c904abe2ebdf88eb6c2b00058b84f9748c6174ff11fc45f1ed F test/window9.test 349c71eab4288a1ffc19e2f65872ec2c37e6cf8a1dda2ad300364b7450ae4836 F test/windowA.test 6d63dc1260daa17141a55007600581778523a8b420629f1282d2acfc36af23be -F test/windowB.test 6e601f8178ba8ba28b2f19e74fe613815084bb4a8d2ad942defc7d42e191e521 +F test/windowB.test b67bda5645f3226790e1a360c4225241840b84adb5aa2e69bfb0b27eef3b84d9 F test/windowerr.tcl f5acd6fbc210d7b5546c0e879d157888455cd4a17a1d3f28f07c1c8a387019e0 F test/windowerr.test a8b752402109c15aa1c5efe1b93ccb0ce1ef84fa964ae1cd6684dd0b3cc1819b F test/windowfault.test 21919e601f20b976ea2a73aa401220c89ed0e8d203c4f69476ea55bce3726496 @@ -1926,7 +1926,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 706322c2b5bb31e14c1120a94520b21fa623ff119e3890170e36b37d8bde721a -R 29fc8916ca6b5a432d2826152a802182 -U drh -Z 2df13617e37950dc266054e33266713a +P 7a8fcf6d2c8e3c8f10ff515c8c00c761d15a28eef8e0e31e09e22feb06c9443b +R f06fcf734c6cb0151864e67091ca6f93 +T *branch * group_concat_varsep +T *sym-group_concat_varsep * +T -sym-trunk * +U larrybr +Z a3e2174302f0231fe7e552f570c10b10 diff --git a/manifest.uuid b/manifest.uuid index 57f5790ed6..90cedf20e5 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -7a8fcf6d2c8e3c8f10ff515c8c00c761d15a28eef8e0e31e09e22feb06c9443b \ No newline at end of file +98e0f2bf67cdee1da1edadeb54ff8564728b3f28fc821e46e8de201247c3fc87 \ No newline at end of file diff --git a/src/func.c b/src/func.c index b47378a3be..d348aa6574 100644 --- a/src/func.c +++ b/src/func.c @@ -1716,23 +1716,36 @@ static void minMaxFinalize(sqlite3_context *context){ /* ** group_concat(EXPR, ?SEPARATOR?) */ +typedef struct { + StrAccum str; /* The accumulated concatenation */ +#ifndef SQLITE_OMIT_WINDOWFUNC + int nAccum; /* Number of strings presently concatenated */ + int nFirstSepLength; /* Used to detect separator length change */ + /* If pnSepLengths!=0, refs an array of inter-string separator lengths, + * stored as actually incorporated into presently accumulated result. + * (Hence, its slots in use number nAccum-1 between method calls.) + * If pnSepLengths==0, nFirstSepLength is the length used throughout. + */ + int *pnSepLengths; +#endif +} GroupConcatCtx; + static void groupConcatStep( sqlite3_context *context, int argc, sqlite3_value **argv ){ const char *zVal; - StrAccum *pAccum; + GroupConcatCtx *pGCC; const char *zSep; int nVal, nSep; assert( argc==1 || argc==2 ); if( sqlite3_value_type(argv[0])==SQLITE_NULL ) return; - pAccum = (StrAccum*)sqlite3_aggregate_context(context, sizeof(*pAccum)); - - if( pAccum ){ + pGCC = (GroupConcatCtx*)sqlite3_aggregate_context(context, sizeof(*pGCC)); + if( pGCC ){ sqlite3 *db = sqlite3_context_db_handle(context); - int firstTerm = pAccum->mxAlloc==0; - pAccum->mxAlloc = db->aLimit[SQLITE_LIMIT_LENGTH]; + int firstTerm = pGCC->str.mxAlloc==0; + pGCC->str.mxAlloc = db->aLimit[SQLITE_LIMIT_LENGTH]; if( !firstTerm ){ if( argc==2 ){ zSep = (char*)sqlite3_value_text(argv[1]); @@ -1741,49 +1754,92 @@ static void groupConcatStep( zSep = ","; nSep = 1; } - if( zSep ) sqlite3_str_append(pAccum, zSep, nSep); + if( zSep ) + sqlite3_str_append(&pGCC->str, zSep, nSep); +#ifndef SQLITE_OMIT_WINDOWFUNC + else + nSep = 0; + if( nSep != pGCC->nFirstSepLength || pGCC->pnSepLengths != 0 ){ + int * pnsl = pGCC->pnSepLengths; + if( pnsl == 0 ){ + /* First separator length variation seen, start tracking them. */ + pnsl = (int*)sqlite3_malloc64((pGCC->nAccum+1) * sizeof(int)); + if( pnsl!=0 ){ + int i = 0, nA = pGCC->nAccum-1; + while( inFirstSepLength; + } + }else{ + pnsl = (int*)sqlite3_realloc64(pnsl, pGCC->nAccum * sizeof(int)); + } + if( pnsl!=0 ){ + if( pGCC->nAccum>0 ) + pnsl[pGCC->nAccum-1] = nSep; + pGCC->pnSepLengths = pnsl; + }else{ + setStrAccumError(&pGCC->str, SQLITE_NOMEM); + } + } +#endif } +#ifndef SQLITE_OMIT_WINDOWFUNC + else{ + pGCC->nFirstSepLength = (argc==2)? sqlite3_value_bytes(argv[1]) : 1; + } + pGCC->nAccum += 1; +#endif zVal = (char*)sqlite3_value_text(argv[0]); nVal = sqlite3_value_bytes(argv[0]); - if( zVal ) sqlite3_str_append(pAccum, zVal, nVal); + if( zVal ) sqlite3_str_append(&pGCC->str, zVal, nVal); } } + #ifndef SQLITE_OMIT_WINDOWFUNC static void groupConcatInverse( sqlite3_context *context, int argc, sqlite3_value **argv ){ - int n; - StrAccum *pAccum; + GroupConcatCtx *pGCC; assert( argc==1 || argc==2 ); if( sqlite3_value_type(argv[0])==SQLITE_NULL ) return; - pAccum = (StrAccum*)sqlite3_aggregate_context(context, sizeof(*pAccum)); - /* pAccum is always non-NULL since groupConcatStep() will have always + pGCC = (GroupConcatCtx*)sqlite3_aggregate_context(context, sizeof(*pGCC)); + /* pGCC is always non-NULL since groupConcatStep() will have always ** run frist to initialize it */ - if( ALWAYS(pAccum) ){ - n = sqlite3_value_bytes(argv[0]); - if( argc==2 ){ - n += sqlite3_value_bytes(argv[1]); + if( ALWAYS(pGCC) ){ + int nVS = sqlite3_value_bytes(argv[0]); + pGCC->nAccum -= 1; + if( pGCC->pnSepLengths!=0 ){ + assert(pGCC->nAccum >= 0); + if( pGCC->nAccum>0 ){ + nVS += *pGCC->pnSepLengths; + memmove(pGCC->pnSepLengths, pGCC->pnSepLengths+1, + (pGCC->nAccum-1)*sizeof(int)); + } }else{ - n++; + /* If removing single accumulated string, harmlessly over-do. */ + nVS += pGCC->nFirstSepLength; } - if( n>=(int)pAccum->nChar ){ - pAccum->nChar = 0; + if( nVS>=(int)pGCC->str.nChar ){ + pGCC->str.nChar = 0; }else{ - pAccum->nChar -= n; - memmove(pAccum->zText, &pAccum->zText[n], pAccum->nChar); + pGCC->str.nChar -= nVS; + memmove(pGCC->str.zText, &pGCC->str.zText[nVS], pGCC->str.nChar); + } + if( pGCC->str.nChar==0 ){ + pGCC->str.mxAlloc = 0; + sqlite3_free(pGCC->pnSepLengths); + pGCC->pnSepLengths = 0; } - if( pAccum->nChar==0 ) pAccum->mxAlloc = 0; } } #else # define groupConcatInverse 0 #endif /* SQLITE_OMIT_WINDOWFUNC */ static void groupConcatFinalize(sqlite3_context *context){ - StrAccum *pAccum; - pAccum = sqlite3_aggregate_context(context, 0); - if( pAccum ){ + GroupConcatCtx *pGCC + = (GroupConcatCtx*)sqlite3_aggregate_context(context, 0); + if( pGCC ){ + StrAccum *pAccum = &pGCC->str; if( pAccum->accError==SQLITE_TOOBIG ){ sqlite3_result_error_toobig(context); }else if( pAccum->accError==SQLITE_NOMEM ){ @@ -1792,13 +1848,17 @@ static void groupConcatFinalize(sqlite3_context *context){ sqlite3_result_text(context, sqlite3StrAccumFinish(pAccum), -1, sqlite3_free); } +#ifndef SQLITE_OMIT_WINDOWFUNC + sqlite3_free(pGCC->pnSepLengths); +#endif } } #ifndef SQLITE_OMIT_WINDOWFUNC static void groupConcatValue(sqlite3_context *context){ - sqlite3_str *pAccum; - pAccum = (sqlite3_str*)sqlite3_aggregate_context(context, 0); - if( pAccum ){ + GroupConcatCtx *pGCC + = (GroupConcatCtx*)sqlite3_aggregate_context(context, 0); + if( pGCC ){ + StrAccum *pAccum = &pGCC->str; if( pAccum->accError==SQLITE_TOOBIG ){ sqlite3_result_error_toobig(context); }else if( pAccum->accError==SQLITE_NOMEM ){ diff --git a/test/windowB.test b/test/windowB.test index 30380aee20..52221c445c 100644 --- a/test/windowB.test +++ b/test/windowB.test @@ -9,6 +9,7 @@ # #*********************************************************************** # Test cases for RANGE BETWEEN and especially with NULLS LAST +# and for varying separator handling by group_concat(). # set testdir [file dirname $argv0] @@ -356,5 +357,12 @@ do_execsql_test 8.1 { FROM t1; } {111 660 938 979} +do_execsql_test 9.0 { + CREATE TABLE seps(x); + INSERT INTO seps(x) VALUES ('1'), ('22'), ('333'), ('4444'); + SELECT group_concat('-', x) + OVER ( ORDER BY x ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING ) + FROM seps; +} {-22- -22-333- -333-4444- -4444-} finish_test