feat(gql): implement standalone INSERT statement - #154
Open
qishipengqsp wants to merge 7 commits into
Open
Conversation
Add monotonic AtomicU64 counters on MemoryGraph so callers that don't supply explicit ids (notably the upcoming GQL INSERT executor) can ask storage for fresh ones. Counters start at 1, are observed past any externally provided id inside create_vertex/create_edge, and are positioned past the largest replayed id during WAL replay and checkpoint restore so allocator output never collides with persisted state. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the empty LinearDataModifyingStatement stub with a real DataModifyingStatement enum carrying InsertStatement, where each path is a list of alternating node/edge ElementPatterns reusing the existing ElementPattern/Filler/Predicate AST (so labels, properties, and variables share the parser code with MATCH). The parser dispatches on the INSERT keyword and parses `node (edge node)*` chains separated by commas. No quantifiers, alternation, or path prefix are accepted -- INSERT only takes simple shapes in this iteration. Snapshot tests cover single vertex, multiple vertices, vertex+edge, and reverse-direction edge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Wire INSERT end-to-end through binder, bound AST, logical planner, plan node, and optimizer pass-through. - BoundStatement::Data variant carries a Vec of BoundDataModifyingStatement - BoundInsertStatement holds resolved vertices (label_id + ordered property literals) and edges (label_id + endpoint indices into the vertex list), so the executor never has to touch the parser AST or the catalog provider - Binder resolves labels against current_graph.graph_type(), validates endpoint type compatibility, coerces property literals to declared types, and reports a focused error variant for each failure mode (unknown label/property, missing required property, type mismatch, duplicate variable, undirected edge, etc.) - Plan node PhysicalInsert flows through the optimizer untouched - Standalone INSERT only -- MATCH-context bindings and SET/REMOVE/DELETE are deferred to follow-up work Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
InsertBuilder follows the existing catalog_modify generator pattern: acquire the current graph's MemoryGraph, open a Serializable transaction, allocate fresh vids/eids via the new storage allocators, build Vertex/Edge values from the bound property records, and commit. The executor yields no rows (StatementComplete) -- INSERT has no output schema in this iteration. ExecutorBuilder dispatches PhysicalInsert to this builder. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds create_insert_test_graph, a fixture procedure that registers a
small schema using only primitive-typed properties (PERSON{name,age},
COMPANY{name,revenue}, KNOWS{since}, WORKS_AT{role}). GQL has no
vector-literal syntax yet, so the existing create_test_graph_data
schema (with non-nullable vector embeddings) is unusable for INSERT
tests; this fixture sidesteps that without disturbing existing
vector-search tests.
Adds minigu-test/gql/dml/insert.slt covering:
- 7 happy paths: single vertex, multiple vertices, vertex+edge,
self-type edges, reused vertex variables across edges, zero-value
boundary, reverse-direction edge
- 8 error paths: unknown label/property, missing required property,
property/edge type mismatch, undefined edge endpoint, duplicate
variable, edge endpoint type mismatch
- 4 MATCH verifications confirming inserted data is readable
Updates 8 insta snapshots in ddl/dml/misc whose INSERT statements
previously failed at parse time and now reach the binder (where they
fail with CurrentGraphNotSpecified because those test programs don't
set the current graph before INSERT -- a separate gap).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end support for standalone GQL INSERT statements (no MATCH context) across the parser → binder/planner → executor pipeline, and introduces storage-side ID allocation so inserts can use system-assigned vertex/edge IDs without colliding with recovered state.
Changes:
- Parser: implements a real
DataModifyingStatement::InsertAST + parsing forINSERT <path>(, <path>)*, reusingElementPatternfor nodes/edges. - Planner/Executor: binds
INSERTinto a physical plan node and executes it by allocating IDs, inserting vertices first then edges in a serializable transaction. - Storage/Tests: adds monotone
AtomicU64ID allocators toMemoryGraph(observed during WAL replay/checkpoint restore) and adds a sqllogictest suite + fixture procedure for INSERT.
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| minigu/storage/src/tp/memory_graph.rs | Adds atomic ID allocators (next_vertex_id/next_edge_id) + observe helpers to keep allocator monotone across recovery and explicit IDs. |
| minigu/storage/src/tp/checkpoint.rs | Observes restored vertex/edge IDs during checkpoint restore to advance allocators past persisted maxima. |
| minigu/gql/planner/src/plan/mod.rs | Registers a new physical plan node variant for INSERT. |
| minigu/gql/planner/src/plan/data_modify.rs | Introduces Insert physical plan node and explain output. |
| minigu/gql/planner/src/optimizer/mod.rs | Passes through PhysicalInsert during physical plan creation. |
| minigu/gql/planner/src/logical_planner/procedure_spec.rs | Routes bound data-modifying statements into the data-modify planner path. |
| minigu/gql/planner/src/logical_planner/mod.rs | Adds the data logical planner module. |
| minigu/gql/planner/src/logical_planner/data.rs | Plans BoundDataModifyingStatement::Insert into PlanNode::PhysicalInsert. |
| minigu/gql/planner/src/bound/procedure_spec.rs | Adds BoundStatement::Data(Vec<...>) to carry bound data-modifying statements. |
| minigu/gql/planner/src/bound/mod.rs | Exposes new bound data-modify structures. |
| minigu/gql/planner/src/bound/data.rs | Defines BoundInsertStatement (vertices + edges referencing endpoints by vertex index). |
| minigu/gql/planner/src/binder/procedure_spec.rs | Enables binding of Statement::Data into BoundStatement::Data. |
| minigu/gql/planner/src/binder/mod.rs | Adds the binder data module. |
| minigu/gql/planner/src/binder/error.rs | Adds INSERT-specific bind errors (unknown labels/properties, endpoint/type mismatch, etc.). |
| minigu/gql/planner/src/binder/data.rs | Implements binding for standalone INSERT, including schema/type checking and literal coercions. |
| minigu/gql/parser/src/parser/impls/data.rs | Implements parsing for linear data-modifying statements and INSERT paths; adds parser snapshot tests. |
| minigu/gql/parser/src/ast/data.rs | Replaces the stub linear DML AST with DataModifyingStatement::Insert + InsertStatement/InsertPath. |
| minigu/gql/parser/src/parser/impls/snapshots/gql_parser__parser__impls__data__tests__insert_single_vertex.snap | New parser snapshot for single-vertex INSERT. |
| minigu/gql/parser/src/parser/impls/snapshots/gql_parser__parser__impls__data__tests__insert_multiple_vertices.snap | New parser snapshot for multi-vertex INSERT. |
| minigu/gql/parser/src/parser/impls/snapshots/gql_parser__parser__impls__data__tests__insert_vertex_edge_vertex.snap | New parser snapshot for vertex-edge-vertex INSERT. |
| minigu/gql/parser/src/parser/impls/snapshots/gql_parser__parser__impls__data__tests__insert_reverse_edge.snap | New parser snapshot for reverse-direction edge INSERT. |
| minigu/gql/execution/src/executor/mod.rs | Registers the data_modify executor module. |
| minigu/gql/execution/src/executor/data_modify.rs | Adds INSERT executor that allocates IDs and writes vertices then edges in a serializable txn. |
| minigu/gql/execution/src/builder.rs | Wires PlanNode::PhysicalInsert into executor building. |
| minigu/core/src/procedures/mod.rs | Registers a new built-in procedure for INSERT testing. |
| minigu/core/src/procedures/create_insert_test_graph.rs | Adds create_insert_test_graph fixture procedure to set up a primitive-only schema for INSERT tests. |
| minigu-test/gql/dml/insert.slt | Adds sqllogictest coverage for INSERT happy/error paths + MATCH verification queries. |
| minigu-test/gql/misc/vector_index@parser.snap | Updates parser snapshot: INSERT now parses (previously failed at parse time). |
| minigu-test/gql/misc/vector_index@e2e.snap | Updates e2e snapshot: INSERT now reaches bind-time errors when no graph is set. |
| minigu-test/gql/dml/dml_dql@e2e.snap | Updates e2e snapshot: INSERT now reaches bind-time errors (no current graph) rather than parse errors. |
| minigu-test/gql/ddl/ddl_truncate@parser.snap | Updates parser snapshot: INSERT now parses in these programs. |
| minigu-test/gql/ddl/ddl_truncate@e2e.snap | Updates e2e snapshot: INSERT now fails at bind-time (no current graph) rather than parse-time. |
| minigu-test/gql/ddl/ddl_drop@parser.snap | Updates parser snapshot: INSERT now parses in these programs. |
| minigu-test/gql/ddl/ddl_drop@e2e.snap | Updates e2e snapshot: INSERT now fails at bind-time (no current graph) rather than parse-time. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+176
to
+177
| } | ||
|
|
Comment on lines
+312
to
+325
| .copied() | ||
| .ok_or_else(|| BindError::InsertUnknownProperty { | ||
| label: label.clone(), | ||
| property: SmolStr::new(name), | ||
| })?; | ||
| let value = literal_scalar_from_expr(field.value().value.value()).ok_or_else(|| { | ||
| BindError::InsertNonLiteralProperty { | ||
| property: SmolStr::new(name), | ||
| } | ||
| })?; | ||
| let value = | ||
| coerce_scalar_to_type(value, declared_prop.logical_type()).ok_or_else(|| { | ||
| BindError::InsertPropertyTypeMismatch { | ||
| label: label.clone(), |
Comment on lines
+396
to
+398
| .value() | ||
| .float | ||
| .parse::<f64>() |
| /// Bumps the vertex id counter so that any future [`alloc_vertex_id`] call | ||
| /// returns a value strictly greater than `seen`. No-op if the counter is | ||
| /// already ahead. | ||
| pub fn observe_vertex_id(&self, seen: VertexId) { |
| } | ||
|
|
||
| /// Like [`observe_vertex_id`] but for edge ids. | ||
| pub fn observe_edge_id(&self, seen: EdgeId) { |
CI builds gql-parser with --no-default-features against aarch64-unknown-none, where the prelude does not provide alloc's Vec. Other parser modules use `use crate::imports::Vec;` for this; bring the INSERT parser in line. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements GQL
INSERT(standalone, no MATCH context, system-allocated vertex/edge ids) end-to-end across parser → planner → executor, plus a sqllogictest suite and a test-fixture procedure.LinearDataModifyingStatementstub with a realDataModifyingStatement::Insertenum; INSERT paths reuse the existingElementPatternAST so labels, properties, and variables share code withMATCH.BoundInsertStatement(resolved vertices with property literals, edges referenced by endpoint index). Binder reports focused errors for unknown labels/properties, type mismatches, undefined endpoints, duplicate variables, and edge-endpoint type mismatches.AtomicU64vid/eid allocators onMemoryGraph. Counters are observed past externally supplied ids insidecreate_vertex/create_edge, WAL replay, and checkpoint restore so allocator output never collides with persisted state.InsertBuilderfollows the existingcatalog_modifygenerator pattern — open a Serializable txn, allocate ids, insert vertices then edges, commit.minigu-test/gql/dml/insert.sltwith 7 happy paths, 8 error paths, and 4 MATCH-based verifications. Adds thecreate_insert_test_graphfixture procedure (PERSON/COMPANY/KNOWS/WORKS_AT, primitive types only — GQL has no vector literal syntax, socreate_test_graph_datawith its required vector embeddings can't be used for INSERT tests).Supported syntax
All vertex variables used by edges in the same INSERT must be defined inline; MATCH-context bindings, anonymous vertices, undirected edges, quantifiers, and SET/REMOVE/DELETE are explicitly out of scope for this PR.
Side effect on existing snapshots
8 insta snapshots in
ddl/,dml/,misc/are updated. INSERT statements in those test programs previously failed at parse time (Parser(Unexpected)); they now parse cleanly and fall through to bind-time errors likeCurrentGraphNotSpecifiedbecause those programs don't issueSESSION SET GRAPH. New snapshots reflect the new (more accurate) error path.Test plan
cargo test --workspace— all green (one pre-existing flaky test inminigu-storageis unrelated)cargo test -p gql-parser --features serde insert— 4 new parser snapshot tests passcargo test -p minigu-test --test sqllogictest— newdml/insert.sltpasses (all 7statement ok, all 8statement error, all 4 MATCH verifications)🤖 Generated with Claude Code