[HOLD bump queue] feat: Split queries with many parameters into multiple queries to avoid too many SQL variables error

lib/storage/providers/SQLiteProvider.ts

@@ -2,14 +2,21 @@
  * The SQLiteStorage provider stores everything in a key/value store by
  * converting the value to a JSON string
  */
-import type {BatchQueryCommand, NitroSQLiteConnection} from 'react-native-nitro-sqlite';
+import type {BatchQueryCommand, NitroSQLiteConnection, QueryResult} from 'react-native-nitro-sqlite';
 import {open} from 'react-native-nitro-sqlite';
 import {getFreeDiskStorage} from 'react-native-device-info';
 import type {FastMergeReplaceNullPatch} from '../../utils';
 import utils from '../../utils';
 import type StorageProvider from './types';
 import type {StorageKeyList, StorageKeyValuePair} from './types';
 
+/**
+ * The result of the `PRAGMA compile_options`, which lists SQLite compile-time options
+ */
+type CompileOptionsResult = {
+    compile_options: string;
+};
+
 /**
  * The type of the key-value pair stored in the SQLite database
  * @property record_key - the key of the record
@@ -35,6 +42,40 @@ type PageCountResult = {
 };
 
 const DB_NAME = 'OnyxDB';
+const SQLITE_MAX_VARIABLE_NUMBER = 32766;
+const COMPILE_OPTIONS = {
+    MAX_VARIABLE_NUMBER: 'MAX_VARIABLE_NUMBER',
+} as const;
+
+/** SQLite's maximum number of bound parameters per statement, read once from PRAGMA compile_options in init(). */
+let sqliteMaxVariableNumber = SQLITE_MAX_VARIABLE_NUMBER;
+
+/**
+ * Returns the value of a compile option from the rows returned by `PRAGMA compile_options`.
+ * For flag-only options (e.g. `ENABLE_FTS3`), returns an empty string when the option is present.
+ */
+function getCompileOptionValue(compileOptionsResult: QueryResult<CompileOptionsResult>, optionName: string): string | undefined {
+    const optionPrefix = `${optionName}=`;
+    const rowCount = compileOptionsResult.rows?.length ?? 0;
+
+    for (let index = 0; index < rowCount; index++) {
+        const compileOption = compileOptionsResult.rows?.item(index)?.compile_options;
+
+        if (!compileOption) {
+            continue;
+        }
+
+        if (compileOption === optionName) {
+            return '';
+        }
+
+        if (compileOption.startsWith(optionPrefix)) {
+            return compileOption.slice(optionPrefix.length);
+        }
+    }
+
+    return undefined;
+}
 
 /**
  * Prevents the stringifying of the object markers.
@@ -76,6 +117,13 @@ const provider: StorageProvider<NitroSQLiteConnection | undefined> = {
         provider.store.execute('PRAGMA CACHE_SIZE=-20000;');
         provider.store.execute('PRAGMA synchronous=NORMAL;');
         provider.store.execute('PRAGMA journal_mode=WAL;');
+
+        const compileOptionsResult = provider.store.execute<CompileOptionsResult>('PRAGMA compile_options;');
+
+        // Get the value of MAX_VARIABLE_NUMBER from the compile options and
+        // stores it in a global variable, that is going to be used during runtime.
+        const maxVariableNumber = Number(getCompileOptionValue(compileOptionsResult, COMPILE_OPTIONS.MAX_VARIABLE_NUMBER));
+        sqliteMaxVariableNumber = maxVariableNumber > 0 ? maxVariableNumber : SQLITE_MAX_VARIABLE_NUMBER;
     },
     getItem(key) {
         if (!provider.store) {
@@ -100,12 +148,29 @@ const provider: StorageProvider<NitroSQLiteConnection | undefined> = {
             throw new Error('Store is not initialized!');
         }
 
-        const placeholders = keys.map(() => '?').join(',');
-        const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`;
-        return provider.store.executeAsync<OnyxSQLiteKeyValuePair>(command, keys).then(({rows}) => {
-            // eslint-disable-next-line no-underscore-dangle
-            const result = rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]);
-            return (result ?? []) as StorageKeyValuePair[];
+        if (keys.length === 0) {
+            return Promise.resolve([]);
+        }
+
+        const keyChunks = utils.chunkArray(keys, sqliteMaxVariableNumber);
+
+        return Promise.all(
+            keyChunks.map((keyChunk) => {
+                if (!provider.store) {
+                    throw new Error('Store is not initialized!');
+                }
+
+                const placeholders = keyChunk.map(() => '?').join(',');
+                const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`;
+                return provider.store.executeAsync<OnyxSQLiteKeyValuePair>(command, keyChunk);
+            }),
+        ).then((results) => {
+            const result = results.flatMap(
+                ({rows}) =>
+                    // eslint-disable-next-line no-underscore-dangle
+                    rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]) ?? [],
+            );
+            return result as StorageKeyValuePair[];
         });
     },
     setItem(key, value) {
@@ -121,7 +186,7 @@ const provider: StorageProvider<NitroSQLiteConnection | undefined> = {
         }
 
         const query = 'REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);';
-        const params = pairs.map((pair) => [pair[0], JSON.stringify(pair[1] === undefined ? null : pair[1])]);
+        const params = pairs.map(([key, value]) => [key, JSON.stringify(value === undefined ? null : value)]);
         if (utils.isEmptyObject(params)) {
             return Promise.resolve();
         }
@@ -214,9 +279,28 @@ const provider: StorageProvider<NitroSQLiteConnection | undefined> = {
             throw new Error('Store is not initialized!');
         }
 
-        const placeholders = keys.map(() => '?').join(',');
-        const query = `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`;
-        return provider.store.executeAsync(query, keys).then(() => undefined);
+        if (keys.length === 0) {
+            return Promise.resolve();
+        }
+
+        const keyChunks = utils.chunkArray(keys, sqliteMaxVariableNumber);
+
+        const buildDeleteQuery = (keyChunk: readonly string[]) => {
+            const placeholders = keyChunk.map(() => '?').join(',');
+            return `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`;
+        };
+
+        if (keyChunks.length === 1) {
+            const keyChunk = keyChunks[0];
+            return provider.store.executeAsync(buildDeleteQuery(keyChunk), keyChunk).then(() => undefined);
+        }
+
+        const commands: BatchQueryCommand[] = keyChunks.map((keyChunk) => ({
+            query: buildDeleteQuery(keyChunk),
+            params: keyChunk,
+        }));
+
+        return provider.store.executeBatchAsync(commands).then(() => undefined);
     },
     clear() {
         if (!provider.store) {

lib/utils.ts

@@ -322,6 +322,25 @@ function omit<TValue>(obj: Record<string, TValue>, condition: string | string[]
     return filterObject(obj, condition, false);
 }
 
+/**
+ * Splits an array into chunks no larger than maxChunkSize.
+ */
+function chunkArray<T>(items: readonly T[], maxChunkSize: number): T[][] {
+    if (items.length === 0) {
+        return [];
+    }
+
+    if (items.length <= maxChunkSize) {
+        return [items as T[]];
+    }
+
+    const chunks: T[][] = [];
+    for (let i = 0; i < items.length; i += maxChunkSize) {
+        chunks.push(items.slice(i, i + maxChunkSize));
+    }
+    return chunks;
+}
+
 export default {
     fastMerge,
     isEmptyObject,
@@ -330,6 +349,7 @@ export default {
     checkCompatibilityWithExistingValue,
     pick,
     omit,
+    chunkArray,
     ONYX_INTERNALS__REPLACE_OBJECT_MARK,
 };
 export type {FastMergeResult, FastMergeReplaceNullPatch, FastMergeOptions};

tests/unit/mocks/sqliteMock.ts

@@ -7,13 +7,13 @@
  *   - open({name})
  *   - connection.execute(sql)
  *   - connection.executeAsync<T>(sql, params?)
- *   - connection.executeBatchAsync([{query, params}])
+ *   - connection.executeBatchAsync([{query, params}, ...])
  *
  * Result rows are shaped to match Nitro: `{rows: {_array, item, length}}`.
  */
 import BetterSqlite3 from 'better-sqlite3';
 import type {Database} from 'better-sqlite3';
-import type {NitroSQLiteConnection, NitroSQLiteQueryResultRows, QueryResult, QueryResultRow, SQLiteQueryParams} from 'react-native-nitro-sqlite';
+import type {BatchQueryCommand, NitroSQLiteConnection, NitroSQLiteQueryResultRows, QueryResult, QueryResultRow, SQLiteQueryParams} from 'react-native-nitro-sqlite';
 
 // `better-sqlite3` is declared as `export = Database` (CommonJS), so the type is
 // derived from the default import's namespace rather than via a named type import.
@@ -54,7 +54,7 @@ function wrapRows<TRow extends QueryResultRow>(rowsArray: TRow[]): NitroSQLiteQu
     };
 }
 
-function prepareAndBind(database: Database, sql: string, parameters?: SQLiteQueryParams) {
+function prepareAndBind(database: Database, sql: BatchQueryCommand['query'], parameters?: BatchQueryCommand['params']) {
     const namedOrder = extractNamedParameterOrder(sql);
     if (namedOrder) {
         // Map positional parameters array to named bindings object — NitroSQLite's
@@ -71,6 +71,35 @@ function prepareAndBind(database: Database, sql: string, parameters?: SQLiteQuer
     return {statement: database.prepare(sql), boundArguments: parameters ?? []};
 }
 
+/**
+ * Expands batch commands the same way NitroSQLite does in `batchParamsToCommands`:
+ * `params` is either one binding set for the query, or an array of binding sets
+ * (same query executed once per row).
+ */
+function batchParamsToCommands(commands: BatchQueryCommand[]): BatchQueryCommand[] {
+    const expanded: BatchQueryCommand[] = [];
+
+    for (const command of commands) {
+        const {query, params} = command;
+
+        if (!params) {
+            expanded.push({query});
+            continue;
+        }
+
+        if (Array.isArray(params[0])) {
+            for (const rowParams of params as SQLiteQueryParams[]) {
+                expanded.push({query, params: rowParams});
+            }
+            continue;
+        }
+
+        expanded.push({query, params: params as SQLiteQueryParams});
+    }
+
+    return expanded;
+}
+
 function runOne<TRow extends QueryResultRow>(database: Database, sql: string, parameters?: SQLiteQueryParams): QueryResult<TRow> {
     // Multi-statement (CREATE TABLE; SELECT ...; etc.) — better-sqlite3 cannot
     // prepare more than one statement at a time. SQLiteProvider's init() issues
@@ -120,29 +149,13 @@ function makeConnection(name: string): Pick<NitroSQLiteConnection, 'execute' | '
         executeBatchAsync(commands) {
             try {
                 let total = 0;
+                const expandedCommands = batchParamsToCommands(commands);
+
                 connection.transaction(() => {
-                    for (const command of commands) {
-                        const namedOrder = extractNamedParameterOrder(command.query);
-                        const statement = connection.prepare(command.query);
-                        const parameterRows = (command.params ?? []) as SQLiteQueryParams[];
-                        if (parameterRows.length === 0) {
-                            const info = statement.run();
-                            total += info.changes;
-                            continue;
-                        }
-                        for (const row of parameterRows) {
-                            if (namedOrder) {
-                                const bindings: Record<string, unknown> = {};
-                                for (let index = 0; index < namedOrder.length; index++) {
-                                    bindings[namedOrder[index]] = row[index];
-                                }
-                                const info = statement.run(bindings);
-                                total += info.changes;
-                            } else {
-                                const info = statement.run(...row);
-                                total += info.changes;
-                            }
-                        }
+                    for (const command of expandedCommands) {
+                        const {statement, boundArguments} = prepareAndBind(connection, command.query, command.params);
+                        const info = statement.run(...(boundArguments as unknown[]));
+                        total += info.changes;
                     }
                 })();
                 return Promise.resolve({rowsAffected: total});

tests/unit/storage/providers/SQLiteProviderTest.ts

@@ -312,6 +312,99 @@ describe('SQLiteProvider', () => {
         });
     });
 
+    describe('query splitting', () => {
+        const CHUNK_SIZE = 2;
+        const originalChunkArray = utils.chunkArray;
+
+        const createKeyValueEntries = (count: number): Array<[string, unknown]> => Array.from({length: count}, (_, index) => [`chunk_key_${index}`, index]);
+
+        beforeEach(() => {
+            resetAllDatabases();
+            SQLiteProvider.init();
+
+            jest.spyOn(utils, 'chunkArray').mockImplementation((items, _maxChunkSize) => originalChunkArray(items, CHUNK_SIZE));
+        });
+
+        afterEach(() => {
+            jest.restoreAllMocks();
+        });
+
+        describe('multiGet', () => {
+            it('should return all values when keys exceed MAX_VARIABLE_NUMBER', async () => {
+                const entries = createKeyValueEntries(5);
+                await SQLiteProvider.multiSet(entries);
+
+                const keys = entries.map(([key]) => key);
+                const result = await SQLiteProvider.multiGet(keys);
+
+                expect(result).toEqual(expect.arrayContaining(entries));
+                expect(result).toHaveLength(entries.length);
+            });
+
+            it('should issue one IN query per chunk', async () => {
+                const entries = createKeyValueEntries(5);
+                await SQLiteProvider.multiSet(entries);
+
+                const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync');
+                executeAsyncSpy.mockClear();
+
+                const keys = entries.map(([key]) => key);
+                await SQLiteProvider.multiGet(keys);
+
+                const inQueries = executeAsyncSpy.mock.calls.filter(([sql]) => typeof sql === 'string' && sql.includes('WHERE record_key IN'));
+                expect(inQueries).toHaveLength(3);
+            });
+        });
+
+        describe('removeItems', () => {
+            it('should remove all keys when keys exceed MAX_VARIABLE_NUMBER', async () => {
+                const entries = createKeyValueEntries(5);
+                await SQLiteProvider.multiSet(entries);
+
+                const keys = entries.map(([key]) => key);
+                await SQLiteProvider.removeItems(keys);
+
+                expect(await SQLiteProvider.getAllKeys()).toEqual([]);
+            });
+
+            it('should use executeAsync when keys fit in a single chunk', async () => {
+                const entries = createKeyValueEntries(2);
+                await SQLiteProvider.multiSet(entries);
+
+                const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync');
+                const executeBatchAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeBatchAsync');
+                executeAsyncSpy.mockClear();
+                executeBatchAsyncSpy.mockClear();
+
+                const keys = entries.map(([key]) => key);
+                await SQLiteProvider.removeItems(keys);
+
+                expect(executeAsyncSpy).toHaveBeenCalledTimes(1);
+                expect(executeBatchAsyncSpy).not.toHaveBeenCalled();
+            });
+
+            it('should use executeBatchAsync when keys span multiple chunks', async () => {
+                const entries = createKeyValueEntries(5);
+                await SQLiteProvider.multiSet(entries);
+
+                const executeAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeAsync');
+                const executeBatchAsyncSpy = jest.spyOn(SQLiteProvider.store!, 'executeBatchAsync');
+                executeAsyncSpy.mockClear();
+                executeBatchAsyncSpy.mockClear();
+
+                const keys = entries.map(([key]) => key);
+                await SQLiteProvider.removeItems(keys);
+
+                expect(executeAsyncSpy).not.toHaveBeenCalled();
+                expect(executeBatchAsyncSpy).toHaveBeenCalledTimes(1);
+
+                const batchCommands = executeBatchAsyncSpy.mock.calls[0][0];
+                expect(batchCommands).toHaveLength(3);
+                expect(batchCommands.every((command) => command.query.includes('DELETE FROM keyvaluepairs WHERE record_key IN'))).toBe(true);
+            });
+        });
+    });
+
     // SQLite-specific: the IN-list is parameterised, so a key containing SQL
     // fragments must be treated as a literal record_key.
     describe('SQL-injection safety', () => {