From 5b21c0ff279fd886c0f7714a43128be125eb8096 Mon Sep 17 00:00:00 2001 From: Aditya Mukhopadhyay Date: Tue, 20 Oct 2026 19:34:46 +0530 Subject: [PATCH] fix(parser): reject what the grammar rejects (#651) The grammar verifier (#643) found inputs the parser accepted and tests/grammar/grammar.pest rejects, some with wrong answers: - a value after a query option and its argument joined the section before the option ([:find ?a :any-valid-time ?b ...] returned ?b as a column): now PRS-061 - a :where list headed by an operator or a variable, (< ?a ?b) or (?r ?a), was a call of a rule of that name and returned no rows: now PRS-097; a rule named by one: PRS-105 - a rule head argument that is not a variable: new PRS-114 - an aggregate, window or expression named by a variable failed at run time: now PRS-025 / PRS-029 / PRS-063 - (query []): new PRS-111; an empty :find/:where/:with: new PRS-112; an unknown query keyword: new PRS-113 - an unterminated string: new PRS-109; a ':' with no name: new PRS-110 A lone '-' stays the subtraction operator: the grammar's op_sym now lists it. Removes the #651 entries from the verifier's KNOWN_DRIFT. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019kY61Y11gcRD91Y6b7jbGT --- CHANGELOG.md | 11 +- CLAUDE.md | 4 +- README.md | 2 +- docs/ERROR_REFERENCE.md | 118 +++++++- docs/GRAMMAR.md | 3 +- docs/TEST_COVERAGE.md | 3 +- src/error.rs | 54 ++++ src/query/datalog/parser.rs | 57 +++- src/query/datalog/parser/verifier.rs | 252 +----------------- tests/error_codes_prs_test.rs | 125 ++++++++- tests/grammar/grammar.pest | 5 +- .../syntax/aggregate_variable_head.edn | 1 + .../invalid/syntax/clause_operator_head.edn | 1 + .../invalid/syntax/clause_variable_head.edn | 1 + .../invalid/syntax/expr_variable_head.edn | 1 + .../invalid/syntax/keyword_bare_colon.edn | 1 + tests/grammar/invalid/syntax/query_empty.edn | 1 + .../invalid/syntax/query_find_empty.edn | 1 + .../invalid/syntax/query_unknown_keyword.edn | 1 + .../syntax/query_value_after_option.edn | 1 + .../invalid/syntax/query_where_empty.edn | 1 + .../invalid/syntax/query_with_empty.edn | 1 + .../invalid/syntax/rule_head_constant_arg.edn | 1 + .../syntax/rule_head_variable_name.edn | 1 + tests/grammar/valid/expr_subtract.edn | 1 + 25 files changed, 380 insertions(+), 268 deletions(-) create mode 100644 tests/grammar/invalid/syntax/aggregate_variable_head.edn create mode 100644 tests/grammar/invalid/syntax/clause_operator_head.edn create mode 100644 tests/grammar/invalid/syntax/clause_variable_head.edn create mode 100644 tests/grammar/invalid/syntax/expr_variable_head.edn create mode 100644 tests/grammar/invalid/syntax/keyword_bare_colon.edn create mode 100644 tests/grammar/invalid/syntax/query_empty.edn create mode 100644 tests/grammar/invalid/syntax/query_find_empty.edn create mode 100644 tests/grammar/invalid/syntax/query_unknown_keyword.edn create mode 100644 tests/grammar/invalid/syntax/query_value_after_option.edn create mode 100644 tests/grammar/invalid/syntax/query_where_empty.edn create mode 100644 tests/grammar/invalid/syntax/query_with_empty.edn create mode 100644 tests/grammar/invalid/syntax/rule_head_constant_arg.edn create mode 100644 tests/grammar/invalid/syntax/rule_head_variable_name.edn create mode 100644 tests/grammar/valid/expr_subtract.edn diff --git a/CHANGELOG.md b/CHANGELOG.md index 07093d23b..5a9b392d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Every build reorders query clauses; the `wasm` feature does nothing** (#594, #596). The `wasm` Cargo feature kept patterns, `not` clauses and `or` branches in source order and turned off the `or` short-circuits (#250), for small browser datasets and a predictable order. With every answer independent of the plan (#594, #596, #606) it only made queries slower, and no first-party build enabled it (the browser package uses `browser`). The feature is kept, so builds that name it still resolve, but is deprecated and has no effect. - **Aggregate and window answers no longer depend on the order of their input** (#547). Before, aggregates and window functions read their rows in the order the plan produced them, and several answers followed that order. Six cases changed. (1) `sum` over floats is exact: the values are added without rounding and the total is rounded once, so `1e16 + 1.0 + -1e16 + 1.0 + 0.5` gives `2.5` in every order (before, `1.5` or `2.5`). Window `sum` and `avg` likewise. (2) `sum` over integers saturates once, at the end: `i64::MAX + 1 + -1` gives `i64::MAX` (before, `i64::MAX - 1` or `i64::MAX` by order). (3) Window `sum` over integers adds them exactly; before they were added as floats, so a running total above 2^53 lost its low digits. (4) `min` / `max` of floats follow one order: `-0.0` before `0.0`, NaN after every other number (before, the first of two such values seen won). Window `min` / `max` use the same order over any values, so the minimum of keywords is the first in text order. (5) Window functions sort each partition by `:order-by` under one total order of values (numbers by numeric value, an integer before an equal float, then strings, booleans, refs, keywords, `nil`), so keywords, refs and booleans now sort (before, any two of them compared equal and kept their input order). Rows with equal `:order-by` values are peers: `rank` gives them the same rank, and `sum`, `count`, `min`, `max`, `avg` and registered window functions give every peer the value at the last peer, as SQL does; `row-number` numbers peers in the order of the other variables `:find` names. For example, with `?s` of 1.5, 5, 10, 10, `(sum ?s :over (:order-by ?s))` gave 1.5, 6.5, 16.5, 26.5, the 16.5 going to either tied row, and now gives 1.5, 6.5, 26.5, 26.5; ranking `:x :x :y :y` by `:order-by` gave ranks depending on the order the scan met them (1 2 3 4 when interleaved) and now gives 1 1 3 3. (6) In a query with aggregates, a window's argument, `:partition-by` and `:order-by` must be plain `:find` variables, or the query fails at parse time with the new `PRS-082`; before, any other variable read `nil` on every group row. The error a failing aggregate query reports is the first error, in `:find` order, of the first failing group in output order. A registered aggregate still receives its values in input order, which can depend on the plan; one whose result depends on that order may answer equivalent queries differently. (7) A group whose grouping variable holds floats that are equal as groups compare them (`-0.0` and `0.0`, or NaNs) returns the least of them in that order, so `-0.0` when the group holds it; before, it returned the first one the plan met. - **Parse errors that were `INT-019` now carry their own `PRS` codes** (#643). The parser's long-tail syntax and structure errors (a bind slot with no name, a window without `:order-by`, a non-vector query, malformed `:with`, `:find`, valid-time map, retract fact, `not`/`not-join`/`or` body, pattern and rule) used one internal code, `INT-019`, with a `datalog parser: ` prefix. Each now has its own Parser-category code, `PRS-083` to `PRS-107`, documented in `docs/ERROR_REFERENCE.md`; the message is the old text without the prefix. A retract fact without exactly three elements was `INT-000` and is now `PRS-090`. Code that matches on `INT-019` (or `INT-000` for retract arity), or on the `Internal` category, for these errors must match the new codes. `INT-019` stays registered for parser invariants no input is known to reach. +- **The parser rejects what the grammar rejects** (#651). The grammar verifier (#643) found inputs the parser accepted that `tests/grammar/grammar.pest` and `docs/GRAMMAR.md` do not describe, several with wrong answers. Each is now an error: + - A value after a query option and its argument (`:as-of N`, `:valid-at "…"`, `:any-valid-time`, `:max-derived-facts N`, `:max-results N`) joined the section before the option: `[:find ?a :any-valid-time ?b :where …]` returned `?b` as a second column. It now fails with `PRS-061`. + - A `:where` list headed by an operator or a variable, `(< ?a ?b)` or `(?r ?a)`, was a call of a rule of that name and returned no rows. It now fails with `PRS-097`; a comparison is an expression clause, `[(< ?a ?b)]`. + - A rule named by a variable or an operator, `(rule [(?r ?a) …])`, fails with `PRS-105`, and a rule head argument that is not a variable, `(rule [(r ?a :c) …])`, with the new `PRS-114` (it bound the caller's argument to the constant; bind the variable in the body instead). + - An aggregate, window function or expression named by a variable, `(?f ?x)`, failed when the query ran (`QRY-010`, `QRY-007`); it now fails to parse with `PRS-025`, `PRS-029` or `PRS-063`. + - `(query [])` fails with the new `PRS-111`; `:find`, `:where` or `:with` with nothing after it (an empty `:find` returned one empty row) with the new `PRS-112`; a keyword the query vector does not know, such as `:limit`, with the new `PRS-113` (it was ignored when another keyword followed, and `PRS-061` otherwise). + - A string with no closing quote fails with the new `PRS-109` (it ran to the end of the input), and a `:` with no name after it, such as `:+`, with the new `PRS-110` (it was the keyword `:`). + + A lone `-` stays the subtraction operator; the grammar now lists it with the other operators. The verifier's #651 entries are removed. ### Added @@ -105,7 +114,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `tests/snapshot_atomicity_test.rs` races a writer against a reader: a query must see each transaction whole, whether written with `execute` or a `WriteTransaction` (#554), and must read one state for all its patterns (#555), in memory and on file. It also runs the v2.x checkpoint race from #558, which never failed on v3: copy-on-write checkpoints do not overwrite pages a published reader uses. - **The `datalog_eval` fuzz guard counts partial bindings** (#597). The guard skipped inputs whose patterns join to more than 50,000 rows, counting only the rows after the last pattern. A `not-join` body still runs on the executor pattern by pattern in text order, so a 120-pattern body built hundreds of thousands of partial bindings before its last pattern matched nothing, and ran past the 60-second timeout. The guard now counts the bindings after every pattern. On the `v3` corpus (4,783 inputs) coverage drops by 7 of 9,719 edges. The input is added as a seed, as is the `btree_page` input of the cursor fix above. -- **Parser vs grammar verifier (#643).** `src/query/datalog/parser/verifier.rs` checks, for every generated input, that the parser rejects what `tests/grammar/grammar.pest` rejects, and accepts what it accepts unless the error code is semantic (a listed code with a witness pest accepts). It runs every string of 1 to 3 characters over 102 characters (ASCII, `\t \n \r`, four non-ASCII stand-ins) through `parse_edn`, and every bracket-balanced sequence of up to `MINIGRAF_GRAMMAR_BOUND` (default 3) of 32 tokens in 15 command templates through `parse_datalog_command`, also with no separators up to 3 tokens. Deterministic; about 20 s by default and 3.5 min at bound 4 on a 16-thread machine. The drift it found is listed in `KNOWN_DRIFT` by cause (comments and non-ASCII letters #650; empty sections, ignored keywords, rule and call heads, a lone `:` or `-`, unterminated strings #651; a fact of more than four elements losing elements, #649, since fixed); each fix removes its entry. `grammar.pest` now says what the parser means: a fact is exactly `[e a v]` plus an optional valid-time map and a retracted fact exactly `[e a v]`; `not`, `not-join`, `or`, `or-join` (and `and` inside an `or`) are never rule names; literal keywords end at a word boundary (`:find?x` is one keyword). +- **Parser vs grammar verifier (#643).** `src/query/datalog/parser/verifier.rs` checks, for every generated input, that the parser rejects what `tests/grammar/grammar.pest` rejects, and accepts what it accepts unless the error code is semantic (a listed code with a witness pest accepts). It runs every string of 1 to 3 characters over 102 characters (ASCII, `\t \n \r`, four non-ASCII stand-ins) through `parse_edn`, and every bracket-balanced sequence of up to `MINIGRAF_GRAMMAR_BOUND` (default 3) of 32 tokens in 15 command templates through `parse_datalog_command`, also with no separators up to 3 tokens. Deterministic; about 20 s by default and 3.5 min at bound 4 on a 16-thread machine. The drift it found is listed in `KNOWN_DRIFT` by cause (comments and non-ASCII letters #650; empty sections, ignored keywords, rule and call heads, a lone `:` or `-`, unterminated strings #651, since fixed; a fact of more than four elements losing elements, #649, since fixed); each fix removes its entry. `grammar.pest` now says what the parser means: a fact is exactly `[e a v]` plus an optional valid-time map and a retracted fact exactly `[e a v]`; `not`, `not-join`, `or`, `or-join` (and `and` inside an `or`) are never rule names; literal keywords end at a word boundary (`:find?x` is one keyword). ### Documentation diff --git a/CLAUDE.md b/CLAUDE.md index abe022c13..a50259a75 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,7 +129,7 @@ cargo run < demos/demo_negation.txt 3. **`src/query/datalog/`** — Datalog engine - `parser.rs`: EDN/Datalog parser — `transact`, `retract`, `query`, `rule`, `:as-of`, `:valid-at`, `not`, `not-join`; `resolve_pseudo_sources` names each per-fact pseudo-attribute's source (nearest earlier same-entity pattern, lexical scope) and its hidden metadata variable `?#.`, else PRS-081 (#594) - - `parser/verifier.rs`: test-only (not wasm32) bounded-exhaustive check of the parser against `tests/grammar/grammar.pest` (#643): every 1–3 character string (`parse_edn`) and every balanced token filler up to `MINIGRAF_GRAMMAR_BOUND` (default 3) in 15 command templates (`parse_datalog_command`); pest rejects ⇒ parser rejects, pest accepts ⇒ parser accepts or raises a `SEMANTIC` code; today's drift is `KNOWN_DRIFT`, one entry per cause, removed by its fix (#650, #651; #649 fixed) + - `parser/verifier.rs`: test-only (not wasm32) bounded-exhaustive check of the parser against `tests/grammar/grammar.pest` (#643): every 1–3 character string (`parse_edn`) and every balanced token filler up to `MINIGRAF_GRAMMAR_BOUND` (default 3) in 15 command templates (`parse_datalog_command`); pest rejects ⇒ parser rejects, pest accepts ⇒ parser accepts or raises a `SEMANTIC` code; today's drift is `KNOWN_DRIFT`, one entry per cause, removed by its fix (#650; #649, #651 fixed) - `executor.rs`: Query executor — temporal filter (tx-time → net-assert → valid-time), not/not-join post-filter - `matcher.rs`: Pattern matching with variable unification; `edn_to_value`, `edn_to_entity_id` - `aggregate.rs`: `Agg` (one aggregate of one group) and `Running` (a window's running value), shared by the executor and the engine; exact sums (`i128` integers saturated once, fixed-point floats rounded once), `value_order` (one total order of values for window sorts, peers, `rank` and `min`/`max`), `named_vars` (a window's tie-break variables), `windows` (each window's value per row: partitions, sort, peers) (#547) @@ -206,7 +206,7 @@ Auto-migrates v7 → v8 on open (spec §9, with a backup meta page). v1–v6 are ## Test Coverage -**1661 tests** (1651 passing, 10 ignored; unit + integration + doc). +**1671 tests** (1661 passing, 10 ignored; unit + integration + doc). See `docs/TEST_COVERAGE.md` for the full per-file breakdown. **Testing conventions** — see the Testing Conventions section below before writing any tests. diff --git a/README.md b/README.md index bd12c796b..a263fba5e 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ let r2 = pq.execute(&[("tx", BindValue::TxCount(2)), ("entity", BindValue::Entit ```bash cargo run # interactive Datalog REPL -cargo test # run 1661 tests +cargo test # run 1671 tests cargo run < demos/demo_recursive.txt # recursive rules demo ``` diff --git a/docs/ERROR_REFERENCE.md b/docs/ERROR_REFERENCE.md index bd10f7a11..f80d2fed6 100644 --- a/docs/ERROR_REFERENCE.md +++ b/docs/ERROR_REFERENCE.md @@ -129,6 +129,12 @@ with no `CodedError` anywhere in its chain. | PRS-106 | Rule body clause is not a vector or list | Parser | | PRS-107 | Rule body has no clause | Parser | | PRS-108 | Fact has more than 4 elements | Parser | +| PRS-109 | Unterminated string literal | Parser | +| PRS-110 | Keyword without a name | Parser | +| PRS-111 | Empty query vector | Parser | +| PRS-112 | Empty query section | Parser | +| PRS-113 | Unknown query keyword | Parser | +| PRS-114 | Rule head argument is not a variable | Parser | | QRY-001 | Invalid entity | Query Execution | | QRY-002 | Attribute must be a keyword | Query Execution | | QRY-003 | Cannot transact a pseudo-attribute | Query Execution | @@ -677,7 +683,7 @@ See the [Datalog Reference](https://project-minigraf.github.io/minigraf-docs/lat **Error text**: `Aggregate function name must be a symbol, got {}` -**Cause**: The aggregate function name must be an unqualified symbol, not a keyword. +**Cause**: The aggregate function name must be an unqualified symbol, not a keyword or a variable. Through v2.0.x a variable, `(?f ?x)`, was read as a function named `?f` and failed when the query ran (#651). **Resolution**: - Use `sum`, not `:sum`. @@ -745,7 +751,7 @@ See the [Datalog Reference](https://project-minigraf.github.io/minigraf-docs/lat **Error text**: `window function name must be a symbol` -**Cause**: The window function name token is not a symbol (e.g. a keyword was used). +**Cause**: The window function name token is not a symbol (e.g. a keyword was used), or is a variable. Through v2.0.x a variable was read as a function named after it and failed when the query ran (#651). **Resolution**: - Use `rank`, not `:rank`. @@ -1276,18 +1282,17 @@ See the [Datalog Reference](https://project-minigraf.github.io/minigraf-docs/lat **Error text**: `Unexpected element in query: {}` -**Cause**: An unrecognised key appeared at the top level of the query map. +**Cause**: A value in the query vector belongs to no section: it comes before the first keyword, or after an option and its argument (`:as-of N`, `:valid-at "…"`, `:any-valid-time`, `:max-derived-facts N`, `:max-results N`). Through v2.0.x a value after an option joined the section before the option, so `[:find ?a :any-valid-time ?b :where …]` returned `?b` as a second column (#651). **Resolution**: -- Valid query map keys are `:find`, `:where`, `:with`, `:as-of`, `:valid-at`. -- Check spelling. +- Put every `:find` spec after `:find` and every clause after `:where`, before or after the options. - See the [Datalog Reference](https://project-minigraf.github.io/minigraf-docs/latest/datalog-reference/). **Example**: ```datalog -(query {:find [?e] :where [[?e :name ?n]] :limit 10}) +(query [:find ?a :any-valid-time ?b :where [?a :edge ?b]]) ``` -*`:limit` is not supported; results are not paginated in this version* +*Write `[:find ?a ?b :any-valid-time :where [?a :edge ?b]]`* ### PRS-062 Expression list cannot be empty @@ -1310,7 +1315,7 @@ See the [Datalog Reference](https://project-minigraf.github.io/minigraf-docs/lat **Error text**: `expression head must be a symbol, got {}` -**Cause**: The first element of an expression must be a symbol naming a function, not a keyword. +**Cause**: The first element of an expression must be a symbol naming a function, not a keyword or a variable. Through v2.0.x a variable, `[(?f ?x) ?y]`, was read as a function named `?f` and failed when the query ran (#651). **Resolution**: - Use `+` not `:+`. @@ -1853,10 +1858,11 @@ $<4097-char-slot-name> **Error text**: `Rule invocation must start with predicate name (symbol), got {}` -**Cause**: A list in `:where` or a rule body does not start with a symbol, so it is not a rule invocation (nor `not`, `or` or another clause form). Before v3.0.0 this was `INT-019`. +**Cause**: A list in `:where` or a rule body does not start with a rule name, so it is not a rule invocation (nor `not`, `or` or another clause form). A rule name is a symbol that is not a variable or an operator. Before v3.0.0 this was `INT-019`, and a list headed by a variable or an operator, such as `(< ?a ?b)`, was a call of a rule of that name, which matched nothing: the query returned no rows (#651). **Resolution**: - Start the list with the rule's name: `(reachable ?a ?b)`. +- Write a comparison as an expression clause, in a vector: `[(< ?a ?b)]`. **Example**: ```datalog @@ -1973,7 +1979,7 @@ $<4097-char-slot-name> **Error text**: `Rule head must start with a symbol (predicate name)` -**Cause**: The rule head does not start with a symbol, so the rule has no name. Before v3.0.0 this was `INT-019`. +**Cause**: The rule head does not start with a rule name: a symbol that is not a variable or an operator. Before v3.0.0 this was `INT-019`, and a variable or an operator such as `(?r ?a)` or `(< ?a)` was accepted as a name (#651). **Resolution**: - Start the head with the rule's name: `(reachable ?a ?b)`. @@ -2035,6 +2041,98 @@ with `PRS-047`. ``` *Remove `99 :junk`, or make it its own fact* +### PRS-109 Unterminated string literal + +**Error text**: `Unterminated string literal` + +**Cause**: A string has no closing `"`. Through v2.0.x the string ran to the end of the input, and the command was read without its closing brackets (#651). + +**Resolution**: +- Close the string. A `"` inside a string is written `\"`. + +**Example**: +```datalog +(transact [[:alice :person/name "Alice]]) +``` +*Write `"Alice"`* + +### PRS-110 Keyword without a name + +**Error text**: `Keyword has no name after ':'` + +**Cause**: A `:` is not followed by a letter, digit or one of `- _ / ?`, so the keyword has no name. Through v2.0.x this was the keyword `:` (#651). + +**Resolution**: +- Write the keyword's name right after the colon: `:person/name`, not `: person/name`. +- An operator in an expression is a symbol: `(+ ?a 1)`, not `(:+ ?a 1)`. + +**Example**: +```datalog +(transact [[:alice : "Alice"]]) +``` +*`:` alone is not a keyword* + +### PRS-111 Empty query vector + +**Error text**: `Query vector is empty` + +**Cause**: The query is `(query [])`. Through v2.0.x it was accepted (#651). + +**Resolution**: +- Add a `:find` and a `:where` section: `(query [:find ?e :where [?e :person/name ?n]])`. + +**Example**: +```datalog +(query []) +``` + +### PRS-112 Empty query section + +**Error text**: `Query section {} is empty` + +**Cause**: `:find`, `:where` or `:with` is followed by another keyword or by the end of the query vector. Through v2.0.x an empty `:find` returned one empty row and an empty `:where` or `:with` was ignored (#651). + +**Resolution**: +- Add what the section is for: variables or aggregates after `:find`, clauses after `:where`, variables after `:with`; or remove the keyword. + +**Example**: +```datalog +(query [:find :where [?e :person/name ?n]]) +``` +*Write `[:find ?e :where …]`* + +### PRS-113 Unknown query keyword + +**Error text**: `Unknown query keyword {}` + +**Cause**: The query vector holds a keyword it does not know. The query keywords are `:find`, `:where`, `:with`, `:as-of`, `:valid-at`, `:any-valid-time`, `:max-derived-facts` and `:max-results`. Through v2.0.x an unknown keyword followed by another keyword was ignored, and one followed by a value raised `PRS-061` (#651). + +**Resolution**: +- Check the spelling. +- `:in` is not supported: pass inputs through bind slots (`$name`) of a prepared query. +- `:limit` is not supported yet (#306). + +**Example**: +```datalog +(query [:find ?e :limit 10 :where [?e :person/name ?n]]) +``` +*`:limit` is not a query keyword* + +### PRS-114 Rule head argument is not a variable + +**Error text**: `Rule head argument must be a variable, got {}` + +**Cause**: An argument in a rule head is not a variable. Through v2.0.x a constant was accepted, `(r ?a :c)`, and a call `(r ?a ?x)` bound `?x` to it (#651). + +**Resolution**: +- Use a variable in the head and bind it in the body, e.g. with a pattern or an expression clause: `(rule [(r ?a ?x) [?a :edge ?x] [(= ?x :c)]])`. + +**Example**: +```datalog +(rule [(reachable-5 ?a ?b 0) [?a :edge ?b]]) +``` +*`0` is not a variable* + --- ## QRY — Query Execution Errors diff --git a/docs/GRAMMAR.md b/docs/GRAMMAR.md index 743c09a10..2b6c382e7 100644 --- a/docs/GRAMMAR.md +++ b/docs/GRAMMAR.md @@ -41,7 +41,8 @@ uuid-lit ::= "#uuid" string-lit bind-slot ::= "$" (letter | digit | "_" | "-")+ (* token *) keyword ::= ":" word-cont+ (* token *) variable ::= "?" word-cont* (* token *) -op-sym ::= "<=" | ">=" | "!=" | "<" | ">" | "=" | "+" | "*" | "/" (* token *) +op-sym ::= "<=" | ">=" | "!=" | "<" | ">" | "=" | "+" | "*" | "/" + | "-" (* not followed by word-cont *) (* token *) plain-sym ::= (* not followed by boolean-lit *) (* not followed by nil-lit *) (letter | "_") word-cont* | "-" word-cont+ (* token *) diff --git a/docs/TEST_COVERAGE.md b/docs/TEST_COVERAGE.md index 452d7fe64..08a5d95c4 100644 --- a/docs/TEST_COVERAGE.md +++ b/docs/TEST_COVERAGE.md @@ -25,7 +25,8 @@ This covers the unreleased file-format-v8 work (#287, #371): the index-key value - **Aggregates and windows** (`tests/aggregate_window_rules_test.rs`, 8 tests; #547): exact float `sum` and window `avg`, integer `sum` saturating once, exact window `sum` of integers above 2^53, `min`/`max` of `-0.0` and `0.0`, `rank` by keywords (peers 1 1 3 3), a running `sum` giving peers the value at the last peer, `row-number` breaking ties by the other `:find` variables, and `INT-041` from a failing group. `src/query/datalog/aggregate.rs` (15 unit tests) checks every accumulator over every order of small inputs and that `value_order` is a total order; `tests/window_functions_test.rs` checks `PRS-082` and a window over the groups of an aggregate query, and `tests/plan_independence_test.rs` runs aggregate, window and mixed queries with reordering on and off. `Aggregate` on the engine (#547 PR 2, `src/query/engine/tests.rs`, 3 tests): every built-in aggregate grouped by keywords, refs, booleans, floats and long strings, over `nil` values, keywords and refs, with `:with` and `or`, at six temporal settings in memory, after a checkpoint and after reopen, against the executor; an ungrouped aggregate reads every row and returns one batch of one row; the zero row and empty answers; groups in value order over several batches; `INT-041` from a later group; a group dropped by `min` of no value. `tests/aggregation_test.rs` checks that a group of `0.0` and `-0.0` returns `-0.0` in either insertion order. - **Parser error codes** (`tests/error_codes_prs_test.rs`, 26 new tests; #643): each of `PRS-083` to `PRS-107` (the parse errors that were `INT-019`, and the retract arity that was `INT-000`) from its `docs/ERROR_REFERENCE.md` example, in the Parser category, with its argument in the message and without the old `datalog parser: ` prefix; three of them through `prepare`. - **Fact arity** (`tests/error_codes_prs_test.rs`, 2 tests; #649): a fact of five or six elements, with or without a map 4th, fails with `PRS-108`, and a transaction holding one stores none of its facts. -- **Parser vs grammar** (`src/query/datalog/parser/verifier.rs`, 4 tests; #643): about 1.07M lexical strings and 555K structural inputs at the default bound (7.6M at `MINIGRAF_GRAMMAR_BOUND=4`) checked against `tests/grammar/grammar.pest`; known drift by cause in `KNOWN_DRIFT` (#650, #651), semantic codes with witnesses in `SEMANTIC`. +- **Parser tightening** (`tests/error_codes_prs_test.rs`, 10 tests; #651): `PRS-109` to `PRS-114` from their inputs (an unterminated string, a `:` with no name, `(query [])`, an empty `:find`/`:where`/`:with` in each position, an unknown query keyword with and without a value after it, a constant rule head argument); `PRS-061` for a value after each query option; `PRS-097` for a clause list headed by an operator or a variable, also inside `not`, `or` and a rule body; `PRS-105` for a rule named by a variable or an operator; `PRS-025`/`PRS-029`/`PRS-063` for an aggregate, window or expression named by a variable; the forms next to them still run (an option followed by a keyword, `(- ?n 1)`, a zero-argument rule). Thirteen new `tests/grammar/invalid/syntax/` fixtures. +- **Parser vs grammar** (`src/query/datalog/parser/verifier.rs`, 4 tests; #643): about 1.07M lexical strings and 555K structural inputs at the default bound (7.6M at `MINIGRAF_GRAMMAR_BOUND=4`) checked against `tests/grammar/grammar.pest`; known drift by cause in `KNOWN_DRIFT` (#650; #651 fixed), semantic codes with witnesses in `SEMANTIC`. - **Grammar page** (`tests/grammar_doc.rs`, 9 tests; #643): `docs/GRAMMAR.md` matches the EBNF rendered from `tests/grammar/grammar.pest`; each rendering rule (sequence, choice and grouping, names and builtins, `SOI`/`EOI` dropped, lookaheads as comments, bounded repetition, quoting, token marks, section titles, line breaking, the whitespace note) on a small grammar. - **Plan independence** (`tests/plan_independence_test.rs`, 15 tests, `test-oracle` feature only; #596): a matrix of queries (joins in both text orders, cross products, keyword and ref, floats, long strings, filter and binding expressions, recursive rules and rules with `not`, `or`/`or-join` including the #250 short-circuit shapes, placed and deferred `not`/`not-join`, aggregates, `:as-of`, `:valid-at`, `:any-valid-time`), in memory and on file after a checkpoint and a reopen, through `execute` and a cursor, gives the same bag of rows with clause reordering on and off (`minigraf::__reorder`) and with the engine's negation bodies and `or` branches chosen, forced hashed and forced correlated (`minigraf::__subplans`, #546; the matrix gains #546's negation and disjunction shapes); #596's example and the unification cases (an expression whose output is its input, two expressions on one variable, integer vs float, `-0.0` vs `0.0`); and `or` / `or-join` branches whose expressions read or bind from a variable only the outer row binds, nested and in rule bodies (#620); and expressions, chains of them and a `not` after them, reading a variable only an `or` binds, in queries, with rules and in rule bodies (#625; `tests/disjunction_test.rs` repeats them without `test-oracle`). Under `test-oracle` the model-based and property tests also rerun every generated query with reordering off (the property test also with negation bodies and `or` branches forced hashed; it generates `or` / `or-join` over bound variables, #546). Unit tests cover `branch_outer_reads` (#620), the planner deferring an expression only an `or` can bind for, after the expressions placed before it and ahead of the deferred `not`s (#625), `apply_expr` (equal, different, integer vs float, NaN, `-0.0`/`0.0` with and without replacing, keyword vs ref both ways, a failing expression), `Binders::expression`, the switch (source order kept, per thread, restored on drop), and the engine planner's `Bind` modes and build side with the switch off; the engine's join matrix gains six unification shapes. - **Public diagnostics**: structured parser, query, storage, WAL, API, and internal error codes; the error-code registry is checked against [`ERROR_REFERENCE.md`](ERROR_REFERENCE.md). diff --git a/src/error.rs b/src/error.rs index 4556853f9..2c4172fd2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -155,6 +155,12 @@ pub(crate) enum ErrorCode { Prs106, Prs107, Prs108, + Prs109, + Prs110, + Prs111, + Prs112, + Prs113, + Prs114, Qry001, Qry002, Qry003, @@ -961,6 +967,42 @@ pub(crate) const REGISTRY: &[(ErrorCode, &str, &str, ErrorCategory)] = &[ "Fact must have at most 4 elements (E A V and an optional map), got {}", ErrorCategory::Parser, ), + ( + ErrorCode::Prs109, + "PRS-109", + "Unterminated string literal", + ErrorCategory::Parser, + ), + ( + ErrorCode::Prs110, + "PRS-110", + "Keyword has no name after ':'", + ErrorCategory::Parser, + ), + ( + ErrorCode::Prs111, + "PRS-111", + "Query vector is empty", + ErrorCategory::Parser, + ), + ( + ErrorCode::Prs112, + "PRS-112", + "Query section {} is empty", + ErrorCategory::Parser, + ), + ( + ErrorCode::Prs113, + "PRS-113", + "Unknown query keyword {}", + ErrorCategory::Parser, + ), + ( + ErrorCode::Prs114, + "PRS-114", + "Rule head argument must be a variable, got {}", + ErrorCategory::Parser, + ), ( ErrorCode::Qry001, "QRY-001", @@ -2208,6 +2250,12 @@ mod tests { | ErrorCode::Prs106 | ErrorCode::Prs107 | ErrorCode::Prs108 + | ErrorCode::Prs109 + | ErrorCode::Prs110 + | ErrorCode::Prs111 + | ErrorCode::Prs112 + | ErrorCode::Prs113 + | ErrorCode::Prs114 | ErrorCode::Qry001 | ErrorCode::Qry002 | ErrorCode::Qry003 @@ -2462,6 +2510,12 @@ mod tests { ErrorCode::Prs106, ErrorCode::Prs107, ErrorCode::Prs108, + ErrorCode::Prs109, + ErrorCode::Prs110, + ErrorCode::Prs111, + ErrorCode::Prs112, + ErrorCode::Prs113, + ErrorCode::Prs114, ErrorCode::Qry001, ErrorCode::Qry002, ErrorCode::Qry003, diff --git a/src/query/datalog/parser.rs b/src/query/datalog/parser.rs index 784d0c425..96614353c 100644 --- a/src/query/datalog/parser.rs +++ b/src/query/datalog/parser.rs @@ -70,9 +70,11 @@ fn tokenize(input: &str) -> Result> { '"' => { chars.next(); // consume opening quote let mut string = String::new(); + let mut closed = false; while let Some(&ch) = chars.peek() { if ch == '"' { chars.next(); // consume closing quote + closed = true; break; } else if ch == '\\' { chars.next(); @@ -96,6 +98,9 @@ fn tokenize(input: &str) -> Result> { chars.next(); } } + if !closed { + bail_coded!(ErrorCode::Prs109); + } tokens.push(Token::String(string)); } // Keywords (start with :) @@ -116,6 +121,9 @@ fn tokenize(input: &str) -> Result> { break; } } + if keyword.len() == 1 { + bail_coded!(ErrorCode::Prs110); + } tokens.push(Token::Keyword(keyword)); } // Tagged literals (start with #) @@ -533,7 +541,7 @@ fn parse_aggregate(elems: &[EdnValue]) -> Result { // SAFETY: len == 2 check above guarantees indices 0 and 1 exist #[allow(clippy::indexing_slicing)] let func_name = match &elems[0] { - EdnValue::Symbol(s) => s.clone(), + EdnValue::Symbol(s) if !s.starts_with('?') => s.clone(), other => { let repr = format!("{:?}", other); bail_coded!(ErrorCode::Prs025, repr); @@ -572,7 +580,7 @@ fn parse_window_expr(elems: &[EdnValue]) -> Result { // SAFETY: is_empty() check above guarantees index 0 exists #[allow(clippy::indexing_slicing)] let func_name = match &elems[0] { - EdnValue::Symbol(s) => s.as_str(), + EdnValue::Symbol(s) if !s.starts_with('?') => s.as_str(), _ => bail_coded!(ErrorCode::Prs029), }; @@ -702,6 +710,15 @@ fn parse_query(elements: &[EdnValue]) -> Result { let query_vector = elements[0] .as_vector() .ok_or_else(|| err_coded!(ErrorCode::Prs085))?; + if query_vector.is_empty() { + bail_coded!(ErrorCode::Prs111); + } + // Whether the item at `j` is a value, not a keyword or the end of the vector. + let value_at = |j: usize| { + query_vector + .get(j) + .is_some_and(|v| v.as_keyword().is_none()) + }; let mut find_specs: Vec = Vec::new(); let mut with_vars: Vec = Vec::new(); @@ -720,6 +737,7 @@ fn parse_query(elements: &[EdnValue]) -> Result { if let Some(keyword) = current_item.as_keyword() { match keyword { ":as-of" => { + current_clause = None; // Next element is the value i += 1; let as_of_val = query_vector @@ -750,6 +768,7 @@ fn parse_query(elements: &[EdnValue]) -> Result { continue; } ":valid-at" => { + current_clause = None; i += 1; let valid_at_val = query_vector .get(i) @@ -771,6 +790,7 @@ fn parse_query(elements: &[EdnValue]) -> Result { continue; } ":any-valid-time" => { + current_clause = None; // Shorthand for `:valid-at :any-valid-time`; disables automatic // valid-time filtering so pseudo-attribute patterns are accessible. query_valid_at = Some(ValidAt::AnyValidTime); @@ -778,6 +798,7 @@ fn parse_query(elements: &[EdnValue]) -> Result { continue; } ":max-derived-facts" => { + current_clause = None; if query_max_derived_facts.is_some() { bail_coded!(ErrorCode::Int012, ":max-derived-facts"); } @@ -803,6 +824,7 @@ fn parse_query(elements: &[EdnValue]) -> Result { continue; } ":max-results" => { + current_clause = None; if query_max_results.is_some() { bail_coded!(ErrorCode::Int012, ":max-results"); } @@ -829,6 +851,9 @@ fn parse_query(elements: &[EdnValue]) -> Result { } ":with" => { // Collect ?-prefixed symbols until the next keyword or end of vector + if !value_at(i + 1) { + bail_coded!(ErrorCode::Prs112, ":with"); + } i += 1; while i < query_vector.len() { let with_item = query_vector.get(i).ok_or_else(|| { @@ -847,11 +872,17 @@ fn parse_query(elements: &[EdnValue]) -> Result { } continue; } - _ => { + ":find" | ":where" => { + if !value_at(i + 1) { + bail_coded!(ErrorCode::Prs112, keyword); + } current_clause = Some(keyword); i += 1; continue; } + other => { + bail_coded!(ErrorCode::Prs113, other); + } } } @@ -1174,7 +1205,7 @@ fn parse_expr(list: &[EdnValue]) -> Result { // SAFETY: is_empty() check above guarantees index 0 exists #[allow(clippy::indexing_slicing)] let head = match &list[0] { - EdnValue::Symbol(s) => s.as_str(), + EdnValue::Symbol(s) if !s.starts_with('?') => s.as_str(), other => { let repr = format!("{:?}", other); bail_coded!(ErrorCode::Prs063, repr); @@ -1317,6 +1348,12 @@ fn parse_expr_clause(vec: &[EdnValue]) -> Result { /// Parse a list item (EDN List) appearing in a :where clause or rule body. /// Returns Err if the list is empty, has an unknown form, or contains nested `not`. +/// Whether `s` can name a rule: a symbol that is neither a variable nor an operator +/// (the grammar's `plain_sym`). +fn is_name(s: &str) -> bool { + s.starts_with(|c: char| c.is_alphabetic() || c == '_') || (s.len() > 1 && s.starts_with('-')) +} + fn parse_list_as_where_clause(list: &[EdnValue], allow_not: bool) -> Result { if list.is_empty() { bail_coded!(ErrorCode::Prs050); @@ -1459,7 +1496,7 @@ fn parse_list_as_where_clause(list: &[EdnValue], allow_not: bool) -> Result { + EdnValue::Symbol(predicate) if is_name(predicate) => { let args = list .get(1..) .ok_or_else(|| err_coded!(ErrorCode::Int011, "rule invocation"))? @@ -2069,9 +2106,17 @@ fn parse_rule(elements: &[EdnValue]) -> Result { // SAFETY: is_empty() check above guarantees index 0 exists #[allow(clippy::indexing_slicing)] match &head_list[0] { - EdnValue::Symbol(_) => {} + EdnValue::Symbol(name) if is_name(name) => {} _ => bail_coded!(ErrorCode::Prs105), } + // SAFETY: head_list is non-empty, so [1..] is valid + #[allow(clippy::indexing_slicing)] + if let Some(arg) = head_list[1..] + .iter() + .find(|a| !matches!(a, EdnValue::Symbol(s) if s.starts_with('?'))) + { + bail_coded!(ErrorCode::Prs114, format!("{arg:?}")); + } // Rest of body_vec are patterns, rule invocations, or (not ...) clauses let mut body_clauses: Vec = Vec::new(); diff --git a/src/query/datalog/parser/verifier.rs b/src/query/datalog/parser/verifier.rs index b39b4505f..245be607e 100644 --- a/src/query/datalog/parser/verifier.rs +++ b/src/query/datalog/parser/verifier.rs @@ -21,7 +21,6 @@ use super::{parse_datalog_command, parse_edn}; use crate::error::MinigrafError; -use crate::query::datalog::types::EdnValue; use pest::Parser as _; use std::fmt::Write as _; @@ -226,46 +225,14 @@ enum Cause { // Lexical. /// `;` starts a line comment in the parser; the grammar has no comments. Comment, - /// A string with no closing quote reads to the end of the input. - UnterminatedString, /// A non-ASCII letter or digit in a symbol or keyword (the grammar is ASCII). UnicodeLetter, - /// `:` with no keyword character after it is the keyword `:`. - BareColon, - /// `-` with no word character after it is the symbol `-`. - LoneMinus, - // Structural. - /// `(query [])`. - EmptyQuery, - /// `:find` with no find spec before the next keyword or the end. - EmptyFind, - /// `:where` with no clause before the next keyword or the end. - EmptyWhere, - /// `:with` with no variable before the next keyword or the end. - EmptyWith, - /// A keyword the query vector does not know, with no value after it, is ignored. - UnknownKeyword, - /// A value after `:as-of`, `:valid-at`, `:any-valid-time` or `:max-*` and its - /// argument continues the section before that keyword. - ResumedSection, - /// A clause list whose head is a variable or an operator is read as a rule call. - CallHead, - /// A `:find` list whose head is a variable is read as an aggregate. - AggregateHead, - /// A rule head whose name is not a plain symbol or whose argument is not a - /// variable. - RuleHead, } impl Cause { fn level(self) -> Level { match self { - Cause::Comment - | Cause::UnterminatedString - | Cause::UnicodeLetter - | Cause::BareColon - | Cause::LoneMinus => Level::Lexical, - _ => Level::Structural, + Cause::Comment | Cause::UnicodeLetter => Level::Lexical, } } } @@ -283,103 +250,30 @@ const KNOWN_DRIFT: &[Drift] = &[ issue: 650, min_k: 0, }, - Drift { - cause: Cause::UnterminatedString, - issue: 651, - min_k: 0, - }, Drift { cause: Cause::UnicodeLetter, issue: 650, min_k: 0, }, - Drift { - cause: Cause::BareColon, - issue: 651, - min_k: 0, - }, - Drift { - cause: Cause::LoneMinus, - issue: 651, - min_k: 0, - }, - Drift { - cause: Cause::EmptyQuery, - issue: 651, - min_k: 0, - }, - Drift { - cause: Cause::EmptyFind, - issue: 651, - min_k: 0, - }, - Drift { - cause: Cause::EmptyWhere, - issue: 651, - min_k: 1, - }, - Drift { - cause: Cause::EmptyWith, - issue: 651, - min_k: 1, - }, - Drift { - cause: Cause::UnknownKeyword, - issue: 651, - min_k: 1, - }, - Drift { - cause: Cause::ResumedSection, - issue: 651, - min_k: 3, - }, - Drift { - cause: Cause::CallHead, - issue: 651, - min_k: 3, - }, - Drift { - cause: Cause::AggregateHead, - issue: 651, - min_k: 4, - }, - Drift { - cause: Cause::RuleHead, - issue: 651, - min_k: 1, - }, ]; -/// Characters the parser reads as part of a symbol after its first. -fn symbol_char(c: char) -> bool { - c.is_alphanumeric() || matches!(c, '?' | '_' | '-' | '/') -} - /// The first cause, in text order, of a lexical failure. fn lexical_cause(text: &str) -> Option { let c: Vec = text.chars().collect(); let mut i = 0; while i < c.len() { - let next = c.get(i + 1).copied(); match c[i] { '"' => { i += 1; - loop { - match c.get(i) { - None => return Some(Cause::UnterminatedString), - Some('\\') => i += 2, - Some('"') => break, - Some(_) => i += 1, + while let Some(&ch) = c.get(i) { + match ch { + '\\' => i += 2, + '"' => break, + _ => i += 1, } } } ';' => return Some(Cause::Comment), - ':' if !next.is_some_and(symbol_char) => return Some(Cause::BareColon), - '-' if !next.is_some_and(symbol_char) - && !(i > 0 && (symbol_char(c[i - 1]) || matches!(c[i - 1], ':' | '#' | '$'))) => - { - return Some(Cause::LoneMinus); - } ch if !ch.is_ascii() && ch.is_alphanumeric() => return Some(Cause::UnicodeLetter), _ => {} } @@ -388,139 +282,13 @@ fn lexical_cause(text: &str) -> Option { None } -/// The first cause, in text order, of a structural failure, read from the parser's -/// EDN tree of the input (glued tokens split as the parser splits them). -fn structural_cause(text: &str) -> Option { - let EdnValue::List(cmd) = parse_edn(text).ok()? else { - return None; - }; - let arg = cmd.get(1); - match cmd.first()? { - EdnValue::Symbol(s) if s == "query" => query_cause(arg?.as_vector()?), - EdnValue::Symbol(s) if s == "rule" => rule_cause(arg?.as_vector()?), - _ => None, - } -} - -/// Walks the query vector's sections as `parse_query` does. -fn query_cause(items: &[EdnValue]) -> Option { - if items.is_empty() { - return Some(Cause::EmptyQuery); - } - let value_at = |j: usize| items.get(j).is_some_and(|v| v.as_keyword().is_none()); - // The section that values go to: `:find`, `:where`, or an unknown keyword. - let mut section: Option<&str> = None; - let mut i = 0; - while i < items.len() { - let Some(k) = items[i].as_keyword() else { - let cause = match section { - Some(":where") => clause_cause(&items[i]), - Some(":find") => find_cause(&items[i]), - _ => None, - }; - if cause.is_some() { - return cause; - } - i += 1; - continue; - }; - match k { - ":as-of" | ":valid-at" | ":max-derived-facts" | ":max-results" | ":any-valid-time" => { - i += if k == ":any-valid-time" { 1 } else { 2 }; - if section.is_some() && value_at(i) { - return Some(Cause::ResumedSection); - } - } - ":with" => { - if !value_at(i + 1) { - return Some(Cause::EmptyWith); - } - i += 1; - while value_at(i) { - i += 1; - } - } - _ if !value_at(i + 1) => { - return Some(match k { - ":find" => Cause::EmptyFind, - ":where" => Cause::EmptyWhere, - _ => Cause::UnknownKeyword, - }); - } - _ => { - section = Some(k); - i += 1; - } - } - } +/// The first cause, in text order, of a structural failure. No structural drift is +/// known (#651 removed the last); a new cause is read here from the parser's EDN tree +/// of the input ([`parse_edn`]), which splits glued tokens as the parser does. +fn structural_cause(_text: &str) -> Option { None } -/// Whether pest's `plain_sym` reads all of `s`. -fn is_plain_symbol(s: &str) -> bool { - let word = |c: char| c.is_ascii_alphanumeric() || matches!(c, '?' | '_' | '-' | '/'); - let mut chars = s.chars(); - match chars.next() { - Some(c) if c.is_ascii_alphabetic() || c == '_' => chars.all(word), - Some('-') => !chars.as_str().is_empty() && chars.all(word), - _ => false, - } -} - -/// Whether pest's `op_sym` reads all of `s`. -fn is_op_symbol(s: &str) -> bool { - matches!(s, "<=" | ">=" | "!=" | "<" | ">" | "=" | "+" | "*" | "/") -} - -/// The cause in a `:find` spec: an aggregate or window list must start with a name. -fn find_cause(spec: &EdnValue) -> Option { - match spec.as_list()?.first()? { - EdnValue::Symbol(f) if !is_plain_symbol(f) && !is_op_symbol(f) => { - Some(Cause::AggregateHead) - } - _ => None, - } -} - -/// The first cause in a `:where` clause, a negation or disjunction body, or a rule body. -fn clause_cause(clause: &EdnValue) -> Option { - let list = clause.as_list()?; - let EdnValue::Symbol(head) = list.first()? else { - return None; - }; - let rest = &list[1..]; - match head.as_str() { - "not" => rest.iter().find_map(clause_cause), - "not-join" => rest.get(1..)?.iter().find_map(clause_cause), - "or" => rest.iter().find_map(branch_cause), - "or-join" => rest.get(1..)?.iter().find_map(branch_cause), - h if !is_plain_symbol(h) => Some(Cause::CallHead), - _ => None, - } -} - -fn branch_cause(branch: &EdnValue) -> Option { - match branch.as_list().map(Vec::as_slice) { - Some([EdnValue::Symbol(and), body @ ..]) if and == "and" => { - body.iter().find_map(clause_cause) - } - _ => clause_cause(branch), - } -} - -fn rule_cause(rule: &[EdnValue]) -> Option { - let head = rule.first()?.as_list()?; - let name_ok = matches!(head.first(), Some(EdnValue::Symbol(s)) if is_plain_symbol(s)); - let args_ok = head - .iter() - .skip(1) - .all(|a| matches!(a, EdnValue::Symbol(s) if s.starts_with('?'))); - if !(name_ok && args_ok) { - return Some(Cause::RuleHead); - } - rule[1..].iter().find_map(clause_cause) -} - // ── The check ───────────────────────────────────────────────────────────── /// Which clause of the property an input breaks, if any. diff --git a/tests/error_codes_prs_test.rs b/tests/error_codes_prs_test.rs index b2ad62033..0cce7ddfc 100644 --- a/tests/error_codes_prs_test.rs +++ b/tests/error_codes_prs_test.rs @@ -286,7 +286,7 @@ fn prs_codes_cover_expressions() { let cases: &[(&str, &str)] = &[ (r#"(query [:find ?x :where [?x :a ?v] [()]])"#, "PRS-062"), ( - r#"(query [:find ?x :where [?x :a ?v] [(:+ ?v 1)]])"#, + r#"(query [:find ?x :where [?x :a ?v] [(:plus ?v 1)]])"#, "PRS-063", ), ( @@ -451,7 +451,7 @@ fn prs_codes_reach_through_prepare() { ("(query [:find ?e :where [?e :name ?n] :as-of])", "PRS-014"), (r#"(query [:find ?x :where [?x :a ?v] 42])"#, "PRS-060"), ( - r#"(query [:find ?x :where [?x :a ?v] [(:+ ?v 1)]])"#, + r#"(query [:find ?x :where [?x :a ?v] [(:plus ?v 1)]])"#, "PRS-063", ), ( @@ -755,3 +755,124 @@ fn prs_108_rejects_the_whole_transaction() { "no fact of the rejected transaction is stored" ); } + +// ── #651: inputs the parser accepted and the grammar rejects ─────────────── + +#[test] +fn prs_109_unterminated_string() { + assert_code_and_message(r#"(transact [[:a :b "abc]])"#, "PRS-109", "Unterminated"); + assert_execute_code(r#"(transact [[:a :b "abc\"#, "PRS-109"); +} + +#[test] +fn prs_110_keyword_without_a_name() { + assert_code_and_message("(transact [[:a : 1]])", "PRS-110", "':'"); + assert_execute_code("(query [:find ?e :where [?e :(]])", "PRS-110"); +} + +#[test] +fn prs_111_empty_query_vector() { + assert_code_and_message("(query [])", "PRS-111", "empty"); +} + +#[test] +fn prs_112_empty_query_section() { + assert_code_and_message("(query [:find :where [?e :a ?v]])", "PRS-112", ":find"); + assert_code_and_message("(query [:find ?e :where])", "PRS-112", ":where"); + assert_code_and_message("(query [:where])", "PRS-112", ":where"); + assert_code_and_message( + "(query [:find (count ?e) :with :where [?e :a ?v]])", + "PRS-112", + ":with", + ); + assert_execute_code("(query [:find ?e :where [?e :a ?v] :with])", "PRS-112"); +} + +#[test] +fn prs_113_unknown_query_keyword() { + assert_code_and_message( + "(query [:find ?e :foo :where [?e :a ?v]])", + "PRS-113", + ":foo", + ); + // With a value after it the keyword used to open a section of its own (PRS-061). + assert_code_and_message( + "(query [:find ?e :in ?v :where [?e :a ?v]])", + "PRS-113", + ":in", + ); +} + +#[test] +fn prs_114_rule_head_argument_not_a_variable() { + assert_code_and_message("(rule [(r ?a :c) [?a :edge :c]])", "PRS-114", ":c"); + assert_execute_code("(rule [(r ?a 0) [?a :edge ?b]])", "PRS-114"); +} + +#[test] +fn prs_105_rule_name_is_a_variable_or_an_operator() { + assert_execute_code("(rule [(?r ?a) [?a :edge ?b]])", "PRS-105"); + assert_execute_code("(rule [(< ?a) [?a :edge ?b]])", "PRS-105"); +} + +#[test] +fn prs_061_value_after_a_query_option() { + // A value after an option and its argument used to join the section before it. + for q in [ + "(query [:find ?a :any-valid-time ?b :where [?a :edge ?b]])", + "(query [:find ?a :as-of 1 ?b :where [?a :edge ?b]])", + r#"(query [:find ?a :valid-at "2024-01-01T00:00:00Z" ?b :where [?a :edge ?b]])"#, + "(query [:find ?a :max-results 5 ?b :where [?a :edge ?b]])", + "(query [:find ?a :where [?a :edge ?b] :max-derived-facts 5 [?b :edge ?c]])", + ] { + assert_execute_code(q, "PRS-061"); + } +} + +#[test] +fn prs_097_call_head_is_a_variable_or_an_operator() { + // `(< ?a ?b)` as a clause was a call of a rule named `<` and returned no rows. + for q in [ + "(query [:find ?a :where [?a :edge ?b] (< ?a ?b)])", + "(query [:find ?a :where [?a :edge ?b] (?r ?a)])", + "(query [:find ?a :where [?a :edge ?b] (not (?r ?a))])", + "(query [:find ?a :where [?a :edge ?b] (or (?r ?a) [?a :x 1])])", + ] { + assert_execute_code(q, "PRS-097"); + } + assert_execute_code("(rule [(r ?a) [?a :edge ?b] (= ?a ?b)])", "PRS-097"); +} + +#[test] +fn prs_025_029_063_function_named_by_a_variable() { + assert_execute_code("(query [:find (?f ?a) :where [?a :edge ?b]])", "PRS-025"); + assert_execute_code( + "(query [:find ?a (?f ?b :over (:order-by ?b)) :where [?a :edge ?b]])", + "PRS-029", + ); + assert_execute_code( + "(query [:find ?a :where [?a :edge ?b] [(?f ?a) ?z]])", + "PRS-063", + ); +} + +#[test] +fn accepted_forms_next_to_the_tightened_ones() { + let db = db(); + db.execute("(transact [[:a :edge :b] [:a :n 5]])").unwrap(); + for q in [ + // An option between sections, then a keyword: the next section starts. + "(query [:find ?a :any-valid-time :where [?a :edge ?b]])", + "(query [:find ?a :as-of 1 :where [?a :edge ?b]])", + // `-` alone is the subtraction operator. + "(query [:find ?d :where [?a :n ?n] [(- ?n 1) ?d]])", + // A rule call named by a symbol, and an aggregate named by an operator-free symbol. + "(query [:find (count ?a) :where [?a :edge ?b]])", + ] { + db.execute(q).expect("query must parse and run"); + } + db.execute("(rule [(linked ?a ?b) [?a :edge ?b]])").unwrap(); + db.execute("(rule [(none) [?a :edge ?b]])").unwrap(); + db.execute("(query [:find ?a :where (linked ?a ?b)])") + .unwrap(); +} diff --git a/tests/grammar/grammar.pest b/tests/grammar/grammar.pest index 5fae33350..6dca2e84e 100644 --- a/tests/grammar/grammar.pest +++ b/tests/grammar/grammar.pest @@ -60,8 +60,9 @@ keyword = @{ ":" ~ word_cont+ } // Logic variable: starts with "?" variable = @{ "?" ~ word_cont* } -// Operator symbols used in expressions -op_sym = @{ "<=" | ">=" | "!=" | "<" | ">" | "=" | "+" | "*" | "/" } +// Operator symbols used in expressions. A "-" followed by a word character starts a +// symbol, and one followed by a digit a number. +op_sym = @{ "<=" | ">=" | "!=" | "<" | ">" | "=" | "+" | "*" | "/" | "-" ~ !word_cont } // Plain symbol: letter or underscore start; or "-" followed by at least one word char. // Negative lookaheads prevent "true", "false", "nil" from matching as plain symbols. diff --git a/tests/grammar/invalid/syntax/aggregate_variable_head.edn b/tests/grammar/invalid/syntax/aggregate_variable_head.edn new file mode 100644 index 000000000..a498fff97 --- /dev/null +++ b/tests/grammar/invalid/syntax/aggregate_variable_head.edn @@ -0,0 +1 @@ +(query [:find (?f ?b) :where [?a :edge ?b]]) diff --git a/tests/grammar/invalid/syntax/clause_operator_head.edn b/tests/grammar/invalid/syntax/clause_operator_head.edn new file mode 100644 index 000000000..b481e3626 --- /dev/null +++ b/tests/grammar/invalid/syntax/clause_operator_head.edn @@ -0,0 +1 @@ +(query [:find ?a :where [?a :age ?x] [?b :age ?y] (< ?x ?y)]) diff --git a/tests/grammar/invalid/syntax/clause_variable_head.edn b/tests/grammar/invalid/syntax/clause_variable_head.edn new file mode 100644 index 000000000..21bd830b1 --- /dev/null +++ b/tests/grammar/invalid/syntax/clause_variable_head.edn @@ -0,0 +1 @@ +(query [:find ?a :where [?a :edge ?b] (?r ?a)]) diff --git a/tests/grammar/invalid/syntax/expr_variable_head.edn b/tests/grammar/invalid/syntax/expr_variable_head.edn new file mode 100644 index 000000000..7d98e7ff5 --- /dev/null +++ b/tests/grammar/invalid/syntax/expr_variable_head.edn @@ -0,0 +1 @@ +(query [:find ?a :where [?a :n ?n] [(?f ?n) ?z]]) diff --git a/tests/grammar/invalid/syntax/keyword_bare_colon.edn b/tests/grammar/invalid/syntax/keyword_bare_colon.edn new file mode 100644 index 000000000..ee0d9948d --- /dev/null +++ b/tests/grammar/invalid/syntax/keyword_bare_colon.edn @@ -0,0 +1 @@ +(transact [[:alice : "Alice"]]) diff --git a/tests/grammar/invalid/syntax/query_empty.edn b/tests/grammar/invalid/syntax/query_empty.edn new file mode 100644 index 000000000..80a3e9792 --- /dev/null +++ b/tests/grammar/invalid/syntax/query_empty.edn @@ -0,0 +1 @@ +(query []) diff --git a/tests/grammar/invalid/syntax/query_find_empty.edn b/tests/grammar/invalid/syntax/query_find_empty.edn new file mode 100644 index 000000000..1bf2bb737 --- /dev/null +++ b/tests/grammar/invalid/syntax/query_find_empty.edn @@ -0,0 +1 @@ +(query [:find :where [?e :person/name ?n]]) diff --git a/tests/grammar/invalid/syntax/query_unknown_keyword.edn b/tests/grammar/invalid/syntax/query_unknown_keyword.edn new file mode 100644 index 000000000..6c96caa2c --- /dev/null +++ b/tests/grammar/invalid/syntax/query_unknown_keyword.edn @@ -0,0 +1 @@ +(query [:find ?e :limit :where [?e :person/name ?n]]) diff --git a/tests/grammar/invalid/syntax/query_value_after_option.edn b/tests/grammar/invalid/syntax/query_value_after_option.edn new file mode 100644 index 000000000..44d806fac --- /dev/null +++ b/tests/grammar/invalid/syntax/query_value_after_option.edn @@ -0,0 +1 @@ +(query [:find ?e :any-valid-time ?n :where [?e :person/name ?n]]) diff --git a/tests/grammar/invalid/syntax/query_where_empty.edn b/tests/grammar/invalid/syntax/query_where_empty.edn new file mode 100644 index 000000000..512120432 --- /dev/null +++ b/tests/grammar/invalid/syntax/query_where_empty.edn @@ -0,0 +1 @@ +(query [:find ?e :where]) diff --git a/tests/grammar/invalid/syntax/query_with_empty.edn b/tests/grammar/invalid/syntax/query_with_empty.edn new file mode 100644 index 000000000..0e62e0de0 --- /dev/null +++ b/tests/grammar/invalid/syntax/query_with_empty.edn @@ -0,0 +1 @@ +(query [:find (count ?e) :with :where [?e :person/name ?n]]) diff --git a/tests/grammar/invalid/syntax/rule_head_constant_arg.edn b/tests/grammar/invalid/syntax/rule_head_constant_arg.edn new file mode 100644 index 000000000..3e8b64dd5 --- /dev/null +++ b/tests/grammar/invalid/syntax/rule_head_constant_arg.edn @@ -0,0 +1 @@ +(rule [(reachable-5 ?a ?b 0) [?a :edge ?b]]) diff --git a/tests/grammar/invalid/syntax/rule_head_variable_name.edn b/tests/grammar/invalid/syntax/rule_head_variable_name.edn new file mode 100644 index 000000000..43b7aef35 --- /dev/null +++ b/tests/grammar/invalid/syntax/rule_head_variable_name.edn @@ -0,0 +1 @@ +(rule [(?r ?a) [?a :edge ?b]]) diff --git a/tests/grammar/valid/expr_subtract.edn b/tests/grammar/valid/expr_subtract.edn new file mode 100644 index 000000000..6b6c4ceae --- /dev/null +++ b/tests/grammar/valid/expr_subtract.edn @@ -0,0 +1 @@ +(query [:find ?d :where [?a :n ?n] [(- ?n 1) ?d]])