diff --git a/src/builder/valid.ts b/src/builder/valid.ts index 942fdaec..52c98f6f 100644 --- a/src/builder/valid.ts +++ b/src/builder/valid.ts @@ -16,12 +16,11 @@ // under the License. import { DataType } from '../type.js'; -import { valueToString } from '../util/pretty.js'; /** - * Dynamically compile the null values into an `isValid()` function whose - * implementation is a switch statement. Microbenchmarks in v8 indicate - * this approach is 25% faster than using an ES6 Map. + * Creates a validity predicate without compiling strings as JavaScript. + * A single null sentinel uses a direct comparison; multiple sentinels use a + * snapshot Set, including NaN and treating positive and negative zero equally. * * @example * console.log(createIsValidFunction([null, 'N/A', NaN])); @@ -45,30 +44,12 @@ export function createIsValidFunction(nul return function isValid(value: any) { return true; }; } - let fnBody = ''; - const noNaNs = nullValues.filter((x) => x === x); - - if (noNaNs.length > 0) { - fnBody = ` - switch (x) {${noNaNs.map((x) => ` - case ${valueToCase(x)}:`).join('')} - return false; - }`; - } - - // NaN doesn't equal anything including itself, so it doesn't work as a - // switch case. Instead we must explicitly check for NaN before the switch. - if (nullValues.length !== noNaNs.length) { - fnBody = `if (x !== x) return false;\n${fnBody}`; - } - - return new Function(`x`, `${fnBody}\nreturn true;`) as (value: T['TValue'] | TNull) => boolean; -} - -/** @ignore */ -function valueToCase(x: any) { - if (typeof x !== 'bigint') { - return valueToString(x); + if (nullValues.length === 1) { + const nullValue = nullValues[0]; + return nullValue !== nullValue + ? function isValid(value: T['TValue'] | TNull) { return value === value; } + : function isValid(value: T['TValue'] | TNull) { return value !== nullValue; }; } - return `${valueToString(x)}n`; + const invalidValues = new Set(nullValues); + return function isValid(value: T['TValue'] | TNull) { return !invalidValues.has(value); }; } diff --git a/test/unit/builders/valid-tests.ts b/test/unit/builders/valid-tests.ts new file mode 100644 index 00000000..ec0c60ea --- /dev/null +++ b/test/unit/builders/valid-tests.ts @@ -0,0 +1,62 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import { createIsValidFunction } from 'apache-arrow/builder/valid'; + +describe('Null sentinel validity predicates', () => { + test('empty sentinel lists accept every value, including NaN', () => { + for (const nullValues of [undefined, []]) { + const isValid = createIsValidFunction(nullValues); + expect([null, undefined, Number.NaN, 0, ''].every(value => isValid(value))).toBe(true); + } + }); + + test.each([null, undefined, Number.NaN, 0, false, '', 'quoted "text"\nwith\\slashes', 42n])( + 'matches a single sentinel without coercion: %p', nullValue => { + const isValid = createIsValidFunction([nullValue]); + expect(isValid(nullValue)).toBe(false); + expect(isValid(Symbol('different'))).toBe(true); + } + ); + + test('multiple sentinels preserve NaN, zero and primitive type distinctions', () => { + const isValid = createIsValidFunction([null, undefined, Number.NaN, -0, 42n, '42', false]); + expect([null, undefined, Number.NaN, 0, -0, 42n, '42', false].map(value => isValid(value))).toEqual(new Array(8).fill(false)); + expect([42, true, '', 'NaN', 1n].every(value => isValid(value))).toBe(true); + }); + + test('snapshots sentinel lists, including duplicate entries', () => { + const nullValues = ['missing', 'missing']; + const isValid = createIsValidFunction(nullValues); + nullValues.splice(0, 2, 'changed'); + expect(isValid('missing')).toBe(false); + expect(isValid('changed')).toBe(true); + }); + + test('does not require the Function constructor permitted by unsafe-eval', () => { + const originalFunction = globalThis.Function; + let results: boolean[]; + try { + globalThis.Function = (() => { throw new Error('Dynamic compilation blocked'); }) as unknown as FunctionConstructor; + const isValid = createIsValidFunction([null, 'quoted "text"\nwith\\slashes', Number.NaN]); + results = [null, Number.NaN, 'valid'].map(value => isValid(value)); + } finally { + globalThis.Function = originalFunction; + } + expect(results!).toEqual([false, false, true]); + }); +});