diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 19249d8..8ed9316 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,6 +66,22 @@ jobs: # Real engines, hostile schemas, no model. A mixed-case Postgres schema once failed every query # and shipped that way, because every database test used tables we had written ourselves. + room-regression: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + - uses: pnpm/action-setup@v5 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + - run: pnpm install --frozen-lockfile + - run: pnpm -r --filter='./packages/*' build + # An Android app's database: Long ids, INTEGER booleans, epoch millis, Room bookkeeping, FTS + # shadow tables and a WAL sidecar. Every fixture the suite owned used real types, so none of it + # was covered. No model involved, so the result is deterministic. + - run: pnpm test:room + schema-regression: runs-on: ubuntu-latest services: diff --git a/package.json b/package.json index 5f7edfd..a30efde 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "coverage": "vitest run --coverage", "test:packaged": "node tools/packaged-consumer-test.mjs", "test:schemas": "node tools/schema-regression.mjs", + "test:room": "node tools/room-regression.mjs", "test:real-db:load": "node tools/real-db-load.mjs", "test:real-db": "node tools/real-db-e2e.mjs", "test:schema-sweep": "node tools/schema-sweep.mjs", diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md index fdf48d9..90305cf 100644 --- a/packages/core/CHANGELOG.md +++ b/packages/core/CHANGELOG.md @@ -1,5 +1,36 @@ # @asksql/core +## 0.8.0 + +### Minor Changes + +- Catch a date comparison that answered confidently and wrongly, and let full-text search run. + + SQLite has no date type, so a timestamp is a bare number: Room writes epoch milliseconds into an + INTEGER column. The SQLite guidance told the model to use `date('now','-30 days')`, which is right for + a TEXT column and wrong for that one - and SQLite compares by storage class, so nothing matches, no + error is raised, and "how many users signed up in the last 7 days" answers zero. Guessing epoch + seconds instead is worse: a milliseconds column is a thousand times larger, so every row matches and + the answer is the whole table. Measured against a Room-shaped database: a 7B model answered 0 where + the truth was 2, and a 30B model answered 5. + + The guidance now says which units an INTEGER column is in and how to build a bound in the same units, + and a semantic floor catches a numeric column compared against a date and sends it back to be + corrected. Both models now answer 2. The floor is held to a sweep of every column type, date + expression, operator, query shape and dialect, because firing on a TEXT column would refuse SQL that + is correct. + + Full-text search works. SQLite parses under the Postgresql grammar, which has no `MATCH`, so every + `WHERE messages_fts MATCH 'term'` was refused as unparseable - valid read-only SQL that a Room `@Fts4` + entity is queried with. `MATCH` is now validated as a comparison and the statement that runs keeps it + verbatim; a right side that is a column, a parameter or a subquery is still refused, as are writes, + stacked statements and denied functions. Verified on FTS4 and FTS5, including `rank` ordering. + + `rowid` is no longer reported as an invented column. SQLite gives every table `rowid`, `oid` and + `_rowid_` without listing them in `PRAGMA table_info`, and FTS tables answer to `docid` and `rank`, so + `SELECT rowid FROM users` was refused after every correction attempt. On a `WITHOUT ROWID` table the + database rejects the name, which the correction loop can act on. + ## 0.7.0 ### Minor Changes diff --git a/packages/core/package.json b/packages/core/package.json index 2312659..ee1e6e4 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@asksql/core", - "version": "0.7.0", + "version": "0.8.0", "description": "AskSQL engine: schema catalog, AST SQL guard, prompt pipeline, LLM orchestration. Zero database drivers.", "type": "module", "main": "./dist/index.js", diff --git a/packages/core/src/dialects.ts b/packages/core/src/dialects.ts index 3bbee49..950ab1b 100644 --- a/packages/core/src/dialects.ts +++ b/packages/core/src/dialects.ts @@ -40,7 +40,10 @@ export const SQLITE_DIALECT: DialectInfo = Object.freeze({ promptLabel: 'SQLite', limitStyle: 'limit', promptNotes: Object.freeze([ - "Use date/datetime/strftime for date math (e.g. date('now','-30 days')).", + "Dates: a TEXT column holds ISO text, so compare it with date/datetime/strftime (e.g. date('now','-30 days')). " + + 'An INTEGER column holds a number - usually epoch seconds, or milliseconds if the values are ~1000x larger - ' + + "so build the bound as a number in the SAME units, e.g. (strftime('%s','now') - 30*86400) * 1000 for " + + 'milliseconds. Never compare an INTEGER column with a text date: nothing matches and no error is raised.', 'There are no schemas; refer to tables by bare name.', "Combine values into one string with group_concat(col, ', ').", ]), diff --git a/packages/core/src/engine.ts b/packages/core/src/engine.ts index 27095e7..0a73c07 100644 --- a/packages/core/src/engine.ts +++ b/packages/core/src/engine.ts @@ -20,7 +20,7 @@ import { withoutFetchTail } from './strip.js'; import { AskSqlError } from './errors.js'; import { extractImpossible, extractSql } from './extract.js'; import { guardSql, resolveGuardPolicy } from './guard.js'; -import { fanOutAggregate, nestedAggregate, ungroupedAggregate } from './semantics.js'; +import { epochUnitMismatch, fanOutAggregate, nestedAggregate, ungroupedAggregate } from './semantics.js'; import { historyId, MemoryHistoryStore } from './history.js'; import { callModel } from './llm.js'; import { @@ -927,6 +927,32 @@ export function createAskSql(config: AskSqlConfig): AskSqlEngine { continue; } + // Semantic floor: a column that stores a moment as a number, compared against a date. Wrong + // whichever way the engine resolves it - an empty result reported as zero, or every row + // matching because seconds were compared with milliseconds - and it never errors. + const epoch = epochUnitMismatch(verdict.sql, conn.dialect.grammar, fullCatalog); + if (epoch && attempt >= MAX_REPAIRS) { + semanticNotes.push( + `This compares "${epoch.column}", which is ${epoch.dbType}, against ${epoch.comparedTo}. A number and a ` + + 'date are not the same kind of value, so the rows selected are not the rows the question asked for.', + ); + } + if (epoch && attempt < MAX_REPAIRS) { + userPrompt = buildRepairUser({ + question: q, + failedSql: verdict.sql, + failure: + `"${epoch.column}" is ${epoch.dbType}, so it holds a number, not a date, and comparing it with ` + + `${epoch.comparedTo} does not select the rows intended: against text nothing matches, and against ` + + 'epoch seconds a column of milliseconds matches everything. Compare it in its own units - build the ' + + "bound as a number, for example (strftime('%s','now') - 7*86400) * 1000 for milliseconds - or convert " + + 'the column with the matching divisor before comparing.', + schemaText, + dialect: conn.dialect, + }); + continue; + } + // Column-level hallucination floor: a column attributed to a real base table must exist on it. const unknownColumn = firstUnknownColumn(verdict.sql, fullCatalog, conn.dialect.grammar); if (unknownColumn) { @@ -1358,6 +1384,13 @@ function collectSelectAliases(sql: string): ReadonlySet { return names; } +/** + * Columns SQLite gives every table without listing them, so `PRAGMA table_info` never reports them. + * On a WITHOUT ROWID table the database rejects the name, which the repair loop can act on; refusing + * here blocked SQL that works. + */ +const SQLITE_IMPLICIT_COLUMNS: ReadonlySet = new Set(['rowid', 'oid', '_rowid_', 'docid', 'rank']); + /** * Returns the first column reference whose base table exists in the catalog but * does not have that column - the column-level hallucination floor. Fails open (returns null) on @@ -1473,6 +1506,7 @@ export function firstUnknownColumn(sql: string, catalog: SchemaCatalog, grammar: if (!table || table === 'null') { // Unqualified: skip aliases, require every base table known, then flag it if no table has it. if (!attributable || aliases.has(column) || queryTables.length === 0) continue; + if (catalog.engine === 'sqlite' && SQLITE_IMPLICIT_COLUMNS.has(column)) continue; if (queryTables.some((t) => byTable.get(t)!.has(column))) continue; const available = new Set(); for (const t of queryTables) for (const c of realColumns.get(t) ?? []) available.add(c); @@ -1488,6 +1522,7 @@ export function firstUnknownColumn(sql: string, catalog: SchemaCatalog, grammar: const known = byTable.get(table); if (!known) continue; // derived/subquery alias or table not in catalog - fail open if (known.has(column)) continue; // real column + if (catalog.engine === 'sqlite' && SQLITE_IMPLICIT_COLUMNS.has(column)) continue; return { table, column, available: [...known].sort() }; } return null; diff --git a/packages/core/src/guard.ts b/packages/core/src/guard.ts index 6f10565..75d82a9 100644 --- a/packages/core/src/guard.ts +++ b/packages/core/src/guard.ts @@ -397,6 +397,32 @@ const ORACLE_DENY_PREFIXES = [ */ const ORACLE_SEQUENCE_PSEUDO_COLUMNS = new Set(['nextval']); +/** + * SQLite's full-text search operator, which the Postgresql grammar has no notion of, so every Room + * @Fts4 query was refused as unparseable. Only the operator with a single-quoted literal is rewritten; + * a column, parameter or subquery on the right still fails closed. + */ +const SQLITE_MATCH_RE = /(\s)match(\s+'(?:[^']|'')*')/giu; + +/** Parse-only, and length-preserving, so the validated text and the text that runs share every offset. */ +function rewriteSqliteMatch(sql: string): { rewritten: string; count: number } { + let count = 0; + const masked = maskCommentsAndStrings(sql); + const rewritten = sql.replace(SQLITE_MATCH_RE, (whole, lead: string, right: string, offset: number) => { + // Only outside a string or comment: a literal containing the word "match" is not the operator. + if ( + !masked + .slice(offset, offset + whole.length) + .toLowerCase() + .includes('match') + ) + return whole; + count++; + return `${lead}= ${right}`; + }); + return { rewritten, count }; +} + /** Every known-dangerous function is denied on every dialect, closing the "dangerous in A, allowed in B" gap. */ const UNIVERSAL_DENY: readonly string[] = [ ...PG_DENY_FUNCTIONS, @@ -944,8 +970,11 @@ export function guardSql(input: GuardInput): GuardVerdict { // ---- Parse once (fail-closed): `parse` yields the AST and the table list together. ---- let ast: unknown; let tableList: string[] = []; + // MATCH is validated as a comparison. The rewrite is length-preserving and used only for parsing, + // so every text position still lines up and the statement that runs keeps MATCH as written. + const toParse = dialect.engine === 'sqlite' ? rewriteSqliteMatch(inner).rewritten : inner; try { - const parsed = parser.parse(inner, { database: dialect.grammar }); + const parsed = parser.parse(toParse, { database: dialect.grammar }); ast = parsed.ast; tableList = Array.isArray(parsed.tableList) ? parsed.tableList : []; } catch { diff --git a/packages/core/src/semantics.ts b/packages/core/src/semantics.ts index cbc415b..4b146cb 100644 --- a/packages/core/src/semantics.ts +++ b/packages/core/src/semantics.ts @@ -262,3 +262,116 @@ function aggregateName(node: Node): string { : ''; return text.toUpperCase(); } + +/** A comparison whose two sides cannot mean the same thing: an integer column against a date. */ +export interface EpochMismatch { + /** The column as the catalog spells it. */ + readonly column: string; + readonly dbType: string; + /** The date expression it was compared against, rendered for the message. */ + readonly comparedTo: string; +} + +interface TypedCatalog { + readonly tables: readonly { + readonly name: string; + readonly columns: readonly { readonly name: string; readonly dbType?: string }[]; + }[]; +} + +/** + * A column that stores a moment as a number: SQLite has no date type, so Room writes epoch + * milliseconds into INTEGER, and a hand-rolled schema may write epoch seconds. + */ +const INTEGER_DB_TYPE = + /^(?:big\s*int|int|integer|int2|int4|int8|smallint|tinyint|mediumint|unsigned\s+big\s+int|numeric|number)\b/i; + +/** SQLite's date builders, plus the standard keywords. All of them produce text or a day number. */ +const DATE_FUNCTION = + /^(?:date|datetime|time|strftime|julianday|unixepoch|current_date|current_time|current_timestamp|now|getdate|sysdate)$/i; + +/** A literal a person writes for a day or an instant, which is text however it is compared. */ +const DATE_LITERAL = /^\d{4}-\d{2}-\d{2}(?:[ T]\d{2}:\d{2}(?::\d{2})?)?$/; + +function renderDateSide(node: Node): string | null { + const type = node['type']; + if (type === 'function' || type === 'aggr_func') { + const name = aggregateName(node); + return DATE_FUNCTION.test(name) ? `${name}(...)` : null; + } + // CURRENT_DATE and friends arrive as a bare keyword rather than a call. + if (type === 'origin' || type === 'keyword') { + const value = node['value']; + return typeof value === 'string' && DATE_FUNCTION.test(value.replace(/\s+/g, '_')) ? value : null; + } + if (type === 'single_quote_string' || type === 'string') { + const value = node['value']; + return typeof value === 'string' && DATE_LITERAL.test(value.trim()) ? `'${value}'` : null; + } + // date('now','-7 days') nested under a cast, or strftime wrapped in one. + if (type === 'cast' && isNode(node['expr'])) return renderDateSide(node['expr'] as Node); + return null; +} + +/** The catalog type of a column named anywhere in the query, or null when it is not attributable. */ +function dbTypeOf(column: string, catalog: TypedCatalog): string | null { + const matches: string[] = []; + for (const table of catalog.tables) { + for (const c of table.columns) { + if (c.name.toLowerCase() === column.toLowerCase() && typeof c.dbType === 'string') matches.push(c.dbType); + } + } + // Two tables typing the same name differently is not attributable from the name alone. + if (matches.length === 0) return null; + const first = matches[0]!; + return matches.every((m) => m.toLowerCase() === first.toLowerCase()) ? first : null; +} + +/** + * A column holding a number compared against a date. Against text nothing matches and zero is reported; + * against epoch seconds a milliseconds column matches every row. Neither errors. + */ +export function epochUnitMismatch(sql: string, grammar: string, catalog: TypedCatalog): EpochMismatch | null { + let ast: unknown; + try { + ast = parser.parse(withoutFetchTail(sql), { database: grammar }).ast; + } catch { + return null; + } + + let found: EpochMismatch | null = null; + const visit = (node: unknown): void => { + if (found || !isNode(node)) return; + if (Array.isArray(node)) { + for (const item of node) visit(item); + return; + } + if (node['type'] === 'binary_expr') { + const left = node['left']; + const right = node['right']; + for (const [maybeColumn, maybeDate] of [ + [left, right], + [right, left], + ] as const) { + if (!isNode(maybeColumn) || maybeColumn['type'] !== 'column_ref') continue; + const column = columnNameOf(maybeColumn); + if (!column) continue; + const dbType = dbTypeOf(column, catalog); + if (!dbType || !INTEGER_DB_TYPE.test(dbType.trim())) continue; + // BETWEEN carries its bounds as a list; either bound being a date is the same mistake. + const candidates = + isNode(maybeDate) && Array.isArray(maybeDate['value']) ? (maybeDate['value'] as unknown[]) : [maybeDate]; + for (const candidate of candidates) { + const rendered = isNode(candidate) ? renderDateSide(candidate as Node) : null; + if (rendered) { + found = { column, dbType, comparedTo: rendered }; + return; + } + } + } + } + for (const value of Object.values(node)) visit(value); + }; + visit(ast); + return found; +} diff --git a/packages/core/test/epoch-mismatch-sweep.test.ts b/packages/core/test/epoch-mismatch-sweep.test.ts new file mode 100644 index 0000000..fe165d2 --- /dev/null +++ b/packages/core/test/epoch-mismatch-sweep.test.ts @@ -0,0 +1,170 @@ +/** + * A numeric column compared against a date answers zero against text, or the whole table against epoch + * seconds, and never errors. Measured on a Room fixture: 7B returned 0 where the truth was 2, 30B + * returned 5. Sweeps the cross product of column type, date expression, operator, shape and dialect, + * because firing on a TEXT column would refuse correct SQL. + */ +import { describe, expect, it } from 'vitest'; +import { epochUnitMismatch } from '../src/semantics.js'; +import { DUCKDB_DIALECT, MYSQL_DIALECT, ORACLE_DIALECT, POSTGRES_DIALECT, SQLITE_DIALECT } from '../src/dialects.js'; +import type { DialectInfo } from '../src/types.js'; + +/** Types that hold a number, so a date on the other side cannot mean the same thing. */ +const NUMERIC_TYPES = ['INTEGER', 'INT', 'int', 'BIGINT', 'SMALLINT', 'TINYINT', 'MEDIUMINT', 'INT8', 'NUMERIC']; +/** Types that hold a date or text, where comparing against a date is exactly right. */ +const DATE_SAFE_TYPES = ['TEXT', 'VARCHAR(32)', 'DATE', 'TIMESTAMP', 'DATETIME', 'REAL', 'BLOB', 'BOOLEAN']; + +/** Expressions that produce a date or an instant. */ +const DATE_EXPRESSIONS = [ + "date('now')", + "date('now','-7 days')", + "datetime('now')", + "strftime('%s','now')", + "strftime('%Y-%m-%d','now')", + "julianday('now')", + 'CURRENT_DATE', + 'CURRENT_TIMESTAMP', + "'2026-08-09'", + "'2026-08-09 12:30:00'", +]; +/** Right-hand sides that are legitimately numeric, or not a date at all. */ +const SAFE_EXPRESSIONS = [ + '1755300000000', + '0', + "(strftime('%s','now') - 7*86400) * 1000", + "(strftime('%s','now') - 7*86400)", + 'other_number', + "'not-a-date'", + "'2026'", +]; + +const OPERATORS = ['>=', '>', '<', '<=', '=', '<>']; + +/** Query shapes the same comparison can hide in. */ +const SHAPES: readonly ((lhs: string, op: string, rhs: string) => string)[] = [ + (l, o, r) => `SELECT * FROM events WHERE ${l} ${o} ${r}`, + (l, o, r) => `SELECT * FROM events e WHERE e.${l} ${o} ${r}`, + (l, o, r) => `SELECT * FROM events WHERE ${r} ${o} ${l}`, + (l, o, r) => `SELECT * FROM events WHERE label = 'x' AND ${l} ${o} ${r}`, + (l, o, r) => `SELECT * FROM events WHERE label = 'x' OR ${l} ${o} ${r}`, + (l, o, r) => `SELECT COUNT(*) FROM events WHERE ${l} ${o} ${r}`, + (l, o, r) => `SELECT label, COUNT(*) FROM events WHERE ${l} ${o} ${r} GROUP BY label`, + (l, o, r) => `SELECT * FROM events JOIN people ON people.id = events.person_id WHERE ${l} ${o} ${r}`, +]; + +const DIALECTS: [string, DialectInfo][] = [ + ['postgres', POSTGRES_DIALECT], + ['mysql', MYSQL_DIALECT], + ['sqlite', SQLITE_DIALECT], + ['duckdb', DUCKDB_DIALECT], + ['oracle', ORACLE_DIALECT], +]; + +const catalogWith = (dbType: string) => ({ + tables: [ + { + name: 'events', + columns: [ + { name: 'happened_at', dbType }, + { name: 'other_number', dbType: 'INTEGER' }, + { name: 'label', dbType: 'TEXT' }, + { name: 'person_id', dbType: 'INTEGER' }, + ], + }, + { name: 'people', columns: [{ name: 'id', dbType: 'INTEGER' }] }, + ], +}); + +describe('a numeric column compared against a date is always caught', () => { + for (const [dialectName, dialect] of DIALECTS) { + for (const dbType of NUMERIC_TYPES) { + it(`${dialectName}/${dbType}: every date expression, operator and shape is flagged`, () => { + const catalog = catalogWith(dbType); + const missed: string[] = []; + let checked = 0; + for (const expr of DATE_EXPRESSIONS) { + for (const op of OPERATORS) { + for (const shape of SHAPES) { + const sql = shape('happened_at', op, expr); + checked++; + if (epochUnitMismatch(sql, dialect.grammar, catalog) === null) missed.push(sql); + } + } + } + expect(checked).toBe(DATE_EXPRESSIONS.length * OPERATORS.length * SHAPES.length); + expect(missed, `${missed.length} of ${checked} not flagged, e.g. ${missed[0]}`).toEqual([]); + }); + } + } +}); + +describe('a column that legitimately holds a date is never flagged', () => { + for (const [dialectName, dialect] of DIALECTS) { + for (const dbType of DATE_SAFE_TYPES) { + it(`${dialectName}/${dbType}: comparing it with a date stays quiet`, () => { + const catalog = catalogWith(dbType); + const wrong: string[] = []; + for (const expr of DATE_EXPRESSIONS) { + for (const op of OPERATORS) { + for (const shape of SHAPES) { + const sql = shape('happened_at', op, expr); + if (epochUnitMismatch(sql, dialect.grammar, catalog) !== null) wrong.push(sql); + } + } + } + expect(wrong, `${wrong.length} correct queries refused, e.g. ${wrong[0]}`).toEqual([]); + }); + } + } +}); + +describe('a numeric column compared numerically is never flagged', () => { + for (const [dialectName, dialect] of DIALECTS) { + for (const dbType of NUMERIC_TYPES) { + it(`${dialectName}/${dbType}: a numeric bound is the right way to write it`, () => { + const catalog = catalogWith(dbType); + const wrong: string[] = []; + for (const expr of SAFE_EXPRESSIONS) { + for (const op of OPERATORS) { + for (const shape of SHAPES) { + const sql = shape('happened_at', op, expr); + if (epochUnitMismatch(sql, dialect.grammar, catalog) !== null) wrong.push(sql); + } + } + } + expect(wrong, `${wrong.length} correct queries refused, e.g. ${wrong[0]}`).toEqual([]); + }); + } + } +}); + +describe('the shapes that must never be judged at all', () => { + const catalog = catalogWith('INTEGER'); + const g = SQLITE_DIALECT.grammar; + + it('a date expression in the SELECT list is not a comparison', () => { + expect(epochUnitMismatch("SELECT date('now') AS today, happened_at FROM events", g, catalog)).toBeNull(); + }); + + it('a column the catalog does not know is left alone', () => { + expect(epochUnitMismatch("SELECT * FROM events WHERE unknown_col >= date('now')", g, catalog)).toBeNull(); + }); + + it('a name two tables type differently is not attributable', () => { + const ambiguous = { + tables: [ + { name: 'events', columns: [{ name: 'happened_at', dbType: 'INTEGER' }] }, + { name: 'logs', columns: [{ name: 'happened_at', dbType: 'TEXT' }] }, + ], + }; + expect(epochUnitMismatch("SELECT * FROM events WHERE happened_at >= date('now')", g, ambiguous)).toBeNull(); + }); + + it('unparsable SQL fails open rather than blocking', () => { + expect(epochUnitMismatch('SELECT FROM WHERE', g, catalog)).toBeNull(); + }); + + it('IS NULL on a numeric column is not a date comparison', () => { + expect(epochUnitMismatch('SELECT * FROM events WHERE happened_at IS NOT NULL', g, catalog)).toBeNull(); + }); +}); diff --git a/packages/core/test/sqlite-match.test.ts b/packages/core/test/sqlite-match.test.ts new file mode 100644 index 0000000..660c8c6 --- /dev/null +++ b/packages/core/test/sqlite-match.test.ts @@ -0,0 +1,64 @@ +/** + * SQLite parses under the Postgresql grammar, which has no MATCH, so every Room @Fts4 query was refused + * as unparseable. Verified against a populated FTS4 table: MATCH returns the matching rows, and the + * `= 'term'` form a prompt might steer to returns none. The statement that runs keeps MATCH verbatim. + */ +import { describe, expect, it } from 'vitest'; +import { guardSql } from '../src/guard.js'; +import { POSTGRES_DIALECT, SQLITE_DIALECT } from '../src/dialects.js'; + +const sqlite = (sql: string) => guardSql({ sql, dialect: SQLITE_DIALECT }); + +describe('a full-text query is allowed, and runs as written', () => { + it('accepts MATCH against the table and against a column', () => { + for (const sql of [ + "SELECT rowid FROM messages_fts WHERE messages_fts MATCH 'memory'", + "SELECT body FROM messages_fts WHERE body MATCH 'memory'", + "SELECT m.body FROM messages m JOIN messages_fts f ON f.rowid = m.id WHERE f.body MATCH 'rope memory'", + ]) { + const v = sqlite(sql); + expect(v.allowed, `${sql} -> ${v.reason ?? ''}`).toBe(true); + // The operator survives: rewritten to `=` it would silently return nothing on FTS4. + expect(v.sql).toMatch(/\bMATCH\b/); + } + }); + + it('keeps the search term exactly, spaces and quotes included', () => { + const v = sqlite("SELECT rowid FROM t_fts WHERE t_fts MATCH 'rope memory'"); + expect(v.sql).toContain("MATCH 'rope memory'"); + }); + + it('still applies the row cap to a full-text query', () => { + const v = sqlite("SELECT body FROM messages_fts WHERE body MATCH 'a'"); + expect(v.autoLimited).toBe(true); + expect(v.sql).toMatch(/LIMIT \d+/); + }); +}); + +describe('nothing else slips in behind MATCH', () => { + const mustBlock: [string, string][] = [ + ['a write', "DELETE FROM messages WHERE body MATCH 'x'"], + ['an update', "UPDATE messages SET body = 'x' WHERE body MATCH 'y'"], + ['a stacked statement', "SELECT 1 FROM t WHERE a MATCH 'x'; DROP TABLE t"], + ['an ATTACH after it', "SELECT 1 FROM t WHERE a MATCH 'x'; ATTACH DATABASE '/tmp/y.db' AS y"], + ['a denied function', "SELECT load_extension('x') FROM t WHERE a MATCH 'y'"], + ['a right side that is a column', 'SELECT * FROM t WHERE a MATCH b'], + ['a right side that is a subquery', 'SELECT * FROM t WHERE a MATCH (SELECT x FROM y)'], + ['a right side that is a parameter', 'SELECT * FROM t WHERE a MATCH ?'], + ]; + for (const [label, sql] of mustBlock) { + it(`refuses ${label}`, () => { + expect(sqlite(sql).allowed, sql).toBe(false); + }); + } + + it('leaves the word alone inside a string literal', () => { + const v = sqlite("SELECT * FROM t WHERE note = 'a match here'"); + expect(v.allowed).toBe(true); + expect(v.sql).toContain("'a match here'"); + }); + + it('does not accept MATCH on an engine that has no such operator', () => { + expect(guardSql({ sql: "SELECT * FROM t WHERE a MATCH 'x'", dialect: POSTGRES_DIALECT }).allowed).toBe(false); + }); +}); diff --git a/packages/jetbrains/CHANGELOG.md b/packages/jetbrains/CHANGELOG.md index e0a0628..ab57a85 100644 --- a/packages/jetbrains/CHANGELOG.md +++ b/packages/jetbrains/CHANGELOG.md @@ -3,6 +3,32 @@ All notable changes to the AskSQL JetBrains plugin are documented here. Format loosely follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [0.5.4] - 2026-08-18 + +Android Studio is where this plugin is mostly installed, and an Android app's database is SQLite via +Room. Every fixture the test suite owned used real types, so none of this was covered until a +Room-shaped database was tried. + +### Fixed +- A query over a table with a `ByteArray` column returned nothing at all. SQLite's driver reports those + cells as BLOB and implements none of the streaming interface, so reading them threw and the whole + result set was lost, reported as "the database didn't accept that query". +- `SELECT rowid FROM users` was refused as an invented column. SQLite gives every table `rowid`, `oid` + and `_rowid_` without listing them, and FTS tables answer to `docid` and `rank`. +- Opening a file that is not a readable database - encrypted, truncated, or the `-wal` sidecar picked by + mistake - reopened it about 21,000 times a second until the operation timed out, pegging a core and + then reporting something unrelated. It now fails once, saying that a database pulled from a device + needs its `-wal` and `-shm` files alongside it. +- A date comparison against an integer timestamp answered confidently and wrongly: Room stores epoch + milliseconds, and comparing that with a text date matches nothing while comparing it with epoch + seconds matches everything. Neither raised an error. The guidance now states the units, and a check + catches the comparison and sends it back to be corrected. +- Full-text search ran at all. A Room `@Fts4` entity is queried with `MATCH`, which the SQL parser has + no notion of, so every full-text query was refused as unparseable. Writes, stacked statements and + denied functions are still refused. +- A relationship question about two collections is answered in prose on MongoDB, as it already was on + the SQL engines, rather than returning documents. + ## [0.5.3] - 2026-08-15 ### Security diff --git a/packages/jetbrains/gradle.properties b/packages/jetbrains/gradle.properties index 69f69c4..6649643 100644 --- a/packages/jetbrains/gradle.properties +++ b/packages/jetbrains/gradle.properties @@ -4,7 +4,7 @@ pluginGroup = com.rahulmahadik.asksql pluginName = AskSQL -pluginVersion = 0.5.3 +pluginVersion = 0.5.4 # IntelliJ Platform target used to COMPILE and RUN the sandbox. Broad # compatibility is governed by pluginSinceBuild/pluginUntilBuild in diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistry.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistry.kt index c50afaa..caad70c 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistry.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistry.kt @@ -4,6 +4,8 @@ import com.intellij.openapi.Disposable import com.intellij.openapi.components.Service import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.project.Project +import com.rahulmahadik.asksql.ide.errors.AskSqlErrorCode +import com.rahulmahadik.asksql.ide.errors.AskSqlException import com.rahulmahadik.asksql.ide.model.EngineKind import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Deferred @@ -56,8 +58,12 @@ class ConnectionRegistry(private val project: Project, private val scope: Corout } } + /** A dropped connection deserves one fresh open; more than that is a loop, not a retry. */ + private val MAX_STALE_RETRIES = 2 + private suspend fun acquire(descriptor: ConnectionDescriptor, password: String?, duckDbDriverJarPath: String?, oracleDriverJarPath: String?): Pair { val generation = generations.getOrPut(descriptor.id) { AtomicInteger(0) }.get() + var staleRetries = 0 while (true) { // compute() runs its remapping function at most once per key, so racers for one not-yet-cached id share a single open. @@ -74,12 +80,12 @@ class ConnectionRegistry(private val project: Project, private val scope: Corout throw e } // DuckDB's isValid() runs a real SELECT; take JdbcExecutor's per-connection lock like any statement. - val valid = if (descriptor.engine == EngineKind.DUCKDB) { - JdbcExecutor.withConnectionLock(connection) { isValid(connection) } + val state = if (descriptor.engine == EngineKind.DUCKDB) { + JdbcExecutor.withConnectionLock(connection) { health(connection) } } else { - isValid(connection) + health(connection) } - if (valid) return slot to connection + if (state is Health.Ok) return slot to connection // Removes only this exact stale instance; a replacement another caller already installed is adopted instead. if (slots.remove(descriptor.id, slot)) { @@ -87,6 +93,25 @@ class ConnectionRegistry(private val project: Project, private val scope: Corout slot.superseded = true if (slot.leases.get() == 0) closeQuietly(connection) } + + if (state is Health.Broken) { + throw AskSqlException( + AskSqlErrorCode.DB_UNREACHABLE, + userMessage = "That file opened, but it is not a readable database. A database pulled from a device " + + "needs its -wal and -shm files alongside it, and an encrypted database cannot be read directly.", + detail = state.detail, + retryable = false, + ) + } + // One fresh open for a dropped connection; a second failure is not going to be different. + if (++staleRetries >= MAX_STALE_RETRIES) { + throw AskSqlException( + AskSqlErrorCode.DB_UNREACHABLE, + userMessage = "The database connection could not be established. Check that it is running and reachable.", + detail = "connection did not validate after $MAX_STALE_RETRIES attempts", + retryable = true, + ) + } } } @@ -98,10 +123,29 @@ class ConnectionRegistry(private val project: Project, private val scope: Corout }, ) - private fun isValid(connection: Connection): Boolean = try { - !connection.isClosed && connection.isValid(2) + /** Whether a cached connection can still be used, and if not, whether opening again could help. */ + private sealed interface Health { + object Ok : Health + + /** Dropped or timed out: a fresh open is worth one try. */ + object Stale : Health + + /** The file itself is not a database. Opening again gives the same answer, forever. */ + data class Broken(val detail: String) : Health + } + + /** + * SQLite opens lazily: a file that is not a database connects and fails here, by throwing rather + * than returning false. Read as "stale, open again" that spun at ~21,000 opens a second until the + * caller's timeout. These result codes never recover. + */ + private val TERMINAL_CODES = Regex("SQLITE_(NOTADB|CORRUPT|CANTOPEN|PERM|AUTH|READONLY_DBMOVED)", RegexOption.IGNORE_CASE) + + private fun health(connection: Connection): Health = try { + if (!connection.isClosed && connection.isValid(2)) Health.Ok else Health.Stale } catch (e: Exception) { - false + val message = e.message ?: "" + if (TERMINAL_CODES.containsMatchIn(message)) Health.Broken(message.take(200)) else Health.Stale } /** Bumps the generation so the next [withConnection] rebuilds it. If still leased, the lease holder closes it on completion instead of closing here. */ diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/JdbcExecutor.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/JdbcExecutor.kt index b8fb978..b1562af 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/JdbcExecutor.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/db/JdbcExecutor.kt @@ -210,8 +210,21 @@ object JdbcExecutor { if (rs.wasNull() || bytes == null) CellValue.Null else binaryPreview(bytes) } Types.BLOB -> { - val blob = rs.getBlob(index) - if (rs.wasNull() || blob == null) { + // SQLite's driver reports BLOB but implements none of java.sql.Blob, so getBlob throws and + // the whole result set is lost. getBytes reads the same column, and only a preview is kept. + val blob = try { + rs.getBlob(index) + } catch (e: java.sql.SQLFeatureNotSupportedException) { + null + } + if (blob == null) { + val bytes = rs.getBytes(index) + if (rs.wasNull() || bytes == null) { + CellValue.Null + } else { + CellValue.Binary(BinaryPreview(bytes.size.toLong(), toHex(bytes.take(HEX_PREVIEW_BYTES).toByteArray()))) + } + } else if (rs.wasNull()) { CellValue.Null } else { val length = blob.length() diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/EnginePipeline.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/EnginePipeline.kt index 36202bd..ed7f8e2 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/EnginePipeline.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/EnginePipeline.kt @@ -685,6 +685,24 @@ class EnginePipeline( continue } + // Epoch floor, mirroring packages/core/src/engine.ts: SQLite has no date type, so Room writes + // epoch milliseconds into an INTEGER. Compared with a text date nothing matches and the answer + // is reported as zero; compared with epoch seconds every row matches. Neither errors. + val epoch = Semantics.epochUnitMismatch(verdict.sql, fullCatalog) + if (epoch != null && attempt < MAX_REPAIRS) { + userPrompt = Prompts.buildRepairUser( + question = q, failedSql = verdict.sql, + failure = "\"${epoch.column}\" is ${epoch.dbType}, so it holds a number, not a date, and comparing it " + + "with ${epoch.comparedTo} does not select the rows intended: against text nothing matches, and " + + "against epoch seconds a column of milliseconds matches everything. Compare it in its own units - " + + "build the bound as a number, for example (strftime('%s','now') - 7*86400) * 1000 for " + + "milliseconds - or convert the column with the matching divisor before comparing.", + schemaText = schemaText, dialect = dialect, + ) + attempt++ + continue + } + val unknownColumn = HallucinationChecks.firstUnknownColumn(verdict.sql, fullCatalog) if (unknownColumn != null) { if (attempt >= MAX_REPAIRS) { diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/HallucinationChecks.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/HallucinationChecks.kt index a8d8997..ead431f 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/HallucinationChecks.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/HallucinationChecks.kt @@ -1,5 +1,6 @@ package com.rahulmahadik.asksql.ide.engine +import com.rahulmahadik.asksql.ide.model.EngineKind import com.rahulmahadik.asksql.ide.model.SchemaCatalog import net.sf.jsqlparser.expression.ExpressionVisitorAdapter import net.sf.jsqlparser.parser.CCJSqlParserUtil @@ -140,6 +141,12 @@ object HallucinationChecks { return null } + /** + * Columns SQLite gives every table without listing them, so `PRAGMA table_info` never reports them. + * On a WITHOUT ROWID table the database rejects the name, which the repair loop can act on. + */ + private val SQLITE_IMPLICIT_COLUMNS = setOf("rowid", "oid", "_rowid_", "docid", "rank") + fun firstUnknownColumn(sql: String, catalog: SchemaCatalog): UnknownColumn? { val statement = try { CCJSqlParserUtil.parse(sql) @@ -201,6 +208,7 @@ object HallucinationChecks { if (table == null) { if (!attributable || aliases.contains(column) || queryTables.isEmpty()) continue + if (catalog.engine == EngineKind.SQLITE && column.lowercase() in SQLITE_IMPLICIT_COLUMNS) continue if (queryTables.any { byTable[it]?.contains(column) == true }) continue val available = queryTables.flatMap { byTable[it].orEmpty() }.toSortedSet() return UnknownColumn(queryTables.first(), column, available.toList()) @@ -212,6 +220,7 @@ object HallucinationChecks { if (cteNames.contains(resolvedTable) || SYSTEM_SCHEMAS.contains(resolvedTable)) continue val known = byTable[resolvedTable] ?: continue // derived/subquery alias or unknown table; fail open if (known.contains(column)) continue + if (catalog.engine == EngineKind.SQLITE && column.lowercase() in SQLITE_IMPLICIT_COLUMNS) continue return UnknownColumn(resolvedTable, column, known.toSortedSet().toList()) } return null diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/Semantics.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/Semantics.kt index cfeb525..3aedf60 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/Semantics.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/engine/Semantics.kt @@ -164,6 +164,86 @@ object Semantics { return null } + /** A comparison whose two sides cannot mean the same thing: a numeric column against a date. */ + data class EpochMismatch(val column: String, val dbType: String, val comparedTo: String) + + /** Types that hold a number, so a date on the other side cannot mean the same thing. */ + private val INTEGER_DB_TYPE = + Regex("""^(?:big\s*int|int|integer|int2|int4|int8|smallint|tinyint|mediumint|unsigned\s+big\s+int|numeric|number)\b""", RegexOption.IGNORE_CASE) + + /** SQLite's date builders plus the standard keywords; all produce text or a day number. */ + private val DATE_FUNCTION = + Regex("""^(?:date|datetime|time|strftime|julianday|unixepoch|current_date|current_time|current_timestamp|now|getdate|sysdate)$""", RegexOption.IGNORE_CASE) + + /** A literal a person writes for a day or an instant, which is text however it is compared. */ + private val DATE_LITERAL = Regex("""^\d{4}-\d{2}-\d{2}(?:[ T]\d{2}:\d{2}(?::\d{2})?)?$""") + + private fun dateSideOf(expression: Expression?): String? = when (expression) { + null -> null + is net.sf.jsqlparser.expression.Function -> + if (DATE_FUNCTION.matches(expression.name ?: "")) "${expression.name}(...)" else null + is net.sf.jsqlparser.expression.TimeKeyExpression -> + if (DATE_FUNCTION.matches((expression.stringValue ?: "").replace(" ", "_"))) expression.stringValue else null + is net.sf.jsqlparser.expression.StringValue -> + if (DATE_LITERAL.matches(expression.value.trim())) "'${expression.value}'" else null + is net.sf.jsqlparser.expression.CastExpression -> dateSideOf(expression.leftExpression) + else -> null + } + + /** The catalog type of a column named anywhere in the query, or null when it is not attributable. */ + private fun dbTypeOf(column: String, catalog: com.rahulmahadik.asksql.ide.model.SchemaCatalog): String? { + val types = catalog.tables.flatMap { t -> t.columns.filter { it.name.equals(column, true) }.map { it.dbType } } + if (types.isEmpty()) return null + // Two tables typing the same name differently is not attributable from the name alone. + return types.first().takeIf { first -> types.all { it.equals(first, true) } } + } + + /** + * A numeric column compared against a date: against text nothing matches, against epoch seconds a + * milliseconds column matches every row, and neither errors. Mirrors core's semantics.ts. + */ + fun epochUnitMismatch(sql: String, catalog: com.rahulmahadik.asksql.ide.model.SchemaCatalog): EpochMismatch? { + val statement = try { + CCJSqlParserUtil.parse(sql) + } catch (e: Exception) { + return null // the guard already parsed it; never double-block here + } + val select = statement as? Select ?: return null + + fun checkPair(maybeColumn: Expression?, maybeDate: Expression?): EpochMismatch? { + val col = maybeColumn as? net.sf.jsqlparser.schema.Column ?: return null + val name = col.columnName ?: return null + val dbType = dbTypeOf(name, catalog) ?: return null + if (!INTEGER_DB_TYPE.containsMatchIn(dbType.trim())) return null + val rendered = dateSideOf(maybeDate) ?: return null + return EpochMismatch(name, dbType, rendered) + } + + // The file's own reflective walk, rather than a visitor: JSqlParser's visitor is generic here + // and every comparison would need its own override. + fun scanExpression(expression: Expression?, depth: Int = 0): EpochMismatch? { + if (expression == null || depth > MAX_DEPTH) return null + when (expression) { + is net.sf.jsqlparser.expression.operators.relational.ComparisonOperator -> + checkPair(expression.leftExpression, expression.rightExpression) + ?: checkPair(expression.rightExpression, expression.leftExpression) + is net.sf.jsqlparser.expression.operators.relational.Between -> + checkPair(expression.leftExpression, expression.betweenExpressionStart) + ?: checkPair(expression.leftExpression, expression.betweenExpressionEnd) + else -> null + }?.let { return it } + for (child in childrenOf(expression)) scanExpression(child, depth + 1)?.let { return it } + return null + } + + for (plain in plainSelects(select)) { + scanExpression(plain.where)?.let { return it } + scanExpression(plain.having)?.let { return it } + plain.joins?.forEach { j -> j.onExpressions?.forEach { on -> scanExpression(on)?.let { return it } } } + } + return null + } + fun ungroupedAggregate(sql: String): String? { val statement = try { CCJSqlParserUtil.parse(sql) diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/guard/SqlGuard.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/guard/SqlGuard.kt index c7ab2ff..1fb9a0c 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/guard/SqlGuard.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/guard/SqlGuard.kt @@ -126,8 +126,12 @@ object SqlGuard { } // ---- Parse once, fail-closed ---- + // MATCH is not in JSqlParser's grammar, so every Room @Fts4 query was refused as unparseable. + // Validated as a comparison; the rewrite is parse-only and length-preserving, so the statement + // that runs keeps MATCH verbatim. Mirrors core's guard.ts. + val toParse = if (dialect.engine == EngineKind.SQLITE) rewriteSqliteMatch(inner) else inner val statement: Statement = try { - CCJSqlParserUtil.parse(inner) + CCJSqlParserUtil.parse(toParse) } catch (e: JSQLParserException) { return blocked(original, "parse_failed", "The statement could not be verified as safe SQL for this database, so it was blocked.") } catch (e: StackOverflowError) { @@ -257,6 +261,22 @@ object SqlGuard { ) } + /** + * `match` becomes `=` and four spaces, so every offset is preserved. Only the operator with a + * single-quoted literal is rewritten; anything else still fails closed. + */ + private val SQLITE_MATCH_RE = Regex("""(\s)match(\s+'(?:[^']|'')*')""", RegexOption.IGNORE_CASE) + + private fun rewriteSqliteMatch(sql: String): String { + val masked = SqlLexer.stripCommentsAndStrings(sql) + return SQLITE_MATCH_RE.replace(sql) { m -> + // Only outside a string or comment: a literal containing the word "match" is not the operator. + val span = masked.substring(m.range.first, minOf(m.range.last + 1, masked.length)) + if (!span.contains("match", ignoreCase = true)) m.value + else "${m.groupValues[1]}= ${m.groupValues[2]}" + } + } + private fun blocked(sql: String, ruleId: String, reason: String) = GuardVerdict(allowed = false, sql = sql, ruleId = ruleId, reason = reason) diff --git a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/model/Dialect.kt b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/model/Dialect.kt index 77fa6c4..1f1bdf8 100644 --- a/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/model/Dialect.kt +++ b/packages/jetbrains/src/main/kotlin/com/rahulmahadik/asksql/ide/model/Dialect.kt @@ -75,7 +75,11 @@ object Dialects { promptLabel = "SQLite", limitStyle = LimitStyle.LIMIT, promptNotes = listOf( - "Use date/datetime/strftime for date math (e.g. date('now','-30 days')).", + "Dates: a TEXT column holds ISO text, so compare it with date/datetime/strftime " + + "(e.g. date('now','-30 days')). An INTEGER column holds a number - usually epoch seconds, or " + + "milliseconds if the values are ~1000x larger - so build the bound as a number in the SAME " + + "units, e.g. (strftime('%s','now') - 30*86400) * 1000 for milliseconds. Never compare an " + + "INTEGER column with a text date: nothing matches and no error is raised.", "There are no schemas; refer to tables by bare name.", "Combine values into one string with group_concat(col, ', ').", ), diff --git a/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistryTest.kt b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistryTest.kt index 2df0ea8..c047813 100644 --- a/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistryTest.kt +++ b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/ConnectionRegistryTest.kt @@ -1,6 +1,8 @@ package com.rahulmahadik.asksql.ide.db import com.rahulmahadik.asksql.ide.model.EngineKind +import com.rahulmahadik.asksql.ide.errors.AskSqlErrorCode +import com.rahulmahadik.asksql.ide.errors.AskSqlException import com.rahulmahadik.asksql.ide.test.fakeProject import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope @@ -15,6 +17,7 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertNotSame import org.junit.Assert.assertSame import org.junit.Assert.assertTrue +import org.junit.Assert.assertNotNull import org.junit.Test import java.sql.Connection @@ -32,6 +35,36 @@ class ConnectionRegistryTest { filePath = ":memory:", ) + @Test + fun `a file that is not a database fails once, instead of being reopened forever`() = runTest { + // SQLite opens lazily: an encrypted, truncated or wrong file connects and fails validation, and it + // fails by throwing. Read as "stale, open again" this reopened at ~21,000 opens a second until the + // caller's timeout, burning a core and then reporting something unrelated. Picking a `.db-wal` + // sidecar in the file chooser is enough to reach it. + val notADatabase = java.io.File.createTempFile("asksql-notadb", ".db").also { + it.writeText("this is not a database at all, just some text") + it.deleteOnExit() + } + val descriptor = sqliteDescriptor("bad-file").copy(filePath = notADatabase.absolutePath) + val registry = registry() + + val started = System.nanoTime() + var thrown: AskSqlException? = null + try { + registry.withConnection(descriptor, null) { it } + } catch (e: AskSqlException) { + thrown = e + } + val elapsedMs = (System.nanoTime() - started) / 1_000_000 + + assertNotNull("expected a classified failure, not a spin", thrown) + assertEquals(AskSqlErrorCode.DB_UNREACHABLE, thrown!!.code) + assertFalse("a file that is not a database will not become one on retry", thrown.retryable) + assertTrue("gave up in ${elapsedMs}ms, which is not a fast failure", elapsedMs < 5_000) + // The message has to say something a person can act on. + assertTrue(thrown.userMessage, thrown.userMessage.contains("-wal")) + } + @Test fun `withConnection reuses the same connection across calls`() = runTest { val registry = registry() diff --git a/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/SqliteCellFidelityTest.kt b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/SqliteCellFidelityTest.kt new file mode 100644 index 0000000..9f3539b --- /dev/null +++ b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/db/SqliteCellFidelityTest.kt @@ -0,0 +1,99 @@ +package com.rahulmahadik.asksql.ide.db + +import com.rahulmahadik.asksql.ide.model.CellValue +import com.rahulmahadik.asksql.ide.model.EngineKind +import com.rahulmahadik.asksql.ide.ui.displayString +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.sql.DriverManager + +/** + * A Room primary key is a Kotlin Long, so an id past 2^53 is ordinary. It survives only because + * [JdbcExecutor] reads the column type per ROW on SQLite: the driver reports the current row's storage + * class, answering INTEGER for a small value and BIGINT for a large one. Read once per result set, a + * column whose first row is small would round every later row. Nothing else covers that. + */ +class SqliteCellFidelityTest { + + private fun connect() = DriverManager.getConnection("jdbc:sqlite::memory:").also { + Class.forName("org.sqlite.JDBC") + } + + /** 2^53 + 1: the first integer a double cannot represent. */ + private val pastDoubleRange = "9007199254740993" + + private suspend fun read(sql: String, prepare: String) = connect().let { c -> + c.createStatement().use { st -> prepare.split(";\n").forEach { st.execute(it) } } + JdbcExecutor.execute(c, sql, maxRows = 10, timeoutMs = 5000, EngineKind.SQLITE).also { c.close() } + } + + @Test + fun `an id past a double's range is exact whatever type the column declares`() = runTest { + // Room writes INTEGER; a hand-rolled schema may write BIGINT, INT, or no type at all. + val result = read( + "SELECT * FROM t", + "CREATE TABLE t (declared_integer INTEGER, declared_bigint BIGINT, declared_int INT, untyped);\n" + + "INSERT INTO t VALUES ($pastDoubleRange, $pastDoubleRange, $pastDoubleRange, $pastDoubleRange)", + ) + for ((i, cell) in result.rows.first().withIndex()) { + assertEquals(result.columns[i].name, pastDoubleRange, displayString(cell)) + } + } + + @Test + fun `a column holding both a small id and a huge one keeps every digit of both`() = runTest { + // The case a per-result-set read would corrupt: the first row decides the type for the rest. + val result = read("SELECT id FROM t ORDER BY id", "CREATE TABLE t (id INTEGER);\nINSERT INTO t VALUES (5), ($pastDoubleRange)") + assertEquals(listOf("5", pastDoubleRange), result.rows.map { displayString(it.first()) }) + } + + @Test + fun `the same column read in the other order is still exact`() = runTest { + val result = read("SELECT id FROM t ORDER BY id DESC", "CREATE TABLE t (id INTEGER);\nINSERT INTO t VALUES (5), ($pastDoubleRange)") + assertEquals(listOf(pastDoubleRange, "5"), result.rows.map { displayString(it.first()) }) + } + + @Test + fun `a Room boolean and an epoch timestamp read as the app wrote them`() = runTest { + // Room has no boolean and no date type: 0/1 and epoch millis are what an Android schema holds. + val result = read( + "SELECT is_active, created_at FROM users", + "CREATE TABLE users (id INTEGER PRIMARY KEY, is_active INTEGER, created_at INTEGER);\n" + + "INSERT INTO users VALUES (1, 1, 1755300000000)", + ) + val row = result.rows.first() + // Not "1.0": an integer must never gain a decimal the database did not have. + assertEquals("1", displayString(row[0])) + assertEquals("1755300000000", displayString(row[1])) + } + + @Test + fun `a BLOB column does not take the whole result set down with it`() = runTest { + // A Room ByteArray field is a BLOB column. SQLite's driver reports Types.BLOB and implements + // none of java.sql.Blob, so reading it that way threw and every row of the query was lost, + // reported to the user as "the database didn't accept that query". + val result = read( + "SELECT id, thumb FROM photo ORDER BY id", + "CREATE TABLE photo (id INTEGER PRIMARY KEY, thumb BLOB);\n" + + "INSERT INTO photo VALUES (1, x'89504E470D0A1A0A'), (2, NULL)", + ) + assertEquals(2, result.rows.size) + val preview = result.rows[0][1] + assertTrue("expected a binary preview, got $preview", preview is CellValue.Binary) + assertEquals(8L, (preview as CellValue.Binary).preview.bytes) + // A NULL blob is still a NULL, not an empty preview. + assertTrue(result.rows[1][1] is CellValue.Null) + } + + @Test + fun `a NULL in an integer column stays distinguishable from zero`() = runTest { + val result = read( + "SELECT n FROM t ORDER BY rowid", + "CREATE TABLE t (n INTEGER);\nINSERT INTO t VALUES (0), (NULL)", + ) + assertEquals("0", displayString(result.rows[0].first())) + assertTrue(result.rows[1].first() is CellValue.Null) + } +} diff --git a/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/engine/EpochMismatchSweepTest.kt b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/engine/EpochMismatchSweepTest.kt new file mode 100644 index 0000000..22fd17d --- /dev/null +++ b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/engine/EpochMismatchSweepTest.kt @@ -0,0 +1,134 @@ +package com.rahulmahadik.asksql.ide.engine + +import com.rahulmahadik.asksql.ide.model.ColumnInfo +import com.rahulmahadik.asksql.ide.model.EngineKind +import com.rahulmahadik.asksql.ide.model.SchemaCatalog +import com.rahulmahadik.asksql.ide.model.TableInfo +import com.rahulmahadik.asksql.ide.model.TableKind +import org.junit.Assert.assertEquals +import org.junit.Test + +/** + * Room has no date type: a timestamp is an INTEGER of epoch milliseconds. Against a text date nothing + * matches and zero is reported; against epoch seconds every row matches. Measured: 7B answered 0 where + * the truth was 2, 30B answered 5. Sweeps the cross product, since firing on TEXT would refuse correct + * SQL. Mirrors epoch-mismatch-sweep.test.ts. + */ +class EpochMismatchSweepTest { + + private val numericTypes = listOf("INTEGER", "INT", "int", "BIGINT", "SMALLINT", "TINYINT", "MEDIUMINT", "INT8", "NUMERIC") + private val dateSafeTypes = listOf("TEXT", "VARCHAR(32)", "DATE", "TIMESTAMP", "DATETIME", "REAL", "BLOB", "BOOLEAN") + + private val dateExpressions = listOf( + "date('now')", + "date('now','-7 days')", + "datetime('now')", + "strftime('%s','now')", + "strftime('%Y-%m-%d','now')", + "julianday('now')", + "CURRENT_DATE", + "CURRENT_TIMESTAMP", + "'2026-08-09'", + "'2026-08-09 12:30:00'", + ) + private val safeExpressions = listOf( + "1755300000000", + "0", + "(strftime('%s','now') - 7*86400) * 1000", + "(strftime('%s','now') - 7*86400)", + "other_number", + "'not-a-date'", + "'2026'", + ) + private val operators = listOf(">=", ">", "<", "<=", "=", "<>") + + private val shapes: List<(String, String, String) -> String> = listOf( + { l, o, r -> "SELECT * FROM events WHERE $l $o $r" }, + { l, o, r -> "SELECT * FROM events e WHERE e.$l $o $r" }, + { l, o, r -> "SELECT * FROM events WHERE $r $o $l" }, + { l, o, r -> "SELECT * FROM events WHERE label = 'x' AND $l $o $r" }, + { l, o, r -> "SELECT * FROM events WHERE label = 'x' OR $l $o $r" }, + { l, o, r -> "SELECT COUNT(*) FROM events WHERE $l $o $r" }, + { l, o, r -> "SELECT label, COUNT(*) FROM events WHERE $l $o $r GROUP BY label" }, + { l, o, r -> "SELECT * FROM events JOIN people ON people.id = events.person_id WHERE $l $o $r" }, + ) + + private fun col(name: String, dbType: String) = ColumnInfo(name = name, dbType = dbType, nullable = true) + + private fun catalogWith(dbType: String) = SchemaCatalog( + engine = EngineKind.SQLITE, + schemas = emptyList(), + tables = listOf( + TableInfo( + schema = null, + name = "events", + kind = TableKind.TABLE, + columns = listOf(col("happened_at", dbType), col("other_number", "INTEGER"), col("label", "TEXT"), col("person_id", "INTEGER")), + ), + TableInfo(schema = null, name = "people", kind = TableKind.TABLE, columns = listOf(col("id", "INTEGER"))), + ), + ) + + @Test fun `every date expression against a numeric column is flagged, in every shape`() { + val missed = mutableListOf() + var checked = 0 + for (dbType in numericTypes) { + val catalog = catalogWith(dbType) + for (expr in dateExpressions) for (op in operators) for (shape in shapes) { + val sql = shape("happened_at", op, expr) + checked++ + if (Semantics.epochUnitMismatch(sql, catalog) == null) missed += "$dbType: $sql" + } + } + assertEquals(numericTypes.size * dateExpressions.size * operators.size * shapes.size, checked) + assertEquals("${missed.size} of $checked not flagged, e.g. ${missed.firstOrNull()}", emptyList(), missed) + } + + @Test fun `a column that legitimately holds a date is never flagged`() { + val wrong = mutableListOf() + for (dbType in dateSafeTypes) { + val catalog = catalogWith(dbType) + for (expr in dateExpressions) for (op in operators) for (shape in shapes) { + val sql = shape("happened_at", op, expr) + if (Semantics.epochUnitMismatch(sql, catalog) != null) wrong += "$dbType: $sql" + } + } + assertEquals("${wrong.size} correct queries refused, e.g. ${wrong.firstOrNull()}", emptyList(), wrong) + } + + @Test fun `a numeric column compared numerically is never flagged`() { + val wrong = mutableListOf() + for (dbType in numericTypes) { + val catalog = catalogWith(dbType) + for (expr in safeExpressions) for (op in operators) for (shape in shapes) { + val sql = shape("happened_at", op, expr) + if (Semantics.epochUnitMismatch(sql, catalog) != null) wrong += "$dbType: $sql" + } + } + assertEquals("${wrong.size} correct queries refused, e.g. ${wrong.firstOrNull()}", emptyList(), wrong) + } + + @Test fun `shapes that must never be judged at all`() { + val catalog = catalogWith("INTEGER") + // A date expression in the SELECT list is not a comparison. + assertEquals(null, Semantics.epochUnitMismatch("SELECT date('now') AS today, happened_at FROM events", catalog)) + // A column the catalog does not know is left alone. + assertEquals(null, Semantics.epochUnitMismatch("SELECT * FROM events WHERE unknown_col >= date('now')", catalog)) + // IS NULL is not a date comparison. + assertEquals(null, Semantics.epochUnitMismatch("SELECT * FROM events WHERE happened_at IS NOT NULL", catalog)) + // Unparsable SQL fails open rather than blocking. + assertEquals(null, Semantics.epochUnitMismatch("SELECT FROM WHERE", catalog)) + } + + @Test fun `a name two tables type differently is not attributable`() { + val ambiguous = SchemaCatalog( + engine = EngineKind.SQLITE, + schemas = emptyList(), + tables = listOf( + TableInfo(schema = null, name = "events", kind = TableKind.TABLE, columns = listOf(col("happened_at", "INTEGER"))), + TableInfo(schema = null, name = "logs", kind = TableKind.TABLE, columns = listOf(col("happened_at", "TEXT"))), + ), + ) + assertEquals(null, Semantics.epochUnitMismatch("SELECT * FROM events WHERE happened_at >= date('now')", ambiguous)) + } +} diff --git a/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/guard/SqliteMatchTest.kt b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/guard/SqliteMatchTest.kt new file mode 100644 index 0000000..ed75694 --- /dev/null +++ b/packages/jetbrains/src/test/kotlin/com/rahulmahadik/asksql/ide/guard/SqliteMatchTest.kt @@ -0,0 +1,57 @@ +package com.rahulmahadik.asksql.ide.guard + +import com.rahulmahadik.asksql.ide.model.Dialects +import com.rahulmahadik.asksql.ide.model.EngineKind +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * A Room @Fts4 entity is queried with MATCH, which JSqlParser's grammar has no notion of, so every + * full-text query was refused. Verified against a populated FTS4 table: MATCH returns the matching + * rows and the `= 'term'` form returns none. Mirrors sqlite-match.test.ts. + */ +class SqliteMatchTest { + + private fun guard(sql: String) = SqlGuard.guard(sql, Dialects.of(EngineKind.SQLITE)) + + @Test fun `a full-text query is allowed and keeps its operator`() { + for (sql in listOf( + "SELECT rowid FROM messages_fts WHERE messages_fts MATCH 'memory'", + "SELECT body FROM messages_fts WHERE body MATCH 'memory'", + "SELECT m.body FROM messages m JOIN messages_fts f ON f.rowid = m.id WHERE f.body MATCH 'rope memory'", + )) { + val v = guard(sql) + assertTrue("$sql -> ${v.reason}", v.allowed) + // Rewritten to `=` it would silently return nothing on FTS4, so the operator must survive. + assertTrue("$sql -> ${v.sql}", v.sql.contains("MATCH", ignoreCase = true)) + } + } + + @Test fun `the search term is kept exactly`() { + assertTrue(guard("SELECT rowid FROM t_fts WHERE t_fts MATCH 'rope memory'").sql.contains("MATCH 'rope memory'")) + } + + @Test fun `nothing else slips in behind it`() { + for (sql in listOf( + "DELETE FROM messages WHERE body MATCH 'x'", + "UPDATE messages SET body = 'x' WHERE body MATCH 'y'", + "SELECT 1 FROM t WHERE a MATCH 'x'; DROP TABLE t", + "SELECT load_extension('x') FROM t WHERE a MATCH 'y'", + "SELECT * FROM t WHERE a MATCH b", + "SELECT * FROM t WHERE a MATCH (SELECT x FROM y)", + )) { + assertFalse(sql, guard(sql).allowed) + } + } + + @Test fun `the word inside a string literal is left alone`() { + val v = guard("SELECT * FROM t WHERE note = 'a match here'") + assertTrue(v.allowed) + assertTrue(v.sql.contains("'a match here'")) + } + + @Test fun `an engine without the operator still refuses it`() { + assertFalse(SqlGuard.guard("SELECT * FROM t WHERE a MATCH 'x'", Dialects.of(EngineKind.POSTGRES)).allowed) + } +} diff --git a/packages/sqlite/CHANGELOG.md b/packages/sqlite/CHANGELOG.md index 6bdbfb0..b09178e 100644 --- a/packages/sqlite/CHANGELOG.md +++ b/packages/sqlite/CHANGELOG.md @@ -1,5 +1,23 @@ # @asksql/sqlite +## 0.5.0 + +### Minor Changes + +- Say what an integer timestamp counts, and say when a database is missing its `-wal`. + + Nothing in a SQLite schema records whether an integer timestamp holds seconds or milliseconds, so a + model guesses - and guessing seconds against a milliseconds column matches every row, reporting a whole + table as "this week". The unit is now stated in the schema the model reads, decided from an aggregate: + only the classification is recorded, never a value, so no cell value reaches the model. Bounded to 40 + columns per catalog read and measured at 41ms per million rows. + + Room defaults to WAL, so an Android database on disk is three files. Copying only `app.db` - what an + `adb pull` of the database gives you - left SQLite reporting no tables at all, and every question + answered "no such table" against what looked like an empty database. It now says the `-wal` file is + missing and what to do about it. The check reads the file header and the sidecar's size before opening, + because SQLite creates an empty `-wal` itself as soon as the file is opened. + ## 0.4.0 ### Minor Changes diff --git a/packages/sqlite/package.json b/packages/sqlite/package.json index 1f1bdfd..122f049 100644 --- a/packages/sqlite/package.json +++ b/packages/sqlite/package.json @@ -1,6 +1,6 @@ { "name": "@asksql/sqlite", - "version": "0.4.0", + "version": "0.5.0", "description": "SQLite connector for AskSQL. Works with better-sqlite3 or the built-in node:sqlite; full PRAGMA-based introspection.", "type": "module", "main": "./dist/index.js", @@ -28,7 +28,7 @@ } }, "devDependencies": { - "@asksql/core": "workspace:>=0.6.1" + "@asksql/core": "workspace:>=0.8.0" }, "license": "Apache-2.0", "engines": { diff --git a/packages/sqlite/src/index.ts b/packages/sqlite/src/index.ts index 0696f06..d1901ec 100644 --- a/packages/sqlite/src/index.ts +++ b/packages/sqlite/src/index.ts @@ -5,6 +5,7 @@ * cancellation are cooperative: a pre-flight abort check and a row cap, no mid-statement stop. */ +import { closeSync, openSync, readSync, statSync } from 'node:fs'; import { AskSqlError, SQLITE_DIALECT, @@ -74,6 +75,31 @@ function isSampleableSqliteType(dbType: string): boolean { return /char|clob|text/i.test(dbType); } +/** How many columns a catalog read will probe for their epoch unit before it stops. */ +const MAX_UNIT_PROBES = 40; + +/** An integer column whose name says it holds a moment; the unit is not in the type. */ +const TIMEISH_NAME = + /(?:^|_)(?:at|ts|time|date|timestamp|created|updated|modified|deleted|expires?|expiry|last_seen|sent|received|due|start|end|since|until)(?:_|$)|(?:time|date|timestamp)$/i; + +/** SQLite integer affinity: the declared type says nothing about seconds versus milliseconds. */ +const INTEGERISH = + /^(?:big\s*int|int|integer|int2|int4|int8|smallint|tinyint|mediumint|unsigned\s+big\s+int|numeric)\b/i; + +/** + * Which epoch unit a magnitude is in. Nothing in a SQLite schema says whether an integer timestamp + * counts seconds or milliseconds, and guessing seconds against milliseconds matches every row. Decided + * from an aggregate, so only the unit is stated, never a value. + */ +function epochUnitOf(max: number): string | null { + if (!Number.isFinite(max) || max <= 0) return null; + if (max >= 1e17) return 'epoch nanoseconds'; + if (max >= 1e14) return 'epoch microseconds'; + if (max >= 1e11) return 'epoch milliseconds'; + if (max >= 1e8) return 'epoch seconds'; + return null; // too small to be a modern timestamp; saying nothing beats guessing +} + export class SqliteConnector implements Connector { readonly engine = 'sqlite' as const; readonly dialect = SQLITE_DIALECT; @@ -82,6 +108,8 @@ export class SqliteConnector implements Connector { readonly name: string; readonly database?: string; private db: SqliteDriver | null = null; + /** Whether a -wal file held rows when we opened the database; SQLite makes an empty one itself. */ + private walBeforeOpen = false; /** Set once the handle in use has been proven read-only, so the check runs once per handle. */ private readOnlyAsserted = false; @@ -108,6 +136,14 @@ export class SqliteConnector implements Connector { userMessage: 'No SQLite database was configured.', }); } + // Before opening: SQLite creates an empty -wal itself, so asking afterwards always finds one. + this.walBeforeOpen = ((): boolean => { + try { + return statSync(`${this.config.file}-wal`).size > 0; + } catch { + return false; + } + })(); // Load the driver and open the file in separate steps, so each failure gets its own message. let Ctor: new (f: string, o?: object) => SqliteDriver; let openOptions: object = { readonly: true, fileMustExist: true }; @@ -251,6 +287,35 @@ export class SqliteConnector implements Connector { return vals.length > 0 ? vals : undefined; } + /** + * A WAL database whose -wal file is not beside it opens, reports no tables, and answers "no such + * table" - an empty database with nothing to explain it. Header byte 18 is 2 for WAL, 1 for a + * rollback journal. + */ + private missingWalWarning(): string | null { + const file = this.config.file; + if (!file || file === ':memory:') return null; + // A -wal that held rows when we opened it is being read; nothing is missing. + if (this.walBeforeOpen) return null; + try { + const fd = openSync(file, 'r'); + try { + const header = Buffer.alloc(20); + readSync(fd, header, 0, 20, 0); + if (header[18] !== 2) return null; + } finally { + closeSync(fd); + } + return ( + 'This database is in WAL mode and no "-wal" file was beside it, so it reads as empty. If it is not ' + + 'actually empty, the rows are in the "-wal" file that was left behind: copy the "-wal" and "-shm" ' + + 'files next to the database, or checkpoint the database before copying it.' + ); + } catch { + return null; // unreadable header is the connect path's problem, not this one's + } + } + async introspect(): Promise { const warnings: string[] = []; const objs = this.rows( @@ -259,6 +324,9 @@ export class SqliteConnector implements Connector { const tables: TableInfo[] = []; const triggers: TriggerInfo[] = []; let sampleBudget = MAX_SAMPLED_COLUMNS; + // One aggregate per candidate column: 41ms per million rows, so bounded like value sampling is, + // in case a schema has dozens of timestamp columns across dozens of tables. + let unitBudget = MAX_UNIT_PROBES; for (const o of objs) { const name = String(o['name']); @@ -296,6 +364,23 @@ export class SqliteConnector implements Connector { nullable: Number(c['notnull']) === 0, default: c['dflt_value'] == null ? null : String(c['dflt_value']), })); + // The unit of an integer timestamp, stated as a comment so the model stops guessing seconds for a + // milliseconds column. One aggregate per candidate column, name-filtered so a wide table is not + // scanned for nothing, and base tables only: an aggregate over a view runs the view's query. + if (type !== 'view') { + columns = columns.map((col) => { + if (unitBudget <= 0 || col.comment || !INTEGERISH.test(col.dbType.trim()) || !TIMEISH_NAME.test(col.name)) + return col; + unitBudget--; + try { + const rows = this.rows(`SELECT MAX(${quoteIdent(col.name)}) AS m FROM ${quoteIdent(name)}`); + const unit = epochUnitOf(Number(rows[0]?.['m'])); + return unit ? { ...col, comment: unit } : col; + } catch { + return col; // best-effort: an unreadable column simply goes unannotated + } + }); + } // Opt-in: observe the distinct codes a short text column holds; base tables only, as sampling a view runs its query. if (this.config.sampleColumnValues && type !== 'view') { columns = columns.map((col) => { @@ -347,7 +432,11 @@ export class SqliteConnector implements Connector { sequences: [], triggers, routines: [], - warnings, + // Only when the database looks empty: with tables present the sidecar is not the story. + warnings: + tables.length === 0 + ? [...warnings, ...[this.missingWalWarning()].filter((w): w is string => w !== null)] + : warnings, fetchedAt: new Date().toISOString(), }; } diff --git a/packages/sqlite/test/epoch-unit-hint.test.ts b/packages/sqlite/test/epoch-unit-hint.test.ts new file mode 100644 index 0000000..e21541f --- /dev/null +++ b/packages/sqlite/test/epoch-unit-hint.test.ts @@ -0,0 +1,93 @@ +/** + * Nothing in a SQLite schema says whether an integer timestamp counts seconds or milliseconds, and + * guessing seconds against milliseconds matches every row. Measured on the Room fixture: 30B answered + * 5 of 5 users, 7B answered 0. The unit comes from an aggregate, so no value reaches the model. + */ +import { describe, expect, it, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DatabaseSync } from 'node:sqlite'; +import { SqliteConnector } from '../src/index.js'; + +const dirs: string[] = []; +afterEach(() => { + for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }); +}); + +/** A table with one integer column holding `value`, introspected. */ +async function commentFor( + columnName: string, + dbType: string, + value: number | bigint, +): Promise { + const dir = mkdtempSync(join(tmpdir(), 'asksql-epoch-')); + dirs.push(dir); + const file = join(dir, 'app.db'); + const db = new DatabaseSync(file); + // No second column when the column under test is itself named id. + const extra = columnName === 'id' ? '' : 'id INTEGER PRIMARY KEY, '; + db.exec(`CREATE TABLE t (${extra}"${columnName}" ${dbType})`); + const stmt = db.prepare(`INSERT INTO t ("${columnName}") VALUES (?)`); + stmt.run(typeof value === 'bigint' ? value : Math.trunc(value)); + db.close(); + const c = new SqliteConnector({ id: 's', name: 's', file }); + await c.connect(); + const catalog = await c.introspect(); + await c.close(); + return catalog.tables.find((t) => t.name === 't')?.columns.find((col) => col.name === columnName)?.comment; +} + +describe('the unit of an integer timestamp is stated', () => { + it('calls a milliseconds column milliseconds', async () => { + expect(await commentFor('created_at', 'INTEGER', 1_755_300_000_000)).toBe('epoch milliseconds'); + }); + + it('calls a seconds column seconds', async () => { + expect(await commentFor('created_at', 'INTEGER', 1_755_300_000)).toBe('epoch seconds'); + }); + + it('recognises microseconds and nanoseconds', async () => { + expect(await commentFor('sent_at', 'INTEGER', 1_755_300_000_000_000)).toBe('epoch microseconds'); + expect(await commentFor('sent_at', 'BIGINT', 1_755_300_000_000_000_000n)).toBe('epoch nanoseconds'); + }); + + it('reads the name in the shapes Room and hand-written schemas use', async () => { + for (const name of ['created_at', 'updated_at', 'sent_at', 'timestamp', 'start_time', 'expires', 'due_date']) { + expect(await commentFor(name, 'INTEGER', 1_755_300_000_000), name).toBe('epoch milliseconds'); + } + }); +}); + +describe('what it must not annotate', () => { + it('says nothing about a column that is not a timestamp by name', async () => { + // A large integer that is an id, a byte count or a price is not a moment. + expect(await commentFor('size_bytes', 'INTEGER', 1_755_300_000_000)).toBeFalsy(); + expect(await commentFor('id', 'INTEGER', 1_755_300_000_000)).toBeFalsy(); + }); + + it('says nothing about a text or real column', async () => { + expect(await commentFor('created_at', 'TEXT', 1_755_300_000_000)).toBeFalsy(); + expect(await commentFor('created_at', 'REAL', 1_755_300_000_000)).toBeFalsy(); + }); + + it('says nothing when the magnitude is too small to be a modern timestamp', async () => { + // A duration in seconds, or a year: guessing here would be worse than silence. + expect(await commentFor('start_time', 'INTEGER', 3_600)).toBeFalsy(); + expect(await commentFor('created_at', 'INTEGER', 2026)).toBeFalsy(); + }); + + it('says nothing about an empty table', async () => { + const dir = mkdtempSync(join(tmpdir(), 'asksql-epoch-empty-')); + dirs.push(dir); + const file = join(dir, 'app.db'); + const db = new DatabaseSync(file); + db.exec('CREATE TABLE t (created_at INTEGER)'); + db.close(); + const c = new SqliteConnector({ id: 's', name: 's', file }); + await c.connect(); + const catalog = await c.introspect(); + await c.close(); + expect(catalog.tables[0]?.columns[0]?.comment).toBeFalsy(); + }); +}); diff --git a/packages/sqlite/test/wal-sidecar.test.ts b/packages/sqlite/test/wal-sidecar.test.ts new file mode 100644 index 0000000..cffdfa1 --- /dev/null +++ b/packages/sqlite/test/wal-sidecar.test.ts @@ -0,0 +1,75 @@ +/** + * Room defaults to WAL, so an Android database is three files. An `adb pull app.db` takes only the + * first, and SQLite then reports no tables at all - an empty database with nothing to say why. The + * sidecar's size is read BEFORE opening, because SQLite creates an empty -wal itself on open. + */ +import { describe, expect, it, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, copyFileSync, statSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DatabaseSync } from 'node:sqlite'; +import { SqliteConnector } from '../src/index.js'; + +const dirs: string[] = []; +afterEach(() => { + for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }); +}); + +/** A WAL database whose rows are still in the -wal, as a running app leaves it. */ +function walDatabase(): { dir: string; file: string } { + const dir = mkdtempSync(join(tmpdir(), 'asksql-wal-')); + dirs.push(dir); + const file = join(dir, 'app.db'); + const db = new DatabaseSync(file); + db.exec('PRAGMA journal_mode=WAL'); + db.exec('PRAGMA wal_autocheckpoint=0'); + db.exec('CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT)'); + db.exec("INSERT INTO users VALUES (1,'Ada'),(2,'Grace')"); + // Copy the main file while the -wal still holds the rows, then close. + const pulledDir = mkdtempSync(join(tmpdir(), 'asksql-pull-')); + dirs.push(pulledDir); + const pulled = join(pulledDir, 'app.db'); + copyFileSync(file, pulled); + db.close(); + return { dir, file: pulled }; +} + +describe('a WAL database copied without its sidecars', () => { + it('says the -wal is missing instead of reporting an empty database', async () => { + const { file } = walDatabase(); + // Guard the fixture: if the copy already had the rows, the test would prove nothing. + expect(statSync(file).size).toBeGreaterThan(0); + const c = new SqliteConnector({ id: 's', name: 's', file }); + await c.connect(); + const catalog = await c.introspect(); + expect(catalog.tables).toHaveLength(0); + expect(catalog.warnings.join(' ')).toMatch(/-wal/); + await c.close(); + }); + + it('says nothing when the sidecars are present and the rows are readable', async () => { + const { dir } = walDatabase(); + const c = new SqliteConnector({ id: 's', name: 's', file: join(dir, 'app.db') }); + await c.connect(); + const catalog = await c.introspect(); + expect(catalog.tables.map((t) => t.name)).toContain('users'); + expect(catalog.warnings.filter((w) => w.includes('-wal'))).toEqual([]); + const rows = await c.execute('SELECT id, name FROM users ORDER BY id', { maxRows: 10 }); + expect(rows.rowCount).toBe(2); + await c.close(); + }); + + it('says nothing about a database that uses a rollback journal', async () => { + const dir = mkdtempSync(join(tmpdir(), 'asksql-journal-')); + dirs.push(dir); + const file = join(dir, 'plain.db'); + const db = new DatabaseSync(file); + db.exec('CREATE TABLE t (a INTEGER)'); + db.close(); + const c = new SqliteConnector({ id: 's', name: 's', file }); + await c.connect(); + const catalog = await c.introspect(); + expect(catalog.warnings.filter((w) => w.includes('-wal'))).toEqual([]); + await c.close(); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e9291cf..5564e56 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -423,7 +423,7 @@ importers: version: 12.11.1 devDependencies: '@asksql/core': - specifier: workspace:>=0.6.1 + specifier: workspace:>=0.8.0 version: link:../core packages/vscode: diff --git a/tests/bundle-size.test.ts b/tests/bundle-size.test.ts index 9f4e43f..fe13ad5 100644 --- a/tests/bundle-size.test.ts +++ b/tests/bundle-size.test.ts @@ -28,7 +28,10 @@ const BUDGETS: Record = { // 74->91 step was not growth but a fix, when the walk became recursive and dist/mongo was counted // for the first time. 96->97: the catalog checks read the statement with its row-limit tail removed, // and a repair now names the table that holds the missing column and the join that reaches it. - core: 97, + // 97->100: the epoch floor, which catches a numeric column compared against a date, the SQLite date + // note that tells the model which units an INTEGER column is in, and the MATCH rewrite that lets a + // full-text query be validated at all. + core: 100, // 20 -> 23: copy controls, streamed-token progress, cell tooltips, export feedback, result-grid copy. react: 23, // 12 -> 14: the CSRF/Host gate every adapter inherits, client-path confinement for file engines, @@ -76,6 +79,6 @@ describe('bundle-size budgets (gzipped, own code)', () => { if (core === null || react === null) return; // Own code only (React is a peer). The same recursion correction as core's accounts for 96->113; // the rest is identifier normalisation, the reserved-word lists and the routing work. - expect(core + react).toBeLessThan(119); + expect(core + react).toBeLessThan(122); }); }); diff --git a/tools/room-regression.mjs b/tools/room-regression.mjs new file mode 100644 index 0000000..9cf6b5e --- /dev/null +++ b/tools/room-regression.mjs @@ -0,0 +1,209 @@ +#!/usr/bin/env node +/** + * The engine against a database shaped the way Room leaves one, which is what an Android Studio user + * opens. No model: every expectation is a fact about the schema, so this can gate CI. + * + * Every fixture the suite owned used real types - Chinook's InvoiceDate is a DATETIME - so nothing + * exercised epoch-millis dates, Long ids past a double, BLOBs, or Room's bookkeeping and FTS tables. + * Five defects surfaced the first time such a database was tried. + * + * Usage: node tools/room-regression.mjs + */ +import { mkdtempSync, copyFileSync, rmSync, statSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DatabaseSync } from 'node:sqlite'; + +const core = await import('../packages/core/dist/index.js'); +const semantics = await import('../packages/core/dist/semantics.js'); +const engineInternals = await import('../packages/core/dist/engine.js'); +const { SqliteConnector } = await import('../packages/sqlite/dist/index.js'); + +const results = []; +const check = (name, fn) => { + try { + const detail = fn(); + results.push({ name, ok: true, detail: detail ?? '' }); + } catch (e) { + results.push({ name, ok: false, detail: e instanceof Error ? e.message : String(e) }); + } +}; +const asyncCheck = async (name, fn) => { + try { + const detail = await fn(); + results.push({ name, ok: true, detail: detail ?? '' }); + } catch (e) { + results.push({ name, ok: false, detail: e instanceof Error ? e.message : String(e) }); + } +}; +const assert = (cond, message) => { + if (!cond) throw new Error(message); +}; + +const DAY = 86_400_000; +const now = Date.now(); +const BIG_ID = 9007199254740993n; +const dir = mkdtempSync(join(tmpdir(), 'asksql-room-')); +const file = join(dir, 'app.db'); + +// ---- the fixture: what Room actually writes ---- +{ + const db = new DatabaseSync(file); + db.exec(` + CREATE TABLE users ( + id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, email TEXT NOT NULL, + is_active INTEGER NOT NULL DEFAULT 1, created_at INTEGER NOT NULL, + prefs TEXT, avatar BLOB + ); + CREATE UNIQUE INDEX index_users_email ON users (email); + CREATE TABLE messages ( + id INTEGER PRIMARY KEY NOT NULL, sender_id INTEGER NOT NULL, body TEXT NOT NULL, + sent_at INTEGER NOT NULL, is_read INTEGER NOT NULL DEFAULT 0, + FOREIGN KEY (sender_id) REFERENCES users(id) ON DELETE CASCADE + ); + CREATE INDEX index_messages_sender_id ON messages (sender_id); + CREATE VIEW active_users AS SELECT id, name FROM users WHERE is_active = 1; + CREATE TABLE room_master_table (id INTEGER PRIMARY KEY, identity_hash TEXT); + CREATE TABLE android_metadata (locale TEXT); + INSERT INTO android_metadata VALUES ('en_US'); + CREATE VIRTUAL TABLE messages_fts USING fts4(body, content=messages); + `); + const u = db.prepare('INSERT INTO users (id,name,email,is_active,created_at,prefs,avatar) VALUES (?,?,?,?,?,?,?)'); + u.run(1n, 'Ada', 'ada@example.com', 1, now - 2 * DAY, '{"theme":"dark"}', null); + u.run(2n, 'Grace', 'grace@example.com', 1, now - 5 * DAY, null, null); + u.run(3n, 'Alan', 'alan@example.com', 0, now - 90 * DAY, null, null); + u.run(BIG_ID, 'Margaret', 'margaret@example.com', 1, now - 400 * DAY, null, Buffer.from([0x89, 0x50, 0x4e, 0x47])); + const m = db.prepare('INSERT INTO messages (id,sender_id,body,sent_at,is_read) VALUES (?,?,?,?,?)'); + m.run(1n, 1n, 'Rope memory is woven, not written', now - 3 * DAY, 1); + m.run(2n, BIG_ID, 'The simulator agrees', now - 2 * DAY, 0); + db.exec("INSERT INTO messages_fts(messages_fts) VALUES('rebuild')"); + db.close(); +} + +const connector = new SqliteConnector({ id: 'room', name: 'Room app', file }); +await connector.connect(); +const catalog = await connector.introspect(); +const dialect = core.SQLITE_DIALECT; +const guard = (sql) => core.guardSql({ sql, dialect }); + +await asyncCheck('a Long id past a double range survives the driver', async () => { + const r = await connector.execute("SELECT id FROM users WHERE name = 'Margaret'", { maxRows: 1 }); + const got = String(r.rows[0][0]); + assert(got === BIG_ID.toString(), `id came back as ${got}`); + return got; +}); + +await asyncCheck('a BLOB column does not destroy the result set', async () => { + const r = await connector.execute('SELECT id, avatar FROM users ORDER BY id', { maxRows: 10 }); + assert(r.rowCount === 4, `expected 4 rows, got ${r.rowCount}`); + return `${r.rowCount} rows, blob preserved`; +}); + +check('rowid is not reported as an invented column', () => { + const found = engineInternals.firstUnknownColumn('SELECT rowid, name FROM users', catalog, dialect.grammar); + assert(found === null, `flagged ${found?.table}.${found?.column}`); + return 'rowid accepted'; +}); + +check('a full-text query is allowed', () => { + const v = guard("SELECT rowid FROM messages_fts WHERE messages_fts MATCH 'memory'"); + assert(v.allowed, `refused: ${v.reason}`); + assert(/\bMATCH\b/.test(v.sql), 'the operator did not survive the guard'); + return 'MATCH accepted and preserved'; +}); + +await asyncCheck('the full-text query returns the matching rows', async () => { + const v = guard("SELECT rowid FROM messages_fts WHERE messages_fts MATCH 'memory'"); + const r = await connector.execute(v.sql, { maxRows: 10 }); + assert(r.rowCount === 1, `expected 1 match, got ${r.rowCount}`); + return `${r.rowCount} row`; +}); + +check('an epoch column compared with a text date is caught', () => { + const bad = "SELECT COUNT(*) FROM users WHERE created_at >= date('now','-7 days')"; + const found = semantics.epochUnitMismatch(bad, dialect.grammar, catalog); + assert(found !== null, 'the comparison was not flagged'); + return `${found.column} (${found.dbType}) vs ${found.comparedTo}`; +}); + +check('the same column compared in its own units is left alone', () => { + const good = "SELECT COUNT(*) FROM users WHERE created_at >= (strftime('%s','now') - 7*86400) * 1000"; + assert(semantics.epochUnitMismatch(good, dialect.grammar, catalog) === null, 'correct SQL was flagged'); + return 'accepted'; +}); + +await asyncCheck('the correct epoch form returns the right count', async () => { + const r = await connector.execute( + "SELECT COUNT(*) FROM users WHERE created_at >= (strftime('%s','now') - 7*86400) * 1000", + { maxRows: 1 }, + ); + const got = Number(r.rows[0][0]); + assert(got === 2, `expected 2 users in the last week, got ${got}`); + return `${got} users`; +}); + +check('an INTEGER boolean is readable as a number, not text', () => { + const v = guard('SELECT COUNT(*) FROM users WHERE is_active = 1'); + assert(v.allowed, `refused: ${v.reason}`); + return 'accepted'; +}); + +check("Room's bookkeeping and FTS shadow tables are visible to the catalog", () => { + const names = catalog.tables.map((t) => t.name); + for (const expected of ['users', 'messages', 'room_master_table', 'android_metadata']) { + assert(names.includes(expected), `${expected} missing from the catalog`); + } + return `${names.length} objects`; +}); + +await asyncCheck('a write is still refused against an app database', async () => { + const attempts = ['DELETE FROM messages', 'UPDATE users SET is_active = 0', 'DROP TABLE users']; + for (const sql of attempts) { + const v = guard(sql); + assert(!v.allowed, `${sql} was allowed`); + } + const after = await connector.execute('SELECT COUNT(*) FROM users', { maxRows: 1 }); + assert(Number(after.rows[0][0]) === 4, 'the table changed'); + return `${attempts.length} writes refused, 4 users intact`; +}); + +await connector.close(); + +// ---- a WAL database copied without its sidecars ---- +await asyncCheck('a WAL database missing its -wal says so', async () => { + const walDir = mkdtempSync(join(tmpdir(), 'asksql-room-wal-')); + const walFile = join(walDir, 'app.db'); + const db = new DatabaseSync(walFile); + db.exec('PRAGMA journal_mode=WAL'); + db.exec('PRAGMA wal_autocheckpoint=0'); + db.exec('CREATE TABLE t (a INTEGER)'); + db.exec('INSERT INTO t VALUES (1)'); + const pulledDir = mkdtempSync(join(tmpdir(), 'asksql-room-pull-')); + const pulled = join(pulledDir, 'app.db'); + copyFileSync(walFile, pulled); + db.close(); + assert(statSync(pulled).size > 0, 'the copy is empty'); + + const c = new SqliteConnector({ id: 'w', name: 'w', file: pulled }); + await c.connect(); + const cat = await c.introspect(); + await c.close(); + rmSync(walDir, { recursive: true, force: true }); + rmSync(pulledDir, { recursive: true, force: true }); + assert(cat.tables.length === 0, 'the copy unexpectedly had tables'); + assert( + cat.warnings.some((w) => w.includes('-wal')), + `no sidecar warning: ${JSON.stringify(cat.warnings)}`, + ); + return 'warned instead of reporting an empty database'; +}); + +rmSync(dir, { recursive: true, force: true }); + +console.log('\n### A Room-shaped SQLite database\n'); +console.log('| Check | Result | Detail |'); +console.log('|---|---|---|'); +for (const r of results) console.log(`| ${r.name} | ${r.ok ? 'ok' : 'FAIL'} | ${r.detail} |`); +const failed = results.filter((r) => !r.ok); +console.log(`\n${failed.length === 0 ? `All ${results.length} checks passed.` : `${failed.length} CHECK(S) FAILED`}`); +process.exit(failed.length === 0 ? 0 : 1);