Get group_concat() to handle varying separator lengths when windowing

FossilOrigin-Name: 98e0f2bf67cdee1da1edadeb54ff8564728b3f28fc821e46e8de201247c3fc87

manifest

@@ -1,5 +1,5 @@
-C Load\srecent\sdbsqlfuzz\scases\sinto\stest/fuzzdata8.db.
+C Get\sgroup_concat()\sto\shandle\svarying\sseparator\slengths\swhen\swindowing
-D 2021-09-25T20:28:39.895
+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
+P 7a8fcf6d2c8e3c8f10ff515c8c00c761d15a28eef8e0e31e09e22feb06c9443b
-R 29fc8916ca6b5a432d2826152a802182
+R f06fcf734c6cb0151864e67091ca6f93
-U drh
+T *branch * group_concat_varsep
-Z 2df13617e37950dc266054e33266713a
+T *sym-group_concat_varsep *
+T -sym-trunk *
+U larrybr
+Z a3e2174302f0231fe7e552f570c10b10

manifest.uuid

@@ -1 +1 @@
-7a8fcf6d2c8e3c8f10ff515c8c00c761d15a28eef8e0e31e09e22feb06c9443b
+98e0f2bf67cdee1da1edadeb54ff8564728b3f28fc821e46e8de201247c3fc87

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));
+  pGCC = (GroupConcatCtx*)sqlite3_aggregate_context(context, sizeof(*pGCC));
-
+  if( pGCC ){
-  if( pAccum ){
     sqlite3 *db = sqlite3_context_db_handle(context);
-    int firstTerm = pAccum->mxAlloc==0;
+    int firstTerm = pGCC->str.mxAlloc==0;
-    pAccum->mxAlloc = db->aLimit[SQLITE_LIMIT_LENGTH];
+    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( i<nA ) pnsl[i++] = pGCC->nFirstSepLength;
 	  }
+	}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;
+  GroupConcatCtx *pGCC;
-  StrAccum *pAccum;
   assert( argc==1 || argc==2 );
   if( sqlite3_value_type(argv[0])==SQLITE_NULL ) return;
-  pAccum = (StrAccum*)sqlite3_aggregate_context(context, sizeof(*pAccum));
+  pGCC = (GroupConcatCtx*)sqlite3_aggregate_context(context, sizeof(*pGCC));
-  /* pAccum is always non-NULL since groupConcatStep() will have always
+  /* pGCC is always non-NULL since groupConcatStep() will have always
   ** run frist to initialize it */
-  if( ALWAYS(pAccum) ){
+  if( ALWAYS(pGCC) ){
-    n = sqlite3_value_bytes(argv[0]);
+    int nVS = sqlite3_value_bytes(argv[0]);
-    if( argc==2 ){
+    pGCC->nAccum -= 1;
-      n += sqlite3_value_bytes(argv[1]);
+    if( pGCC->pnSepLengths!=0 ){
-    }else{
+      assert(pGCC->nAccum >= 0);
-      n++;
+      if( pGCC->nAccum>0 ){
+	nVS += *pGCC->pnSepLengths;
+	memmove(pGCC->pnSepLengths, pGCC->pnSepLengths+1,
+		(pGCC->nAccum-1)*sizeof(int));
       }
-    if( n>=(int)pAccum->nChar ){
-      pAccum->nChar = 0;
     }else{
-      pAccum->nChar -= n;
+      /* If removing single accumulated string, harmlessly over-do. */
-      memmove(pAccum->zText, &pAccum->zText[n], pAccum->nChar);
+      nVS += pGCC->nFirstSepLength;
+    }
+    if( nVS>=(int)pGCC->str.nChar ){
+      pGCC->str.nChar = 0;
+    }else{
+      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;
+  GroupConcatCtx *pGCC
-  pAccum = sqlite3_aggregate_context(context, 0);
+    = (GroupConcatCtx*)sqlite3_aggregate_context(context, 0);
-  if( pAccum ){
+  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;
+  GroupConcatCtx *pGCC
-  pAccum = (sqlite3_str*)sqlite3_aggregate_context(context, 0);
+    = (GroupConcatCtx*)sqlite3_aggregate_context(context, 0);
-  if( pAccum ){
+  if( pGCC ){
+    StrAccum *pAccum = &pGCC->str;
     if( pAccum->accError==SQLITE_TOOBIG ){
       sqlite3_result_error_toobig(context);
     }else if( pAccum->accError==SQLITE_NOMEM ){

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