fix(query): project a bare node variable as its full record - #667
Merged
azimafroozeh merged 3 commits intoSep 6, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
What & why
Closes #631.
return { $p }returned the node's id as a string ({"p": "alice"}) whilelint --jsondescribed the field as anobject: the compiler typed a bare node binding as a struct of the node type's schema, the executor projected the binding'sidcolumn. 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.Blob(not a read value,T24) andVector(an embedding is never context an agent reads;$p.embeddingstays the explicit route):{"id": "alice", "name": "alice", "age": 30}.NodeType::node_object_fieldsgives typecheck its struct fields and the executor its column list. It is keyed on the declared property types, not on Arrow types: the engine rewritesBlobcolumns to their storage field before executing, so a filter onLargeBinaryis dead at runtime.<var>.*column instead was rejected: search scans add_distanceand_scorethere.count($p)is unchanged (counts rows); a$pgroup key shows the object;sum/avg/min/maxover a bare node binding or an aliased node projection are refused atT8(before,min($p)returned the smallest id andsum($p)failed in the executor); an aggregate-typed argument (a nested aggregate or a forward alias reference) is refused atT8for all five functions instead of failing in the executor..gqtshape 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-Vectorproperties);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
issue_631_bare_node_projection_returns_node.gqt, the report's schema, seed, and query with the expected rows;Fix Regression Gate)Checklist
T8refusals that struct makes necessary).gqtcases, 13 steps:issue_631_bare_node_projection_returns_node.gqt, 3 steps: bare projection with a null property, a traversal-reached node, a$pgroup key withcount;bare_node_projection_object_fields.gqt, 8 steps on aBlob+Vectorschema: 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)andmin(person)refused,count(count($p))refused;bare_node_projection_search_scan.gqt, 2 steps: a bm25-ordered scan and an rrf-fused scan, no_scoreand no vector in the object; every rows step carries its--- expect shapesection withp: Person-style lines; no engine Rust tests, every behavior is visible in rows or errors; one harness self-test for the node spelling)docs/user/queries/index.md§Return,skills/omnigraph/references/queries.md,docs/releases/v0.11.0.mdhighlight)Local verification
On the branch rebased onto
mainatc253f111.cargo test -p omnigraph-compiler— 339 greencargo 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 greencargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --all --check— cleanpython3 scripts/check-docs.py(129 files),scripts/check-agents-md.sh,python3 scripts/check-release-vocabulary-gates.py— cleancargo test --workspace— not run: the engine lib, DST, server, and CLI suites are off the projection path and compile under--all-targetsNotes for reviewers
--- expect shapesection 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.idon 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.c253f111: theT8arm was moved into one function here and edited by fix(query): acceptBool,Date, andDateTimeinminandmax#637 upstream (min/maxacceptBool,Date,DateTime); the function carries fix(query): acceptBool,Date, andDateTimeinminandmax#637's condition and message.