From 5f879fd9a41dd0e8b12b38ff2e860eeabe803200 Mon Sep 17 00:00:00 2001 From: Bart Broere Date: Tue, 3 Nov 2026 07:54:11 +0100 Subject: [PATCH] Create draft plans for addressing each limitation --- README.md | 25 ++- plans/01-return-star-explain-profile.md | 138 ++++++++++++ plans/02-write-statistics.md | 107 ++++++++++ plans/03-age-unsupported-features.md | 267 ++++++++++++++++++++++++ plans/04-users-transactions-sysinfo.md | 154 ++++++++++++++ plans/05-tls.md | 105 ++++++++++ plans/06-streaming-results.md | 118 +++++++++++ plans/README.md | 50 +++++ 8 files changed, 954 insertions(+), 10 deletions(-) create mode 100644 plans/01-return-star-explain-profile.md create mode 100644 plans/02-write-statistics.md create mode 100644 plans/03-age-unsupported-features.md create mode 100644 plans/04-users-transactions-sysinfo.md create mode 100644 plans/05-tls.md create mode 100644 plans/06-streaming-results.md create mode 100644 plans/README.md diff --git a/README.md b/README.md index 6e395e6..9fcab91 100644 --- a/README.md +++ b/README.md @@ -384,17 +384,22 @@ the full image, default `9.18`). ## Limitations -This is a first version. Things that do not work (yet): +This is a first version. Things that do not work (yet), each with a plan for +addressing it in [`plans/`](plans/README.md): -- `RETURN *`, `EXPLAIN` and `PROFILE`. -- Write statistics: write queries report "(no changes, no records)" even when they - changed the graph. -- Anything AGE does not support, for example multiple labels per node, pattern - comprehensions, temporal and spatial types, indexes and constraints through Cypher. -- User and role management, `SHOW TRANSACTIONS`, `:sysinfo`. -- TLS (`bolt+s://`, HTTPS). -- Each query's full result is fetched from PostgreSQL before it is streamed to the - client, so very large results use memory in boltage. +1. `RETURN *`, `EXPLAIN` and `PROFILE` + ([plan](plans/01-return-star-explain-profile.md)). +2. Write statistics: write queries report "(no changes, no records)" even when they + changed the graph ([plan](plans/02-write-statistics.md)). +3. Anything AGE does not support, for example multiple labels per node, pattern + comprehensions, temporal and spatial types, indexes and constraints through Cypher + ([plan](plans/03-age-unsupported-features.md)). +4. User and role management, `SHOW TRANSACTIONS`, `:sysinfo` + ([plan](plans/04-users-transactions-sysinfo.md)). +5. TLS (`bolt+s://`, HTTPS) ([plan](plans/05-tls.md)). +6. Each query's full result is fetched from PostgreSQL before it is streamed to the + client, so very large results use memory in boltage + ([plan](plans/06-streaming-results.md)). ## Development diff --git a/plans/01-return-star-explain-profile.md b/plans/01-return-star-explain-profile.md new file mode 100644 index 0000000..621b9fc --- /dev/null +++ b/plans/01-return-star-explain-profile.md @@ -0,0 +1,138 @@ +# `RETURN *`, `EXPLAIN` and `PROFILE` + +## Problem + +- **`RETURN *`**: AGE's `cypher()` needs a column definition list + (`AS (c0 agtype, c1 agtype, ...)`) and Bolt needs column names in the `RUN` + response. `return_columns()` (`boltage/cypher.py:89`) derives both from the text of + the final `RETURN`, and raises `FeatureNotSupported` for `*` + (`boltage/cypher.py:115`), because the names of the variables in scope are not in + the `RETURN` clause. +- **`EXPLAIN` / `PROFILE`**: rejected in `Session._run_cypher` + (`boltage/backend.py:215`). neo4j-browser shows a plan view when the `PULL` + summary carries a `plan` (EXPLAIN) or `profile` (PROFILE) entry. + +## Part 1: `RETURN *` + +### Approach + +Rewrite `RETURN *` into an explicit list of the variables in scope, before +`return_columns()` runs. This needs the scope analysis from the +[shared groundwork](README.md#shared-groundwork). + +1. **Clause splitter.** Using `scan()`'s masked text and depth, split the query at + top-level clause keywords (`MATCH`, `OPTIONAL MATCH`, `WHERE`, `WITH`, `UNWIND`, + `CALL`, `YIELD`, `CREATE`, `MERGE`, `SET`, `REMOVE`, `DELETE`, `DETACH DELETE`, + `RETURN`, `ORDER BY`, `SKIP`, `LIMIT`, `UNION`). Only the part after the last + top-level `UNION` matters for the final `RETURN`; for `UNION`, every branch must + end up with the same columns, so expand each branch's `RETURN *` separately. +2. **Scope tracker.** Walk the clauses of the relevant part: + - `MATCH`, `OPTIONAL MATCH`, `CREATE`, `MERGE`: add named node variables + (`(name` / `(name:`), relationship variables (`[name` / `[name:`) and path + variables (`name =` before a pattern). Skip bracket contents of property maps + and `WHERE` inside patterns. + - `UNWIND expr AS name`: add `name`. + - `CALL ... YIELD a, b AS c`: add the yielded names. + - `WITH`: reset the scope to the projected names (aliases, or the bare variable + for `WITH n`). `WITH *` keeps the scope, plus any extra aliases + (`WITH *, x AS y`). + - Anonymous entities (`()`, `[]`, `-->`) add nothing. +3. **Expansion.** Replace `*` with the names in scope, in Neo4j's order (check + against a real Neo4j 5: it is believed to be alphabetical), quoting names that + need backticks. `RETURN *, expr AS x` keeps the extra items. `RETURN DISTINCT *` + keeps `DISTINCT`. +4. **No variables in scope** (`MATCH () RETURN *`): Neo4j raises + `RETURN * is not allowed when there are no variables in scope`; raise the same + `SyntaxError`. + +AGE itself does not need to accept `RETURN *`; it only sees the explicit list. + +### Alternative considered + +Let PostgreSQL tell us the column count: AGE turns `cypher()` into a sub-query at +parse analysis, and `EXPLAIN VERBOSE` might show the output columns. It would give +the count but not the names, and costs an extra round trip per query, so static +analysis is the better fit. + +### Tests + +- Unit tests in `tests/test_cypher.py`: plain `MATCH (a)-[r]->(b) RETURN *`, paths, + `WITH` resets scope, `WITH *`, `UNWIND`, `OPTIONAL MATCH`, `UNION` branches, + strings and comments containing `*` or keywords, no variables in scope. +- Smoke test: `MATCH (a:Person)-[r]->(b) RETURN *` returns columns `a`, `b`, `r` with + a Node, Node and Relationship. + +## Part 2: `EXPLAIN` + +### Approach + +1. Strip the leading `EXPLAIN` keyword (keeping the rest of the query, including a + `RETURN *` expansion), build the usual `cypher()` SQL, and run + `EXPLAIN (FORMAT JSON, VERBOSE) `. +2. Convert the PostgreSQL plan tree to Neo4j's plan structure, recursively: + + | Neo4j plan field | From the PostgreSQL plan node | + | --- | --- | + | `operatorType` | `Node Type`, plus AGE's custom scan names (`Custom Scan (Cypher Create)` and friends) | + | `identifiers` | `Output` columns where available, else `[]` | + | `args` | `EstimatedRows` from `Plan Rows`, `Details` from `Relation Name` / `Index Name` / `Filter` / `Index Cond`, plus `planner`, `runtime` = `"PostgreSQL"`, `version` | + | `children` | `Plans` | + + The root also gets `Neo4j`-style top-level args (`planner`, `runtime`, + `runtime-impl`) so neo4j-browser renders its header. + Label tables show up as `Relation Name`; map them back to labels through + `ag_catalog.ag_label` so the plan shows `Person` instead of `neo4j."Person"`. +3. Return a result with the columns of the query but no records, and put the plan in + the `PULL` summary. This needs a new `Result.summary` dict that `BoltConnection.pull` + (`boltage/bolt.py:327`) merges into its final `SUCCESS` metadata. +4. `query_type` is `r` for a read plan and `rw`/`w` when the plan contains AGE's + write nodes, which is more reliable than the regex at `boltage/backend.py:232`. + +`EXPLAIN` must not execute the query; PostgreSQL's plain `EXPLAIN` does not. + +## Part 3: `PROFILE` + +Neo4j's `PROFILE` runs the query, returns its records **and** a `profile` summary +with actual row counts, db hits and times per operator. + +### Approach + +1. Run `EXPLAIN (ANALYZE, BUFFERS, FORMAT JSON, VERBOSE) ` inside a savepoint and + roll back to the savepoint afterwards, so writes do not happen twice. +2. Then run the query normally to get its records (and, with 02, its write + statistics). +3. Convert the plan as for `EXPLAIN`, adding: + - `rows` = `Actual Rows` × `Actual Loops` + - `dbHits` = `Shared Hit Blocks` + `Shared Read Blocks` (the closest PostgreSQL + analogue; document that it counts pages, not records) + - `time` from `Actual Total Time` (Neo4j uses nanoseconds) + - `pageCacheHits` / `pageCacheMisses` from the shared hit and read blocks. +4. Put the plan in the summary under `profile`. + +This runs a read query twice. The alternative, profiling only and returning no +records, is cheaper but diverges from Neo4j and breaks clients that read the +records. Start with running twice and document it; queries that use `rand()` or +`timestamp()` can differ between the two runs, which is acceptable for a profile. + +Auto-commit `PROFILE` needs a transaction for the savepoint, which the +[shared groundwork](README.md#shared-groundwork) provides; inside an explicit +transaction a savepoint is enough. + +### Tests + +- Smoke test: `EXPLAIN MATCH (n:Person) RETURN n` returns no records and a summary + `plan` with an `operatorType`; `PROFILE` returns the same records as the plain query + and a `profile` with `rows`; `PROFILE CREATE (:Temp)` creates exactly one node. +- Manual check in neo4j-browser that both plan views render. + +## Packstream / Bolt notes + +The plan is a nested map of strings, numbers and lists, which the existing packer +handles. Drivers expose it as `summary.plan` / `summary.profile`; the Python driver +test can assert on that. + +## README changes + +Remove `RETURN *`, `EXPLAIN` and `PROFILE` from Limitations. Add a sentence under "How +it works" that `EXPLAIN` and `PROFILE` show PostgreSQL's plan, that `dbHits` counts +pages, and that `PROFILE` runs a query twice. diff --git a/plans/02-write-statistics.md b/plans/02-write-statistics.md new file mode 100644 index 0000000..f2467fc --- /dev/null +++ b/plans/02-write-statistics.md @@ -0,0 +1,107 @@ +# Write statistics + +## Problem + +Neo4j reports what a write query changed in the `stats` entry of the final `PULL` +summary (`nodes-created`, `relationships-deleted`, `properties-set`, ...). +neo4j-browser shows these as "Created 2 nodes, set 4 properties". boltage sends no +`stats` (`BoltConnection.pull`, `boltage/bolt.py:327`), so the browser says +"(no changes, no records)". Drivers' `summary.counters.contains_updates` is always +false, which also breaks code that checks it. + +AGE's `cypher()` does not report counts, and AGE writes label tables without firing +triggers, so triggers cannot count either. + +## Approach: PostgreSQL's transaction-local table statistics + +`pg_stat_xact_user_tables` has, per table, the number of rows inserted, updated +and deleted **by the current transaction so far** (`n_tup_ins`, `n_tup_upd`, +`n_tup_del`). Every AGE label is a table in the graph's schema, and +`ag_catalog.ag_label` says whether it holds vertices or edges. Snapshot those +counters for the graph's schema before and after the query and take the difference. + +1. **Spike first.** Confirm on the supported PostgreSQL versions (the image uses 18): + - the counters include the changes of a statement that has just finished, within + the same transaction; + - a non-superuser can read them for tables they wrote to; + - how AGE's `SET` and `REMOVE` show up (an `UPDATE` of the row, so `n_tup_upd`), + and `DETACH DELETE` (deletes on edge tables and the vertex table); + - `MERGE` that matches counts nothing, `MERGE` that creates counts inserts. +2. **Transaction per auto-commit query.** The counters reset at commit, and the + connection runs in autocommit mode (`boltage/backend.py:126`). Wrap each + auto-commit Cypher query in `BEGIN` / `COMMIT` in `Session.run` + (see [shared groundwork](README.md#shared-groundwork)). Inside an explicit Bolt + transaction nothing changes: the counters accumulate, and the diff per query + still works. +3. **Snapshot query.** One query, run before and after: + + ```sql + SELECT l.kind, s.relname <> ALL(%(defaults)s) AS labelled, + sum(s.n_tup_ins), sum(s.n_tup_upd), sum(s.n_tup_del) + FROM pg_stat_xact_user_tables s + JOIN ag_catalog.ag_label l ON l.relation = s.relid + JOIN ag_catalog.ag_graph g ON g.graphid = l.graph + WHERE g.name = %(graph)s + GROUP BY 1, 2 + ``` + + To avoid two extra round trips per query, send the "before" snapshot in the same + pipeline as `BEGIN` (psycopg's `conn.pipeline()`), and skip both snapshots for + queries that cannot write (no `CREATE`/`MERGE`/`SET`/`DELETE`/`REMOVE` keyword). + The existing keyword check at `boltage/backend.py:232` runs on the raw text, so a + string literal like `'SET'` counts as a write; run it on `scan().masked` instead. + A false positive only costs the snapshots, a false negative loses the counts. +4. **Mapping to Neo4j counters:** + + | Neo4j counter | Source | + | --- | --- | + | `nodes-created` / `nodes-deleted` | inserts / deletes on vertex tables | + | `relationships-created` / `relationships-deleted` | inserts / deletes on edge tables | + | `labels-added` | inserts on vertex tables other than `_ag_label_vertex` (AGE has one label per node) | + | `labels-removed` | deletes on vertex tables other than `_ag_label_vertex` | + | `properties-set` | see below | + | `contains-updates` | true when any of the above is non-zero | + + Only send non-zero counters, like Neo4j, plus `contains-updates`. +5. **`properties-set`** has no exact source: an update rewrites a row's whole + `properties` map. Start with an approximation and document it: + - for inserts: not counted (Neo4j counts the properties in `CREATE`; leaving this + out under-reports, but never claims changes that did not happen); + - for updates: the number of updated rows. + A more exact count would need comparing old and new `properties` maps, which AGE + does not expose. Revisit if users ask for it. +6. **System commands.** `CREATE DATABASE` / `DROP DATABASE` (and later user + management, plan 04) report `system-updates: 1` and `contains-system-updates: true`. +7. **Plumbing.** Add a `stats` dict to `Result`, filled in `Session._run_cypher` and + sent by `BoltConnection.pull` in the final `SUCCESS`. Use the counts to set + `query_type` (`r`, `w` or `rw`, depending on whether there are columns and writes) + instead of the regex. + +With streaming (plan 06) the "after" snapshot must be taken once the cursor is +exhausted or drained, just before the final summary. + +## Alternatives considered + +- **Counting rows before and after** with `count(*)` per label table: slow on large + graphs, wrong under concurrent writers, and blind to updates. +- **Rewriting the query to count**, for example appending a `RETURN count(...)`: + changes the query's result and does not work for queries that already return. +- **Statement-level triggers with transition tables:** AGE does not fire triggers on + its label tables. + +## Tests + +Smoke test, using the driver's `summary.counters`: + +- `CREATE (a:Person)-[:KNOWS]->(b:Person)`: 2 nodes, 1 relationship, 2 labels, + `contains_updates`. +- `MATCH (n:Person) SET n.x = 1`: properties set equals the number of persons. +- `MATCH (n:Person) DETACH DELETE n`: nodes and relationships deleted. +- `MATCH (n) RETURN n`: no updates. +- Same queries in an explicit transaction, two queries in one transaction report + their own counts. +- `CREATE DATABASE`: system updates. + +## README changes + +Remove the write statistics bullet; add a note that `properties-set` is approximate. diff --git a/plans/03-age-unsupported-features.md b/plans/03-age-unsupported-features.md new file mode 100644 index 0000000..7456201 --- /dev/null +++ b/plans/03-age-unsupported-features.md @@ -0,0 +1,267 @@ +# Features AGE does not support + +## Problem + +The README lists "anything AGE does not support", with examples: multiple labels per +node, pattern comprehensions, temporal and spatial types, indexes and constraints +through Cypher. These are very different in size, so this plan treats each as a +separate part that can be done (or dropped) on its own. In order of value per effort: + +- [D. Indexes and constraints](#d-indexes-and-constraints-through-cypher): mostly + translation to SQL DDL; high value. +- [B. Pattern comprehensions](#b-pattern-comprehensions): a generalisation of an + existing rewrite. +- [C. Temporal types](#c-temporal-types): useful, but needs a convention for + storing values. +- [A. Multiple labels](#a-multiple-labels-per-node): invasive; needs a real parser. +- [E. Spatial types](#e-spatial-types): least demand; do only on request. + +For all parts, boltage should keep failing clearly (`FeatureNotSupported` with a +message that names the feature) for whatever is still unsupported, rather than +passing AGE's sometimes cryptic errors through. + +Where an approach depends on AGE behaviour that is not yet verified, the plan says +"spike": check it against the AGE version in the image (`apache/age:release_PG18_1.8.0`) +before building on it. Also check AGE's issue tracker for each part: if AGE adds the +feature itself, boltage only has to pass it through. + +--- + +## D. Indexes and constraints through Cypher + +Today `SHOW INDEXES` and `SHOW CONSTRAINTS` return empty results +(`boltage/backend.py` `_run_system`), and `CREATE INDEX` reaches AGE, which rejects it. +Label tables are ordinary PostgreSQL tables, so indexes and constraints can be real +PostgreSQL indexes and constraints on them. + +### Commands to support + +| Cypher | SQL | +| --- | --- | +| `CREATE INDEX [name] [IF NOT EXISTS] FOR (n:L) ON (n.p [, n.q])` | `CREATE INDEX name ON g."L" ( [, ])` | +| `CREATE INDEX ... FOR ()-[r:T]-() ON (r.p)` | same, on the edge table `g."T"` | +| `CREATE CONSTRAINT [name] FOR (n:L) REQUIRE n.p IS UNIQUE` | `CREATE UNIQUE INDEX name ON g."L" ()` | +| `... REQUIRE (n.p, n.q) IS NODE KEY` | unique index on both, plus a `CHECK` that both exist | +| `... REQUIRE n.p IS NOT NULL` | `ALTER TABLE g."L" ADD CONSTRAINT name CHECK (

)` | +| `DROP INDEX name [IF EXISTS]`, `DROP CONSTRAINT name [IF EXISTS]` | `DROP INDEX` / `ALTER TABLE ... DROP CONSTRAINT` | +| `SHOW INDEXES`, `SHOW CONSTRAINTS` | read `pg_index` / `pg_constraint` for the graph's schema | + +Full-text, vector, point and `TEXT` indexes, and property type constraints, stay +unsupported with a clear error. `RANGE` indexes map to the default b-tree. + +### Steps + +1. **Spike: the index expression.** An index only helps if it matches the expression + AGE generates for `n.p` in a `WHERE`. Use `EXPLAIN` on + `SELECT * FROM cypher('g', $$ MATCH (n:L) WHERE n.p = 1 RETURN n $$) AS (n agtype)` + to find it (believed to be + `agtype_access_operator(VARIADIC ARRAY[properties, '"p"'::agtype])`), and check that + an index on it is used for `=`, range comparisons, and for `MATCH (n:L {p: 1})` + (which may need a GIN index on `properties` instead). Record the findings in the + code, since they are AGE implementation details. +2. **Label creation.** `CREATE INDEX` on a label that does not exist yet creates it + first with `create_vlabel` / `create_elabel`, as Neo4j allows indexes on labels with + no nodes. +3. **Names.** Use the Cypher name as the PostgreSQL index/constraint name; generate + one like Neo4j does (`index_`, `constraint_`) when none is given. Store + the Cypher-level kind (`RANGE` index, `UNIQUENESS` constraint, ...) in a + `COMMENT ON INDEX` so `SHOW` can report it without guessing. +4. **`SHOW INDEXES` / `SHOW CONSTRAINTS`.** Query the catalog for indexes and + constraints on the graph's label tables, map table → label through `ag_label`, + and parse the property name back out of the indexed expression (or the comment). + Indexes AGE creates itself (on `id`, `start_id`, `end_id`) are shown as Neo4j's + built-in `LOOKUP` indexes or hidden; decide after looking at what neo4j-browser + does with them. Also fill `indexes` / `constraints` in `db.schema.visualization`. +5. **Errors.** A unique violation from AGE's `CREATE`/`MERGE` already maps to + `ConstraintValidationFailed` (sqlstate 23, `failure_from_pg`); make the message + name the constraint and property like Neo4j does. Creating a unique constraint on + data that has duplicates fails in PostgreSQL; map it to + `Neo.ClientError.Schema.ConstraintCreationFailed`. +6. **Privileges.** Only the table owner can create indexes; map the permission + error to `Neo.ClientError.Security.Forbidden`. +7. Parsing: these commands have a fixed shape, so regexes in `_run_system` (like the + existing `CREATE DATABASE` handling) are enough. + +### Tests + +Smoke test: create an index and see it used (via `EXPLAIN` once plan 01 lands, or +via `SHOW INDEXES`); a unique constraint rejects a duplicate `CREATE` with +`ConstraintError`; `IF NOT EXISTS` / `IF EXISTS`; drop both. + +--- + +## B. Pattern comprehensions + +`[(a)-[r:T]->(b) WHERE b.x > 1 | b.name]` is a list built from a sub-pattern. +boltage rewrites one specific neo4j-browser query (`rewrite_for_age`, +`boltage/cypher.py:208`); everything else fails in AGE. + +### Approach + +Rewrite a pattern comprehension in clause `C` into an `OPTIONAL MATCH` plus +`collect()` before `C`, carrying the variables in scope: + +``` +MATCH (a) RETURN a.name, [(a)-->(b) | b.name] AS friends +``` + +becomes + +``` +MATCH (a) +OPTIONAL MATCH (a)-->(boltage_b1) WITH a, collect(boltage_b1.name) AS boltage_pc1 +RETURN a.name, boltage_pc1 AS friends +``` + +1. Find pattern comprehensions: a `[` whose contents start with a pattern + (`(`…`)` followed by a relationship) and contain a top-level `|`. List + comprehensions (`[x IN list | ...]`) must not match. +2. Rename the comprehension's own new variables (here `b`) so they cannot clash + with outer variables. +3. Use the scope tracker from the [shared groundwork](README.md#shared-groundwork) to + list the variables to carry in the `WITH`. +4. Each comprehension gets its own `OPTIONAL MATCH ... WITH ... collect()` step, one + after the other, so several comprehensions in one clause do not multiply rows. +5. A `WHERE` inside the comprehension becomes the `WHERE` of the `OPTIONAL MATCH`. +6. Replace the existing neo4j-browser-specific regex with the general rewrite, and + keep its smoke test. + +### Semantic differences to handle or document + +- `collect()` drops nulls, a comprehension keeps them: `[(a)-->(b) | b.missing]` + gives `[null, null]` in Neo4j. Collect a wrapper (`collect([b.missing])`) and unwrap + it with a list comprehension, if AGE supports list comprehensions (spike); + otherwise document the difference. +- `OPTIONAL MATCH` with no match yields one row with nulls, which `collect()` turns + into `[]`: correct. +- Comprehensions inside aggregations, `WHERE` clauses, `ORDER BY`, or nested in + another comprehension: support the `WITH`/`RETURN`/`WHERE` projection positions + first; reject the rest with `FeatureNotSupported`. +- A `WITH` that carries the variables also ends the scope for aggregation in the + original clause; when `C` itself aggregates (`RETURN a, count(*), [(a)-->(b)|b]`), + the rewrite must keep grouping keys intact. Reject that case at first. + +Pattern **expressions** used as predicates (`WHERE (a)-->(b)`) and `EXISTS { }` / +`COUNT { }` subqueries are related; check which ones AGE accepts and extend the +same rewrite where it does not. + +### Tests + +Unit tests for detection (pattern vs list comprehension, strings) and the rewrite +text; smoke tests comparing results with hand-written `OPTIONAL MATCH` equivalents. + +--- + +## C. Temporal types + +AGE has no date, time or duration values. Neo4j clients send and receive them as +Bolt structures (`Date`, `Time`, `LocalTime`, `DateTime`, `LocalDateTime`, +`Duration`), which boltage's packstream does not handle: a temporal parameter +reaches `literal()` (`boltage/cypher.py`) as an unknown `Structure` and fails. + +The README already recommends storing points in time as Unix timestamps or ISO 8601 +strings with a UTC offset. The plan builds on that convention. + +### Phase 1: accept temporal parameters + +1. Unpack Bolt temporal structures in `packstream.py` into Python + `date`/`time`/`datetime` values and a small duration type: `D` (Date), `T` (Time), + `t` (LocalTime), `d` (LocalDateTime), `I` / `i` (DateTime with offset / zone id, + Bolt 5 and 4.4 with the `utc` patch), `F` / `f` (the legacy DateTime variants of + Bolt 4.4 without the patch) and `E` (Duration). +2. `literal()` turns them into ISO 8601 strings (with offset for zoned values, the + way the README recommends). `Duration` becomes an ISO 8601 duration string + (`P1DT2H`). +3. Document that the values come back as strings. + +This alone makes drivers able to write temporal values, and the README's views and +generated columns work on them. + +### Phase 2: temporal functions in Cypher + +1. **Spike:** AGE can call PostgreSQL functions from Cypher when they are + schema-qualified and take/return `agtype`. Check this works, and its performance. +2. Ship a small SQL extension script (a `boltage` schema with `boltage.datetime(agtype)`, + `boltage.date(agtype)`, `boltage.duration(agtype)`, `boltage.datetime_truncate`, + ...) that returns normalised ISO 8601 strings, implemented with PostgreSQL's + `timestamptz` arithmetic. Install it from the container's init script; for other + setups document `psql -f`. +3. Rewrite calls to `date()`, `datetime()`, `localdatetime()`, `time()`, + `duration()` and `duration.between()` in the Cypher text to the qualified + functions (outside strings, using `scan()`). Leave `timestamp()`, which AGE has. +4. Property access such as `d.year` on a temporal value cannot be rewritten + reliably, because boltage does not know which values are temporal. Support the + function forms (`date.truncate`, ...) and document that accessors are not + available. + +ISO strings normalised to UTC compare and sort correctly as strings, so `ORDER BY` +and range filters keep working. + +### Phase 3 (optional): typed output + +Returning strings means drivers give users `str` instead of `datetime`. To return +real temporal values, the type has to be stored with the value, for example as a +map `{"__boltage_type": "datetime", "value": "..."}`. That breaks string comparison +and the README's SQL views, so only do this behind a setting, if users ask for it. + +### Tests + +Smoke test: write a `datetime` and a `date` parameter and read them back as ISO +strings; `RETURN datetime('2024-01-02T03:04:05+01:00')` returns the normalised UTC +string; ordering by a datetime property. + +--- + +## A. Multiple labels per node + +In AGE a label is a table and a vertex lives in exactly one. Supporting +`CREATE (:A:B)`, `MATCH (n:B)` where `B` is a second label, `SET n:B` and +`REMOVE n:B` needs boltage to keep extra labels elsewhere and rewrite queries. + +### Approach + +- Keep the first label as the AGE label (the table), and extra labels in a reserved + property, `__labels` (a list). +- Rewrite: + - `CREATE (n:A:B {..})` → `CREATE (n:A {.., __labels: ['B']})` + - `MATCH (n:B)` → `MATCH (n) WHERE (label(n) = 'B' OR 'B' IN n.__labels)` + - `MATCH (n:A:B)` → `MATCH (n:A) WHERE 'B' IN n.__labels` + - `n:B` as a predicate → the same expression + - `SET n:B` → `SET n.__labels = coalesce(n.__labels, []) + 'B'` (when `B` is not + the table label); `REMOVE n:B` similarly. +- Output: `agtype.parse` merges `__labels` into the node's labels and hides the + property; `keys(n)`, `properties(n)` and `n {.*}` must hide it too (rewrite to + filter it out). +- Catalog: `db.labels`, `SHOW` and counts must include labels that only exist in + `__labels` (needs a scan, or a GIN index on `properties`). + +### Why it is last + +Every rewrite needs to understand patterns, predicates and projections exactly; +regexes on masked text will not be correct enough. This needs a real Cypher parser +(for example generated from the openCypher grammar), which is a larger change than +everything else in these plans together. `MATCH (n:B)` for a secondary label also +turns into a scan of all vertices. Do this only if there is real demand, and look +at AGE's own plans for multiple labels first. + +--- + +## E. Spatial types + +Same shape as temporal types: AGE has no point type, and Bolt has `Point2D` (`X`) +and `Point3D` (`Y`) structures. + +1. Unpack Bolt points and store them as maps `{srid, x, y[, z]}` via `literal()`. +2. Optionally, behind a setting, pack maps with exactly those keys back into Bolt + points, so neo4j-browser and drivers show points. +3. Provide `point()` and `point.distance()` as `boltage` schema functions (as in + part C), using plain Euclidean distance for Cartesian SRIDs and haversine for + WGS-84 (4326/4979). + +Spatial indexes would need PostGIS and are out of scope. + +## README changes + +Per part as it lands: replace the item in the Limitations bullet with a short +section under "How it works" (indexes and constraints, pattern comprehension +rewriting, how temporal values are stored). diff --git a/plans/04-users-transactions-sysinfo.md b/plans/04-users-transactions-sysinfo.md new file mode 100644 index 0000000..ea2bd73 --- /dev/null +++ b/plans/04-users-transactions-sysinfo.md @@ -0,0 +1,154 @@ +# User and role management, `SHOW TRANSACTIONS`, `:sysinfo` + +## Problem + +- Neo4j's user and role commands on the `system` database (`SHOW USERS`, + `CREATE USER`, `GRANT ROLE`, ...) are rejected by `_run_system` + (`NotSystemDatabaseCommand`). `dbms.showCurrentUser` always reports the role + `admin` (`boltage/backend.py:247`). +- `SHOW TRANSACTIONS` / `TERMINATE TRANSACTIONS` are not emulated. +- neo4j-browser's `:sysinfo` frame shows nothing useful, because the queries behind + it are not emulated. + +Authentication already maps Neo4j users to PostgreSQL roles one to one, so the +natural model is that Neo4j users and roles **are** PostgreSQL roles. + +## Part 1: users and roles + +### Mapping + +| Neo4j | PostgreSQL | +| --- | --- | +| user | role with `LOGIN` | +| role | role without `LOGIN` | +| user has role | role membership (`GRANT role TO user`) | +| suspended user | `NOLOGIN` on a role that has a password (or a marker, see below) | +| `admin` role | superuser, or membership in a configurable role | +| home database | role setting `boltage.home_database` (`ALTER ROLE ... SET boltage.home_database = 'g'`, a custom GUC) | + +### Commands + +| Cypher (on `system`) | SQL | +| --- | --- | +| `SHOW USERS` | `pg_roles` where `rolcanlogin` (or suspended), with memberships from `pg_auth_members` | +| `SHOW CURRENT USER` | same, for `current_user` | +| `CREATE USER u [IF NOT EXISTS] SET [PLAINTEXT] PASSWORD 'p' [CHANGE [NOT] REQUIRED] [SET STATUS ...] [SET HOME DATABASE g]` | `CREATE ROLE u LOGIN PASSWORD 'p'`, plus `ALTER ROLE ... SET` | +| `ALTER USER u SET PASSWORD 'p'`, `SET STATUS SUSPENDED/ACTIVE`, `SET HOME DATABASE` | `ALTER ROLE` | +| `ALTER CURRENT USER SET PASSWORD FROM 'old' TO 'new'` | check `old` by opening a short connection with it, then `ALTER ROLE current_user PASSWORD 'new'` | +| `DROP USER u [IF EXISTS]` | `DROP ROLE u` | +| `SHOW ROLES [WITH USERS]`, `CREATE ROLE r`, `DROP ROLE r`, `RENAME ROLE/USER` | `pg_roles`, `CREATE/DROP ROLE`, `ALTER ROLE ... RENAME TO` | +| `GRANT ROLE r TO u`, `REVOKE ROLE r FROM u` | `GRANT r TO u`, `REVOKE r FROM u` | + +Steps: + +1. Parse the commands with regexes in `_run_system`, like `CREATE DATABASE`; the + grammar is small and fixed. Run them only on the `system` database. +2. Build SQL with `psycopg.sql.Identifier` for names. Passwords cannot be bind + parameters in `CREATE ROLE`; use `sql.Literal`, and make sure the debug log at + `boltage/backend.py` (`log.debug("SQL: ...")`) and `_sql` never log statements + containing passwords (add a `secret=True` flag to `_sql` that redacts). +3. `CHANGE REQUIRED`: PostgreSQL has no equivalent. Accept `CHANGE NOT REQUIRED` + and reject `CHANGE REQUIRED`, or store a marker in a role setting + (`boltage.password_change_required`) and enforce it at login by allowing only + `ALTER CURRENT USER SET PASSWORD` (Neo4j's behaviour, signalled with + `Neo.ClientError.Security.CredentialsExpired`). Start with rejecting. +4. Suspension: `NOLOGIN` makes a suspended user indistinguishable from a role. + Store `boltage.suspended = 'true'` as a role setting next to `NOLOGIN` so + `SHOW USERS` still lists the user. +5. Use the home database at login: after `connect()`, read + `current_setting('boltage.home_database', true)` and use it instead of + `BOLTAGE_DEFAULT_GRAPH` in `resolve_db` and `SHOW HOME DATABASE`. +6. `dbms.showCurrentUser` and `SHOW CURRENT USER` report the real roles; report + `admin` only for superusers / members of the admin role, so neo4j-browser does not + offer admin actions that will fail. +7. Errors: map PostgreSQL's `42501` (insufficient privilege) on these commands to + `Neo.ClientError.Security.Forbidden`, and duplicate/missing roles to Neo4j's + `Neo.ClientError.General.InvalidArguments` messages. + +### Privileges (`GRANT ... ON GRAPH`) + +Neo4j's fine-grained privileges (`TRAVERSE`, `READ {prop}`, per label) do not map +onto PostgreSQL. A coarse subset does, per graph schema: + +| Cypher | SQL | +| --- | --- | +| `GRANT ACCESS ON DATABASE g TO r` | `GRANT USAGE ON SCHEMA g TO r` | +| `GRANT MATCH {*} ON GRAPH g TO r` | `GRANT SELECT ON ALL TABLES IN SCHEMA g TO r` (+ `ALTER DEFAULT PRIVILEGES` for new labels) | +| `GRANT WRITE ON GRAPH g TO r` | `INSERT, UPDATE, DELETE` on all tables and `USAGE` on sequences in the schema | +| `GRANT MATCH {*} ON GRAPH g NODES L TO r` | `SELECT` on `g."L"` only | + +Anything finer (property-level, `TRAVERSE` without `READ`, `DENY`) is rejected with +`FeatureNotSupported`. Do this after the user commands, and only if wanted; the +README's trigger section already explains granting SQL privileges directly. + +### neo4j-browser + +The browser's `:server user list` / `:server user add` frames send these commands. +Capture the exact queries (run boltage with `BOLTAGE_LOG_LEVEL=DEBUG`, or read the +failure log lines) and make sure the shapes they use are covered. + +## Part 2: `SHOW TRANSACTIONS` and `TERMINATE TRANSACTIONS` + +### Approach + +1. Make PostgreSQL backends identifiable: set `application_name` to + `boltage ` (`boltage/backend.py:126`), and remember each session's + backend pid (`SELECT pg_backend_pid()` at connect). +2. Keep a process-wide registry in boltage of live sessions: connection id, user, + client address (`peername`), driver `user_agent` from `HELLO`, current graph, + transaction start time, and the current **Cypher** text (not the generated SQL). +3. `SHOW TRANSACTIONS [ids] [YIELD ...]` joins the registry with `pg_stat_activity` + (by pid) for `state`, `xact_start`, `query_start`, wait events, and lists: + `database`, `transactionId`, `currentQueryId`, `connectionId`, `clientAddress`, + `username`, `currentQuery`, `startTime`, `status`, `elapsedTime`. + Transaction ids look like Neo4j's: `-transaction-`, with `n` from a + per-process counter incremented at each `BEGIN` / auto-commit query. +4. Visibility: show only the user's own transactions unless they are a superuser or + member of `pg_read_all_stats`, matching what `pg_stat_activity` allows. +5. `TERMINATE TRANSACTIONS 'id'`: `pg_cancel_backend(pid)` on the session's backend, + which aborts the running statement and its transaction. Mark the Bolt session so + its next message fails with `Neo.TransientError.Transaction.Terminated`. Only + allowed for one's own transactions or with `pg_signal_backend`. +6. Several boltage processes against one database (for example behind a load + balancer) each know only their own registry. Fall back to `pg_stat_activity` + alone for backends with `application_name` starting with `boltage` that are not + in the local registry, extracting the Cypher text from between the dollar quotes + of the `cypher()` call. + +Also emulate the older `CALL dbms.listTransactions()` and +`CALL dbms.listQueries()` with the same data if neo4j-browser or cypher-shell still +send them. + +## Part 3: `:sysinfo` + +1. **Find out what the frame queries.** neo4j-browser's `:sysinfo` frame uses + different queries depending on the server version it detects (boltage reports + `5.26.0`): typically `SHOW DATABASES` plus `CALL dbms.queryJmx(...)` for store + sizes, id usage, page cache and transaction counts. Capture them from the + failure log, or read `SysInfoFrame` in the neo4j-browser version pinned by + `NEO4J_BROWSER_REF`. +2. **Emulate `dbms.queryJmx`** for the beans the frame reads, from PostgreSQL: + + | Sysinfo field | Source | + | --- | --- | + | store sizes | `pg_total_relation_size` summed over the graph schema's tables; `pg_database_size` for the total | + | node / relationship id counts | row counts or `reltuples` of `_ag_label_vertex` / `_ag_label_edge` (already used by `_counts`) | + | page cache hits / faults / hit ratio | `pg_statio_user_tables` (`heap_blks_hit`, `heap_blks_read`) for the schema | + | transactions (open, committed, rolled back, peak) | `pg_stat_database` (`xact_commit`, `xact_rollback`) and `pg_stat_activity` | + | version / edition | the existing `dbms.components` values, plus PostgreSQL and AGE versions | + +3. Fields that have no PostgreSQL equivalent return empty values rather than errors, + so the frame still renders. + +## Tests + +Smoke test, as the superuser of the test instance: create a user with a password, +log in as that user, grant and revoke a role, `SHOW USERS` / `SHOW ROLES`, change +the user's own password, drop the user. `SHOW TRANSACTIONS` from one session sees a +transaction open in another, and `TERMINATE TRANSACTIONS` makes the other fail. +Manual check of `:sysinfo` and `:server user list` in neo4j-browser. + +## README changes + +Replace the bullet with a "Users and roles" section explaining that they are +PostgreSQL roles, the supported subset of privileges, and what `:sysinfo` shows. diff --git a/plans/05-tls.md b/plans/05-tls.md new file mode 100644 index 0000000..c9038b0 --- /dev/null +++ b/plans/05-tls.md @@ -0,0 +1,105 @@ +# TLS (`bolt+s://`, HTTPS) + +## Problem + +The Bolt port (`asyncio.start_server` in `boltage/__main__.py:46`) and the +neo4j-browser HTTP server (uvicorn) only speak plain text. Drivers configured with +`bolt+s://` / `neo4j+s://`, and neo4j-browser served over HTTPS (which must use +`wss://`, since browsers block plain `ws://` from an HTTPS page), cannot connect. +Credentials are sent in the clear, which matters because they are PostgreSQL +passwords. + +## Design constraints + +- The Bolt port serves raw Bolt, Bolt over WebSocket and plain HTTP (discovery, + CORS preflight) on one port, by sniffing the first four bytes + (`handle_client`, `boltage/bolt.py:349`). TLS has to fit in front of that sniffing. +- Neo4j has three modes for Bolt: TLS disabled, optional (both on one port), and + required. Offering the same makes migration easy. +- The WebSocket transport already uses a Sans-I/O protocol over the raw stream. TLS + can be added the same way, with `ssl.SSLObject` and `ssl.MemoryBIO`, which avoids + needing to "un-read" the sniffed bytes. + +## Approach + +### Configuration + +| Variable | Default | Purpose | +| --- | --- | --- | +| `BOLTAGE_TLS_CERT` | (unset) | PEM certificate chain. | +| `BOLTAGE_TLS_KEY` | (unset) | PEM private key. | +| `BOLTAGE_BOLT_TLS` | `optional` when a certificate is set, else `disabled` | `disabled`, `optional` or `required`. | +| `BOLTAGE_HTTPS` | `true` when a certificate is set | Serve neo4j-browser over HTTPS on `BOLTAGE_HTTP_PORT`. | + +Fail at start-up when only one of cert/key is set, or the files cannot be loaded. + +### Bolt port + +1. Build an `ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)` at start-up, minimum TLS 1.2, + and pass it in `Config`. +2. In `handle_client`, read the first byte. `0x16` is a TLS handshake record: + - `disabled`: close the connection. + - otherwise wrap the stream in a new `TlsTransport` (an `SSLObject` over two + `MemoryBIO`s, fed with the bytes already read), complete the handshake, then + read the first four **decrypted** bytes and continue with the existing sniffing: + `MAGIC` → raw Bolt over TLS, `GET ` → WebSocket over TLS (`wss://`), and so on. + - Anything else in `required` mode: close (for an HTTP request, first answer + with a plain `400` explaining that TLS is required, so the error is + understandable in a browser). +3. Refactor the transports so they stack: `RawTransport` reads from the asyncio + stream, `TlsTransport` reads from a lower transport, and `WebSocketTransport` + reads from either. Today `WebSocketTransport` subclasses `RawTransport` and + reads `self.reader` directly; change it to take a lower transport. +4. Handshake errors (client does not trust the certificate, wrong protocol) are + logged at `DEBUG` and the connection closed, like other connection errors. + +An alternative is `loop.start_tls()` on the accepted connection, which uses +asyncio's own TLS. It cannot be given the byte already read for sniffing, so it only +works for `required` mode without sniffing. The `SSLObject` approach covers all +three modes with one code path. + +### HTTP port + +1. Pass `ssl_certfile` / `ssl_keyfile` to `uvicorn.Config` when HTTPS is on. +2. The discovery document (`boltage/discovery.py`) advertises `neo4j+s://` and + `bolt+s://` when Bolt TLS is `required`, and the plain schemes otherwise. + neo4j-browser picks `wss://` itself when the page is served over HTTPS. +3. Optionally redirect plain HTTP to HTTPS; this needs a second port, so leave it out + at first. + +### Container images + +1. Mount a certificate and key and set `BOLTAGE_TLS_CERT` / `BOLTAGE_TLS_KEY`. +2. For a quick start, let the entrypoint generate a self-signed certificate when + `BOLTAGE_TLS=self-signed` is set (with `openssl`, which the full image has; check + the slim image), stored in a volume so it survives restarts. Clients then use + `bolt+ssc://` / `neo4j+ssc://`, and the browser needs a one-time exception. +3. pgAdmin in the full image has its own TLS settings; document them rather than + wiring them up. + +### Certificate reload + +Read the certificate again on `SIGHUP` by building a new context, used for new +connections, so certificates from e.g. Let's Encrypt can be renewed without +restarting. Optional; do it after the basics. + +## Related: TLS to PostgreSQL + +boltage to PostgreSQL already supports TLS through libpq's environment variables +(`PGSSLMODE`, `PGSSLROOTCERT`, ...), since psycopg uses libpq. That is not +documented; add it to the Configuration section of the README in the same change. + +## Tests + +- Smoke test against an instance with a self-signed certificate: + `python tests/smoke_test.py bolt+ssc://localhost:7687 ...` runs the full suite + unchanged, which covers raw Bolt over TLS. +- `required` mode rejects `bolt://`; `disabled` mode rejects `bolt+ssc://`; + `optional` accepts both on one port. +- Manual check: neo4j-browser over `https://localhost:7474/` connects over `wss://`. + +## README changes + +Remove the TLS bullet. Add the variables to the Configuration table and a short +"TLS" section: modes, client URI schemes (`+s` vs `+ssc`), the container's +self-signed option, and libpq's variables for the PostgreSQL side. diff --git a/plans/06-streaming-results.md b/plans/06-streaming-results.md new file mode 100644 index 0000000..4c9a046 --- /dev/null +++ b/plans/06-streaming-results.md @@ -0,0 +1,118 @@ +# Streaming results instead of fetching them fully + +## Problem + +`Session._sql` calls `fetchall()` (`boltage/backend.py:178`), `_run_cypher` parses +every row with `agtype.parse`, and the whole list sits in `Result.rows` until the +client has pulled it (`BoltConnection.pull`, `boltage/bolt.py:327`). A +`MATCH (n) RETURN n` over a large graph holds the full result in boltage's memory +three times over (PostgreSQL's wire data, the agtype text, the parsed objects), and +the first record only reaches the client after the last one has been read from +PostgreSQL. Bolt's `PULL {n}` already lets the client ask for batches; boltage just +does not pass that on to PostgreSQL. + +## Approach: server-side cursors + +Declare a PostgreSQL cursor for the query and `FETCH` from it as the client pulls. + +### Why named cursors, not single-row mode + +psycopg offers two ways to stream: + +- **`cursor.stream()`** (libpq single-row / chunked mode) works without a + transaction, but occupies the connection until the result is fully read. Bolt + allows several open results in one explicit transaction (each with a `qid`), and a + client may `RUN` again before pulling the previous result. With one PostgreSQL + connection per Bolt connection, that would deadlock or force buffering anyway. +- **Named (server-side) cursors** (`conn.cursor(name=...)`, `DECLARE ... CURSOR`) + can be open side by side within a transaction and fetched in any order. They need + a transaction: in autocommit mode only `WITH HOLD` cursors survive, and those + materialise the whole result at commit, which is what this plan wants to avoid. + +So: named cursors, inside a transaction. + +### Auto-commit queries get a transaction + +Neo4j commits an auto-commit query when its result has been fully consumed or +discarded, which matches a transaction around the cursor: + +1. `RUN` outside an explicit transaction: `BEGIN`, `DECLARE boltage_c NO SCROLL + CURSOR FOR SELECT * FROM cypher(...)`, then `FETCH` the first batch to surface + errors and measure `t_first`, as Neo4j reports errors on `RUN` where it can. +2. `PULL {n}`: `FETCH n` (or batches of e.g. 1000 rows for `n = -1`), parse, send the + records, reply `has_more: true` while rows remain. +3. When the cursor is exhausted: `CLOSE`, `COMMIT`, then the final `SUCCESS` with + the bookmark (and, with plan 02, the write statistics). +4. `DISCARD`: `MOVE FORWARD ALL` (so writes in the rest of the query still execute, + as in Neo4j), `CLOSE`, `COMMIT`. +5. `RESET`, a failure, or the client disconnecting before the end: `ROLLBACK`, + like Neo4j rolls back an unconsumed auto-commit query. + +This is the per-query transaction from the +[shared groundwork](README.md#shared-groundwork); plan 02 uses the same one. + +### Explicit transactions + +1. Each `RUN` declares its own cursor (`boltage_c`), and `PULL {qid}` fetches + from the matching cursor. `self.results` in `BoltConnection` holds cursors + instead of row lists. +2. `COMMIT` with results still open: drain them (`MOVE FORWARD ALL`) before + committing, so their writes happen, then `COMMIT`. `ROLLBACK` just rolls back. +3. A failure while fetching (PostgreSQL errors can surface mid-result, for example + a division by zero on row 5000) fails the transaction, as in Neo4j. + +### Writes inside cursors + +AGE's write clauses run as custom scan nodes that execute as rows are pulled through +them. **Spike:** confirm that + +- `DECLARE CURSOR` accepts a `cypher()` call containing `CREATE`/`MERGE`/`SET`/`DELETE` + (it is syntactically a `SELECT`, but PostgreSQL rejects data-modifying statements + in cursors in some cases); +- partially fetching and then `MOVE FORWARD ALL` gives the same end state as + `fetchall()`. + +If either fails, keep the current fully-buffered path for queries that write (known +from the masked-text keyword check, see plan 02) and stream only reads. Write +queries rarely return large results, so that is an acceptable fallback. + +### Smaller improvements in the same change + +- `pull()` sends each record with its own `write` + `drain`. Pack a batch of records + into one buffer and write it once per batch. +- Parse agtype per fetched batch rather than up front. +- Queries without a `RETURN` (no columns) have no rows to stream; skip the cursor + and execute them directly inside the transaction. +- System and emulated procedure results (`_run_system`) are small and stay as lists; + give `Result` a common interface (`fetch(n)`) with a list-backed and a + cursor-backed implementation. + +## Memory and performance + +- boltage's memory per query becomes one batch instead of the whole result. +- PostgreSQL holds the cursor's state, not the result, for plain plans; sorts and + `DISTINCT` still materialise on the PostgreSQL side, which is where they belong. +- A slow client keeps a transaction open for as long as it takes to pull. Neo4j has + the same behaviour; document that long-open auto-commit results hold locks and + delay vacuum, and that `idle_in_transaction_session_timeout` in PostgreSQL bounds it. +- Extra round trips for `BEGIN`/`DECLARE`/`COMMIT`: use psycopg's pipeline mode to + send `BEGIN` and `DECLARE` together. + +## Tests + +- The existing "large result over several PULLs" smoke test keeps passing. +- A query returning more rows than one batch, pulled with `fetch_size` small, gives + all rows in order. +- `DISCARD` of a write query's result still applies the write. +- Two results open in one explicit transaction, pulled alternately. +- A runtime error halfway through a result fails the `PULL`, and the session + recovers after `RESET`. +- A disconnect mid-result rolls back an auto-commit write. +- Optional: measure boltage's peak memory on `UNWIND range(1, 5000000) AS i RETURN i` + before and after. + +## README changes + +Remove the last Limitations bullet. Mention in "How it works" that results are +streamed through a PostgreSQL cursor, and that an auto-commit query's transaction +stays open until its result is consumed. diff --git a/plans/README.md b/plans/README.md new file mode 100644 index 0000000..204030c --- /dev/null +++ b/plans/README.md @@ -0,0 +1,50 @@ +# Plans for the README's limitations + +One plan per bullet in the README's [Limitations](../README.md#limitations) section: + +| Plan | Limitation | Size | +| --- | --- | --- | +| [01](01-return-star-explain-profile.md) | `RETURN *`, `EXPLAIN` and `PROFILE` | medium | +| [02](02-write-statistics.md) | Write statistics | small | +| [03](03-age-unsupported-features.md) | Multiple labels, pattern comprehensions, temporal and spatial types, indexes and constraints | large, split in parts | +| [04](04-users-transactions-sysinfo.md) | User and role management, `SHOW TRANSACTIONS`, `:sysinfo` | medium | +| [05](05-tls.md) | TLS (`bolt+s://`, HTTPS) | small to medium | +| [06](06-streaming-results.md) | Results fetched fully before streaming | medium | + +## Shared groundwork + +Several plans need the same building blocks. Building them once, first, keeps the +plans small: + +1. **Variable scope analysis in `cypher.py`.** `RETURN *` (01) and general pattern + comprehension rewriting (03) both need to know which variables are in scope at a + given clause. Today `cypher.py` only masks strings and tracks bracket depth. A + small clause splitter plus a scope tracker (variables introduced by `MATCH`, + `OPTIONAL MATCH`, `UNWIND ... AS`, `YIELD`, `WITH` projections, path assignments) + is enough; a full Cypher parser is not needed yet. If 03's multiple-labels part is + ever done, that is the point to reconsider a real parser (for example one + generated from the openCypher grammar). +2. **One transaction per auto-commit query.** Write statistics (02) read + transaction-local counters, streaming (06) needs a transaction to keep a server-side + cursor open, and `PROFILE` (01) needs a savepoint. All three want `Session.run` to + wrap auto-commit queries in `BEGIN ... COMMIT` instead of relying on the + connection's autocommit mode. Do this once, in 06 or 02, whichever comes first. +3. **Unit tests.** The only test is the end-to-end `tests/smoke_test.py`. Most of + this work is text rewriting in `cypher.py`, which is cheap to unit test without a + database. Add `tests/test_cypher.py` (pytest) alongside the first plan that + touches `cypher.py`, and keep extending the smoke test for behaviour that needs + AGE. + +## Suggested order + +1. 02 write statistics: small, very visible in neo4j-browser, and introduces the + per-query transaction. +2. 06 streaming: builds on the same transaction handling. +3. 01 `RETURN *` / `EXPLAIN` / `PROFILE`: introduces the scope analysis. +4. 03 indexes and constraints (part D), then pattern comprehensions (part B), then + temporal types (part C). Multiple labels (part A) and spatial types (part E) last, + or not at all. +5. 05 TLS and 04 user management, independent of the rest, whenever they are needed. + +Each plan ends with the README changes to make when it lands, so the Limitations +section shrinks as the work is done.