Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
31 changes: 31 additions & 0 deletions packages/core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/dialects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, ', ').",
]),
Expand Down
37 changes: 36 additions & 1 deletion packages/core/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1358,6 +1384,13 @@ function collectSelectAliases(sql: string): ReadonlySet<string> {
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<string> = 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
Expand Down Expand Up @@ -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<string>();
for (const t of queryTables) for (const c of realColumns.get(t) ?? []) available.add(c);
Expand All @@ -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;
Expand Down
31 changes: 30 additions & 1 deletion packages/core/src/guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
113 changes: 113 additions & 0 deletions packages/core/src/semantics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading
Loading