From: larrybr Date: Thu, 30 Sep 2021 17:20:23 +0000 (+0000) Subject: Simplify windowed group_concat() by allowing a change in undocumented behavior (fails... X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ed2bfb4d7646ff9cd5626ad263e7725baa0c649a;p=thirdparty%2Fsqlite.git Simplify windowed group_concat() by allowing a change in undocumented behavior (fails some new tests) FossilOrigin-Name: 9d8e6167380cdb8520e4893e36862450b8bbe990f33668b5546dec32eab41df9 --- ed2bfb4d7646ff9cd5626ad263e7725baa0c649a diff --cc manifest index 6f2d72cf83,a26921d663..ec727c3211 --- a/manifest +++ b/manifest @@@ -1,5 -1,5 +1,5 @@@ - C Sync\sw/trunk - D 2021-09-29T16:35:14.868 -C Update\sa\stest\scase\sin\srtreedoc.test\sto\saccount\sfor\sthe\sfact\sthat\srelease\sbuilds\sgenerate\sfewer\sVM\sinstructions\sthan\sdebug\sbuilds. -D 2021-09-30T10:47:10.633 ++C Simplify\swindowed\sgroup_concat()\sby\sallowing\sa\schange\sin\sundocumented\sbehavior\s(fails\ssome\snew\stests) ++D 2021-09-30T17:20:23.080 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@@ -497,12 -499,12 +499,12 @@@ F src/complete.c a3634ab1e687055cd002e1 F src/ctime.c 8159d5f706551861c18ec6c8f6bdf105e15ea00367f05d9ab65d31a1077facc1 F src/date.c e0632f335952b32401482d099321bbf12716b29d6e72836b53ae49683ebae4bf F src/dbpage.c 8a01e865bf8bc6d7b1844b4314443a6436c07c3efe1d488ed89e81719047833a - F src/dbstat.c bea044cfe99eab6c527837e196a5335c128989bdb354cf1b4973b85ea561d66b + F src/dbstat.c 861e08690fcb0f2ee1165eff0060ea8d4f3e2ea10f80dab7d32ad70443a6ff2d F src/delete.c 3ce6af6b64c8b476de51ccc32da0cb3142d42e65754e1d8118addf65b8bcba15 - F src/expr.c f2e0f5dd07d1b202f700f26b0851f2ea485e36ec8f335b05aec2cd91cd08853f + F src/expr.c 82797e5d82422d34ede9a95ba459f40c317b2daadb21109a21abfd42f84e3ed8 F src/fault.c 460f3e55994363812d9d60844b2a6de88826e007 F src/fkey.c 1905af1821b88321e1bb9d6a69e704495b6844a9b6c29398d40117cc251e893c - F src/func.c c852d68d0a984263f969e3b84d602c6d821147a32905fecca65b1d86098367b4 -F src/func.c 812ac5383067bed7150d8597e83c47b714d73db0e62af55811d1a145243e58e1 ++F src/func.c 0fc0672cc663e31aab57c2d42e820c394f3a987a296734821583adecd7dab108 F src/global.c 612ea60c9acbcb45754c2ed659b4a56936a06814718e969636fedc7e3b889808 F src/hash.c 8d7dda241d0ebdafb6ffdeda3149a412d7df75102cecfc1021c98d6219823b19 F src/hash.h 9d56a9079d523b648774c1784b74b89bd93fac7b365210157482e4319a468f38 @@@ -1797,11 -1799,10 +1799,11 @@@ F test/window8.tcl 5e02e41d9d9a80f59706 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/windowC.test ecf1831b995408b03f708386b37ece7a05108faf2288c0c55cff873c100e145f ++F test/windowC.test 1e290a18ffaf406b6da7a7be90a4ec17cc39624413055274406b4a4dece019fb F test/windowerr.tcl f5acd6fbc210d7b5546c0e879d157888455cd4a17a1d3f28f07c1c8a387019e0 F test/windowerr.test a8b752402109c15aa1c5efe1b93ccb0ce1ef84fa964ae1cd6684dd0b3cc1819b -F test/windowfault.test 21919e601f20b976ea2a73aa401220c89ed0e8d203c4f69476ea55bce3726496 +F test/windowfault.test 15094c1529424e62f798bc679e3fe9dfab6e8ba2f7dfe8c923b6248c31660a7c F test/windowpushd.test d8895d08870b7226f7693665bd292eb177e62ca06799184957b3ca7dc03067df F test/with1.test 7bc5abfe4c80c0cef8a90f5a66d60b9982e8ccd7350c8eb70611323a3b8e07ba F test/with2.test f803743b2c746ecdd0b638783c7235654b947b0f1c4bb551ca10e1d813317153 @@@ -1927,7 -1928,7 +1929,7 @@@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a9 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 - P 3d148615f9d9c6a3d63d8eb015f3d70f453a66de49b28e665831254387c700b9 df0d7e36dbf98ab5405d8366ce92fb85176d4388b47a57b0ca1aa1ba6ae5212e - R 4764117e20f212be49211212953ddad0 -P 5d771f3554f3c98872cd0c9f12f415e685f26fcb923e3fb7f1a7a760c7a53255 -R 7836a39a0844715bc15a7a735276f745 -U dan -Z 6f61e641e71e433b9c88547a5dd22a12 ++P a4c18b2f0ce4a0f4d0c4f4c25dc69fbed4cb4876d2b69e3e5e0e756410892d74 7d16b302826fec3606dbc6e20df0d2182f6946a2ed4076d2412d1df30c552ecb ++R 7a3fb3a8eabd209987f42e1233885851 +U larrybr - Z 559da611469829addf62248db5ff3864 ++Z 2579a77f1b00e3b217e06a6d591fd46d diff --cc manifest.uuid index 43f8a3d717,06ada9e62a..394b3ec54a --- a/manifest.uuid +++ b/manifest.uuid @@@ -1,1 -1,1 +1,1 @@@ - a4c18b2f0ce4a0f4d0c4f4c25dc69fbed4cb4876d2b69e3e5e0e756410892d74 -7d16b302826fec3606dbc6e20df0d2182f6946a2ed4076d2412d1df30c552ecb ++9d8e6167380cdb8520e4893e36862450b8bbe990f33668b5546dec32eab41df9 diff --cc src/func.c index d348aa6574,b47378a3be..e6dbdf75c7 --- a/src/func.c +++ b/src/func.c @@@ -1716,20 -1716,6 +1716,11 @@@ static void minMaxFinalize(sqlite3_cont /* ** 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 ++ int nLastSepLength; /* Length of last-appended separator*/ +} GroupConcatCtx; + static void groupConcatStep( sqlite3_context *context, int argc, @@@ -1741,116 -1727,71 +1732,77 @@@ 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 ){ - sqlite3 *db = sqlite3_context_db_handle(context); - int firstTerm = pAccum->mxAlloc==0; - pAccum->mxAlloc = db->aLimit[SQLITE_LIMIT_LENGTH]; - if( !firstTerm ){ - if( argc==2 ){ - zSep = (char*)sqlite3_value_text(argv[1]); - nSep = sqlite3_value_bytes(argv[1]); - }else{ - zSep = ","; - nSep = 1; - } - if( zSep ) sqlite3_str_append(pAccum, zSep, nSep); + pGCC = (GroupConcatCtx*)sqlite3_aggregate_context(context, sizeof(*pGCC)); + if( pGCC ){ - sqlite3 *db = sqlite3_context_db_handle(context); - 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]); - nSep = sqlite3_value_bytes(argv[1]); - }else{ - zSep = ","; - nSep = 1; - } - 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; ++ if( argc==2 ){ ++ zSep = (char*)sqlite3_value_text(argv[1]); ++ nSep = sqlite3_value_bytes(argv[1]); ++ }else{ ++ zSep = ","; ++ nSep = 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( pGCC->str.mxAlloc==0 ){ ++ sqlite3 *db = sqlite3_context_db_handle(context); ++ pGCC->str.mxAlloc = db->aLimit[SQLITE_LIMIT_LENGTH]; ++ } + if( zVal ) sqlite3_str_append(&pGCC->str, zVal, nVal); ++ if( zSep ) sqlite3_str_append(&pGCC->str, zSep, nSep); ++ else nSep = 0; ++ pGCC->nLastSepLength = nSep; } } + #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{ - /* If removing single accumulated string, harmlessly over-do. */ - nVS += pGCC->nFirstSepLength; - } ++ nVS += (argc==2)? sqlite3_value_bytes(argv[1]) : 1; + if( nVS>=(int)pGCC->str.nChar ){ + pGCC->str.nChar = 0; }else{ - n++; + pGCC->str.nChar -= nVS; + memmove(pGCC->str.zText, &pGCC->str.zText[nVS], pGCC->str.nChar); } - if( n>=(int)pAccum->nChar ){ - pAccum->nChar = 0; - }else{ - pAccum->nChar -= n; - memmove(pAccum->zText, &pAccum->zText[n], pAccum->nChar); + if( pGCC->str.nChar==0 ){ + pGCC->str.mxAlloc = 0; - sqlite3_free(pGCC->pnSepLengths); - pGCC->pnSepLengths = 0; ++ pGCC->nLastSepLength = 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 ){ - if( pAccum->accError==SQLITE_TOOBIG ){ - sqlite3_result_error_toobig(context); - }else if( pAccum->accError==SQLITE_NOMEM ){ - sqlite3_result_error_nomem(context); - }else{ - sqlite3_result_text(context, sqlite3StrAccumFinish(pAccum), -1, - sqlite3_free); + 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 ){ - sqlite3_result_error_nomem(context); - }else{ - sqlite3_result_text(context, sqlite3StrAccumFinish(pAccum), -1, - sqlite3_free); ++ switch( pGCC->str.accError ){ ++ case SQLITE_TOOBIG: ++ sqlite3_result_error_toobig(context); ++ break; ++ case SQLITE_NOMEM: ++ sqlite3_result_error_nomem(context); ++ break; ++ default:{ ++ int nc = pGCC->str.nChar - pGCC->nLastSepLength; ++ assert(nc >= 0); ++ pGCC->str.nChar = nc; ++ sqlite3_result_text(context, sqlite3StrAccumFinish(&pGCC->str), ++ nc, sqlite3_free); ++ } } - #ifndef SQLITE_OMIT_WINDOWFUNC - sqlite3_free(pGCC->pnSepLengths); - #endif } } #ifndef SQLITE_OMIT_WINDOWFUNC @@@ -1865,7 -1805,7 +1817,9 @@@ static void groupConcatValue(sqlite3_co sqlite3_result_error_nomem(context); }else{ const char *zText = sqlite3_str_value(pAccum); -- sqlite3_result_text(context, zText, -1, SQLITE_TRANSIENT); ++ int nc = pGCC->str.nChar - pGCC->nLastSepLength; ++ assert(nc >= 0); ++ sqlite3_result_text(context, zText, nc, SQLITE_TRANSIENT); } } } diff --cc test/windowC.test index 54eb7cadc4,0000000000..8c04501208 mode 100644,000000..100644 --- a/test/windowC.test +++ b/test/windowC.test @@@ -1,66 -1,0 +1,66 @@@ +# 2021-09-29 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# Test cases for varying separator handling by group_concat(). +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl - set testprefix windowB ++set testprefix windowC + +ifcapable !windowfunc { + finish_test + return +} + +do_execsql_test 1.0 { + CREATE TABLE x1(i INTEGER PRIMARY KEY, x); +} + +foreach {tn bBlob seps} { + 1 0 {a b c def g} + 2 0 {abcdefg {} {} abcdefg} + 3 0 {a bc def ghij klmno pqrstu} + 4 1 {a bc def ghij klmno pqrstu} + 5 1 {, , , , , , , , , , , , ....... , ,} +} { + foreach type {text blob} { + do_test 1.$type.$tn.1 { + execsql { DELETE FROM x1 } + foreach s $seps { + if {$type=="text"} { + execsql {INSERT INTO x1 VALUES(NULL, $s)} + } else { + execsql {INSERT INTO x1 VALUES(NULL, CAST ($s AS blob))} + } + } + } {} + + foreach {tn2 win} { + 1 "ROWS BETWEEN 1 PRECEDING AND 1 FOLLOWING" + 2 "ROWS BETWEEN 2 PRECEDING AND CURRENT ROW" + 3 "ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING" + } { + do_test 1.$type.$tn.2.$tn2 { + db eval " + SELECT group_concat('val', x) OVER ( ORDER BY i $win ) AS val FROM x1 + " { + if {[string range $val 0 2]!="val" + || [string range $val end-2 end]!="val" + } { + error "unexpected return value: $val" + } + } + } {} + } + } +} + +finish_test