Skip to content

fix(ogar-adapter-surrealql): quote non-bare identifiers in emitted DDL#36

Merged
AdaWorldAPI merged 1 commit into
mainfrom
claude/fix-ddl-identifier-quoting
Jun 5, 2026
Merged

fix(ogar-adapter-surrealql): quote non-bare identifiers in emitted DDL#36
AdaWorldAPI merged 1 commit into
mainfrom
claude/fix-ddl-identifier-quoting

Conversation

@AdaWorldAPI

Copy link
Copy Markdown
Owner

Summary

Closes Codex P2 on PR #33 (now merged) — register_class_knowable_from with surrealql-hint was rendering invalid SurrealQL for Odoo-style class names like sale.order because the emitter wrote DEFINE TABLE sale.order (dot terminates the bare identifier).

Per your instruction, opening this as a fresh PR for visibility since #33 is already on main.

The bug Codex flagged

Enabling surrealql-hint for Odoo-style classes stores an invalid schema hint instead of a self-describing registry entry.

OGAR class names legitimately use the dotted form (sale.order, account.move.line, res.partner) — ogar-ontology::class_identity documents this. SurrealQL bare identifiers match [A-Za-z_][A-Za-z0-9_]*; dots, hyphens, leading digits, and spaces need backtick-quoting.

The fix

New helper at crates/ogar-adapter-surrealql/src/lib.rs:

fn surrealql_ident(name: &str) -> String {
    if is_bare_surrealql_identifier(name) {
        name.to_string()
    } else {
        format!("`{}`", name.replace('`', "``"))
    }
}

Applied at every identifier-emission site:

Site Identifier Now quoted?
emit_classDEFINE TABLE <name> class.name
emit_field_attrDEFINE FIELD <name> ON <table> attr.name + table (pre-quoted at emit_class)
emit_field_assocDEFINE FIELD <name> ON <table> TYPE record<<target>> assoc.name, target, table
emit_field_enumDEFINE FIELD <column> ON <table> TYPE … enum_decl.column, table

The non-owning-side comment marker (-- {table} HasMany …) is left un-quoted — it's not parsed; readable text serves better.

Walker side: zero changes needed

parse_surrealql_ddl extracts identifiers via Expr::Path { start: Ident { text }, parts: None }ast[ident.text]. The SurrealDB parser unwraps backtick-quoting at the lexer level, so the walker yields "sale.order" (unquoted text) whether the DDL had backticks or not. Round-trip works automatically — verified by the new round-trip tests below.

Tests added (8 new)

Helper unit tests:

  • bare_identifier_passes_through_unquoted
  • non_bare_identifier_gets_backtick_quoted — covers sale.order, account.move.line, res.partner, kebab-case, leading digit, space
  • embedded_backticks_get_doubled
  • empty_string_is_not_a_bare_identifier

Emit-site tests:

  • emit_class_with_odoo_dotted_name_quotes_the_table — asserts both DEFINE TABLE and DEFINE FIELD ON use the quoted form
  • emit_class_with_odoo_belongs_to_quotes_record_target — asserts record<\res.partner`>` lands correctly

Round-trip (feature-on) tests — the load-bearing ones:

  • round_trip_odoo_dotted_table_name — builds IR with sale.order + belongs_to → res.partner, emits, parses, asserts the IR survives the trip with names byte-identical.
  • round_trip_three_segment_odoo_name — same for account.move.line.

Verification

$ cargo test -p ogar-adapter-surrealql                          -> 20/20
$ cargo test -p ogar-adapter-surrealql --features surrealdb-parser -> 33/33
$ cargo test -p ogar-knowable-from --features surrealql-hint    -> 10/10
$ cargo check --workspace --all-targets                         -> clean

PII abort-guard (word-boundary): CLEAN.

Scope

This fix is at the emit layer only — same change-policy as ADR-023 (the IR is fixed; adapter emit logic absorbs source-dialect concerns). No Class IR change; no adapter signature change; existing call sites with bare identifiers (widget, account, WorkPackage, etc.) pass through unchanged.

Note on the queue

PR #34 (docs/SPLAT-NATIVE-CUSTOMER.md) is the other session's work — leaving its Codex P2 to them per your instruction. This PR addresses only the OGAR session's own work.

https://claude.ai/code/session_01PBTGaPCSnnt6u3pjXpbLwY

Closes Codex P2 on PR #33 (now merged).

# The bug

When `surrealql-hint` is enabled (PR #33), `register_class_knowable_from`
renders the schema DDL for every registered Class via
`emit_surrealql_ddl`. For Odoo-style class names — which legitimately
use the dotted form (`sale.order`, `account.move.line`,
`res.partner`) per `ogar-ontology::class_identity` — the emitter was
producing raw `DEFINE TABLE sale.order …`. That's invalid SurrealQL:
the dot terminates the bare identifier, so the parser either rejects
the DDL or mis-parses the dot as a path operator.

Codex flagged this on PR #33 line 272: "enabling this feature for
Odoo-style classes stores an invalid schema hint instead of a self-
describing registry entry".

# The fix

New helper in `ogar-adapter-surrealql`:

    fn surrealql_ident(name: &str) -> String

Returns the bare identifier if `name` matches `[A-Za-z_][A-Za-z0-9_]*`,
or backtick-quoted otherwise (with embedded backticks doubled per
SurrealQL convention). Examples:

    surrealql_ident("widget")             -> "widget"
    surrealql_ident("sale.order")         -> "`sale.order`"
    surrealql_ident("account.move.line")  -> "`account.move.line`"
    surrealql_ident("kebab-case")         -> "`kebab-case`"
    surrealql_ident("weird`name")         -> "`weird``name`"

Applied at every identifier-emission site:

  - `emit_class` — DEFINE TABLE <name> (once at the top; `class.name`
    pre-quoted into `table_ident` and reused as `table` parameter for
    the field emitters).
  - `emit_field_attr` — DEFINE FIELD <name> ON <table>; quotes
    `attr.name` locally.
  - `emit_field_assoc` — DEFINE FIELD <name> ON <table> TYPE
    record<<target>>; quotes `assoc.name` + record target locally
    (so `record<\`res.partner\`>` lands correctly).
  - `emit_field_enum` — DEFINE FIELD <column> ON <table> TYPE …;
    quotes `enum_decl.column` locally.

The comment-marker line for non-owning sides (`-- {table} HasMany …`)
is left un-quoted — it's not parsed; readable text is more useful.

# Walker side is automatically correct

`parse_surrealql_ddl` extracts identifiers via
`Expr::Path { start: Ident { text }, parts: None }` -> `ast[ident.text]`.
The SurrealDB parser unwraps backtick-quoting at the lexer level, so
the walker yields `"sale.order"` (unquoted text) whether the DDL had
backticks or not. Round-trip works without walker changes — verified
by the new round-trip tests.

# Tests added (8 new; 20 default + 33 with parser feature)

Helper unit tests:
  - `bare_identifier_passes_through_unquoted`
  - `non_bare_identifier_gets_backtick_quoted` (covers `sale.order`,
    `account.move.line`, `res.partner`, kebab-case, leading digit,
    space)
  - `embedded_backticks_get_doubled`
  - `empty_string_is_not_a_bare_identifier`

Emit-site tests:
  - `emit_class_with_odoo_dotted_name_quotes_the_table` — asserts
    both DEFINE TABLE and DEFINE FIELD ON clauses use the quoted form.
  - `emit_class_with_odoo_belongs_to_quotes_record_target` — asserts
    `record<\`res.partner\`>` lands correctly.

Round-trip (feature-on) tests — the load-bearing ones:
  - `round_trip_odoo_dotted_table_name` — builds IR with
    `sale.order` + belongs_to → `res.partner`, emits, parses,
    asserts the IR survives the trip with names byte-identical.
  - `round_trip_three_segment_odoo_name` — same for
    `account.move.line`.

# Verification

  cargo test -p ogar-adapter-surrealql                          -> 20/20
  cargo test -p ogar-adapter-surrealql --features surrealdb-parser -> 33/33
  cargo test -p ogar-knowable-from --features surrealql-hint    -> 10/10
  cargo check --workspace --all-targets                         -> clean

PII abort-guard (word-boundary): CLEAN.

# Note on scope

This is a fix for the merged PR #33 — adding the missing identifier
quoting at the emit layer. Same change-policy as ADR-023 (the IR is
fixed; adapter emit logic is where source-dialect concerns get
absorbed). No `Class` IR change; no adapter signature change;
existing call sites unaffected (all the bare identifiers they use
pass through `surrealql_ident` unchanged).

https://claude.ai/code/session_01PBTGaPCSnnt6u3pjXpbLwY
@AdaWorldAPI AdaWorldAPI merged commit a25812a into main Jun 5, 2026
1 check passed
AdaWorldAPI pushed a commit that referenced this pull request Jun 29, 2026
Operator small veto (2026-06-29): the ClassView shape is mapped from the class's
inherited format and selected by classid — don't restate or LOCK it. 'Quadruplet'
was a misread of the 3×4 GUID shape; the shapes are per-class: Rails 6×2, other
frameworks 4×3, GUID 3×4. So 4×3 is legitimate per-class, not a 'worst case'.

- CLASSVIEW-FIELDVIEW-ASKAMA-BITMASK.md: 'Wide classes' recast from '[u64;4]
  quadruplet, locked 256' to class-conditioned shape (CascadeShape::from_levels,
  classid-selected); the only fixed bound is the byte-cardinality god-object
  signal (< 256 clean, >= 256 split). Simple-rules + summary + cross-refs updated
  off the removed field_mask_buckets/FIELD_MASK_MAX_BUCKETS API.
- OGAR-TRANSPILE-SUBSTRATE.md §1.5a + status: shapes table reframed Rails 6×2 /
  other frameworks 4×3 / GUID 3×4; 4×3 legitimate (its divide is a per-class
  cost, not a prohibition); is_byte_aligned/shift/ALIGNED = shift-vs-divide
  distinction, not a reject gate. Mirrors lance-graph #621 + ruff #36.

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants