Fix virtual table limit offset (WiseLibs#873)

* Don't care limit and offset constraints of user-defined virtual tables * Add a test case for WiseLibs#872
octodb · Oct 12, 2022 · 15652ad · 15652ad
1 parent 0c42307
commit 15652ad
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 8 deletions.
diff --git a/src/better_sqlite3.cpp b/src/better_sqlite3.cpp
@@ -1748,6 +1748,13 @@ int CustomTable::xBestIndex (sqlite3_vtab * vtab, sqlite3_index_info * output)
                         auto item = output->aConstraint[i];
 
 
+
+
+
+                        if (item.op == SQLITE_INDEX_CONSTRAINT_LIMIT || item.op == SQLITE_INDEX_CONSTRAINT_OFFSET) {
+                                continue;
+                        }
+
                         if (item.iColumn >= 0 && item.iColumn < parameter_count) {
                                 if (item.op != SQLITE_INDEX_CONSTRAINT_EQ) {
                                         sqlite3_free(vtab->zErrMsg);
@@ -1782,9 +1789,9 @@ int CustomTable::xBestIndex (sqlite3_vtab * vtab, sqlite3_index_info * output)
                 output->estimatedCost = output->estimatedRows = 1000000000 / (argument_count + 1);
                 return SQLITE_OK;
 }
-#line 387 "./src/util/custom-table.lzz"
+#line 394 "./src/util/custom-table.lzz"
 void CustomTable::PropagateJSError ()
-#line 387 "./src/util/custom-table.lzz"
+#line 394 "./src/util/custom-table.lzz"
                                 {
                 assert(db->GetState()->was_js_error == false);
                 db->GetState()->was_js_error = true;

diff --git a/src/better_sqlite3.hpp b/src/better_sqlite3.hpp
@@ -666,17 +666,17 @@ class CustomTable
   static int xRowid (sqlite3_vtab_cursor * cursor, sqlite_int64 * output);
 #line 343 "./src/util/custom-table.lzz"
   static int xBestIndex (sqlite3_vtab * vtab, sqlite3_index_info * output);
-#line 387 "./src/util/custom-table.lzz"
+#line 394 "./src/util/custom-table.lzz"
   void PropagateJSError ();
-#line 392 "./src/util/custom-table.lzz"
+#line 399 "./src/util/custom-table.lzz"
   Addon * const addon;
-#line 393 "./src/util/custom-table.lzz"
+#line 400 "./src/util/custom-table.lzz"
   v8::Isolate * const isolate;
-#line 394 "./src/util/custom-table.lzz"
+#line 401 "./src/util/custom-table.lzz"
   Database * const db;
-#line 395 "./src/util/custom-table.lzz"
+#line 402 "./src/util/custom-table.lzz"
   std::string const name;
-#line 396 "./src/util/custom-table.lzz"
+#line 403 "./src/util/custom-table.lzz"
   CopyablePersistent <v8::Function> const factory;
 };
 #line 65 "./src/util/data.lzz"

diff --git a/src/util/custom-table.lzz b/src/util/custom-table.lzz
@@ -348,6 +348,13 @@ private:
 		for (int i = 0, len = output->nConstraint; i < len; ++i) {
 			auto item = output->aConstraint[i];
 
+			// The SQLITE_INDEX_CONSTRAINT_LIMIT and SQLITE_INDEX_CONSTRAINT_OFFSET
+			// operators have no left-hand operand, and so for those operators the
+			// corresponding item.iColumn is meaningless.
+			// We don't care those constraints.
+			if (item.op == SQLITE_INDEX_CONSTRAINT_LIMIT || item.op == SQLITE_INDEX_CONSTRAINT_OFFSET) {
+				continue;
+			}
 			// We only care about constraints on parameters, not regular columns.
 			if (item.iColumn >= 0 && item.iColumn < parameter_count) {
 				if (item.op != SQLITE_INDEX_CONSTRAINT_EQ) {

diff --git a/test/34.database.table.js b/test/34.database.table.js
@@ -668,4 +668,28 @@ describe('Database#table()', function () {
 			throw new TypeError('Expected the statement to throw an exception');
 		});
 	});
+	it('should correctly handle limit and offset clause', function () {
+		let lastValue;
+		this.db.table('vtab', {
+			columns: ['x'],
+			*rows() {
+				lastValue = 1;
+				yield { x: lastValue };
+				lastValue = 2;
+				yield { x: lastValue };
+				lastValue = 3;
+				yield { x: lastValue };
+				lastValue = null;
+			},
+		});
+		expect(this.db.prepare('SELECT * FROM vtab LIMIT 1').all())
+			.to.deep.equal([{ x: 1 }]);
+		expect(lastValue).to.equal(1);
+		expect(this.db.prepare('SELECT * FROM vtab LIMIT 1 OFFSET 2').all())
+			.to.deep.equal([{ x: 3 }]);
+		expect(lastValue).to.equal(3);
+		expect(this.db.prepare('SELECT * FROM vtab LIMIT 100 OFFSET 1').all())
+			.to.deep.equal([{ x: 2 }, { x: 3 }]);
+		expect(lastValue).to.be.null;
+	});
 });