Repository navigation
Conversation
A node type and an edge type may share a name, and the type checker always
selected the node namespace, so \insert Link {from: ..., to: ...}\ was
refused with \T11: type 'Link' has no property 'from'\ and an edge of a
shared name stayed unreachable through GQ.
A mutation target now names its namespace, \
ode:Person\ or \edge:Knows\,
the spelling \load csv\ already uses, and one resolver serves insert,
update and delete. A bare name still resolves in the namespace that
declares it; a bare name both namespaces declare is refused with \T60\
instead of silently selecting one of them.
lab1207
changed the title
|
Request changes. A node type and an edge type may share a name. The two names live in separate namespaces. A mutation must say which namespace it means. This change adds that spelling, This is the layer that owns the contract. Invariant 9 says the compiler selects the node or edge target, and execution keeps that choice. Execution already carries Do not merge this head. The compiler crate does not build, and the user-guide length check fails. The resolution behavior is correct once the crate builds. I proved that with a one-token local compile fix that only changes an unreachable panic format. I restored that edit. It is not in this pull request. Blocking
Optional
The embedded skill still shows only the bare Tradeoff and liabilityThe reported statement The break is limited to a schema that declares one name in both namespaces. A bare mutation of a name that exists in only one namespace stays valid. The changelog and the mutation guide say who must change a query. There is no storage, protocol, or manifest change. The change is reversible. The resolver replaces three node-then-edge lookups with one function. Later kinds, and the schema-namespace names from open RFC #1015 ( Lance does not own this name. The compiler resolves the target before the engine opens a dataset. I read the insert, update, and delete arms in ValidationBefore, main FAIL in 0.63s. After, head
A temporary case on that same build, deleted afterward, passed in 0.74s. It covers
The committed head does not compile, so these runs are not a green result for CI on I did not run DST, S3, or Azure. This change does not touch storage or deployment. Fix the format string and the page length. The design can merge after those two. Inline comments that could not be attached:
|
`Rule` has no `Display` implementation, so the arm that formats it
with `{other}` did not compile. Use the Debug form, as every other match
in the file does.
|
Thanks for the precise review — both blockers are fixed, and the optional point was a good catch. 1. The build. other => unreachable!("grammar: mutation_target holds no {other:?}"),2. The user page length. The lines came from re-wrapping two paragraphs that were raggedly wrapped, not from removing content. The schema guide already states the namespace rule and points here for the mutation spelling, so the two pages now agree. 3. The optional point.
Same limit as the other branch: no cached crates.io dependencies here, so |
|
Re-review of A node type and an edge type may share a name. The two names live in separate namespaces. A mutation must say which namespace it means when both declare the name. This change adds that spelling, This is the layer that owns the contract. Invariant 9 says the compiler selects the node or edge target, and execution keeps that choice. Execution already carries Earlier findings
BlockingNone. Optional
Tradeoff and liabilityThe reported statement The break is limited to a schema that declares one name in both namespaces. A bare mutation of a name that exists in only one namespace stays valid. The changelog says who must change a query. There is no storage, protocol, or manifest change. The change is reversible. The resolver replaces three node-then-edge lookups with one function. Later kinds, and the schema-namespace names from open RFC #1015 ( Lance does not own this name. The compiler resolves the target before the engine opens a dataset. I read the insert, update, and delete arms in ValidationBefore, workspace The case is the report from issue 921 and expects one edge. FAIL in 0.59s. The same binary on this pull request's case file fails before the first mutation: After, head 4 passed. passes.
The reported statement, in a temporary case that expects T60,
CI on I did not run DST, S3, Azure, the full workspace suite, or the full GQT corpus. This change does not touch storage or deployment. The server and the CLI use this compiler. I did not run those processes. Inline comment that could not be attached, |
Closes #921.
The problem
A schema can declare a node type and an edge type with the same name — the two live in separate namespaces, and the catalog only refuses duplicates within one kind:
The compiler always resolved a mutation's target to the node type when both namespaces contained the name, so the edge could never be mutated:
failed with
deletehad the same defect —delete Link where @src = "a"also selected the node type and failed on the@srcmeta-field. Loading and reading those edges worked, and so did inserting the node type, which is what made the failure look like a property bug rather than a name-resolution one.The fix
A mutation may now name the namespace it targets, using the spelling
load csvalready uses:query.pest),MutationKind::Node/MutationKind::Edgeon insert, update and delete (ast.rs),typecheck.rs).A bare name that both namespaces declare is now refused with a new code, T60 — "a mutation target names a type in both the node and the edge namespace" — instead of silently picking the node type. A statement that does not say which one it means should not quietly mean one of them; that is the only case where the old behaviour was wrong silently.
This is a breaking change for schemas that use a shared name, which is why it ships with
changelog.d/mutation-target-namespace.breaking.md. A target that only one namespace declares keeps working unqualified, so queries are unaffected unless the schema really does declare the name twice.Test
crates/omnigraph-compiler/src/query/typecheck_tests.rs— the existing same-named-namespace test now spellsinsert node:Shared, and a new test coversinsert edge:Shared { from: "a", to: "b" }resolving toMutationTarget::Edge. A second new test pins the refusal: the bare spelling of a name both namespaces declare is rejected.crates/omnigraph-compiler/src/query/lint_tests.rs— the lint-level test now uses the qualified spelling.crates/omnigraph-gqt/cases/issue_921_shared_node_and_edge_mutation_target.gqt— an end-to-end case in the repository's own GQT format: the schema from the issue, the seed from the issue, the mutation, andexpect affected: nodes=0 edges=1.Note on validation
I did not build or run the test suite locally: CI on this pull request is the judge. The change is contained in the compiler crate plus the GQT case, and it adds no new dependency and no new process or filesystem lifecycle.
Model Used
MiniMax M3.1 Flash Free via the opencode agent harness (
minimax-m3.1-flash-free)