Revise generate_series() extension (in CLI) to address overflow reported in [forum:754e2d4db2a5|forum post #754e2d4db2a5] and to make behavior better match the like-named PostgreSQL function.

FossilOrigin-Name: beeea3e1b010dace9789f27172462b912819d0f8142a67e3e1e7335211e0e9a8

ext/misc/series.c

@@ -1,5 +1,5 @@
 /*
-** 2015-08-18
+** 2015-08-18, 2023-04-28
 **
 ** The author disclaims copyright to this source code.  In place of
 ** a legal notice, here is a blessing:
@@ -12,7 +12,19 @@
 **
 ** This file demonstrates how to create a table-valued-function using
 ** a virtual table.  This demo implements the generate_series() function
-** which gives similar results to the eponymous function in PostgreSQL.
+** which gives the same results as the eponymous function in PostgreSQL,
+** within the limitation that its arguments are signed 64-bit integers.
+**
+** Considering its equivalents to generate_series(start,stop,step): A
+** value V[n] sequence is produced for integer n ascending from 0 where
+**  ( V[n] == start + n * step  &&  sgn(V[n] - stop) * sgn(step) >= 0 )
+** for each produced value (independent of production time ordering.)
+**
+** All parameters must be either integer or convertable to integer.
+** The start parameter is required.
+** The stop parameter defaults to (1<<32)-1 (aka 4294967295 or 0xffffffff)
+** The step parameter defaults to 1 and 0 is treated as 1.
+**
 ** Examples:
 **
 **      SELECT * FROM generate_series(0,100,5);
@@ -28,6 +40,14 @@
 **
 ** Integers 20 through 29.
 **
+**      SELECT * FROM generate_series(0,-100,-5);
+**
+** Integers 0 -5 -10 ... -100.
+**
+**      SELECT * FROM generate_series(0,-1);
+**
+** Empty sequence.
+**
 ** HOW IT WORKS
 **
 ** The generate_series "function" is really a virtual table with the
@@ -40,6 +60,9 @@
 **       step HIDDEN
 **     );
 **
+** The virtual table also has a rowid, logically equivalent to n+1 where
+** "n" is the ascending integer in the aforesaid production definition.
+**
 ** Function arguments in queries against this virtual table are translated
 ** into equality constraints against successive hidden columns.  In other
 ** words, the following pairs of queries are equivalent to each other:
@@ -72,9 +95,93 @@
 SQLITE_EXTENSION_INIT1
 #include <assert.h>
 #include <string.h>
+#include <limits.h>
 
 #ifndef SQLITE_OMIT_VIRTUALTABLE
+/*
+** Return that member of a generate_series(...) sequence whose 0-based
+** index is ix. The 0th member is given by smBase. The sequence members
+** progress per ix increment by smStep.
+*/
+static sqlite3_int64 genSeqMember(sqlite3_int64 smBase,
+                                  sqlite3_int64 smStep,
+                                  sqlite3_uint64 ix){
+  if( ix>=(sqlite3_uint64)LLONG_MAX ){
+    /* Get ix into signed i64 range. */
+    ix -= (sqlite3_uint64)LLONG_MAX;
+    smBase += LLONG_MAX * smStep;
+  }
+  return smBase + ((sqlite3_int64)ix)*smStep;
+}
 
+typedef unsigned char u8;
+
+typedef struct SequenceSpec {
+  sqlite3_int64 iBase;         /* Starting value ("start") */
+  sqlite3_int64 iTerm;         /* Given terminal value ("stop") */
+  sqlite3_int64 iStep;         /* Increment ("step") */
+  sqlite3_uint64 uMaxRowidM1;  /* maximum rowid minus 1 */
+  sqlite3_uint64 uRidCurrent;  /* Current rowid-1 during generation */
+  sqlite3_int64 iValueCurrent; /* Current value during generation */
+  u8 isNotEOF;                 /* Sequence generation not exhausted */
+  u8 isReversing;              /* Sequence is being reverse generated */
+} SequenceSpec;
+
+/*
+** Prepare a SequenceSpec for use in generating an integer series
+** given initialized iBase, iTerm and iStep values. Sequence is
+** initialized per given isReversing. Other members are computed.
+*/
+void setupSequence( SequenceSpec *pss ){
+  pss->uMaxRowidM1 = 0;
+  pss->isNotEOF = 0;
+  if( pss->iTerm < pss->iBase ){
+    sqlite3_uint64 nuspan = (sqlite3_uint64)(pss->iBase-pss->iTerm);
+    if( pss->iStep<0 ){
+      pss->isNotEOF = 1;
+      if( nuspan==ULONG_MAX ){
+        pss->uMaxRowidM1 = ( pss->iStep>LLONG_MIN )? nuspan/-pss->iStep : 1;
+      }else if( pss->iStep>LLONG_MIN ){
+        pss->uMaxRowidM1 = nuspan/-pss->iStep;
+      }
+    }
+  }else if( pss->iTerm > pss->iBase ){
+    sqlite3_uint64 puspan = (sqlite3_uint64)(pss->iTerm-pss->iBase);
+    if( pss->iStep>0 ){
+      pss->isNotEOF = 1;
+      pss->uMaxRowidM1 = puspan/pss->iStep;
+    }
+  }
+  pss->uRidCurrent = (pss->isReversing)? pss->uMaxRowidM1 : 0;
+  pss->iValueCurrent = (pss->isReversing)
+    ? genSeqMember(pss->iBase, pss->iStep, pss->uMaxRowidM1)
+    : pss->iBase;
+}
+
+/*
+** Progress sequence generator to yield next value, if any.
+** Leave its state to either yield next value or be at EOF.
+** Return whether there is a next value, or 0 at EOF.
+*/
+int progressSequence( SequenceSpec *pss ){
+  if( !pss->isNotEOF ) return 0;
+  if( pss->isReversing ){
+    if( pss->uRidCurrent > 0 ){
+      pss->uRidCurrent--;
+      pss->iValueCurrent -= pss->iStep;
+    }else{
+      pss->isNotEOF = 0;
+    }
+  }else{
+    if( pss->uRidCurrent < pss->uMaxRowidM1 ){
+      pss->uRidCurrent++;
+      pss->iValueCurrent += pss->iStep;
+    }else{
+      pss->isNotEOF = 0;
+    }
+  }
+  return pss->isNotEOF;
+}
 
 /* series_cursor is a subclass of sqlite3_vtab_cursor which will
 ** serve as the underlying representation of a cursor that scans
@@ -83,12 +190,7 @@ SQLITE_EXTENSION_INIT1
 typedef struct series_cursor series_cursor;
 struct series_cursor {
   sqlite3_vtab_cursor base;  /* Base class - must be first */
-  int isDesc;                /* True to count down rather than up */
+  SequenceSpec ss;           /* (this) Derived class data */
-  sqlite3_int64 iRowid;      /* The rowid */
-  sqlite3_int64 iValue;      /* Current value ("value") */
-  sqlite3_int64 mnValue;     /* Mimimum value ("start") */
-  sqlite3_int64 mxValue;     /* Maximum value ("stop") */
-  sqlite3_int64 iStep;       /* Increment ("step") */
 };
 
 /*
@@ -170,12 +272,7 @@ static int seriesClose(sqlite3_vtab_cursor *cur){
 */
 static int seriesNext(sqlite3_vtab_cursor *cur){
   series_cursor *pCur = (series_cursor*)cur;
-  if( pCur->isDesc ){
+  progressSequence( & pCur->ss );
-    pCur->iValue -= pCur->iStep;
-  }else{
-    pCur->iValue += pCur->iStep;
-  }
-  pCur->iRowid++;
   return SQLITE_OK;
 }
 
@@ -191,10 +288,10 @@ static int seriesColumn(
   series_cursor *pCur = (series_cursor*)cur;
   sqlite3_int64 x = 0;
   switch( i ){
-    case SERIES_COLUMN_START:  x = pCur->mnValue; break;
+    case SERIES_COLUMN_START:  x = pCur->ss.iBase; break;
-    case SERIES_COLUMN_STOP:   x = pCur->mxValue; break;
+    case SERIES_COLUMN_STOP:   x = pCur->ss.iTerm; break;
-    case SERIES_COLUMN_STEP:   x = pCur->iStep;   break;
+    case SERIES_COLUMN_STEP:   x = pCur->ss.iStep;   break;
-    default:                   x = pCur->iValue;  break;
+    default:                   x = pCur->ss.iValueCurrent;  break;
   }
   sqlite3_result_int64(ctx, x);
   return SQLITE_OK;
@@ -207,7 +304,7 @@ static int seriesColumn(
 */
 static int seriesRowid(sqlite3_vtab_cursor *cur, sqlite_int64 *pRowid){
   series_cursor *pCur = (series_cursor*)cur;
-  *pRowid = pCur->iRowid;
+  *pRowid = ((sqlite3_int64)pCur->ss.uRidCurrent + 1);
   return SQLITE_OK;
 }
 
@@ -217,14 +314,10 @@ static int seriesRowid(sqlite3_vtab_cursor *cur, sqlite_int64 *pRowid){
 */
 static int seriesEof(sqlite3_vtab_cursor *cur){
   series_cursor *pCur = (series_cursor*)cur;
-  if( pCur->isDesc ){
+  return !pCur->ss.isNotEOF;
-    return pCur->iValue < pCur->mnValue;
-  }else{
-    return pCur->iValue > pCur->mxValue;
-  }
 }
 
-/* True to cause run-time checking of the start=, stop=, and/or step= 
+/* True to cause run-time checking of the start=, stop=, and/or step=
 ** parameters.  The only reason to do this is for testing the
 ** constraint checking logic for virtual tables in the SQLite core.
 */
@@ -235,7 +328,7 @@ static int seriesEof(sqlite3_vtab_cursor *cur){
 /*
 ** This method is called to "rewind" the series_cursor object back
 ** to the first row of output.  This method is always called at least
-** once prior to any call to seriesColumn() or seriesRowid() or 
+** once prior to any call to seriesColumn() or seriesRowid() or
 ** seriesEof().
 **
 ** The query plan selected by seriesBestIndex is passed in the idxNum
@@ -255,7 +348,7 @@ static int seriesEof(sqlite3_vtab_cursor *cur){
 ** (so that seriesEof() will return true) if the table is empty.
 */
 static int seriesFilter(
-  sqlite3_vtab_cursor *pVtabCursor, 
+  sqlite3_vtab_cursor *pVtabCursor,
   int idxNum, const char *idxStrUnused,
   int argc, sqlite3_value **argv
 ){
@@ -263,46 +356,41 @@ static int seriesFilter(
   int i = 0;
   (void)idxStrUnused;
   if( idxNum & 1 ){
-    pCur->mnValue = sqlite3_value_int64(argv[i++]);
+    pCur->ss.iBase = sqlite3_value_int64(argv[i++]);
   }else{
-    pCur->mnValue = 0;
+    pCur->ss.iBase = 0;
   }
   if( idxNum & 2 ){
-    pCur->mxValue = sqlite3_value_int64(argv[i++]);
+    pCur->ss.iTerm = sqlite3_value_int64(argv[i++]);
   }else{
-    pCur->mxValue = 0xffffffff;
+    pCur->ss.iTerm = 0xffffffff;
   }
   if( idxNum & 4 ){
-    pCur->iStep = sqlite3_value_int64(argv[i++]);
+    pCur->ss.iStep = sqlite3_value_int64(argv[i++]);
-    if( pCur->iStep==0 ){
+    if( pCur->ss.iStep==0 ){
-      pCur->iStep = 1;
+      pCur->ss.iStep = 1;
-    }else if( pCur->iStep<0 ){
+    }else if( pCur->ss.iStep<0 ){
-      pCur->iStep = -pCur->iStep;
       if( (idxNum & 16)==0 ) idxNum |= 8;
     }
   }else{
-    pCur->iStep = 1;
+    pCur->ss.iStep = 1;
   }
   for(i=0; i<argc; i++){
     if( sqlite3_value_type(argv[i])==SQLITE_NULL ){
       /* If any of the constraints have a NULL value, then return no rows.
       ** See ticket https://www.sqlite.org/src/info/fac496b61722daf2 */
-      pCur->mnValue = 1;
+      pCur->ss.iBase = 1;
-      pCur->mxValue = 0;
+      pCur->ss.iTerm = 0;
+      pCur->ss.iStep = 1;
       break;
     }
   }
   if( idxNum & 8 ){
-    pCur->isDesc = 1;
+    pCur->ss.isReversing = pCur->ss.iStep > 0;
-    pCur->iValue = pCur->mxValue;
-    if( pCur->iStep>0 ){
-      pCur->iValue -= (pCur->mxValue - pCur->mnValue)%pCur->iStep;
-    }
   }else{
-    pCur->isDesc = 0;
+    pCur->ss.isReversing = pCur->ss.iStep < 0;
-    pCur->iValue = pCur->mnValue;
   }
-  pCur->iRowid = 1;
+  setupSequence( &pCur->ss );
   return SQLITE_OK;
 }
 

manifest

@@ -1,5 +1,5 @@
-C Use\sa\snew\stechnique\sto\sdetect\sfresh\sOOM\sfaults\sin\scolumnName()\sthat\sdoes\snot\nrely\son\sthere\sbeing\sno\sOOMs\sprior\sto\sentry\sinto\scolumnName(),\sas\n[forum/forumpost/fb6811c2f9|forum\spost\sfb6811c2f9]\sdemonstrates\sa\stechnique\nwhich\scould\scause\san\sOOM\sprior\sto\sentry\sinto\scolumnName().
+C Revise\sgenerate_series()\sextension\s(in\sCLI)\sto\saddress\soverflow\sreported\sin\s[forum:754e2d4db2a5|forum\spost\s#754e2d4db2a5]\sand\sto\smake\sbehavior\sbetter\smatch\sthe\slike-named\sPostgreSQL\sfunction.
-D 2023-04-27T23:59:51.813
+D 2023-04-28T21:25:03.297
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -305,7 +305,7 @@ F ext/misc/regexp.c f50ab59bfa8934b7ed98de069d2c74c187f2ef523fb09e85f8840f6459a9
 F ext/misc/remember.c add730f0f7e7436cd15ea3fd6a90fd83c3f706ab44169f7f048438b7d6baa69c
 F ext/misc/rot13.c 51ac5f51e9d5fd811db58a9c23c628ad5f333c173f1fc53c8491a3603d38556c
 F ext/misc/scrub.c 2a44b0d44c69584c0580ad2553f6290a307a49df4668941d2812135bfb96a946
-F ext/misc/series.c 8d79354f2c3d46b95ee21272a07cf0bcabb58d1f2b06d9e7b8a31dca1dacb3e5
+F ext/misc/series.c 9553821737ea55abcf534b22a63e9d29c2c9c07c1f82dd56cfa2b18e4d59d046
 F ext/misc/sha1.c 4011aef176616872b2a0d5bccf0ecfb1f7ce3fe5c3d107f3a8e949d8e1e3f08d
 F ext/misc/shathree.c 543af7ce71d391cd3a9ab6924a6a1124efc63211fd0f2e240dc4b56077ba88ac
 F ext/misc/showauth.c 732578f0fe4ce42d577e1c86dc89dd14a006ab52
@@ -2060,8 +2060,11 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 4bbebb6bfb9910265d91b777c1711b3b8e0732bcf299f7459b20c4ea110422bd
+P a63346d6a0c0ca7ba4c87499de2e461be9c77e9b5d98f2bebf308cdb6599f33c
-R 59c8d7a772734f630b5bf6fd205de302
+R 13e2ac79f26c72ca3ffff8649a5a99a9
-U drh
+T *branch * generate_series-revamp
-Z d29ba302c14b4aed5db886f9a3fc4bd5
+T *sym-generate_series-revamp *
+T -sym-trunk *
+U larrybr
+Z d68099c137797424f3a3da0f35ed98b8
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-a63346d6a0c0ca7ba4c87499de2e461be9c77e9b5d98f2bebf308cdb6599f33c
+beeea3e1b010dace9789f27172462b912819d0f8142a67e3e1e7335211e0e9a8