fix(runtime): SQL-style null and 3VL in residual WHERE#27
Draft
Felipe705x wants to merge 3 commits into
Draft
Conversation
- AttrLookup: missing property reads as Success(Null), not Failure - eval_binop: 3VL for comparisons, AND/OR/NOT, arithmetic; As passes Null - Unary -/NOT: Null preserved; tighten dynamic error strings - Aggregates: skip Success(Null) and Failure (COUNT tests/comments) - cmp_values doc: clarify bool-only pushdown vs full WHERE 3VL - Docs: CLAUDE.md, iso-gql-gaps; tests: null_test, count_test - Typechecker: null literal maps to Star; IsNull expr doc Co-authored-by: Cursor <cursoragent@cursor.com>
…okup) - ISO: unified property reference semantics for records vs graph elements - FPPC R_a: missing read ok null; Failure must not replace OR/AND 3VL path - CLAUDE.md + Value docs; record_test regression for nested miss OR (1=1) Co-authored-by: Cursor <cursoragent@cursor.com>
- Null.field → Success(Null); record arm uses get().unwrap_or - Fixture: Dave without address; OR regression expects 4 names - CLAUDE.md: chained access on null base Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
Missing keys on bound values read as null; general
WHEREuses SQL-style 3VL (AND/OR/NOT; comparisons with null → unknown). Rows are kept only when the condition is definitely true.Graph properties (
AttrLookup) and record fields (FieldAccess) now match: missing key →Success(Value::Null), notFailure, soOR/ANDdo not short-circuit on a missing nested field (same pattern as missing top-level property).Bug visibility (before)
Carol has no
email;Failureon missing property brokeOR:Expected: Alice, Bob, Carol. Before: Carol dropped.
Record layer (same OR pattern, missing nested field on
address):Before
FieldAccessfix:Failureon missing record key → wholeORbecameFailure→ no rows. After: unknown∨true → true for all users withaddress(seerecord_test).Spec alignment (concise)
ISO/IEC 39075:2024:
<property reference>unifies graph elements and record expressions; missing field → null for nullable / open cases;WHEREuses 3VL (e.g. unknownORtrue = true);IS NULLvs= NULLdiffer as in SQL. Material / non-nullable (22G12) and<property exists>remain out of scope for this PR.FPPC: Rule
R_a: missing attribute read →ok null(notstuck);★for null literal at static meet is sound; nestedFieldAccessaligned withAttrLookupmatches that intent.ISO nuance — §5.3.2.4 vs pushdown
Subclause 5.3.2.4 allows reorderings when observable effect matches the General Rules. Treating unknown as false in a filter often matches
WHERE(both drop the row) when that decision is the full condition.It does not automatically justify every split of a boolean expression across pushed (bool-only, e.g.
cmp_values: null → false) vs residual (full 3VL) layers: underOR, unknown∨true must stay true; a rewrite that evaluates only a subexpression as false where the full expression would be true can diverge. So pushdown remains defensible per literal ISO text only when equivalence is proved for that rewrite; the CLAUDE.md follow-up stays the right place to track unsafe splits.Does this block the PR? No. It sharpens the follow-up: unify or prove observational equivalence case-by-case, not assume §5.3.2.4 covers all null pushdown.
Changes
AttrLookup: missing property →Success(Null).FieldAccess: missing record key →Success(Null)(aligns with graph property reads and ISO unified property reference / FPPCR_a).eval_binop: 3VL;Aspasses null; unary-/NOTpreserve null.Failure; COUNT tests renamed/clarified.CLAUDE.md,docs/iso-gql-gaps.md;cmp_valuescomment;Value::Nulldoc mentions 3VL inWHERE.Tests
parser_test,runtime_test,store_runtime_test,text2gql_test,null_test,count_test,record_test(green locally).Follow-up
WHERE(especially underOR) cannot change row sets vs residual 3VL, or document safe subsets per §5.3.2.4.22G12,<property exists>: separate workstreams.