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

MIGRATING-0.3.md

@@ -177,6 +177,72 @@ if let Error::Server { sqlstate: Some(code), detail, hint, .. } = &err {
 
 ---
 
+## #69 — Transaction API consolidation
+
+The raw transaction methods on `Connection` and `AsyncConnection` are now deprecated and hidden from rustdoc. The RAII guard at `Connection::transaction()` / `AsyncConnection::transaction()` is the recommended (and only documented) way to drive transactions.
+
+### What's deprecated
+
+```rust
+Connection::begin_transaction(&self)        // -> #[doc(hidden)] #[deprecated]
+Connection::commit(&self)                   // -> #[doc(hidden)] #[deprecated]
+Connection::rollback(&self)                 // -> #[doc(hidden)] #[deprecated]
+AsyncConnection::begin_transaction(&self)   // -> #[doc(hidden)] #[deprecated]
+AsyncConnection::commit(&self)              // -> #[doc(hidden)] #[deprecated]
+AsyncConnection::rollback(&self)            // -> #[doc(hidden)] #[deprecated]
+```
+
+These methods still exist and still work — your build will see compiler warnings rather than errors. They will be deleted in a future release; new code must use the RAII guard.
+
+### Migration recipe
+
+```rust
+// Before
+conn.begin_transaction()?;
+conn.execute_command("INSERT INTO t VALUES (1)")?;
+conn.commit()?;
+
+// After (sync)
+let txn = conn.transaction()?;          // requires &mut conn
+txn.execute_command("INSERT INTO t VALUES (1)")?;
+txn.commit()?;
+```
+
+For the async equivalent, the body of the function holding `conn` will need to take `&mut AsyncConnection` instead of `&AsyncConnection`. Where you previously had:
+
+```rust
+pub async fn ingest(conn: &AsyncConnection, ...) -> Result<(), McpError> {
+    conn.begin_transaction().await?;
+    ...
+    conn.commit().await?;
+}
+```
+
+write:
+
+```rust
+pub async fn ingest(conn: &mut AsyncConnection, ...) -> Result<(), McpError> {
+    let txn = conn.transaction().await?;
+    txn.execute_command("...").await?;
+    txn.commit().await?;
+}
+```
+
+Callers that hold a pooled connection (`deadpool::managed::Object<ConnectionManager>`) need `let mut conn = pool.get().await?;` and `&mut conn` at the call site.
+
+### What didn't change
+
+- `Connection::transaction(&mut self) -> Result<Transaction<'_>>` — kept as the canonical entry point.
+- `Transaction::commit(self)` and `Transaction::rollback(self)` — kept; consume `self` to prevent double-commit.
+- The `Drop for Transaction` auto-rollback safety net — kept.
+- `AsyncTransaction` semantics, including the warning-only `Drop` (Rust has no async `Drop`) — kept.
+
+### MCP follow-up
+
+The MCP server's `Engine::execute_in_transaction` helper takes `&self` and so cannot use the RAII guard. It retains the deprecated raw methods with a function-level `#[allow(deprecated, reason = "...")]` annotation. Migrating it requires reshaping `Engine`'s locking model. Two structural paths and an acceptance-criteria checklist are written up in [issue #72](https://github.com/tableau/hyper-api-rust/issues/72).
+
+---
+
 ## #70 (continued) — Ergonomic constructors across all workspace error types
 
 The same ergonomic-constructor pattern was applied to every error type in the workspace that user code might construct, so call sites no longer need `.to_string()` ceremony for string-literal arguments.

docs/TRANSACTIONS.md

@@ -1,31 +1,17 @@
 # Transaction Support
 
-This document describes the transaction API in the Hyper Rust API, covering ACID semantics (A, C, I guaranteed; D not provided by this API), raw `Connection` methods, the RAII `Transaction` / `AsyncTransaction` guards, behavioral notes, and the test inventory.
+This document describes the transaction API in the Hyper Rust API, covering ACID semantics (A, C, I guaranteed; D not provided by this API), the RAII `Transaction` / `AsyncTransaction` guards, behavioral notes, and the test inventory.
 
 ## Overview
 
 Hyper transactions in the Rust API guarantee **A**tomicity, **C**onsistency, and **I**solation. **Durability is not provided by this API.** Committed data is held in the server's memory; the database becomes durable only when it is closed, unloaded, detached, or released — at which point its data is flushed to disk. An unexpected process termination (crash, SIGKILL) before that flush can lose committed transactions.
 
-The API provides two levels of transaction control:
+The recommended way to drive transactions is the **RAII guard** (`Transaction<'conn>` / `AsyncTransaction<'conn>`), which auto-rolls back on drop and uses Rust's borrow checker to make several classes of misuse compile errors.
 
-1. **Raw methods** on `Connection` / `AsyncConnection` — thin wrappers around SQL commands
-2. **RAII guards** (`Transaction<'conn>` / `AsyncTransaction<'conn>`) — auto-rollback on drop
-
-All transaction APIs are always available with no feature flags required.
+> Older raw `Connection::begin_transaction` / `commit` / `rollback` methods exist but are **deprecated** as of v0.3.0 and hidden from generated rustdoc. They will be removed in a future release; new code must use the RAII guard. See "Deprecated raw methods" at the bottom of this doc for the migration recipe.
 
 ## API Reference
 
-### Raw Connection Methods
-
-Available on both `Connection` (sync) and `AsyncConnection` (async, with `.await`):
-
-```rust
-// Transaction control
-conn.begin_transaction()?;
-conn.commit()?;
-conn.rollback()?;
-```
-
 ### RAII Transaction Guard (Sync)
 
 ```rust
@@ -117,33 +103,53 @@ txn.commit().await?;
 
 ### Transactions
 
-- **Nested BEGIN:** Calling `begin_transaction()` inside an active transaction produces a Hyper WARNING notice, not an error. The second BEGIN is ignored.
-- **ROLLBACK outside transaction:** Calling `rollback()` with no active transaction produces a WARNING, not an error.
-- **Error in transaction:** After a SQL error inside a transaction, the entire transaction enters an aborted state (SQLSTATE `25P02`). You must issue `ROLLBACK` before using the connection for anything else.
-- **DDL after DML:** Executing DDL (e.g., `CREATE TABLE`) after DML (e.g., `INSERT`) in the same transaction produces error `0A000`. DDL-only transactions work fine.
+- **Error in transaction:** After a SQL error inside a transaction, the entire transaction enters an aborted state (SQLSTATE `25P02`). You must drop or rollback the guard before using the connection for anything else; the next `txn.execute_command(...)` would error.
+- **DDL after DML:** Executing DDL (e.g. `CREATE TABLE`) after DML (e.g. `INSERT`) in the same transaction produces error `0A000`. DDL-only transactions work fine.
+- **Nested transactions:** Hyper does not support nested transactions. Issuing `BEGIN` while a transaction is open produces a WARNING; the second BEGIN is ignored. The RAII guard's `&mut self` borrow already prevents this in safe Rust code.
 
 ## What Works
 
-- BEGIN / COMMIT / ROLLBACK via raw methods and via SQL strings
 - RAII `Transaction` guard with auto-rollback on drop (sync)
 - RAII `AsyncTransaction` guard (async, with warning-only drop)
+- DDL inside transactions (subject to the DDL-after-DML restriction)
+- Multi-table atomic rollback
 
 ## What Doesn't Work / Limitations
 
-- **Async Drop rollback:** `AsyncTransaction` cannot issue ROLLBACK in Drop due to Rust's sync-only Drop trait. It only prints a warning.
-- **Error recovery within transactions:** After a SQL error inside a transaction, the transaction is fully aborted (SQLSTATE `25P02`). You must ROLLBACK — you cannot continue executing statements.
+- **Async Drop rollback:** `AsyncTransaction` cannot issue ROLLBACK in Drop due to Rust's sync-only Drop trait. It only prints a warning. Always explicitly commit or rollback async transactions before drop.
+- **Error recovery within transactions:** After a SQL error inside a transaction, the transaction is fully aborted (SQLSTATE `25P02`). You must rollback — you cannot continue executing statements.
 - **`information_schema.tables`:** Does not exist in Hyper. Cannot be used to check table existence.
 
+## Deprecated raw methods
+
+The methods `Connection::begin_transaction` / `commit` / `rollback` (and the matching `AsyncConnection` versions) are **deprecated** as of v0.3.0. They are hidden from generated rustdoc, marked `#[deprecated]` so any caller receives a compiler warning, and slated for removal in a future release.
+
+Migration recipe:
+
+```rust
+// Before — deprecated
+conn.begin_transaction()?;
+conn.execute_command("INSERT INTO t VALUES (1, 'hello')")?;
+conn.commit()?;
+
+// After — RAII guard
+let txn = conn.transaction()?;          // requires &mut conn
+txn.execute_command("INSERT INTO t VALUES (1, 'hello')")?;
+txn.commit()?;
+```
+
+The `&mut conn` requirement is intentional — it's the borrow-checker mechanism that makes the safety story compile-enforced. If your code currently holds the connection through a non-mutable reference (e.g. inside an `&self` method on a wrapper struct), you may need to reshape the wrapper's locking model. The MCP server's `engine.rs::execute_in_transaction` is one such caller; it retains the deprecated raw methods until [issue #72](https://github.com/tableau/hyper-api-rust/issues/72) restructures `Engine`'s lock model.
+
 ## Test Inventory
 
 ### transaction_tests.rs
 
-Basic transaction behavior.
+Basic transaction behavior. The `test_raw_*` tests pin behavior of the deprecated raw methods until they are removed.
 
 | Test | Description |
 |------|-------------|
-| `test_raw_begin_commit_methods` | Raw `begin_transaction()` / `commit()` methods |
-| `test_raw_begin_rollback_methods` | Raw `begin_transaction()` / `rollback()` methods |
+| `test_raw_begin_commit_methods` | **(deprecated API)** Raw `begin_transaction()` / `commit()` methods |
+| `test_raw_begin_rollback_methods` | **(deprecated API)** Raw `begin_transaction()` / `rollback()` methods |
 | `test_begin_commit` | BEGIN + INSERT + COMMIT via SQL strings |
 | `test_begin_rollback` | BEGIN + INSERT + ROLLBACK via SQL strings |
 | `test_transaction_guard_commit` | RAII guard: `txn.execute_command()` + `txn.commit()` |

hyperdb-api/examples/additional_examples/transactions.rs

@@ -4,17 +4,28 @@
 //! Example: Transactions
 //!
 //! Demonstrates the transaction API:
-//! - Raw transaction methods (`begin_transaction`, `commit`, `rollback`)
-//! - RAII `Transaction` guard with explicit commit and rollback
+//! - **Recommended:** RAII `Transaction` guard with explicit commit and rollback
 //! - Querying within transactions to see uncommitted data
 //! - Multiple operations (INSERT, UPDATE, DELETE) in a single transaction
 //! - Multi-table atomic rollback (referential integrity across tables)
 //! - Multi-table reconnect semantics: only committed cross-table data is visible after reconnect
 //! - Auto-rollback safety net when the guard is dropped without commit
 //! - DDL inside transactions and known restrictions
+//! - Legacy raw transaction methods (`begin_transaction`, `commit`,
+//!   `rollback`) — included for completeness only; deprecated and
+//!   slated for removal. New code should use the RAII guard.
 //!
 //!   cargo run -p hyperdb-api --example transactions
 
+// The `example_raw_transaction` and `example_raw_error_recovery`
+// helpers below intentionally exercise the deprecated raw transaction
+// API for documentation purposes. New code should use the RAII guard
+// shown in `example_transaction_guard` instead.
+#![allow(
+    deprecated,
+    reason = "example intentionally demonstrates the deprecated raw transaction API alongside the RAII guard"
+)]
+
 use hyperdb_api::{
     Catalog, Connection, CreateMode, HyperProcess, Parameters, Result, SqlType, TableDefinition,
 };

hyperdb-api/src/async_connection.rs

@@ -1014,37 +1014,90 @@ impl AsyncConnection {
     // Transaction Control
     // =========================================================================
 
+    // -------------------------------------------------------------------
+    // Raw transaction control (internal)
+    // -------------------------------------------------------------------
+    //
+    // The `*_raw` methods below are `pub(crate)` and form the canonical
+    // implementation of session-level transaction control. The RAII
+    // guard at `crate::AsyncTransaction` and any internal helper that
+    // genuinely needs `&self` (rather than the guard's `&mut self`)
+    // delegate to these.
+    //
+    // The matching `pub` methods (`begin_transaction`, `commit`,
+    // `rollback`) are thin `#[doc(hidden)] #[deprecated]` wrappers
+    // retained only so any pre-existing downstream caller sees a
+    // compiler warning rather than a hard break. They will be deleted
+    // in a future release; the `_raw` methods stay.
+
+    /// Issues `BEGIN TRANSACTION`. Crate-internal use only.
+    pub(crate) async fn begin_transaction_raw(&self) -> Result<()> {
+        self.execute_command("BEGIN TRANSACTION").await?;
+        Ok(())
+    }
+
+    /// Issues `COMMIT`. Crate-internal use only.
+    pub(crate) async fn commit_raw(&self) -> Result<()> {
+        self.execute_command("COMMIT").await?;
+        Ok(())
+    }
+
+    /// Issues `ROLLBACK`. Crate-internal use only.
+    pub(crate) async fn rollback_raw(&self) -> Result<()> {
+        self.execute_command("ROLLBACK").await?;
+        Ok(())
+    }
+
     /// Begins an explicit transaction (async).
     ///
+    /// **Prefer [`transaction()`](Self::transaction)** — the RAII guard
+    /// auto-rolls back on drop and cannot leak a half-open transaction
+    /// across error paths. Hidden from generated rustdoc and
+    /// deprecated; slated for removal in a future release.
+    ///
     /// # Errors
     ///
     /// Returns [`Error::Server`] if the server rejects `BEGIN TRANSACTION`
     /// (e.g. a transaction is already open on this session).
+    #[doc(hidden)]
+    #[deprecated(
+        note = "Use `AsyncConnection::transaction()` for an RAII guard. This method will be \
+                removed in a future release."
+    )]
     pub async fn begin_transaction(&self) -> Result<()> {
-        self.execute_command("BEGIN TRANSACTION").await?;
-        Ok(())
+        self.begin_transaction_raw().await
     }
 
     /// Commits the current transaction (async).
     ///
+    /// **Prefer [`AsyncTransaction::commit`](crate::AsyncTransaction::commit)**
+    /// on the RAII guard returned by [`transaction()`](Self::transaction).
+    /// Hidden from generated rustdoc and deprecated; slated for removal.
+    ///
     /// # Errors
     ///
-    /// Returns [`Error::Server`] if the server rejects `COMMIT` (e.g. no
-    /// transaction is currently open).
+    /// Returns [`Error::Server`] if the server rejects `COMMIT`.
+    #[doc(hidden)]
+    #[deprecated(note = "Use `AsyncTransaction::commit()` on the RAII guard from \
+                `AsyncConnection::transaction()`. This method will be removed in a future release.")]
     pub async fn commit(&self) -> Result<()> {
-        self.execute_command("COMMIT").await?;
-        Ok(())
+        self.commit_raw().await
     }
 
     /// Rolls back the current transaction (async).
     ///
+    /// **Prefer [`AsyncTransaction::rollback`](crate::AsyncTransaction::rollback)**
+    /// on the RAII guard returned by [`transaction()`](Self::transaction).
+    /// Hidden from generated rustdoc and deprecated; slated for removal.
+    ///
     /// # Errors
     ///
-    /// Returns [`Error::Server`] if the server rejects `ROLLBACK` (e.g. no
-    /// transaction is currently open).
+    /// Returns [`Error::Server`] if the server rejects `ROLLBACK`.
+    #[doc(hidden)]
+    #[deprecated(note = "Use `AsyncTransaction::rollback()` on the RAII guard from \
+                `AsyncConnection::transaction()`. This method will be removed in a future release.")]
     pub async fn rollback(&self) -> Result<()> {
-        self.execute_command("ROLLBACK").await?;
-        Ok(())
+        self.rollback_raw().await
     }
 
     /// Starts a transaction with an async RAII guard (async).

hyperdb-api/src/async_transaction.rs

@@ -37,7 +37,11 @@ pub struct AsyncTransaction<'conn> {
 impl<'conn> AsyncTransaction<'conn> {
     /// Creates a new async transaction by issuing `BEGIN TRANSACTION`.
     pub(crate) async fn new(connection: &'conn mut AsyncConnection) -> Result<Self> {
-        connection.begin_transaction().await?;
+        // Use the crate-internal `_raw` family. The matching `pub`
+        // methods on `AsyncConnection` are `#[deprecated]` for
+        // downstream consumers; this guard is the recommended
+        // replacement.
+        connection.begin_transaction_raw().await?;
         Ok(Self {
             connection,
             completed: false,
@@ -48,22 +52,22 @@ impl<'conn> AsyncTransaction<'conn> {
     ///
     /// # Errors
     ///
-    /// Forwards the error from [`AsyncConnection::commit`]. The transaction
+    /// Forwards the error from the server's `COMMIT`. The transaction
     /// is marked completed regardless, so the drop guard will not warn.
     pub async fn commit(mut self) -> Result<()> {
         self.completed = true;
-        self.connection.commit().await
+        self.connection.commit_raw().await
     }
 
     /// Rolls back the transaction explicitly.
     ///
     /// # Errors
     ///
-    /// Forwards the error from [`AsyncConnection::rollback`]. The
+    /// Forwards the error from the server's `ROLLBACK`. The
     /// transaction is marked completed regardless.
     pub async fn rollback(mut self) -> Result<()> {
         self.completed = true;
-        self.connection.rollback().await
+        self.connection.rollback_raw().await
     }
 
     /// Returns a reference to the underlying async connection.