TML-2944: arrow-path through: for relation-field-less junctions#876
TML-2944: arrow-path through: for relation-field-less junctions#876tensordreams wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@prisma-next/extension-author-tools
@prisma-next/mongo-runtime
@prisma-next/family-mongo
@prisma-next/sql-runtime
@prisma-next/family-sql
@prisma-next/extension-arktype-json
@prisma-next/middleware-cache
@prisma-next/mongo
@prisma-next/extension-paradedb
@prisma-next/extension-pgvector
@prisma-next/extension-postgis
@prisma-next/postgres
@prisma-next/sql-orm-client
@prisma-next/sqlite
@prisma-next/extension-supabase
@prisma-next/target-mongo
@prisma-next/adapter-mongo
@prisma-next/driver-mongo
@prisma-next/contract
@prisma-next/utils
@prisma-next/config
@prisma-next/errors
@prisma-next/framework-components
@prisma-next/operations
@prisma-next/ts-render
@prisma-next/contract-authoring
@prisma-next/ids
@prisma-next/psl-parser
@prisma-next/psl-printer
@prisma-next/cli
@prisma-next/cli-telemetry
@prisma-next/config-loader
@prisma-next/emitter
@prisma-next/language-server
@prisma-next/migration-tools
prisma-next
@prisma-next/vite-plugin-contract-emit
@prisma-next/mongo-codec
@prisma-next/mongo-contract
@prisma-next/mongo-value
@prisma-next/mongo-contract-psl
@prisma-next/mongo-contract-ts
@prisma-next/mongo-emitter
@prisma-next/mongo-schema-ir
@prisma-next/mongo-query-ast
@prisma-next/mongo-orm
@prisma-next/mongo-query-builder
@prisma-next/mongo-lowering
@prisma-next/mongo-wire
@prisma-next/sql-contract
@prisma-next/sql-errors
@prisma-next/sql-operations
@prisma-next/sql-schema-ir
@prisma-next/sql-contract-psl
@prisma-next/sql-contract-ts
@prisma-next/sql-contract-emitter
@prisma-next/sql-lane-query-builder
@prisma-next/sql-relational-core
@prisma-next/sql-builder
@prisma-next/target-postgres
@prisma-next/target-sqlite
@prisma-next/adapter-postgres
@prisma-next/adapter-sqlite
@prisma-next/driver-postgres
@prisma-next/driver-sqlite
commit: |
size-limit report 📦
|
02a463c to
1c404f0
Compare
454be09 to
aae6a61
Compare
| diagnostics: input.diagnostics, | ||
| sourceId: input.sourceId, | ||
| }); | ||
| const targetColumn = resolveArrowColumn({ |
There was a problem hiding this comment.
targetColumn is only checked for existence; the relation node below never uses it, and the assembler fills through.targetColumns from the target model @id. So through: "id -> PostTag.postId -> PostTag.tagSlug -> Tag.slug" succeeds as long as Tag.slug exists, but the generated relation still joins the far column to Tag.id. Please either require arrowPath.targetKey to be the target @id column(s), or thread the resolved target column into the through descriptor so the authored path is honored.
aae6a61 to
471261d
Compare
1c404f0 to
01db649
Compare
Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
Recognise the arrow-path many-to-many form `through: "localKey -> Junction.nearCol -> Junction.farCol -> Target.targetKey"` on terminal models over a junction that carries scalar columns + @@id but no relation fields. The relation-field-based junction recognition cannot fire when the junction declares no relation fields, so the resolver builds the `through` descriptor straight from the path-named columns. Value form: a quoted string (no grammar change). The parser already produces a StringLiteralExpr; the resolver detects the `->` separator, strips quotes, and splits the four segments. An unquoted arrow grammar would require a new tokenizer token plus an expression node, AST class, union/cast extension, printSyntax rendering, and formatter spacing rules — out of scope for this escape-hatch slice. Each end carries its own mirror-imaged path; both ends lower to N:M + through. Diagnostics: malformed path (not four segments / bad arrows), a named column absent on its model, the two junction columns on different models, and a junction that is not a declared model. Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
Add a real emitted PSL fixture (mn-psl-arrow-path) authoring a Post <-> Tag many-to-many over a relation-field-less PostTag junction via the quoted arrow-path through: form, e.g. tags Tag[] @relation(through: "id -> PostTag.postId -> PostTag.tagId -> Tag.id") with the mirror-imaged path on Tag.posts. The resolver builds the through descriptor straight from the named columns and lowers both ends to cardinality: 'N:M' + through over post_tags -- the same runtime-consumable shape the authored-junction (mn-psl-through) and implicit (mn-psl-implicit) forms emit. Wire the fixture into the sql-orm-client emit chain so fixtures:check regen covers it, and add a PGlite integration parity test that drives db.orm.Post.include('tags') over the arrow-path M:N -- whole-row toEqual, explicit .select() plus one implicit-selection readback -- proving the column-based arrow-path contract drives the ORM include end-to-end. Signed-off-by: Alexey Orlenko's AI Agent <robot@aqrln.net>
01db649 to
8affc21
Compare
471261d to
1f38e8f
Compare
tensordreams
left a comment
There was a problem hiding this comment.
Slice 5 lands the arrow-path through: cleanly: the four-segment quoted value parses and lowers to an N:M through descriptor that is structurally consistent with slice-2's relation-field junction form (the lower-to-same-contract parity test plus the PGlite include integration test prove the ORM drives it end-to-end), and it early-exits the backrelation loop so it complements rather than collides with slice-4's implicit synthesis. Two substantive correctness gaps remain: the path's targetKey (4th segment) is resolved for existence but never threaded into the relation node — through.targetColumns is always the target model's PK downstream (build-contract.ts:200) — so naming a non-PK target column silently lowers to the PK regardless of what the author declared; and localKey/targetKey are existence-checked but not validated to be key/unique columns as the spec ("segment 1 = a local @id/key column", "segment 4 = a key column") requires, so a non-key local side would silently build a non-unique parent join where slice-2's FK-referenced columns inherently guarantee one. The parseArrowPath jsdoc also mis-describes the last segment as a bare key, and two diagnostic branches (PSL_ARROW_PATH_TARGET_MISMATCH and the same-column PSL_ARROW_PATH_JUNCTION_MISMATCH) ship without regression tests.
| return undefined; | ||
| } | ||
|
|
||
| return arrowPathManyToManyRelationNode({ |
There was a problem hiding this comment.
[bug] targetColumn (the path's 4th segment) is resolved via resolveArrowColumn for existence but is never threaded into arrowPathManyToManyRelationNode — its arg list carries localColumns/nearColumn/farColumn only. through.targetColumns is then filled downstream by buildThroughDescriptor → targetColumnsForJunction(targetModel, …) (build-contract.ts:200), which always returns the target model's PK. So the 4th path segment is existence-validated but otherwise discarded: through: "... -> PostTag.tagId -> Tag.label" would silently lower to through.targetColumns: ['id'] (the PK), joining tag_id against Tag.id rather than the named label. The path reads as though the author controls the target key; they don't. Either thread targetColumn into the node so the contract honours the named target key, or explicitly require targetKey to be the target's PK and validate it (arrowPath.targetKey ∈ targetModel.id.columns) with an actionable diagnostic — otherwise a typo on the 4th segment is silent.
| return undefined; | ||
| } | ||
|
|
||
| const localColumn = resolveArrowColumn({ |
There was a problem hiding this comment.
[question] localKey (on.parentColumns via localColumns: [localColumn]) is resolved as any existing column on the declaring model, not a key/unique column. The spec calls for "segment 1 = a local @id/key column on the declaring model". Slice-2's relation-field path inherently uses pair.parentFk.referencedColumns (FK-referenced ⇒ guaranteed unique/PK), so the M:N parent side is always a key; the arrow-path has no such guarantee, so a non-unique localKey would silently build a non-unique parent join, letting the runtime include fan out/duplicate parent rows. The same gap applies to targetKey (already partly covered above). Consider validating localKey/targetKey are a declared @id/@@unique column (or, for the target, the PK the assembler will use), matching what slice-2 produces structurally.
| * its named parts. The middle two segments are `Model.column`; the first and | ||
| * last are bare keys (a leading `Model.` qualifier on them is tolerated and | ||
| * dropped, like `from:`/`to:`). Returns undefined on the wrong segment count or |
There was a problem hiding this comment.
[doc] The jsdoc says "the first and last are bare keys (a leading Model. qualifier on them is tolerated and dropped, like from:/to:)", but the last segment is parsed by splitQualifiedColumn(targetSegment) (line 184), which requires Model.column and rejects a bare key — and its model is then validated against candidate.targetModelName at line 992. So the last segment is neither bare nor merely tolerated-and-dropped. Worth correcting to match the implementation (and the test form Tag.id): middle two and last are Model.column; only the first is a bare key with an optional tolerated qualifier. As a related asymmetry, the local segment's qualifier is silently dropped without checking it names the declaring model, whereas the target qualifier is checked — worth noting or aligning.
| expect(diagnostic?.message).toContain('Other'); | ||
| }); | ||
|
|
||
| it('diagnoses an arrow path whose junction is not a declared model', () => { |
There was a problem hiding this comment.
[test] The resolver has six diagnostic branches but only four are exercised here. Two are untested:
PSL_ARROW_PATH_TARGET_MISMATCH(psl-relation-resolution.ts:992) — the path's target model ≠ the list field's element type, e.g.tags Tag[] @relation(through: "id -> PostTag.postId -> PostTag.tagId -> Wrong.id")with aTag-typed list field. This is a valuable typo-catcher and worth a regression test.- The same-column
PSL_ARROW_PATH_JUNCTION_MISMATCH(psl-relation-resolution.ts:1054) —nearColumn === farColumn, e.g."... -> PostTag.postId -> PostTag.postId -> ...".
Add a sibling it(...) for each so the diagnostic text/codes stay pinned.
Linked issue
Refs TML-2944. Fifth and final of the PSL: Directional Relation Syntax stack — stacked on
tml-2943-s4-implicit-mn-synthesis(TML-2943). Spec underprojects/psl-relation-syntax/slices/05-arrow-path-through/.At a glance
The arrow-path declares the many-to-many on the terminal models, naming the join columns directly — so the junction needs no relation fields of its own.
Summary
An escape hatch for declaring a many-to-many over a junction that carries scalar columns +
@@idbut no relation fields: thethrough:arrow-path names the column hops (localKey -> Junction.nearCol -> Junction.farCol -> targetKey). It lowers to the sameN:M+throughdescriptor as the relation-field form — by construction.Decision
Recognise an arrow-path
through:value and build thethroughdescriptor straight from the named columns, bypassing the relation-field-based junction recognition (which can't fire when the junction has no relation fields). The arrow-path is a quoted string value — see Reviewer notes for why the unquoted operator form was not taken here.How it fits together
through:value containing->is detected and split into anArrowPath({ localKey, near/far junction model+column, target model+key }). No grammar change — the parser already emits a string literal.packages/2-sql/2-authoring/contract-psl/src/psl-relation-resolution.ts): a dedicated branch validates the columns against the resolved models and builds theN:Mnode directly — parent walkslocalKey → near col, child walksfar col → targetKey— reusing the same node machinery as the relation-fieldthrough:. The junction must be a declared model (so the contract-builder'sthrough.tablerequirement holds).Behavior changes & evidence
N:M+through— identically to the relation-field form.contract-psl/src/psl-relation-resolution.ts— evidence:packages/2-sql/2-authoring/contract-psl/test/interpreter.relations.arrow-path.test.ts(a test assertstoEqualon theContractagainst the relation-fieldthrough:form — the equivalence holds because both paths return the sameRelationNodeshape into the same assembler).PSL_ARROW_PATH_MALFORMED/_COLUMN_NOT_FOUND/_JUNCTION_MISMATCH/_JUNCTION_NOT_MODEL), each tested.include. evidence:test/integration/test/sql-orm-client/fixtures/mn-psl-arrow-path/contract.prisma+…/mn-psl-arrow-path-parity.test.ts(PGlite,@map'd storage columns exercised end-to-end).Notes for the reviewer
a -> J.b. The unquoted form is a real grammar change (a new tokenizer token, a chained-arrow expression node, AST/union/rendering, formatter tests) — disproportionate for the lowest-priority escape-hatch slice. The quoted form is zero-grammar and functionally identical. The unquoted-operator polish is an open ratification — happy to file it as its own grammar PR if preferred (recorded in the project's decisions log).check:upgrade-coverageapplies (touchespackages/3-extensions/sql-orm-client) — see Skill update.Testing performed
pnpm --filter @prisma-next/sql-contract-psl test; the arrow-path parity test on PGlite. Validated after the rebase:pnpm typecheck:packages(129/129),pnpm --filter -next/sql-contract-psl test(23 files, 314 tests),pnpm fixtures:check(zero drift), andpnpm lint:deps(0 violations).Skill update
n/a for behaviour beyond the additive arrow-path syntax. Touches
packages/3-extensions/sql-orm-client, socheck:upgrade-coverageexpects achanges: []declaration in the in-flight upgrade cycle — flagged as a stack-wide follow-up.Checklist
TML-NNNN: …form.