Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression WHERE (not) in (4.2.0) #1830

Merged
merged 10 commits into from
Nov 12, 2023
34 changes: 12 additions & 22 deletions src/50expression.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)';
Expand Down
66 changes: 66 additions & 0 deletions test/test1829.js
mathiasrw marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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();
});

});
Loading