Repository navigation
feat(mysql): pin joins, derived tables, with queries and views by keys - #108
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No configured credential can serve this run. Your Claude Pro/Max subscription has hit its usage limit, which resets at 2026-10-26 08:00 UTC. Re-trigger after it resets, or add an API key for this provider, which Pullfrog falls back to automatically. The providers' responses
|
e24e196 to
57997a2
Compare
|
No configured credential can serve this run. Your Claude Pro/Max subscription has hit its usage limit, which resets at 2026-10-26 08:00 UTC. Re-trigger after it resets, or add an API key for this provider, which Pullfrog falls back to automatically. The providers' responses
|
57997a2 to
bb0d05a
Compare
There was a problem hiding this comment.
Important
Two of the new pins type a query maybeOne when it can return several rows. I reproduced both on MySQL 8.0.46.
Reviewed changes
This is the first Pullfrog review of this PR, so it covers the whole PR. The commits since 57997a2 only rebase onto main and rebuild the artifact.
- Added
mysql_keys.rs: flattens a SELECT's FROM into items and conjuncts, works out which items are pinned, and reads keys out of derived tables,withqueries and views (a key kept unchanged,group by,distinct). - Added catalog reads in
mysql.rscatalog: the columns and types of tables and views, plus view definitions, with one round per level of views. - Moved oracle answers and updated docs: 17 existing answers go from
manytomaybeOne, there is a newmysql_subquery_keyscase, and the MySQL page, the inference section and the skill now describe the rule.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
ℹ️ Both earlier findings are fixed. One minor note is inline.
Reviewed changes
This run covers 5f97a02, the only commit since the last Pullfrog review (bb0d05a). I checked both fixes on MySQL 8.0.46.
- A view's column list is no longer read past: a view's definition is read only when the view's columns, in
information_schema.columnsorder, match the names the definition gives them.swapped (title, id)now gives no key, and a view whose column list matches its definition is still read. - Added
Class::Realforfloat,doubleandreal: a column-to-column pin now needs exact numbers on both sides. Adoublekey compared with an exact constant still pins, which holds because distinct doubles stay distinct. - Added oracle cases and a docs row:
view_swapped_columns,join_double_column,key_double_cast,key_double_literal,double_key_exact_literal, and a cardinality-table row for a key joined on afloatordoublecolumn.
claude-opus-5-5 ｜ 𝕏
5f97a02 to
69a7b31
Compare
There was a problem hiding this comment.
ℹ️ The earlier
Class::Realfinding is fixed. There's one minor gap left, noted inline.
Reviewed changes
This run covers 84ba9f5, the only new commit since the last Pullfrog review (69a7b31 was a rebuild on main).
- Made a placeholder read through a double a
Class::Realvalue:value_classnow givesClass::Realwhen a value both reads a placeholder and is a double by its form (a cast todouble,floatorreal, a scientific literal, or arithmetic over one). As a result,id = cast(:x as double)andid = :x + 0e0no longer pin an exact key, and thecastof a constant still does. - Added oracle cases and reworded the docs: there are new
key_double_arithmetic(many) andkey_double_constant_cast(maybeOne) cases,key_double_castmoves tomany, theClass::Realcomment states the rule for literals and for placeholders separately, and the MySQL page's row covers a parameter cast to a double. The commit includes the rebuiltengine.wasm.
claude-opus-5-5 ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found. The last open finding is fixed, and no open Pullfrog threads remain.
Reviewed changes
This run covers 7fa831a, the only commit since the last Pullfrog review (84ba9f5).
approximatenow looks inside numeric built-ins: it recurses into the arguments ofabs,round,floor,ceil,ceiling,truncate,sign,mod,greatestandleast. Soid = abs(cast(:x as double))becomesClass::Realand no longer pins an exact key. Only the first five could pin before this change.value_classalready maps the rest toClass::Other, andClass::Othernever pins an exact key.signreturns an integer, so treating it as a double only makes the result more conservative.- Two oracle cases were added:
key_double_function(many) andkey_exact_function(abs(:x),maybeOne). The commit includes the rebuiltengine.wasm. The MySQL page's row ("a parameter cast to one") still describes the rule.
claude-opus-5-5 ｜ 𝕏
7fa831a to
2344ab7
Compare
There was a problem hiding this comment.
Important
The rebase onto main left the old packages/mysql/engine.wasm in place. It was built before #120, so the loader's fingerprint test fails and the artifact is missing #120's MySQL fix.
Reviewed changes
This run covers the rebase from 1b2bbbf onto main at 349e953. The rebased commits match the reviewed ones patch for patch, apart from one context line in skills/using-reylo/SKILL.md that comes from #117's tracing bullet.
- Rebased onto
main: the new base adds #118 (SQLite), #120 (MySQL,mysql.rsandmysql_nulls.rs), #117 (tracing) and #121 (slonik). The PR's MySQL patches apply on top of #120 unchanged.
⚠️ The MySQL artifact was not rebuilt after the rebase
At 2344ab7, packages/mysql/engine.wasm is blob 0308803. That is byte for byte the artifact from 7fa831a, which was built on 1b2bbbf, before #120. #120 changed engine/mysql/src/mysql.rs and mysql_nulls.rs, so the committed artifact no longer matches its sources. node --test tests/fingerprint.test.ts fails on this branch with packages/mysql/engine.wasm was built from other sources than engine/core and engine/mysql, and the CI test job will fail the same way. The loader would also run without #120's rule for an aggregate under group by.
Technical details
# Rebuild packages/mysql/engine.wasm on the rebased branch
## Affected sites
- packages/mysql/engine.wasm — blob 0308803, identical to the blob at 7fa831a (base 1b2bbbf); never rebuilt after the rebase onto 349e953, which includes 8a272b0 (#120) touching engine/mysql/src/mysql.rs and engine/mysql/src/mysql_nulls.rs
- tests/fingerprint.test.ts:89 — fails: actual '47c6848823b87db4', expected '7063c88554362d69'
## Required outcome
- The committed MySQL artifact is built from the rebased sources (#120 plus this PR), and `nub run test` passes with none skipped.
## Suggested approach
- `engine/build.sh` (all three, for reproducible bytes), commit `packages/mysql/engine.wasm`, then rerun `node --test tests/fingerprint.test.ts`.
- Run the MySQL oracle shards with `--strict-coverage` again: #120 changes how aggregates under `group by` are typed, and this PR's new `group_by`/`rollup`/`view_group_by` cases were blessed before it landed.claude-opus-5-5 ｜ 𝕏
The row count reads one table's key no more: each FROM item is pinned by a unique key the WHERE and ON clauses equate, a derived table, WITH query or view by the keys its statement keeps.
A view's stored definition leaves out its column list, so a view whose columns are not the definition's names is not read. An exact key joined on a FLOAT or DOUBLE column compares as a double, where keys past 2^53 meet one value.
The server converts a double constant to the key's type before comparing, but compares a placeholder cast to DOUBLE, or added to a double, as a double.
abs, round, floor and the like return a double for a double argument, so a placeholder read through one compares an exact key as a double.
A grouped read whose GROUP BY column the WHERE pins to a value is one group at most.
There was a problem hiding this comment.
✅ No new issues found. The MySQL artifact has been rebuilt on
main, and the fingerprint check passes.
Reviewed changes
This run covers 3969ada, the only commit since the last Pullfrog review (2344ab7).
- Rebuilt
packages/mysql/engine.wasmon the rebased sources: the new artifact includes #120's rule for an aggregate undergroup by.node --test tests/fingerprint.test.tspasses for all three artifacts, which resolves the earlier finding. - Reblessed
mysql_grouped_aggregates/param.sqlasmaybeOne: #120 added this case,where p.user_id = ? group by p.user_id. The PR's existing GROUP BY rule (mysql_keys.rs:670-678, throughFrom::pinnedandequateswith the class check) now reads it as one group at most, because the WHERE pins the only grouping column to a value. No rule changed here: an already-reviewed rule now covers a case added onmain. I did not rerun the oracle locally. CI's MySQL oracle shards will check this answer and the PR's other blessed cases against #120.
claude-opus-5-5 ｜ 𝕏
3969ada to
8dcfb8d
Compare
The MySQL row count read one table's key and nothing else:
select p.title, u.name from posts p join users u on u.id = p.user_id where p.id = :idwasmany, and so was every derived table, evenselect * from (select id from posts where id = :id) s. It now reads joins, derived tables, WITH queries and views the way #105 and #106 do on Postgres and SQLite.The rule
engine/mysql/src/mysql_keys.rsflattens a SELECT's FROM into items and the equalities that may pin each:A query's rows have a key when one of its FROM items has a unique key the select list keeps unchanged and every other item is pinned one row per row of that item, when its GROUP BY (not WITH ROLLUP) names it, or when it is a DISTINCT over plain columns. A LATERAL derived table is never the source of such a key, since its rows are computed anew for each outer row.
An equality pins a key column only where it compares the column as it is. With a value, that is the class check the engine already makes (a string key compared with a number compares as doubles). With another column, it holds only where both are numbers, since two strings of different collations compare under a collation the server picks.
A view is read from
information_schema.VIEWS, its statement parsed with its schema's qualified names. The keys and columns of the tables it reads are asked a round later, only for a statement that reads a view. A view created with a column list gives no key.Checks
oracle/cases/mysql_subquery_keys: 51 statements, run over seeded edge values with no contradiction.--strict-coverage. Seventeen existing answers, in eight cases, move frommanytomaybeOne, each a statement the old proof could not read: a derived table, WITH query or view read by its key or one row (key_derived,key_with,key_view,int_view,select_view,derived_limit,with_limit,with_in_derived,derived_aggregate,param_derived,param_with), a join of two pinned tables (key_join_both), a lateral derived table (lateral), and a GROUP BY a constant or a pinned key (aggregate_group_by_constant,group_alone,key_group_by_expression,key_group_by_position).nub run testwith both test databases: 253 pass, none skipped. The docs state the rule in the MySQL page's cardinality section and the main page's key matches.