Repository navigation
feat(spec): drop the union's per field - #189
Merged
Merged
Conversation
A union is read per value everywhere. MySQL's per-execution unions come from the plan or from how a driver encodes a bound value, which no parameter's type decides, and nothing read the field.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
I reviewed the whole diff: the format change, every call site that builds a union, and the re-blessed MySQL expectations.
perremoved from the format:UnionType.perand thePerenum are gone from core,spec/typedefs.schema.json,spec/index.tsandsite/public/typedefs/0.1.0.json. The MySQL example moved intoUnionType's description.Type::union flattens every unnamed union: a named union still stays a single member, and a new test covers that. Because of this, MySQL'sboolean | either(…)now becomes one flat union, which is what rule 4 asks for.Type::shortspells every union asa | b: I rewrote each of the 134 removedexpected.txtlines mechanically, turningeither(a, b)intoa | b. The result is exactly the set of added lines, so only the spelling changes and no type does.- Call sites in MySQL, Postgres and SQLite: these only change signature. In the tests, the
eitherhelper is removed and its callers useoneOf.
What I checked:
- No leftovers: nothing remains in
src/,engine/,spec/,site/or the docs that reads or writesperor theeither(…)spelling. The loader never read the field. - Old cached descriptors: core's types do not use
deny_unknown_fields, so a cached descriptor that still holdsperdeserializes as before. - Keeping 0.1.0: this is safe. The latest release is v0.7.0, and the
releasebranch has nosite/public/typedefs/.
claude-opus-5-5 ｜ 𝕏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A union in the type descriptor format is now read per value everywhere:
per: "execution"is gone fromspec/typedefs.schema.json, the generatedspec/index.tsandsite/public/typedefs/0.1.0.json, and fromUnionTypeandType::unionin core.Only MySQL's engine wrote the field, and nothing read it: the loader renders a union's members and decodes each value by the result's own column type. It could not be turned into a link between a parameter and a column either, since neither of the two things that pick the member is a parameter's type:
quarter(day)is a BIGINT, and an INT underSQL_BUFFER_RESULT.select ? + ?with 5 and 5 is a DOUBLE through mysql2'sexecute()and a BIGINT through mariadb'squery(), with both parameters typeddouble.What changes:
boolean | bigint | double | decimal | text).Type::shortwritesa | bwhere it wroteeither(a, b), so 134 lines of the MySQL cases'expected.txtchange spelling and no type changes.The format stays at 0.1.0 rather than moving to a new version: that version is on no release yet (npm serves 0.7.0, and
releasehas nosite/public/typedefs/), so nothing has read a descriptor against it.Checked locally:
cargo build --workspace --all-targetswith no warning, the tests of every crate, the oracle's 170 cases with--strict-coverageon Postgres 18, MySQL 8.4, SQLite and DuckDB,nub run typecheck,nub run testwith none skipped, andengine/build.sh.