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
StefanSteiner added a commit that referenced this pull request May 28, 2026
* feat: stabilize v0.3.0 public API bundle

This commit aggregates the breaking and additive API changes that ship
together as v0.3.0. The individual PRs landed under chore: prefixes to
defer release-please from cutting an early version; this single feat:
commit with a BREAKING CHANGE: footer is the trigger for the v0.3.0
release PR.

Bundle contents (all merged to main):
- #70 (PR #71)  — Flat public Error enum + ergonomic constructors
                  workspace-wide
- #69 (PR #73)  — Transaction API consolidation (RAII guard recommended;
                  raw trio deprecated and #[doc(hidden)])
- #61 + #62
  (PR #74)      — FromRow modernization: #[derive(FromRow)] in new
                  hyperdb-api-derive crate, RowAccessor with cached
                  name->index lookup, breaking trait signature change,
                  blanket tuple impls deleted, #[hyperdb(rename)] and
                  #[hyperdb(index)] attributes
- #76           — Follow-ups A/B/C: typed io::Error sources in process.rs,
                  Error::InvalidOperation variant for caller misuse,
                  structured SQLSTATE on Cancelled/Closed/Connection

Follow-up D (flatten internal client::Error) deferred to v0.3.x as
issue #75 — internal-only, zero external consumers, larger than originally
scoped.

The code change in this commit is a small documentation refresh on the
crate-level rustdoc to (a) include hyperdb-api-derive in the companion
crates list and (b) fix a stale crate name (sea-query-hyper ->
sea-query-hyperdb). The body of the commit is the BREAKING CHANGE:
footer below; release-please uses it to generate the v0.3.0 entry in
CHANGELOG.md.

See MIGRATING-0.3.md for full migration recipes covering every breaking
change in the bundle.

BREAKING CHANGE: v0.3.0 reshapes the public hyperdb_api::Error enum
into a flat canonical structure (no Box<dyn StdError> cause channel,
no kind() method, no Other catch-all variant), and its constructor
surface (Error::new and Error::with_cause are deleted in favor of
domain-specific snake_case constructors). It also changes the FromRow
trait signature from fn from_row(row: &Row) to fn from_row(row:
RowAccessor<'_>), deletes the blanket 1/2/3/4-tuple FromRow impls,
deprecates Connection::begin_transaction/commit/rollback (use the RAII
guard at Connection::transaction() instead), introduces a new
Error::InvalidOperation variant, and changes Error::Cancelled and
Error::Closed from tuple to struct variants carrying structured
sqlstate. Every variant has a snake_case constructor; the FromRow
derive lives in a re-exported hyperdb-api-derive crate. See
MIGRATING-0.3.md for migration recipes.

* chore(ci): publish hyperdb-api-derive in release.yml + dry-run in ci.yml

Pre-release adversarial review of the v0.3.0 rollup CI/CD config caught
that hyperdb-api-derive (added in PR #74) was missing from release.yml's
publish-in-dependency-order step. hyperdb-api/Cargo.toml strictly pins
hyperdb-api-derive = "=0.X.Y", so cargo publish -p hyperdb-api would
fail at release time when crates.io can't resolve the strict version
of derive (because release.yml never published it).

Verified topologically:
- hyperdb-api-derive has zero workspace deps (only syn/quote/proc-macro2
  from the registry), so it can publish before any workspace crate.
- It's a runtime dep of hyperdb-api only.
- Inserted right after hyperdb-api-salesforce; existing order otherwise
  unchanged. Added a dependency-order comment to the publish step
  explaining the topology so future contributors don't break it.

Also adds hyperdb-api-derive to ci.yml's publish dry-run job. The
dry-run job exists exactly to catch this class of bug before release
time. Without this addition, the same blocker could re-emerge after a
future major-version refactor of derive.

Updates the stale "7 workspace-member version rows" comment in
release-please.yml to reflect the current 8-member workspace
(hyperdb-api-derive added in #74). The lockfile-sync sentinel
enumerates members at runtime via cargo metadata, so the count is
informational; correctness is unchanged.

Verified locally:
- cargo publish -p hyperdb-api-derive --dry-run: succeeds
- cargo publish -p sea-query-hyperdb --dry-run: succeeds
- cargo publish -p hyperdb-bootstrap --dry-run: succeeds
- cargo metadata workspace topology check: order in release.yml is
  consistent with non-dev deps across all 7 publishable crates.
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