Regression of "WHERE (NOT) IN" to fix #1829 (#1830)

AlaSQL · Nov 12, 2023 · e5c6632 · e5c6632
1 parent 07b4cd6
commit e5c6632
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 22 deletions.
diff --git a/src/50expression.js b/src/50expression.js
@@ -483,17 +483,12 @@
 					s += '.indexOf(';
 					s += 'alasql.utils.getValueOf(' + leftJS() + '))>-1)';
 				} else if (Array.isArray(this.right)) {
-					// Added patch to have a better performance for when you have a lot of entries in an IN statement
-					if (!alasql.sets) {
-						alasql.sets = {};
-					}
-					const allValues = this.right.map((value) => value.value);
-					const allValuesStr = allValues.join(",");
-					if (!alasql.sets[allValuesStr]) {
-						// leverage JS Set, which is faster for lookups than arrays
-						alasql.sets[allValuesStr] = new Set(allValues);
-					}
-					s = 'alasql.sets["' + allValuesStr + '"].has(alasql.utils.getValueOf(' + leftJS() + '))';
+					// leverage JS Set, which is faster for lookups than arrays
+					s = '(new Set([' +
+						this.right.map(ref).join(',') +
+						']).has(alasql.utils.getValueOf(' +
+						leftJS() +
+						')))';
 				} else {
 					s = '(' + rightJS() + '.indexOf(' + leftJS() + ')>-1)';
 					//console.log('expression',350,s);
@@ -509,17 +504,12 @@
 					s += '.indexOf(';
 					s += 'alasql.utils.getValueOf(' + leftJS() + '))<0)';
 				} else if (Array.isArray(this.right)) {
-					// Added patch to have a better performance for when you have a lot of entries in a NOT IN statement
-					if (!alasql.sets) {
-						alasql.sets = {};
-					}
-					const allValues = this.right.map((value) => value.value);
-					const allValuesStr = allValues.join(",");
-					if (!alasql.sets[allValuesStr]) {
-						// leverage JS Set, which is faster for lookups than arrays
-						alasql.sets[allValuesStr] = new Set(allValues);
-					}
-					s = '!alasql.sets["' + allValuesStr + '"].has(alasql.utils.getValueOf(' + leftJS() + '))';
+					// leverage JS Set, which is faster for lookups than arrays
+					s = '(!(new Set([' +
+						this.right.map(ref).join(',') +
+						']).has(alasql.utils.getValueOf(' +
+						leftJS() +
+						'))))';
 				} else {
 					s = '(' + rightJS() + '.indexOf(';
 					s += leftJS() + ')==-1)';

diff --git a/test/test1829.js b/test/test1829.js
@@ -0,0 +1,66 @@
+if (typeof exports === 'object') {
+	var assert = require('assert');
+	var alasql = require('..');
+} else {
+	__dirname = '.';
+}
+
+describe('Test 1829 - WHERE (NOT) IN Regression when using refs', function () {
+	beforeEach(function () {
+		alasql(`CREATE TABLE test1829 (
+			id varchar(50) NOT NULL,
+			text varchar(10) NOT NULL,
+			PRIMARY KEY (id)
+		  )`);
+	});
+
+	afterEach(function () {
+		alasql('DROP TABLE test1829');
+	});
+
+	it('1. Where IN with refs', function (done) {
+		const rowId1 = 'id#1';
+		const rowId2 = 'id#2';
+
+		alasql('insert into test1829(id, text) values (?, ?)', [
+			rowId1,
+			'first text',
+		]);
+		alasql('insert into test1829(id, text) values (?, ?)', [
+			rowId2,
+			'second text',
+		]);
+
+		const selectedByIdRows = alasql(`select entity.id, entity.text from test1829 as entity where entity.id IN (?,?)`, [
+			rowId1,
+			rowId2
+		]);
+		assert.equal(selectedByIdRows.length, 2);
+		assert.equal(selectedByIdRows[0].id, rowId1);
+		assert.equal(selectedByIdRows[1].id, rowId2);
+
+		done();
+	});
+
+	it('2. Where NOT IN with refs', function (done) {
+		const rowId1 = 'id#1';
+		const rowId2 = 'id#2';
+
+		alasql('insert into test1829(id, text) values (?, ?)', [
+			rowId1,
+			'first text',
+		]);
+		alasql('insert into test1829(id, text) values (?, ?)', [
+			rowId2,
+			'second text',
+		]);
+
+		const selectedByIdRows = alasql(`select entity.id, entity.text from test1829 as entity where entity.id NOT IN (?)`, [
+			rowId1
+		]);
+		assert.equal(selectedByIdRows.length, 1);
+		assert.equal(selectedByIdRows[0].id, rowId2);
+		done();
+	});
+
+});