Repository navigation
feat(engine): narrow try()'s error to what a statement writes through - #89
colinhacks wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Important
Several paths mark a statement closed even though it can run user code, or narrow a field the run time fills with a name outside the list. When that happens the emitted QueryError type is wrong, and .try()'s narrowed type is only useful if it is sound.
Reviewed changes
Initial review of the single commit, which narrows .try()'s error type per statement on all three dialects.
- Core
errorsround:Database::errorsruns besidefresh::factsinwith_facts, and the result is serialized asAnalysis.errors. - Postgres closure query: one catalog query covers triggers, rules, foreign tables,
pg_dependon user code, user-I/O types, same-named user functions, operators and casts, and lists the constraint, table, column and domain names. - MySQL closure query:
information_schemacovers triggers, stored functions called by name, and views that use routines, and lists keys, foreign keys, checks and columns. - SQLite closure query: pragmas cover triggers and list unique keys, rowid, NOT NULL and STRICT columns, each mirrored under the primary
SQLITE_CONSTRAINT. - Types:
PostgresQueryError<N>,MysqlQueryError<N>andSqliteQueryError<N>, plusqueryErrorType, which writes the module'sQueryError. - Oracle: every raised error is checked against its statement's narrowing, and three new cases carry
errors.txt. - Docs and skill: each dialect page,
02-usage.mdxandSKILL.mddescribe the narrowing.
⚠️ The .reylo/ cache can keep a stale narrowing after a migration
Analyses are reused while the schema fingerprint matches (src/analyze.ts:399-424). The errors field now depends on catalog objects that the fingerprint does not hash. On MySQL, create trigger audited_audit before insert on audited … does not change the fingerprint. The cached insert_audited.sql entry therefore keeps its closed narrowing until something unrelated changes the schema. The same happens when a CHECK or a foreign key is added without a new index.
Technical details
# Schema fingerprint misses what `errors` reads
## Affected sites
- src/analyze.ts:416-421 — MySQL FINGERPRINT has columns, statistics, views, routines only: no `information_schema.triggers`, `referential_constraints`, `table_constraints`/`check_constraints`
- src/analyze.ts:400-415 — Postgres FINGERPRINT has no `pg_attrdef` (a new `DEFAULT user_fn()` on an existing column), no `pg_policy`, no `pg_cast` — all of which ERRORS_QUERY reads to open a statement
## Required outcome
- Any catalog change that would flip a statement between open and closed, or change the names it lists, changes the fingerprint, so the cached analysis is not reused.
## Suggested approach
- MySQL: add `union all` rows for triggers (name, event table, timing, event), referential constraints (name, table, referenced table, rules) and check constraints (name, clause).
- Postgres: add rows for `pg_attrdef` (`adrelid`, `adnum`, md5 of `pg_get_expr`), `pg_policy`, and `pg_cast` where `oid >= 16384`.claude-opus-5-5 ｜ 𝕏
A user function opens a statement whatever it takes, and a user operator or cast where the statement can reach its types. MySQL lists every table a multi-table write may reach, reads quoted and commented calls, and looks a stored function up in any schema. SQLite's primary code no longer narrows the constraint, which may be a CHECK's.
|
On the review's fingerprint finding: fixed in bce966d. The schema fingerprint in CI's |
There was a problem hiding this comment.
Important
The new present set misses one case. A user operator can still be chosen for an argument of unknown type, a parameter or a literal, and the statement is then typed closed.
Reviewed changes
Reviewed the two commits since the last review, which address its five threads and the cache concern.
- Postgres closing rules: any user function the statement calls by name now opens it. A user operator or cast opens it only when every user type it takes is in a new recursive
presentset. - MySQL write set:
mysql_params::tablesnow lists every table of a multi-table UPDATE or DELETE as written. A non-query that names nothing it writes opens, and stored functions are matched in every schema. - Word scan:
callednow reads quoted identifiers, looks past comments for the(, and reads inside MySQL's/*! */comments. - SQLite primary code:
constraintis no longer copied toSQLITE_CONSTRAINT. - Schema fingerprint: Postgres now hashes defaults, policies and user casts. MySQL now hashes triggers, foreign keys, constraints and other schemas' functions.
- Oracle cases: new statements cover a function and an operator on an enum, quoted and commented calls, and multi-table DELETEs with and without a trigger.
claude-opus-5-5 ｜ 𝕏
| WHERE a.attnum > 0 AND NOT a.attisdropped AND t.oid IN (SELECT oid FROM coded) \ | ||
| UNION ALL SELECT 'open', 'function', p.proname::text, NULL FROM pg_proc p WHERE p.oid >= 16384 AND p.proname = ANY($3::text[]) \ | ||
| UNION ALL SELECT 'open', 'operator', o.oprname::text, NULL FROM pg_operator o WHERE o.oid >= 16384 AND o.oprname = ANY($4::text[]) \ | ||
| AND (o.oprleft < 16384 OR o.oprleft IN (SELECT oid FROM present)) AND (o.oprright < 16384 OR o.oprright IN (SELECT oid FROM present)) \ |
There was a problem hiding this comment.
The reasoning used for functions also applies to operators: an argument of unknown type takes the type of the only candidate. With the case's ~~~ (mood, int), select :m ~~~ 1 as x reads no table and names no type, so mood is not in present and the statement is closed. Yet $1 resolves to mood, and the run raises U0003. I checked this on a live server: prepare p as select $1 ~~~ 1 gives parameter_types = {mood}, and execute p('sad') raises. select 'sad' ~~~ 1 behaves the same way.
Technical details
# A user operator chosen for an unknown-typed argument is unseen
## Affected sites
- engine/postgres/src/errors.rs:90-91 — the operator row requires each user arg type to be in `present`, but an unknown literal or untyped parameter can resolve to a user type that is not in `present`
- engine/postgres/src/errors.rs:93-94 — casts are probably fine, since a cast names its target type, but check them against the same rule
## Required outcome
- A statement opens when a user operator may be chosen for an argument of unknown type, at least when no server operator (oid < 16384) has that name.
- Add an oracle statement to `oracle/cases/errors/`, e.g. `select :m ~~~ 1 as x`, expected `open`.
## Suggested approach
- Also open a user operator whose `oprname` no built-in operator shares, whatever its argument types. This keeps citext's `=` from opening every statement.
- Alternatively, add the described parameter types to `present`. That still misses a literal such as `'sad' ~~~ 1`.There was a problem hiding this comment.
Fixed in 1b85dc5, now on #96 (#96). A user operator also opens the statement when it is a prefix operator, or when the server has no operator of the name taking the other argument's type on both sides (text when neither type is present), which Postgres would pick first. New oracle statements compare_param.sql (select :m ~~~ 1) and compare_literal.sql (select 'sad' ~~~ 1) are both open.
|
This continues in #96, rebased onto main: this pull request closed when its base branch was deleted. |
Builds on #88. When a query runs none of the user's own code, its
QueryErroris narrowed to the names each error can carry. The engine works this out from the catalog for each statement.A statement is closed when it runs only the database's own code, in the statement itself or in anything it sets off. Each closed statement's narrowing goes into the analysis as
errors, beside the freshness facts and in the same round. A statement that isn't closed keeps the whole type, since a trigger can raise any code with any name.Postgres
One catalog query per statement. It reports anything that opens the statement:
pg_depend);Otherwise
codeis the strict list, and each error names only what the statement writes through:MySQL
information_schemareports:Otherwise each error names only what the statement writes through: duplicate keys as
table.index, foreign keys on either side, checks, NOT NULL columns, and the columns of value errors.SQLite
Pragmas report a trigger on a table a write reaches. Otherwise each error names only what the statement writes through: unique keys' columns as SQLite's message lists them, expression indexes, primary-key, rowid, NOT NULL and STRICT columns. Each name is listed under the primary
SQLITE_CONSTRAINTtoo, for drivers that give only that. SQLite's catalog doesn't name a CHECK, so CHECK errors aren't narrowed.Types and checks
PostgresQueryError<N>,MysqlQueryError<N>andSqliteQueryError<N>each take the narrowing. A module declaresQuery<…, QueryError>.errors,mysql_errors,sqlite_errors) carry anerrors.txt. All 119 cases pass with--strict-coverage.engine.wasmare rebuilt.