Skip to content

fix(query): project a bare node variable as its full record - #667

Merged
azimafroozeh merged 3 commits into
ModernRelay:mainfrom
azimafroozeh:bare-node-projection-descriptor-mismatch
Sep 6, 2026
Merged

fix(query): project a bare node variable as its full record#667
azimafroozeh merged 3 commits into
ModernRelay:mainfrom
azimafroozeh:bare-node-projection-descriptor-mismatch

Conversation

@azimafroozeh

Copy link
Copy Markdown
Contributor

What & why

Closes #631. return { $p } returned the node's id as a string ({"p": "alice"}) while lint --json described the field as an object: the compiler typed a bare node binding as a struct of the node type's schema, the executor projected the binding's id column. This PR makes the executor build the struct the compiler already promises, so an agent reading the result gets the node, not a key it cannot use.

  • The object: the identity column and every declared property except Blob (not a read value, T24) and Vector (an embedding is never context an agent reads; $p.embedding stays the explicit route): {"id": "alice", "name": "alice", "age": 30}.
  • One method feeds both sides: NodeType::node_object_fields gives typecheck its struct fields and the executor its column list. It is keyed on the declared property types, not on Arrow types: the engine rewrites Blob columns to their storage field before executing, so a filter on LargeBinary is dead at runtime.
  • The executor reads each binding's node type off the pipeline it executes and assembles the struct from that binding's columns in the wide batch. Taking every <var>.* column instead was rejected: search scans add _distance and _score there.
  • Aggregates: count($p) is unchanged (counts rows); a $p group key shows the object; sum/avg/min/max over a bare node binding or an aliased node projection are refused at T8 (before, min($p) returned the smallest id and sum($p) failed in the executor); an aggregate-typed argument (a nested aggregate or a forward alias reference) is refused at T8 for all five functions instead of failing in the executor.
  • The .gqt shape section gains a spelling for the object: p: Person, the node type name, checked against the executed struct's field names (the type's identity column plus its declared non-Blob, non-Vector properties); Person? is refused since the object is never null; bless spells a struct column back as the node type whose object it is. RFC 0045 carries the decision-log entry.

Backing issue / RFC

Checklist

  • Change is focused (one logical change: the executor returns the struct the compiler types for a bare node binding, plus the T8 refusals that struct makes necessary)
  • Tests added/updated for behavior changes (three .gqt cases, 13 steps: issue_631_bare_node_projection_returns_node.gqt, 3 steps: bare projection with a null property, a traversal-reached node, a $p group key with count; bare_node_projection_object_fields.gqt, 8 steps on a Blob + Vector schema: the object beside $p.embedding, zero rows, count($p) alone, a multi-row group key with a null inside one key, an alias-ordered struct sort, min($p) and min(person) refused, count(count($p)) refused; bare_node_projection_search_scan.gqt, 2 steps: a bm25-ordered scan and an rrf-fused scan, no _score and no vector in the object; every rows step carries its --- expect shape section with p: Person-style lines; no engine Rust tests, every behavior is visible in rows or errors; one harness self-test for the node spelling)
  • Public docs updated if user-facing surface changed (docs/user/queries/index.md §Return, skills/omnigraph/references/queries.md, docs/releases/v0.11.0.md highlight)
  • Reviewed against docs/dev/invariants.md — no Hard Invariant weakened, no deny-list item hit (no storage, commit, or scan change: the binding's columns were already loaded for a bare variable, the diff only changes how they are projected)

Local verification

On the branch rebased onto main at c253f111.

  • cargo test -p omnigraph-compiler — 339 green
  • cargo test -p omnigraph-gqt — 117 self-tests and 16 corpus cases green (the three new cases pass the mandatory shape section and the executed-vs-inferred schema check)
  • cargo test -p omnigraph-engine --test aggregation --test end_to_end --test search --test traversal --test traversal_indexed --test literal_filters --test ordering --test proptest_equivalence --test lineage_projection — 181 green
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo fmt --all --check — clean
  • python3 scripts/check-docs.py (129 files), scripts/check-agents-md.sh, python3 scripts/check-release-vocabulary-gates.py — clean
  • cargo test --workspace — not run: the engine lib, DST, server, and CLI suites are off the projection path and compile under --all-targets

Notes for reviewers

  • Behavior change with no opt-in: a stored query that returns a bare node variable now returns an object where it returned a string. The release note carries it.
  • The node object's field names and order come from one method on both sides (typecheck and the executor). Inside the object, the field types and nullability are the executed batch's (Lance's physical types) while the descriptor's are the catalog's. The runner's executed-vs-inferred schema check (test(gq): add a mandatory --- expect shape section and a result-schema check #635) holds the two structs equal on every case here, which is why typecheck builds its inner fields as plain name, type and nullability rather than cloning the catalog fields with their Lance metadata.
  • The identity key is the catalog's physical name, id on every graph today; field order is the catalog's (identity first, then properties) and is not contracted. The struct column itself is non-null; nullability inside it is per property.
  • Rebased onto c253f111: the T8 arm was moved into one function here and edited by fix(query): accept Bool, Date, and DateTime in min and max #637 upstream (min/max accept Bool, Date, DateTime); the function carries fix(query): accept Bool, Date, and DateTime in min and max #637's condition and message.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@azimafroozeh
azimafroozeh merged commit ea90cf5 into ModernRelay:main Sep 6, 2026
23 checks passed
@azimafroozeh
azimafroozeh deleted the bare-node-projection-descriptor-mismatch branch September 6, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: returning a node variable gives its id instead of the node

1 participant