diff --git a/src/50expression.js b/src/50expression.js index b6a627be4d..ade2658b1c 100755 --- a/src/50expression.js +++ b/src/50expression.js @@ -187,6 +187,14 @@ } } + // The parser reads -1 as a minus in front of 1, so a negative number counts as a literal here + // too: an IN list with a non-literal builds its Set again for every row + const isInListLiteral = item => + item instanceof yy.NumValue || + item instanceof yy.StringValue || + (item instanceof yy.UniOp && item.op === '-' && item.right instanceof yy.NumValue); + const inListValue = item => (item instanceof yy.UniOp ? -item.right.value : item.value); + const toTypeNumberOps = new Set(['-', '*', '/', '%', '^', '<<', '>>', '&', '|']); const toTypeStringOps = new Set(['||']); // Regex to detect identifiers that need bracket wrapping (spaces, dots, hyphens, square brackets) @@ -383,17 +391,14 @@ leftJS(); s = 'false'; skipNullCheck = true; // Result is deterministic even with null operands - } else if ( - !alasql.options.cache || - this.right.some(value => value instanceof yy.ParamValue) - ) { + } else if (!alasql.options.cache || !this.right.every(isInListLiteral)) { // Leverage JS Set for faster lookups than arrays s = `(new Set([${this.right.map(ref).join(',')}]).has(alasql.utils.getValueOf(${leftJS()})))`; } else { // Use a cache to avoid re-creating the Set on every identical query. The key keeps the // type of each value, so IN (5) and IN ('5') get their own Set (#2912) alasql.sets = alasql.sets || {}; - const allValues = this.right.map(value => value.value); + const allValues = this.right.map(inListValue); const key = JSON.stringify(allValues); alasql.sets[key] = alasql.sets[key] || new Set(allValues); s = `alasql.sets[${JSON.stringify(key)}].has(alasql.utils.getValueOf(${leftJS()}))`; @@ -418,16 +423,13 @@ leftJS(); s = 'true'; skipNullCheck = true; // Result is deterministic even with null operands - } else if ( - !alasql.options.cache || - this.right.some(value => value instanceof yy.ParamValue) - ) { + } else if (!alasql.options.cache || !this.right.every(isInListLiteral)) { // Leverage JS Set for faster lookups than arrays s = `(!(new Set([${this.right.map(ref).join(',')}]).has(alasql.utils.getValueOf(${leftJS()}))))`; } else { // Use a cache to avoid re-creating the Set on every identical query, keyed as for IN alasql.sets = alasql.sets || {}; - const allValues = this.right.map(value => value.value); + const allValues = this.right.map(inListValue); const key = JSON.stringify(allValues); alasql.sets[key] = alasql.sets[key] || new Set(allValues); s = `!alasql.sets[${JSON.stringify(key)}].has(alasql.utils.getValueOf(${leftJS()}))`; diff --git a/test/test2874.js b/test/test2874.js new file mode 100644 index 0000000000..4d2daafaf5 --- /dev/null +++ b/test/test2874.js @@ -0,0 +1,46 @@ +if (typeof exports === 'object') { + var assert = require('assert'); + var alasql = require('..'); +} + +let testId = '2874'; // Use the ID of the issue being fixed by this PR + +describe(`Test ${testId} - IN and NOT IN with items that are not plain literals`, function () { + const data = [ + {a: -1, b: 0}, + {a: 5, b: 0}, + {a: 3, b: 3}, + ]; + + it('A) IN with a negative number', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a IN (-1, 5)', [data]), [{a: -1}, {a: 5}]); + }); + + it('B) IN with an expression', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a IN (1 + 2, 5)', [data]), [ + {a: 5}, + {a: 3}, + ]); + }); + + it('C) IN with a column', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a IN (b, 5)', [data]), [{a: 5}, {a: 3}]); + }); + + it('D) NOT IN with a negative number', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a NOT IN (-1, 5)', [data]), [{a: 3}]); + }); + + it('E) NOT IN with a column', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a NOT IN (b, 5)', [data]), [{a: -1}]); + }); + + it('F) IN with plain literals still works', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a IN (3, 5)', [data]), [{a: 5}, {a: 3}]); + }); + + it('G) A negative number keeps the cached list', function () { + assert.deepStrictEqual(alasql('SELECT a FROM ? WHERE a IN (-1, 3)', [data]), [{a: -1}, {a: 3}]); + assert.ok(alasql.sets['[-1,3]'] instanceof Set); + }); +});