feat(ogar-adapter-surrealql): wire surrealdb-parser deps + syntax validation (ADR-017 partial close)#24
Conversation
…eps under feature flag Activates the surrealdb-parser feature flag — deps from the workspace (AdaWorldAPI/surrealdb#main, pinned in PR #23's [workspace.dependencies]). Available now that PR #23 cleared the 1.85 -> 1.95 rust-version blocker. By default (no features), the deps are not built. With --features surrealdb-parser, both surrealdb-ast and surrealdb-parser are pulled in for the parse_surrealql_ddl + register_class_knowable_from wire-ups (this PR implements syntax validation; AST walk is a follow-up sprint). https://claude.ai/code/session_01PBTGaPCSnnt6u3pjXpbLwY
…ation + smoke tests Now that PR #23 cleared the rust-version blocker, parse_surrealql_ddl: - Without the `surrealdb-parser` feature: returns ParseError::Unimplemented with a 'feature off' rationale (was: todo!() panic). - With the feature: drives Parser::enter_parse::<Query>(input, Config) for syntax validation. On parse-OK, returns Unimplemented with a 'parsed successfully; AST -> Class walk pending follow-up' message. On syntax error, returns ParseError::Parse with the error message. Three smoke tests: - With feature: valid DDL returns Unimplemented(parsed-OK) - With feature: invalid DDL returns Parse(error) - Without feature: any DDL returns Unimplemented(feature off) ParseError gains Unimplemented(String) variant + Display + std::error::Error impls (proper error type, not panic). Crate docs reframed: rust-version blocker note removed, replaced with a 'Status post PR #23' section documenting the wire-up state + a canonical parser invocation pattern for the follow-up walk sprint. The pattern documents the full Query -> TopLevelExpr -> Expr::DefineTable / DefineField -> Ident.text arena-walk chain so the follow-up impl doesn't have to re-derive it. Partially closes ADR-017: deps usable from OGAR's workspace, parser drives real input, smoke tests prove the wiring. Full close awaits the AST walk (substantive sprint — ~250-400 lines + handles record<X>/option<X>/scalar TYPE clauses + ASSERT $value IN [...] for enums per contract §2 mapping). https://claude.ai/code/session_01PBTGaPCSnnt6u3pjXpbLwY
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79884570db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| surrealdb-ast = { workspace = true, optional = true } | ||
| surrealdb-parser = { workspace = true, optional = true } |
There was a problem hiding this comment.
Avoid resolving parser git deps in default builds
With these optional git dependencies in the package manifest, Cargo still resolves/fetches them while generating the dependency graph even when surrealdb-parser is not enabled, so emit-only/default users are no longer insulated from the private/remote SurrealDB fork. I checked cargo test -p ogar-adapter-surrealql --no-default-features, and it failed before compiling because Cargo tried to update https://github.com/AdaWorldAPI/surrealdb for surrealdb-ast; in offline CI or consumers without access to that fork, the default build now breaks despite the feature being off.
Useful? React with 👍 / 👎.
ogar-adapter-surrealqlparse wire-up — deps + syntax validation + smoke testsFollow-up to PR #23 (rust-version 1.85 → 1.95 bump + ecosystem pin matrix). Now that the rust-version blocker is cleared, this PR wires up
surrealdb-ast+surrealdb-parserunder thesurrealdb-parserfeature flag and drives the parser for syntax validation.What lands
Cargo.toml:
surrealdb-parserfeature now activates real deps:surrealdb-ast+surrealdb-parser, both taken from the workspace's pinned git refs (AdaWorldAPI/surrealdb#main, per PR chore: bump rust-version 1.85 -> 1.95 (mandatory ecosystem pin alignment) #23's[workspace.dependencies]).emit_surrealql_ddl.src/lib.rs:ParseErrorgainsUnimplemented(String)variant +Display+std::error::Errorimpls (proper error type, no moretodo!()panic).parse_surrealql_ddl(input):Err(ParseError::Unimplemented("…feature not enabled…")).--features surrealdb-parser: drivesParser::enter_parse::<surrealdb_ast::Query>(input, Config). On parse-OK, returnsErr(ParseError::Unimplemented("DDL parsed successfully; AST → Class walk pending follow-up sprint")). On syntax error, returnsErr(ParseError::Parse(error_message)).Query → TopLevelExpr → Expr::DefineTable / DefineField → Ident.textarena-walk chain. Future follow-up walk impl doesn't have to re-derive it.Scope honesty: partial close, not full close, of ADR-017
This PR moves ADR-017 from "blocked, scaffold only" to "deps wired, parser driven, syntax validation working" — but the AST →
Classwalk itself is the substantive follow-up sprint because:NodeId<T>-indexed viaAst<L>).DefineTable/DefineFieldwalk goes through ~6 layers of indirection (Query.exprs→NodeListId<TopLevelExpr>→TopLevelExpr::Expr→NodeId<Expr>→Expr::DefineTable(NodeId<DefineTable>)→DefineTable.name: NodeId<Expr>→Expr::Ident(NodeId<Ident>)→Ident.text: NodeId<String>).DefineFieldhasty: Option<NodeId<Type>>— theTypeenum representsrecord<X>/option<X>/ scalar / etc., each needing a differentClassmapping perOGAR-AST-CONTRACT.md §2.Typevariants + supportsASSERT $value IN [...]forEnumDecl::Staticmapping.Shipping it as a separate, focused follow-up PR avoids overcommitting on a complex internal-API walk in one PR (the EnterEffect cascade lesson from PR #13/#15).
What this enables for the follow-up
The follow-up walk PR can directly target the canonical pattern documented in
src/lib.rscrate docs. The smoke tests provide the wiring proof; the walk impl just has to fill in the body.register_class_knowable_fromstill stubbedThat's the §10.3 producer side of the
knowable_frommeet-point (ADR-010). It needs a Lance dataset writer in addition to the parser-side wiring. Tracked as a separate follow-up that pairs with thelance-bindSprint-5b boundary.Verification
cargo check --workspaceshould succeed (no consumers of the new deps yet by default).cargo test --workspaceshould succeed (existing 12 emit tests + 1 feature-off smoke test).cargo test --workspace --features ogar-adapter-surrealql/surrealdb-parser(or however the workspace exposes it) should succeed (12 emit + 2 feature-on smoke tests; 1 feature-off test skipped).https://claude.ai/code/session_01PBTGaPCSnnt6u3pjXpbLwY