Skip to content

chore(api): deprecate raw transaction trio; migrate MCP async paths to RAII (#69)#73

Merged
StefanSteiner merged 1 commit into
tableau:mainfrom
StefanSteiner:ssteiner/issue-69-transaction-api-consolidation
May 28, 2026
Merged

chore(api): deprecate raw transaction trio; migrate MCP async paths to RAII (#69)#73
StefanSteiner merged 1 commit into
tableau:mainfrom
StefanSteiner:ssteiner/issue-69-transaction-api-consolidation

Conversation

@StefanSteiner
Copy link
Copy Markdown
Contributor

Resolves #69.

Consolidates the public transaction API around the RAII Transaction<'conn> / AsyncTransaction<'conn> guards. The raw &self begin_transaction / commit / rollback methods on Connection and AsyncConnection are now #[doc(hidden)] #[deprecated] — they remain callable for now but are hidden from generated rustdoc and emit a compiler warning at every call site. The MCP server's async pool-path ingest functions are migrated to use the RAII guard, demonstrating the recommended pattern in 4 production code sites.

Soft-deprecation, not removal. The original issue proposed full removal of the trio. After spike + investigation, the workspace's hyperdb-mcp::Engine::execute_in_transaction helper takes &self and so cannot use the RAII guard without restructuring Engine's lock model. Rather than block #69 on that structural change, this PR ships the public-API safety win (deprecation + hide) and the four mechanical MCP migrations now, and tracks the engine-level migration as a follow-up. See .issue-body-mcp-engine-raii-migration.md for the followup scope and two implementation paths.

Lands as chore: to defer release-please from cutting an early version; the v0.3.0 bump happens after the rest of the bundle merges. See MIGRATING-0.3.md for the consolidated migration guide.

What changed

hyperdb-api

  • Added pub(crate) canonical implementations:
    • Connection::{begin_transaction_raw, commit_raw, rollback_raw} (sync)
    • AsyncConnection::{begin_transaction_raw, commit_raw, rollback_raw} (async)
  • The original pub methods became thin wrappers calling the _raw family, marked #[doc(hidden)] #[deprecated(note = "Use ...transaction() for an RAII guard. ...")].
  • The RAII guards (Transaction, AsyncTransaction) switched their internal calls to the _raw family — no #[allow(deprecated)] annotation needed. They model the recommended pattern from inside the crate.

hyperdb-mcp

Four async pool-path ingest functions migrated to the RAII guard:

  • ingest_csv_file_async
  • ingest_json_async
  • ingest_parquet_file_async
  • ingest_arrow_ipc_file_async

Each rewrote the conn.begin_transaction().await? ... conn.commit().await? pattern into:

let txn = conn.transaction().await.map_err(McpError::from)?;
let inner: Result<u64, McpError> = async {
    /* work via txn.connection() and txn.execute_command(...) */
}.await;
let row_count = match inner {
    Ok(n) => { txn.commit().await.map_err(McpError::from)?; n }
    Err(e) => { let _ = txn.rollback().await; return Err(e); }
};

The function signatures changed from &AsyncConnection to &mut AsyncConnection (the borrow-checker truth: a transaction needs exclusive use of the session). ~7 caller sites in server.rs, watcher.rs, and tests/ingest_arrow_tests.rs updated to bind let mut conn = pool.get().await? and pass &mut conn.

Engine::execute_in_transaction retained the deprecated raw methods under #[allow(deprecated, reason = "...")]. It cannot migrate without reshaping the Engine lock model — tracked as the follow-up issue.

Tests / examples / docs

  • hyperdb-api/tests/transaction_tests.rs and wire_desync_tests.rs exercise the deprecated raw API on purpose to lock in its behavior. File-level #![allow(deprecated, reason = "...")] documents the intent.
  • hyperdb-api/examples/additional_examples/transactions.rs keeps both the raw and RAII demonstrations side-by-side for educational purposes; same file-level allow.
  • docs/TRANSACTIONS.md reorganized: RAII is the only documented path, with a "Deprecated raw methods" section at the bottom carrying the migration recipe.
  • MIGRATING-0.3.md adds a #69 section with sync + async migration recipes and the pooled-connection caveat.

Verification

  • cargo build --workspace --all-targets — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo test --workspace — all targets pass (300+ tests, 0 failures, including doctests)
  • cargo fmt --check — clean

Process notes

  • Plan-driven 5-phase workflow with parallel adversarial review at the plan and pre-PR checkpoints. Both reviewers caught real defects on the integrated diff:
    • Stale rustdoc on create_table_async recommending the deprecated pattern (fixed)
    • Misleading "wire-state quirk" comment on the post-commit COUNT(*) call (fixed)
    • The follow-up issue scope was a one-liner; expanded to two structural paths + acceptance criteria in .issue-body-mcp-engine-raii-migration.md (added)

Related

Test plan

  • CI green on this branch
  • Smoke-test cargo run --example transactions and cargo run --example async_parity_smoke against a local hyperd
  • Cross-link review of MIGRATING-0.3.md recipes against the actual public API

…o RAII (tableau#69)

Consolidates the public transaction API around the RAII Transaction /
AsyncTransaction guards. The raw &self begin_transaction / commit /
rollback methods on Connection and AsyncConnection are now
#[doc(hidden)] #[deprecated] -- still callable, but hidden from rustdoc
and emitting a compiler warning at every call site.

Lands as `chore:` to defer release-please from cutting an early
version. The v0.3.0 bump happens after the rest of the bundle merges.
See MIGRATING-0.3.md for the consolidated migration guide.

What changed:

- hyperdb-api Connection/AsyncConnection: added pub(crate) canonical
  begin_transaction_raw / commit_raw / rollback_raw siblings. The
  original pub methods became thin wrappers calling the _raw family,
  marked #[doc(hidden)] #[deprecated]. The RAII guards (Transaction,
  AsyncTransaction) call _raw directly so they don't self-warn.

- hyperdb-mcp: 4 async pool-path ingest functions migrated to use the
  RAII guard (ingest_csv_file_async, ingest_json_async,
  ingest_parquet_file_async, ingest_arrow_ipc_file_async). Their
  signature changed from &AsyncConnection to &mut AsyncConnection;
  ~7 caller sites in server.rs, watcher.rs, and ingest_arrow_tests.rs
  updated to bind `let mut conn = pool.get().await?` and pass
  `&mut conn`.

- Engine::execute_in_transaction (sync, &self constraint) cannot
  migrate without reshaping the Engine lock model. Annotated with
  #[allow(deprecated, reason = "...")]. Followup scope in
  .issue-body-mcp-engine-raii-migration.md.

- Tests intentionally exercising the deprecated API
  (transaction_tests.rs, wire_desync_tests.rs) and the transactions
  example carry file-level #![allow(deprecated, reason = "...")].

- docs/TRANSACTIONS.md reorganized: RAII as the only documented path,
  "Deprecated raw methods" section at the bottom with the migration
  recipe. MIGRATING-0.3.md adds a tableau#69 section.

Soft-deprecation rationale: the original issue proposed full removal
of the trio, but Engine::execute_in_transaction's &self constraint
makes a clean removal a structural change beyond tableau#69's scope. The
deprecation + hide gets the public-API safety win now; the followup
issue tracks the structural migration.

Verification:
- cargo build --workspace --all-targets -- clean
- cargo clippy --workspace --all-targets -- -D warnings -- clean
- cargo test --workspace -- 300+ tests, 0 failures (including doctests)
- cargo fmt --check -- clean
@StefanSteiner StefanSteiner merged commit 5c7e78b into tableau:main May 28, 2026
11 checks passed
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.

Remove the &self begin_transaction/commit/rollback trio (RAII Transaction is the only safe path)

1 participant