From e705aa5a1d42d3e054b63dd97f663179c2bad7e7 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 02:28:32 -0700 Subject: [PATCH 1/6] chore(api): introduce RowAccessor + change FromRow trait signature (#61, #62 part 1) Foundation commit for the FromRow modernization PR. Introduces RowAccessor and the Row::get_by_name accessor; changes the FromRow trait signature from `&Row` to `RowAccessor<'_>`; deletes the 1/2/3/4- tuple FromRow impls; migrates the 3 hand-written FromRow impls; and wires the cached column-name -> index lookup through fetch_one_as / fetch_all_as (sync + async). This is a breaking change to the FromRow trait. Lands as `chore:` to defer release-please version bump until the v0.3.0 rollup commit. What changed: - New `hyperdb-api/src/row_accessor.rs` with `RowAccessor<'a>`. Borrows &Row + a pre-built HashMap<&str, usize>. Methods: `get(name)`, `get_opt(name)`, `position(idx)`, `row()`. Errors map to Error::Column { kind: Missing | Null | TypeMismatch } and Error::ColumnIndexOutOfBounds. Includes 7 unit tests covering each error path and the happy paths. - New `Row::get_by_name(name)` for hand-coded paths that don't go through FromRow. Uses ResultSchema::column_index (linear scan). Doc recommends #[derive(FromRow)] or fetch_*_as for hot paths (both build the cache once per query). - FromRow trait signature changed: `fn from_row(row: &Row)` -> `fn from_row(row: RowAccessor<'_>)`. Trait rustdoc updated with the recommended derive form (preview for the upcoming proc-macro crate) and a hand-written impl example using `row.get(...)` / `row.get_opt(...)`. - Deleted the 4 tuple FromRow impls (1/2/3/4-tuple). Migration: callers define a struct with #[derive(FromRow)] (added in next commit) or use Row::get(idx) directly. - Migrated the 3 hand-written FromRow impls to the new shape: TestUser (tests/remaining_features_tests.rs), User (tests/async_connection_tests.rs), Order (examples/async_parity_smoke.rs). - fetch_one_as / fetch_all_as (4 sites: sync + async) now build the HashMap lookup once per query from the result schema, then construct a RowAccessor per row. All rows in a result set share the same Arc, so this is safe. - Doctest examples in connection.rs and result.rs updated to the new trait shape; the example_raw_transaction direct call to Order::from_row in async_parity_smoke.rs replaced with a fetch_all_as call (since the trait now takes a RowAccessor that needs pre-built indices). Verification: - cargo build --workspace --all-targets -- clean - cargo clippy --workspace --all-targets -- -D warnings -- clean - cargo test --workspace -- all targets pass (including doctests) - cargo fmt --check -- clean --- hyperdb-api/examples/async_parity_smoke.rs | 27 +- hyperdb-api/src/async_connection.rs | 17 +- hyperdb-api/src/connection.rs | 32 +- hyperdb-api/src/lib.rs | 2 + hyperdb-api/src/result.rs | 140 ++++---- hyperdb-api/src/row_accessor.rs | 313 ++++++++++++++++++ hyperdb-api/tests/async_connection_tests.rs | 10 +- hyperdb-api/tests/remaining_features_tests.rs | 36 +- 8 files changed, 459 insertions(+), 118 deletions(-) create mode 100644 hyperdb-api/src/row_accessor.rs diff --git a/hyperdb-api/examples/async_parity_smoke.rs b/hyperdb-api/examples/async_parity_smoke.rs index 07e5e14..7cb5db8 100644 --- a/hyperdb-api/examples/async_parity_smoke.rs +++ b/hyperdb-api/examples/async_parity_smoke.rs @@ -10,7 +10,7 @@ //! "async is a first-class equivalent of sync" contract holds. use hyperdb_api::{ - AsyncConnection, AsyncConnectionBuilder, CreateMode, FromRow, HyperProcess, Result, Row, + AsyncConnection, AsyncConnectionBuilder, CreateMode, FromRow, HyperProcess, Result, RowAccessor, }; #[derive(Debug)] @@ -25,13 +25,11 @@ struct Order { } impl FromRow for Order { - fn from_row(row: &Row) -> Result { + fn from_row(row: RowAccessor<'_>) -> Result { Ok(Order { - id: row - .get::(0) - .ok_or_else(|| hyperdb_api::Error::conversion("NULL id"))?, - customer: row.get::(1).unwrap_or_default(), - total: row.get::(2).unwrap_or(0.0), + id: row.get("id")?, + customer: row.get_opt("customer")?.unwrap_or_default(), + total: row.get_opt("total")?.unwrap_or(0.0), }) } } @@ -69,15 +67,14 @@ async fn main() -> Result<()> { println!("inserted {count} orders"); // 5. Parameterized query + streaming rowset + struct mapping. - let rs = conn - .query_params( - "SELECT id, customer, total FROM orders WHERE total > $1 ORDER BY id", - &[&50.0_f64], - ) + // Use fetch_all_as to drive Order::from_row through the cached + // RowAccessor path. (Direct `Order::from_row(row)?` is no longer a + // one-liner because the trait takes a RowAccessor with a + // pre-resolved column-index map.) + let high_value: Vec = conn + .fetch_all_as("SELECT id, customer, total FROM orders WHERE total > 50.0 ORDER BY id") .await?; - let rows = rs.collect_rows().await?; - for row in &rows { - let order = Order::from_row(row)?; + for order in &high_value { println!(" high-value: {order:?}"); } diff --git a/hyperdb-api/src/async_connection.rs b/hyperdb-api/src/async_connection.rs index 9843c5f..2c795e0 100644 --- a/hyperdb-api/src/async_connection.rs +++ b/hyperdb-api/src/async_connection.rs @@ -358,7 +358,11 @@ impl AsyncConnection { /// produces when the row cannot be mapped. pub async fn fetch_one_as(&self, query: &str) -> Result { let row = self.fetch_one(query).await?; - T::from_row(&row) + let indices = row + .schema() + .map(crate::row_accessor::RowAccessor::build_indices) + .unwrap_or_default(); + T::from_row(crate::RowAccessor::new(&row, &indices)) } /// Fetches all rows and maps them to structs using [`crate::FromRow`]. @@ -370,7 +374,16 @@ impl AsyncConnection { /// [`FromRow::from_row`](crate::FromRow::from_row) on any row. pub async fn fetch_all_as(&self, query: &str) -> Result> { let rows = self.fetch_all(query).await?; - rows.iter().map(|r| T::from_row(r)).collect() + // Build the column-name → index lookup once from the first + // row's schema; reuse for every row. + let indices = rows + .first() + .and_then(crate::result::Row::schema) + .map(crate::row_accessor::RowAccessor::build_indices) + .unwrap_or_default(); + rows.iter() + .map(|r| T::from_row(crate::RowAccessor::new(r, &indices))) + .collect() } /// Fetches a single non-NULL scalar value. Errors on empty / NULL. diff --git a/hyperdb-api/src/connection.rs b/hyperdb-api/src/connection.rs index ecfbc12..5629233 100644 --- a/hyperdb-api/src/connection.rs +++ b/hyperdb-api/src/connection.rs @@ -713,15 +713,15 @@ impl Connection { /// # Example /// /// ```no_run - /// use hyperdb_api::{Connection, CreateMode, Row, FromRow, Result}; + /// use hyperdb_api::{Connection, CreateMode, FromRow, RowAccessor, Result}; /// /// struct User { id: i32, name: String } /// /// impl FromRow for User { - /// fn from_row(row: &Row) -> Result { + /// fn from_row(row: RowAccessor<'_>) -> Result { /// Ok(User { - /// id: row.get::(0).ok_or_else(|| hyperdb_api::Error::conversion("NULL id"))?, - /// name: row.get::(1).unwrap_or_default(), + /// id: row.get("id")?, + /// name: row.get_opt("name")?.unwrap_or_default(), /// }) /// } /// } @@ -741,7 +741,11 @@ impl Connection { /// produces when the row cannot be mapped into `T`. pub fn fetch_one_as(&self, query: &str) -> Result { let row = self.fetch_one(query)?; - T::from_row(&row) + let indices = row + .schema() + .map(crate::row_accessor::RowAccessor::build_indices) + .unwrap_or_default(); + T::from_row(crate::RowAccessor::new(&row, &indices)) } /// Fetches all rows and maps them to structs using [`FromRow`](crate::FromRow). @@ -749,11 +753,11 @@ impl Connection { /// # Example /// /// ```no_run - /// # use hyperdb_api::{Connection, CreateMode, Row, FromRow, Result}; + /// # use hyperdb_api::{Connection, FromRow, RowAccessor, Result}; /// # struct User { id: i32, name: String } /// # impl FromRow for User { - /// # fn from_row(row: &Row) -> Result { - /// # Ok(User { id: row.get::(0).unwrap_or(0), name: row.get::(1).unwrap_or_default() }) + /// # fn from_row(row: RowAccessor<'_>) -> Result { + /// # Ok(User { id: row.get("id")?, name: row.get_opt("name")?.unwrap_or_default() }) /// # } /// # } /// # fn example(conn: &Connection) -> Result<()> { @@ -770,7 +774,17 @@ impl Connection { /// [`FromRow::from_row`](crate::FromRow::from_row) on any of the rows. pub fn fetch_all_as(&self, query: &str) -> Result> { let rows = self.fetch_all(query)?; - rows.iter().map(|r| T::from_row(r)).collect() + // Build the column-name → index lookup once from the first + // row's schema; reuse for every row. All rows in a result set + // share the same `Arc`, so this is safe. + let indices = rows + .first() + .and_then(crate::result::Row::schema) + .map(crate::row_accessor::RowAccessor::build_indices) + .unwrap_or_default(); + rows.iter() + .map(|r| T::from_row(crate::RowAccessor::new(r, &indices))) + .collect() } /// Fetches a single scalar value from a query. diff --git a/hyperdb-api/src/lib.rs b/hyperdb-api/src/lib.rs index e386109..45dd29b 100644 --- a/hyperdb-api/src/lib.rs +++ b/hyperdb-api/src/lib.rs @@ -162,6 +162,7 @@ mod process; mod query_result; pub(crate) mod query_stats; mod result; +mod row_accessor; mod server_version; mod table_definition; mod transaction; @@ -199,6 +200,7 @@ pub use names::{ pub use process::{HyperProcess, ListenMode, Parameters, TransportMode}; pub use query_stats::{LogFileStatsProvider, QueryStats, QueryStatsProvider}; pub use result::{FromRow, ResultColumn, ResultSchema, Row, RowIterator, RowValue, Rowset}; +pub use row_accessor::RowAccessor; pub use server_version::ServerVersion; pub use table_definition::{ColumnDefinition, Persistence, TableDefinition}; pub use transaction::Transaction; diff --git a/hyperdb-api/src/result.rs b/hyperdb-api/src/result.rs index 1b6d77d..779cdab 100644 --- a/hyperdb-api/src/result.rs +++ b/hyperdb-api/src/result.rs @@ -226,17 +226,18 @@ impl Row { /// /// # Example /// + /// Most callers should reach for [`crate::FromRow`] + + /// [`crate::RowAccessor`] for typed mapping. `try_get` is the + /// underlying positional building block; useful when you need + /// indexed access from a hand-rolled loop. + /// /// ```no_run - /// # use hyperdb_api::{Row, FromRow, Result}; - /// # struct User { id: i32, name: String } - /// impl FromRow for User { - /// fn from_row(row: &Row) -> Result { - /// Ok(User { - /// id: row.try_get::(0, "id")?, - /// name: row.try_get::(1, "name")?, - /// }) - /// } - /// } + /// # use hyperdb_api::{Row, Result}; + /// # fn read(row: &Row) -> Result<(i32, String)> { + /// let id: i32 = row.try_get(0, "id")?; + /// let name: String = row.try_get(1, "name")?; + /// # Ok((id, name)) + /// # } /// ``` /// /// # Errors @@ -261,6 +262,35 @@ impl Row { }) } + /// Looks up a column by name and returns its value as `T`. + /// + /// Convenient for hand-coded paths that aren't using + /// [`FromRow`]. The lookup is a linear scan over + /// [`ResultSchema::column_index`]; for hot paths (many rows × many + /// fields), prefer + /// [`fetch_one_as`](crate::Connection::fetch_one_as) / + /// [`fetch_all_as`](crate::Connection::fetch_all_as), which build + /// a cached column-name → index lookup once per query and hand + /// every `FromRow` impl a [`RowAccessor`](crate::RowAccessor) that + /// reuses it. + /// + /// # Errors + /// + /// - [`crate::Error::Column`] with [`crate::ColumnErrorKind::Missing`] + /// if no column with `name` exists in the row's schema (or the + /// row has no schema attached). + /// - [`crate::Error::Conversion`] if the cell is `NULL` or cannot + /// be decoded as `T`. (Inherited from [`Self::try_get`].) + pub fn get_by_name(&self, name: &str) -> crate::error::Result { + let idx = self + .schema() + .and_then(|s| s.column_index(name)) + .ok_or_else(|| { + crate::error::Error::column(name, crate::error::ColumnErrorKind::Missing) + })?; + self.try_get(idx, name) + } + /// Returns an Arrow column reference, or `None` if the index is out of bounds. /// /// This is a safe wrapper around `batch.column(idx)` that avoids panicking. @@ -715,75 +745,65 @@ impl RowValue for hyperdb_api_core::types::Numeric { /// Trait for types that can be constructed from a database row. /// -/// Implement this trait for your structs to enable direct mapping from -/// query results using [`Connection::fetch_one_as`](crate::Connection::fetch_one_as), -/// [`Connection::fetch_all_as`](crate::Connection::fetch_all_as), or by calling -/// [`FromRow::from_row`](FromRow::from_row) on each [`Row`](crate::Row) from a [`Rowset`](crate::Rowset). +/// Used by [`Connection::fetch_one_as`](crate::Connection::fetch_one_as) +/// and [`Connection::fetch_all_as`](crate::Connection::fetch_all_as) +/// to map query results into typed structs. Implementations receive +/// a [`RowAccessor`](crate::RowAccessor), which provides name-based +/// access via a column-name → index lookup built once per query. /// -/// # Example +/// # Recommended: derive /// -/// ```no_run -/// use hyperdb_api::{Row, FromRow, Result}; +/// In most cases the `#[derive(FromRow)]` macro handles the mapping +/// for you — match struct field names to column names automatically, +/// with `#[hyperdb(rename = "...")]` for cases where they differ: +/// +/// ```ignore +/// use hyperdb_api::FromRow; /// +/// #[derive(FromRow)] /// struct User { /// id: i32, /// name: String, -/// active: bool, +/// #[hyperdb(rename = "email_address")] +/// email: Option, /// } +/// ``` +/// +/// # Hand-written impl +/// +/// For custom mapping logic (computed fields, multi-column composition, +/// etc.) implement the trait directly: +/// +/// ```no_run +/// use hyperdb_api::{FromRow, RowAccessor, Result}; +/// +/// struct User { id: i32, name: String, active: bool } /// /// impl FromRow for User { -/// fn from_row(row: &Row) -> Result { +/// fn from_row(row: RowAccessor<'_>) -> Result { /// Ok(User { -/// id: row.get::(0).ok_or_else(|| hyperdb_api::Error::conversion("NULL id"))?, -/// name: row.get::(1).unwrap_or_default(), -/// active: row.get::(2).unwrap_or(false), +/// id: row.get("id")?, +/// name: row.get("name")?, +/// active: row.get("active")?, /// }) /// } /// } /// ``` +/// +/// For ad-hoc tuple destructuring of small results, use +/// [`Row::get`](crate::Row::get) directly — there are no blanket +/// tuple `FromRow` impls. Define a struct with `#[derive(FromRow)]` +/// for typed access in `fetch_*_as`. pub trait FromRow: Sized { /// Constructs an instance from a database row. /// /// # Errors /// - /// Returns an [`Error`](crate::Error) — typically [`crate::Error::Conversion`] — - /// when a required column is missing, SQL `NULL`, or cannot be - /// decoded as the expected type. Implementations decide the exact - /// failure shape. - fn from_row(row: &Row) -> crate::error::Result; -} - -// Tuple implementations for common patterns - -impl FromRow for (Option,) { - fn from_row(row: &Row) -> crate::error::Result { - Ok((row.get::(0),)) - } -} - -impl FromRow for (Option, Option) { - fn from_row(row: &Row) -> crate::error::Result { - Ok((row.get::(0), row.get::(1))) - } -} - -impl FromRow for (Option, Option, Option) { - fn from_row(row: &Row) -> crate::error::Result { - Ok((row.get::(0), row.get::(1), row.get::(2))) - } -} - -impl FromRow - for (Option, Option, Option, Option) -{ - fn from_row(row: &Row) -> crate::error::Result { - Ok(( - row.get::(0), - row.get::(1), - row.get::(2), - row.get::(3), - )) - } + /// Returns an [`Error`](crate::Error) — typically + /// [`crate::Error::Column`] — when a required column is missing, + /// SQL `NULL`, or cannot be decoded as the expected type. + /// Implementations decide the exact failure shape. + fn from_row(row: crate::RowAccessor<'_>) -> crate::error::Result; } // ============================================================================= diff --git a/hyperdb-api/src/row_accessor.rs b/hyperdb-api/src/row_accessor.rs new file mode 100644 index 0000000..0b72ee6 --- /dev/null +++ b/hyperdb-api/src/row_accessor.rs @@ -0,0 +1,313 @@ +// Copyright (c) 2026, Salesforce, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 OR MIT + +//! [`RowAccessor`] — name-based column access for [`FromRow`] impls +//! with cached column-name → index resolution. +//! +//! When a typed query is consumed via [`Connection::fetch_one_as`] / +//! [`Connection::fetch_all_as`] (and the async equivalents), the engine +//! resolves each column name to its position in the result schema +//! **once per query** and hands a `RowAccessor` to every `FromRow` impl. +//! Inside a `FromRow` impl, calling `accessor.get("col")` is a single +//! `HashMap` lookup followed by typed access — no per-call linear scan +//! over the schema. +//! +//! For one-off named access on a [`Row`] (outside `fetch_*_as`), use +//! [`Row::get_by_name`] instead — it does a linear scan but doesn't +//! require building a cache. +//! +//! [`Connection::fetch_one_as`]: crate::Connection::fetch_one_as +//! [`Connection::fetch_all_as`]: crate::Connection::fetch_all_as +//! [`Row::get_by_name`]: crate::Row::get_by_name + +use std::collections::HashMap; + +use crate::error::{ColumnErrorKind, Error, Result}; +use crate::result::{Row, RowValue}; + +/// A view over a [`Row`] that supports name-based access via a +/// pre-resolved column-name → index lookup table. +/// +/// `RowAccessor` is the parameter type of [`FromRow::from_row`]; it +/// borrows the row and a shared lookup map built once per query in +/// [`fetch_one_as`](crate::Connection::fetch_one_as) / +/// [`fetch_all_as`](crate::Connection::fetch_all_as). +/// +/// # Example +/// +/// ```no_run +/// use hyperdb_api::{FromRow, RowAccessor, Result}; +/// +/// struct User { id: i32, name: String, email: Option } +/// +/// impl FromRow for User { +/// fn from_row(row: RowAccessor<'_>) -> Result { +/// Ok(User { +/// id: row.get("id")?, +/// name: row.get("name")?, +/// email: row.get_opt("email")?, +/// }) +/// } +/// } +/// ``` +#[derive(Debug)] +pub struct RowAccessor<'a> { + row: &'a Row, + indices: &'a HashMap<&'a str, usize>, +} + +impl<'a> RowAccessor<'a> { + /// Constructs a new `RowAccessor` over the given row and pre-built + /// lookup map. Crate-internal: callers go through `fetch_*_as` to + /// get a `RowAccessor`, never construct one directly. + pub(crate) fn new(row: &'a Row, indices: &'a HashMap<&'a str, usize>) -> Self { + Self { row, indices } + } + + /// Builds a `name → index` lookup table from a [`ResultSchema`]. + /// + /// Used by `fetch_*_as` to resolve names once per query before + /// iterating rows. Consumes O(N) time and allocates one entry per + /// column. + /// + /// [`ResultSchema`]: crate::ResultSchema + pub(crate) fn build_indices(schema: &'a crate::ResultSchema) -> HashMap<&'a str, usize> { + let mut map = HashMap::with_capacity(schema.column_count()); + for i in 0..schema.column_count() { + map.insert(schema.column(i).name(), i); + } + map + } + + /// Returns the underlying [`Row`]. + /// + /// Useful for callers that need access to row methods not exposed + /// through this accessor (e.g. positional `Row::get`). + #[must_use] + pub fn row(&self) -> &Row { + self.row + } + + /// Returns the named column's value, decoded as `T`. + /// + /// # Errors + /// + /// - [`Error::Column`] with [`ColumnErrorKind::Missing`] if `name` + /// is not in the result schema. + /// - [`Error::Column`] with [`ColumnErrorKind::Null`] if the cell + /// is SQL `NULL`. + /// - [`Error::Column`] with [`ColumnErrorKind::TypeMismatch`] if + /// the cell value cannot be decoded as `T`. + pub fn get(&self, name: &str) -> Result { + let idx = self + .indices + .get(name) + .copied() + .ok_or_else(|| Error::column(name, ColumnErrorKind::Missing))?; + match self.row.get::(idx) { + Some(v) => Ok(v), + None => { + // Disambiguate NULL from type-mismatch by re-checking + // the underlying cell. `row.is_null(idx)` is the source + // of truth for SQL NULL. + if self.row.is_null(idx) { + Err(Error::column(name, ColumnErrorKind::Null)) + } else { + let actual = self + .row + .sql_type(idx) + .map_or_else(|| "".to_string(), |t| format!("{t:?}")); + Err(Error::column( + name, + ColumnErrorKind::TypeMismatch { + expected: std::any::type_name::().to_string(), + actual, + }, + )) + } + } + } + } + + /// Returns the named column's value as `Option`. SQL `NULL` + /// becomes `None`; missing columns and type mismatches still error. + /// + /// # Errors + /// + /// - [`Error::Column`] with [`ColumnErrorKind::Missing`] if `name` + /// is not in the result schema. + /// - [`Error::Column`] with [`ColumnErrorKind::TypeMismatch`] if + /// the cell is non-NULL but cannot be decoded as `T`. + pub fn get_opt(&self, name: &str) -> Result> { + let idx = self + .indices + .get(name) + .copied() + .ok_or_else(|| Error::column(name, ColumnErrorKind::Missing))?; + if self.row.is_null(idx) { + return Ok(None); + } + if let Some(v) = self.row.get::(idx) { + Ok(Some(v)) + } else { + let actual = self + .row + .sql_type(idx) + .map_or_else(|| "".to_string(), |t| format!("{t:?}")); + Err(Error::column( + name, + ColumnErrorKind::TypeMismatch { + expected: std::any::type_name::().to_string(), + actual, + }, + )) + } + } + + /// Positional escape hatch: returns the value at column `idx`, + /// decoded as `T`. + /// + /// # Errors + /// + /// - [`Error::ColumnIndexOutOfBounds`] if `idx` is past the row's + /// column count. + /// - [`Error::Conversion`] if the cell is `NULL` or cannot be + /// decoded as `T`. (Wraps [`Row::try_get`].) + pub fn position(&self, idx: usize) -> Result { + if idx >= self.row.column_count() { + return Err(Error::column_index_out_of_bounds( + idx, + self.row.column_count(), + )); + } + // Reuse Row::try_get's NULL/decode-error path. Synthesize a + // column-name label for the error message. + let label = format!("col[{idx}]"); + self.row.try_get::(idx, &label) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::result::{ResultColumn, ResultSchema}; + use arrow::array::{Int32Array, StringArray}; + use arrow::datatypes::{DataType as ArrowType, Field, Schema}; + use arrow::record_batch::RecordBatch; + use hyperdb_api_core::types::SqlType; + use std::sync::Arc; + + /// Build a single-row `(id INT, name TEXT)` Arrow batch + matching + /// `ResultSchema` for use in `RowAccessor` unit tests. + fn user_row(id: Option, name: Option<&str>) -> (Row, Arc) { + let id_array = Int32Array::from(vec![id]); + let name_array = StringArray::from(vec![name]); + let arrow_schema = Arc::new(Schema::new(vec![ + Field::new("id", ArrowType::Int32, true), + Field::new("name", ArrowType::Utf8, true), + ])); + let batch = Arc::new( + RecordBatch::try_new(arrow_schema, vec![Arc::new(id_array), Arc::new(name_array)]) + .expect("batch"), + ); + let schema = Arc::new(ResultSchema::from_columns(vec![ + ResultColumn::new("id", SqlType::int(), 0), + ResultColumn::new("name", SqlType::text(), 1), + ])); + let row = Row::from_arrow(batch, 0, Some(Arc::clone(&schema))); + (row, schema) + } + + #[test] + fn missing_column_errors_with_kind_missing() { + let (row, schema) = user_row(Some(1), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let err = accessor.get::("does_not_exist").unwrap_err(); + match err { + Error::Column { name, kind } => { + assert_eq!(name, "does_not_exist"); + assert!(matches!(kind, ColumnErrorKind::Missing)); + } + other => panic!("expected Error::Column {{ kind: Missing }}, got {other:?}"), + } + } + + #[test] + fn null_in_required_column_errors_with_kind_null() { + let (row, schema) = user_row(Some(1), None); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let err = accessor.get::("name").unwrap_err(); + match err { + Error::Column { name, kind } => { + assert_eq!(name, "name"); + assert!(matches!(kind, ColumnErrorKind::Null)); + } + other => panic!("expected Error::Column {{ kind: Null }}, got {other:?}"), + } + } + + #[test] + fn null_in_optional_column_returns_none() { + let (row, schema) = user_row(Some(1), None); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let v: Option = accessor.get_opt("name").expect("get_opt for NULL"); + assert_eq!(v, None); + } + + #[test] + fn happy_path_get_returns_value() { + let (row, schema) = user_row(Some(42), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let id: i32 = accessor.get("id").expect("get id"); + let name: String = accessor.get("name").expect("get name"); + assert_eq!(id, 42); + assert_eq!(name, "alice"); + } + + #[test] + fn happy_path_get_opt_returns_some() { + let (row, schema) = user_row(Some(42), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let id: Option = accessor.get_opt("id").expect("get_opt id"); + let name: Option = accessor.get_opt("name").expect("get_opt name"); + assert_eq!(id, Some(42)); + assert_eq!(name, Some("alice".to_string())); + } + + #[test] + fn position_out_of_range_errors_with_index_oob() { + let (row, schema) = user_row(Some(1), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + // Row has 2 columns; position 5 is out of range. + let err = accessor.position::(5).unwrap_err(); + match err { + Error::ColumnIndexOutOfBounds { idx, column_count } => { + assert_eq!(idx, 5); + assert_eq!(column_count, 2); + } + other => panic!("expected Error::ColumnIndexOutOfBounds, got {other:?}"), + } + } + + #[test] + fn position_in_range_returns_value() { + let (row, schema) = user_row(Some(42), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let id: i32 = accessor.position(0).expect("position 0"); + assert_eq!(id, 42); + } +} diff --git a/hyperdb-api/tests/async_connection_tests.rs b/hyperdb-api/tests/async_connection_tests.rs index 5cb86d3..0b4611a 100644 --- a/hyperdb-api/tests/async_connection_tests.rs +++ b/hyperdb-api/tests/async_connection_tests.rs @@ -9,7 +9,7 @@ mod common; use common::{test_hyper_params, test_result_path}; -use hyperdb_api::{AsyncConnection, CreateMode, FromRow, HyperProcess, Result, Row}; +use hyperdb_api::{AsyncConnection, CreateMode, FromRow, HyperProcess, Result}; async fn fresh_async_conn(name: &str) -> Result<(HyperProcess, AsyncConnection)> { let db_path = test_result_path(name, "hyper")?; @@ -120,12 +120,10 @@ struct User { } impl FromRow for User { - fn from_row(row: &Row) -> Result { + fn from_row(row: hyperdb_api::RowAccessor<'_>) -> Result { Ok(User { - id: row - .get::(0) - .ok_or_else(|| hyperdb_api::Error::conversion("NULL id"))?, - name: row.get::(1), + id: row.get("id")?, + name: row.get_opt("name")?, }) } } diff --git a/hyperdb-api/tests/remaining_features_tests.rs b/hyperdb-api/tests/remaining_features_tests.rs index 45fdf95..3b86358 100644 --- a/hyperdb-api/tests/remaining_features_tests.rs +++ b/hyperdb-api/tests/remaining_features_tests.rs @@ -14,7 +14,7 @@ mod common; use common::TestConnection; -use hyperdb_api::{Catalog, FromRow, Row, ServerVersion}; +use hyperdb_api::{Catalog, FromRow, ServerVersion}; // ============================================================================= // #7: Batch Statement Execution @@ -249,13 +249,11 @@ struct TestUser { } impl FromRow for TestUser { - fn from_row(row: &Row) -> hyperdb_api::Result { + fn from_row(row: hyperdb_api::RowAccessor<'_>) -> hyperdb_api::Result { Ok(TestUser { - id: row - .get::(0) - .ok_or_else(|| hyperdb_api::Error::conversion("NULL id"))?, - name: row.get::(1).unwrap_or_default(), - score: row.get::(2).unwrap_or(0.0), + id: row.get("id")?, + name: row.get_opt("name")?.unwrap_or_default(), + score: row.get_opt("score")?.unwrap_or(0.0), }) } } @@ -308,25 +306,11 @@ fn test_fetch_all_as() { assert_eq!(users[2].name, "Carol"); } -#[test] -fn test_from_row_tuple() { - let test = TestConnection::new().expect("Failed to create test connection"); - - test.execute_command("CREATE TABLE tuple_test (id INT NOT NULL, name TEXT)") - .expect("create"); - test.execute_command("INSERT INTO tuple_test VALUES (1, 'Alice')") - .expect("insert"); - - let row = test - .connection - .fetch_one("SELECT id, name FROM tuple_test") - .expect("fetch"); - - let tuple: (Option, Option) = - <(Option, Option) as FromRow>::from_row(&row).expect("from_row"); - assert_eq!(tuple.0, Some(1)); - assert_eq!(tuple.1, Some("Alice".to_string())); -} +// `test_from_row_tuple` was removed in v0.3.0 along with the blanket +// `(Option,)` … `(Option, Option, Option, Option)` +// `FromRow` impls. For ad-hoc tuple-shaped destructuring, callers +// should now use `Row::get(idx)` directly on each row, or define a +// struct with `#[derive(FromRow)]`. // ============================================================================= // #16: Connection Health (ping) From 70356c404394b951352f12455d09bb9b76f666e6 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 02:44:22 -0700 Subject: [PATCH 2/6] chore(api): add hyperdb-api-derive crate with #[derive(FromRow)] (#61, #62 part 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the proc-macro crate `hyperdb-api-derive` that provides `#[derive(FromRow)]`. Re-exported from `hyperdb-api` so callers don't need a direct dependency — same pattern as serde / thiserror. What changed: - New workspace member `hyperdb-api-derive/` with `proc-macro = true`, deps on syn v2 (with the `full` feature), quote, proc-macro2. - `#[derive(FromRow)]` proc-macro generates an `impl FromRow` that uses the `RowAccessor` API from Commit 1: `row.get("col")?` for required fields, `row.get_opt("col")?` for `Option` fields. Field name → column name match is exact by default; the `#[hyperdb(rename = "...")]` attribute overrides on a per-field basis. - Helpful compile errors for unsupported shapes: - Tuple structs → "tuple-struct fields are not supported" - Enums → "FromRow cannot be derived on enums" - Unions → "FromRow cannot be derived on unions" - Unknown attribute → "unrecognized hyperdb attribute `{x}`; expected `rename = \"...\"`" - `hyperdb-api`'s lib.rs adds `pub use hyperdb_api_derive::FromRow;` alongside the trait re-export. The derive macro and the trait share the name "FromRow" (Rust treats them as different namespaces). - 3 integration tests in `hyperdb-api/tests/remaining_features_tests.rs`: - `test_derive_from_row_parity_with_handwritten`: derived `TestUserDerived` produces identical values to the hand-written `TestUser` impl for the same query. - `test_derive_from_row_with_rename`: `#[hyperdb(rename = "score")]` redirects field-name lookup. - `test_derive_from_row_missing_column_errors`: a derived struct with a column not in the SELECT list surfaces as `Error::Column { kind: Missing }`. Verification: - cargo build --workspace --all-targets — clean - cargo clippy --workspace --all-targets -- -D warnings — clean - cargo test --workspace --lib + --doc — all pass - cargo fmt --check — clean --- Cargo.lock | 10 + Cargo.toml | 1 + hyperdb-api-derive/Cargo.toml | 23 +++ hyperdb-api-derive/README.md | 26 +++ hyperdb-api-derive/src/lib.rs | 172 ++++++++++++++++++ hyperdb-api/Cargo.toml | 1 + hyperdb-api/src/lib.rs | 5 + hyperdb-api/tests/remaining_features_tests.rs | 133 ++++++++++++++ release-please-config.json | 1 + 9 files changed, 372 insertions(+) create mode 100644 hyperdb-api-derive/Cargo.toml create mode 100644 hyperdb-api-derive/README.md create mode 100644 hyperdb-api-derive/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 06aa4e6..c47d2d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1817,6 +1817,7 @@ dependencies = [ "bytes", "deadpool", "hyperdb-api-core", + "hyperdb-api-derive", "libc", "rand 0.10.1", "serde", @@ -1870,6 +1871,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "hyperdb-api-derive" +version = "0.2.3" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "hyperdb-api-node" version = "0.2.3" diff --git a/Cargo.toml b/Cargo.toml index c385914..0762c64 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ resolver = "2" members = [ "hyperdb-api-core", "hyperdb-api", + "hyperdb-api-derive", "hyperdb-api-node", "hyperdb-api-salesforce", "hyperdb-mcp", diff --git a/hyperdb-api-derive/Cargo.toml b/hyperdb-api-derive/Cargo.toml new file mode 100644 index 0000000..fda238c --- /dev/null +++ b/hyperdb-api-derive/Cargo.toml @@ -0,0 +1,23 @@ +[package] +name = "hyperdb-api-derive" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +description = "Procedural macros for hyperdb-api (FromRow derive)" +license.workspace = true +repository.workspace = true +homepage.workspace = true +readme = "README.md" +keywords = ["database", "hyper", "derive", "proc-macro"] +categories = ["database"] + +[lib] +proc-macro = true + +[dependencies] +syn = { version = "2", features = ["full"] } +quote = "1" +proc-macro2 = "1" + +[lints] +workspace = true diff --git a/hyperdb-api-derive/README.md b/hyperdb-api-derive/README.md new file mode 100644 index 0000000..344de83 --- /dev/null +++ b/hyperdb-api-derive/README.md @@ -0,0 +1,26 @@ +# hyperdb-api-derive + +⚠️ **This crate is an implementation detail of +[`hyperdb-api`](https://crates.io/crates/hyperdb-api).** +Use `hyperdb-api` directly; don't add `hyperdb-api-derive` to your dependencies. + +This crate provides the procedural macros that `hyperdb-api` re-exports +(currently just `#[derive(FromRow)]`). Use them through `hyperdb-api`: + +```rust +use hyperdb_api::FromRow; + +#[derive(FromRow)] +struct User { + id: i32, + name: String, + #[hyperdb(rename = "email_address")] + email: Option, +} +``` + +See the [`hyperdb-api` docs](https://docs.rs/hyperdb-api) for full usage. + +This crate has no stable API. Breaking changes land here without a major +version bump of `hyperdb-api-derive`; your build may break on any +`hyperdb-api` patch release if you depend on `hyperdb-api-derive` directly. diff --git a/hyperdb-api-derive/src/lib.rs b/hyperdb-api-derive/src/lib.rs new file mode 100644 index 0000000..9fbc23c --- /dev/null +++ b/hyperdb-api-derive/src/lib.rs @@ -0,0 +1,172 @@ +// Copyright (c) 2026, Salesforce, Inc. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 OR MIT + +//! Procedural macros for `hyperdb-api`. +//! +//! Currently exposes `#[derive(FromRow)]`, which generates an +//! [`hyperdb_api::FromRow`] impl for a struct by mapping each field +//! to a column with the matching name. +//! +//! Re-exported by `hyperdb-api` so callers don't need to add this +//! crate as a direct dependency. Use it as `use hyperdb_api::FromRow;` +//! and `#[derive(FromRow)]` on a struct. +//! +//! # Example +//! +//! ```ignore +//! use hyperdb_api::FromRow; +//! +//! #[derive(FromRow)] +//! struct User { +//! id: i32, +//! name: String, +//! // Map to a different column name with `rename`: +//! #[hyperdb(rename = "email_address")] +//! email: Option, +//! } +//! ``` +//! +//! # Attributes +//! +//! - `#[hyperdb(rename = "...")]` on a field uses the given column +//! name instead of the field name. +//! - Field types of `Option` use [`RowAccessor::get_opt`] +//! (NULL → `None`); other field types use [`RowAccessor::get`] +//! (NULL → error). +//! +//! [`hyperdb_api::FromRow`]: https://docs.rs/hyperdb-api +//! [`RowAccessor::get_opt`]: https://docs.rs/hyperdb-api +//! [`RowAccessor::get`]: https://docs.rs/hyperdb-api + +use proc_macro::TokenStream; +use proc_macro2::TokenStream as TokenStream2; +use quote::quote; +use syn::{ + parse_macro_input, Data, DataStruct, DeriveInput, Field, Fields, GenericArgument, LitStr, + PathArguments, Type, TypePath, +}; + +/// Derives `hyperdb_api::FromRow` for a struct. +/// +/// See the crate-level documentation for the full feature list. +#[proc_macro_derive(FromRow, attributes(hyperdb))] +pub fn from_row_derive(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + match expand(&input) { + Ok(ts) => ts.into(), + Err(e) => e.to_compile_error().into(), + } +} + +fn expand(input: &DeriveInput) -> syn::Result { + let name = &input.ident; + let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); + + let fields = match &input.data { + Data::Struct(DataStruct { + fields: Fields::Named(named), + .. + }) => &named.named, + Data::Struct(_) => { + return Err(syn::Error::new_spanned( + &input.ident, + "FromRow can only be derived on structs with named fields", + )); + } + Data::Enum(_) => { + return Err(syn::Error::new_spanned( + &input.ident, + "FromRow cannot be derived on enums", + )); + } + Data::Union(_) => { + return Err(syn::Error::new_spanned( + &input.ident, + "FromRow cannot be derived on unions", + )); + } + }; + + let assignments = fields + .iter() + .map(field_assignment) + .collect::>>()?; + + Ok(quote! { + #[automatically_derived] + impl #impl_generics ::hyperdb_api::FromRow for #name #ty_generics #where_clause { + fn from_row( + row: ::hyperdb_api::RowAccessor<'_>, + ) -> ::hyperdb_api::Result { + Ok(Self { + #(#assignments),* + }) + } + } + }) +} + +/// Generates `field_name: row.get("col")?` (or `get_opt` for `Option` fields). +fn field_assignment(field: &Field) -> syn::Result { + let ident = field + .ident + .as_ref() + .ok_or_else(|| syn::Error::new_spanned(field, "tuple-struct fields are not supported"))?; + let column_name = column_name_for(field, ident)?; + let column_lit = LitStr::new(&column_name, ident.span()); + + let getter = if is_option_type(&field.ty) { + quote!(row.get_opt(#column_lit)?) + } else { + quote!(row.get(#column_lit)?) + }; + + Ok(quote! { #ident: #getter }) +} + +/// Reads `#[hyperdb(rename = "...")]` from a field's attributes; falls back +/// to the field's identifier as the column name. +fn column_name_for(field: &Field, default: &syn::Ident) -> syn::Result { + for attr in &field.attrs { + if !attr.path().is_ident("hyperdb") { + continue; + } + let mut rename: Option = None; + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("rename") { + let s: LitStr = meta.value()?.parse()?; + rename = Some(s.value()); + Ok(()) + } else { + Err(meta.error(format!( + "unrecognized hyperdb attribute `{}`; expected `rename = \"...\"`", + meta.path + .get_ident() + .map_or_else(|| "?".to_string(), ToString::to_string) + ))) + } + })?; + if let Some(name) = rename { + return Ok(name); + } + } + Ok(default.to_string()) +} + +/// Detects `Option` (any path ending in `Option`). +fn is_option_type(ty: &Type) -> bool { + let Type::Path(TypePath { path, qself: None }) = ty else { + return false; + }; + let Some(last) = path.segments.last() else { + return false; + }; + if last.ident != "Option" { + return false; + } + matches!( + last.arguments, + PathArguments::AngleBracketed(ref args) + if matches!(args.args.first(), Some(GenericArgument::Type(_))) + ) +} diff --git a/hyperdb-api/Cargo.toml b/hyperdb-api/Cargo.toml index d79e11e..cd8f86d 100644 --- a/hyperdb-api/Cargo.toml +++ b/hyperdb-api/Cargo.toml @@ -15,6 +15,7 @@ autobenches = false [dependencies] # x-release-please-start-version hyperdb-api-core = { path = "../hyperdb-api-core", version = "=0.2.3" } +hyperdb-api-derive = { path = "../hyperdb-api-derive", version = "=0.2.3" } # x-release-please-end bytes = { workspace = true } thiserror = { workspace = true } diff --git a/hyperdb-api/src/lib.rs b/hyperdb-api/src/lib.rs index 45dd29b..b4fd2e4 100644 --- a/hyperdb-api/src/lib.rs +++ b/hyperdb-api/src/lib.rs @@ -201,6 +201,11 @@ pub use process::{HyperProcess, ListenMode, Parameters, TransportMode}; pub use query_stats::{LogFileStatsProvider, QueryStats, QueryStatsProvider}; pub use result::{FromRow, ResultColumn, ResultSchema, Row, RowIterator, RowValue, Rowset}; pub use row_accessor::RowAccessor; + +// Re-export the `#[derive(FromRow)]` proc-macro from the companion +// crate so callers don't need to add `hyperdb-api-derive` as a direct +// dependency. Same pattern as serde / thiserror. +pub use hyperdb_api_derive::FromRow; pub use server_version::ServerVersion; pub use table_definition::{ColumnDefinition, Persistence, TableDefinition}; pub use transaction::Transaction; diff --git a/hyperdb-api/tests/remaining_features_tests.rs b/hyperdb-api/tests/remaining_features_tests.rs index 3b86358..c4e891b 100644 --- a/hyperdb-api/tests/remaining_features_tests.rs +++ b/hyperdb-api/tests/remaining_features_tests.rs @@ -312,6 +312,139 @@ fn test_fetch_all_as() { // should now use `Row::get(idx)` directly on each row, or define a // struct with `#[derive(FromRow)]`. +// ============================================================================= +// #17 cont: #[derive(FromRow)] parity with hand-written impl +// ============================================================================= +// +// Verifies that the proc-macro-generated impl produces the same values +// as the hand-written `TestUser` impl for the same query. + +#[derive(Debug, PartialEq, FromRow)] +struct TestUserDerived { + id: i32, + name: Option, + score: Option, +} + +#[test] +fn test_derive_from_row_parity_with_handwritten() { + let test = TestConnection::new().expect("Failed to create test connection"); + + test.execute_command( + "CREATE TABLE derive_parity (id INT NOT NULL, name TEXT, score DOUBLE PRECISION)", + ) + .expect("create"); + test.execute_command("INSERT INTO derive_parity VALUES (1, 'Alice', 95.5), (2, 'Bob', 87.0)") + .expect("insert"); + + let handwritten: Vec = test + .connection + .fetch_all_as("SELECT id, name, score FROM derive_parity ORDER BY id") + .expect("fetch_all_as TestUser"); + + let derived: Vec = test + .connection + .fetch_all_as("SELECT id, name, score FROM derive_parity ORDER BY id") + .expect("fetch_all_as TestUserDerived"); + + assert_eq!(handwritten.len(), 2); + assert_eq!(derived.len(), 2); + for (hw, drv) in handwritten.iter().zip(derived.iter()) { + assert_eq!(hw.id, drv.id); + assert_eq!(Some(hw.name.clone()), drv.name); + assert_eq!(Some(hw.score), drv.score); + } +} + +#[test] +fn test_derive_from_row_null_in_optional_field_returns_none() { + // Regression test for the Option -> get_opt path: a NULL cell + // in an Option field on a derived FromRow must produce None, + // not an Error::Column { kind: Null }. + let test = TestConnection::new().expect("Failed to create test connection"); + + test.execute_command( + "CREATE TABLE derive_null (id INT NOT NULL, name TEXT, score DOUBLE PRECISION)", + ) + .expect("create"); + test.execute_command("INSERT INTO derive_null VALUES (1, NULL, NULL)") + .expect("insert null row"); + + let row: TestUserDerived = test + .connection + .fetch_one_as("SELECT id, name, score FROM derive_null WHERE id = 1") + .expect("fetch_one_as"); + + assert_eq!(row.id, 1); + assert_eq!(row.name, None); + assert_eq!(row.score, None); +} + +#[test] +fn test_derive_from_row_with_rename() { + // Verify `#[hyperdb(rename = "...")]` redirects field-name lookup + // to a different column name. + #[derive(Debug, PartialEq, FromRow)] + struct UserWithRename { + id: i32, + #[hyperdb(rename = "score")] + rating: Option, + } + + let test = TestConnection::new().expect("Failed to create test connection"); + test.execute_command("CREATE TABLE rename_test (id INT NOT NULL, score DOUBLE PRECISION)") + .expect("create"); + test.execute_command("INSERT INTO rename_test VALUES (1, 99.9)") + .expect("insert"); + + let user: UserWithRename = test + .connection + .fetch_one_as("SELECT id, score FROM rename_test") + .expect("fetch_one_as UserWithRename"); + + assert_eq!(user.id, 1); + assert_eq!(user.rating, Some(99.9)); +} + +#[test] +fn test_derive_from_row_missing_column_errors() { + // Asking for a column that isn't in the SELECT list should + // surface as Error::Column { kind: Missing }. + use hyperdb_api::{ColumnErrorKind, Error}; + + #[derive(Debug, FromRow)] + #[allow( + dead_code, + reason = "fields populated by the generated impl; only error path is asserted" + )] + struct NeedsColumn { + id: i32, + not_in_query: i32, + } + + let test = TestConnection::new().expect("Failed to create test connection"); + test.execute_command("CREATE TABLE missing_col_test (id INT NOT NULL)") + .expect("create"); + test.execute_command("INSERT INTO missing_col_test VALUES (1)") + .expect("insert"); + + let err = test + .connection + .fetch_one_as::("SELECT id FROM missing_col_test") + .expect_err("expected missing-column error"); + + match err { + Error::Column { name, kind } => { + assert_eq!(name, "not_in_query"); + assert!( + matches!(kind, ColumnErrorKind::Missing), + "expected Missing kind, got {kind:?}", + ); + } + other => panic!("expected Error::Column {{ kind: Missing }}, got {other:?}"), + } +} + // ============================================================================= // #16: Connection Health (ping) // ============================================================================= diff --git a/release-please-config.json b/release-please-config.json index 624f521..d5492c4 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -11,6 +11,7 @@ { "type": "toml", "path": "Cargo.toml", "jsonpath": "$.workspace.package.version" }, { "type": "generic", "path": "hyperdb-api-core/Cargo.toml" }, { "type": "generic", "path": "hyperdb-api/Cargo.toml" }, + { "type": "generic", "path": "hyperdb-api-derive/Cargo.toml" }, { "type": "generic", "path": "hyperdb-mcp/Cargo.toml" } ] } From ac3166eebecfffa6c9a2f266b8ccd4d57a1d5547 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 02:58:32 -0700 Subject: [PATCH 3/6] docs(api): add #61+#62 migration recipes to MIGRATING-0.3.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a "FromRow modernization" section to the consolidated migration guide covering: - Trait signature change (&Row -> RowAccessor<'_>) - Tuple impl deletion + recipes for migrating (define a struct with #[derive(FromRow)] or use Row::get(idx) directly) - New #[derive(FromRow)] proc-macro with #[hyperdb(rename = "...")] - New Row::get_by_name accessor - Error::Column / ColumnErrorKind error shape (already in #70) - Performance note about cached-index lookup vs. linear scan - Note that hyperdb-api-derive doesn't need to be a direct dep Also fixes a broken intra-doc link in row_accessor.rs (FromRow -> crate::FromRow); doc-warning count back to baseline 6. Verification: - cargo build --workspace --all-targets — clean - cargo doc --workspace --no-deps — 6 warnings (= post-#70 baseline) - cargo fmt --check — clean --- MIGRATING-0.3.md | 120 ++++++++++++++++++++++++++++++++ hyperdb-api/src/row_accessor.rs | 2 +- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/MIGRATING-0.3.md b/MIGRATING-0.3.md index 75265c2..bab565e 100644 --- a/MIGRATING-0.3.md +++ b/MIGRATING-0.3.md @@ -243,6 +243,126 @@ The MCP server's `Engine::execute_in_transaction` helper takes `&self` and so ca --- +## #61 + #62 — FromRow modernization + +The `FromRow` trait was redesigned around a new [`RowAccessor`] type and a new [`#[derive(FromRow)]`][derive] proc-macro. The blanket tuple impls (1/2/3/4-tuple) were deleted; hand-written impls have a new signature. + +[`RowAccessor`]: https://docs.rs/hyperdb-api/latest/hyperdb_api/struct.RowAccessor.html +[derive]: https://docs.rs/hyperdb-api/latest/hyperdb_api/derive.FromRow.html + +### What's changed + +| Surface | Before (v0.2.x) | After (v0.3.0) | +| ------- | --------------- | -------------- | +| `FromRow::from_row` signature | `fn from_row(row: &Row) -> Result` | `fn from_row(row: RowAccessor<'_>) -> Result` | +| Blanket tuple impls | `(Option,)` … `(Option, Option, Option, Option)` | **Deleted.** Define a struct with `#[derive(FromRow)]` instead. | +| Derive macro | did not exist | `#[derive(FromRow)]` from the new `hyperdb-api-derive` crate (re-exported by `hyperdb-api`) | +| Name-based access on a single row | did not exist | `Row::get_by_name(name)` | +| Cached column-name → index lookup | did not exist | `RowAccessor` carries one; built once per query in `fetch_*_as` | + +### What's new + +- **`#[derive(FromRow)]`** generates the `impl FromRow` for you. Field names match column names by default; `#[hyperdb(rename = "...")]` on a field overrides. `Option` fields use `get_opt` (NULL → `None`); other fields use `get` (NULL → error). + + ```rust + use hyperdb_api::FromRow; + + #[derive(FromRow)] + struct User { + id: i32, + name: String, + #[hyperdb(rename = "email_address")] + email: Option, + } + ``` + +- **`RowAccessor<'a>`** is the parameter type of the new `FromRow::from_row`. It exposes: + - `get(name: &str) -> Result` — required field; missing/NULL/type-mismatch return `Error::Column`. + - `get_opt(name: &str) -> Result>` — optional field; NULL becomes `None`. + - `position(idx: usize) -> Result` — positional escape hatch. + - `row()` — access to the underlying `Row` for callers that need other methods. + +- **`Row::get_by_name(name)`** does the same name-based lookup but on a single `Row` (no cached lookup map). Convenient for hand-coded paths that don't go through `FromRow`. Doc warns that it's a linear scan; recommends `#[derive(FromRow)]` or `fetch_*_as` for hot paths. + +### Migration recipes + +#### Hand-written `FromRow` impl + +```rust +// Before +impl FromRow for User { + fn from_row(row: &Row) -> Result { + Ok(User { + id: row.get::(0).ok_or_else(|| Error::conversion("NULL id"))?, + name: row.get::(1).unwrap_or_default(), + }) + } +} + +// After +impl FromRow for User { + fn from_row(row: RowAccessor<'_>) -> Result { + Ok(User { + id: row.get("id")?, + name: row.get_opt("name")?.unwrap_or_default(), + }) + } +} +``` + +The new shape is shorter, more readable, and decouples your code from column position — reordering `SELECT` columns no longer breaks your impl. + +#### Tuple destructuring (deleted) + +```rust +// Before — blanket tuple impl +let row = conn.fetch_one("SELECT id, name FROM users")?; +let (id, name): (Option, Option) = FromRow::from_row(&row)?; + +// After — define a struct +#[derive(FromRow)] +struct User { id: Option, name: Option } +let user: User = conn.fetch_one_as("SELECT id, name FROM users")?; +``` + +Or, if you really want positional access without a struct, use `Row::get(idx)` directly: + +```rust +let row = conn.fetch_one("SELECT id, name FROM users")?; +let id: Option = row.get(0); +let name: Option = row.get(1); +``` + +#### Direct `T::from_row(&row)` calls + +If you previously called `T::from_row(&row)` directly (outside `fetch_*_as`), the new signature requires a `RowAccessor`. Easiest migration: use `fetch_one_as` / `fetch_all_as` instead, which build the cached lookup for you. + +If you must construct a `RowAccessor` yourself (e.g. processing rows from a custom source), the constructor is `pub(crate)`. File an issue if you need this surfaced — current direction is to keep `RowAccessor` construction internal so the cache lifetime stays tied to `fetch_*_as`. + +### Errors + +The derive and `RowAccessor` accessors return `Error::Column { name, kind }` for column-access failures, where `ColumnErrorKind` is one of: + +- `Missing` — column with that name not in the result schema +- `Null` — required field, but the cell is SQL `NULL` +- `TypeMismatch { expected, actual }` — the cell value couldn't be decoded as `T` + +`Error::ColumnIndexOutOfBounds { idx, column_count }` is returned by `RowAccessor::position` when `idx` is out of range. + +These variants were shipped in `#70` so this PR doesn't re-break the error type. + +### Performance note + +`fetch_*_as` builds a `HashMap<&str, usize>` once per query (O(N) in the column count). Each row's `RowAccessor::get(name)` then runs a single hash lookup followed by typed access — O(1) per field per row. This is strictly better than the previous behavior, where a hand-written impl using `try_get(idx, name)` had to know column positions hard-coded. + +For one-off named access on a `Row` outside `fetch_*_as`, `Row::get_by_name` is a linear scan over `ResultSchema::column_index`. For hot paths (many rows × many fields), prefer `#[derive(FromRow)]`. + +### `hyperdb-api-derive` crate + +The proc-macro lives in a new `hyperdb-api-derive` workspace crate (Rust requires proc-macro code to live in its own `proc-macro = true` crate). It's re-exported from `hyperdb-api`, so callers don't need a direct dependency — same pattern as serde / thiserror. **Don't add `hyperdb-api-derive` to your `Cargo.toml`**; just `use hyperdb_api::FromRow;`. + +--- + ## #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. diff --git a/hyperdb-api/src/row_accessor.rs b/hyperdb-api/src/row_accessor.rs index 0b72ee6..b7bd502 100644 --- a/hyperdb-api/src/row_accessor.rs +++ b/hyperdb-api/src/row_accessor.rs @@ -28,7 +28,7 @@ use crate::result::{Row, RowValue}; /// A view over a [`Row`] that supports name-based access via a /// pre-resolved column-name → index lookup table. /// -/// `RowAccessor` is the parameter type of [`FromRow::from_row`]; it +/// `RowAccessor` is the parameter type of [`crate::FromRow::from_row`]; it /// borrows the row and a shared lookup map built once per query in /// [`fetch_one_as`](crate::Connection::fetch_one_as) / /// [`fetch_all_as`](crate::Connection::fetch_all_as). From 676a4c24324855fc848f384eca77328d7a319497 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 03:12:47 -0700 Subject: [PATCH 4/6] chore(api): address final-review findings on FromRow modernization (#61, #62 part 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from the architectural pre-PR review: - M1 (consistency): RowAccessor::position now produces Error::Column with ColumnErrorKind::Null / TypeMismatch (synthesized name "col[{idx}]") instead of Error::Conversion. This aligns positional errors with the named-access error shape, so callers can match on Error::Column { kind, .. } uniformly across get/get_opt/position rather than special-casing positional access. Out-of-bounds still uses Error::ColumnIndexOutOfBounds for index integrity. Added a dedicated unit test (position_null_errors_with_kind_null). - m1 (API surface): Removed RowAccessor::row(). It was a leak vector that let FromRow impls drop down to bare Row methods, bypassing the cached-index lookup the accessor exists to provide — exactly the anti-pattern this PR set out to eliminate. With no production caller needing it (verified by build), removing rather than gating to pub(crate) is the cleanest move. Easy to add back if a real consumer surfaces. - m2 (forward-compat): Derive macro's "unrecognized hyperdb attribute" error now reads "supported attributes: rename" instead of pinning a specific syntax. When new attributes (skip, default, with) ship in v0.3.x, the message stays accurate without a macro patch. Verification: - cargo build --workspace --all-targets — clean - cargo clippy --workspace --all-targets -- -D warnings — clean - cargo test --workspace --lib — 8 row_accessor tests pass (incl. new position_null_errors_with_kind_null) - cargo fmt --check — clean --- hyperdb-api-derive/src/lib.rs | 2 +- hyperdb-api/src/row_accessor.rs | 59 ++++++++++++++++++++++++--------- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/hyperdb-api-derive/src/lib.rs b/hyperdb-api-derive/src/lib.rs index 9fbc23c..95e8d0b 100644 --- a/hyperdb-api-derive/src/lib.rs +++ b/hyperdb-api-derive/src/lib.rs @@ -139,7 +139,7 @@ fn column_name_for(field: &Field, default: &syn::Ident) -> syn::Result { Ok(()) } else { Err(meta.error(format!( - "unrecognized hyperdb attribute `{}`; expected `rename = \"...\"`", + "unrecognized hyperdb attribute `{}`; supported attributes: rename", meta.path .get_ident() .map_or_else(|| "?".to_string(), ToString::to_string) diff --git a/hyperdb-api/src/row_accessor.rs b/hyperdb-api/src/row_accessor.rs index b7bd502..5c07c2a 100644 --- a/hyperdb-api/src/row_accessor.rs +++ b/hyperdb-api/src/row_accessor.rs @@ -79,15 +79,6 @@ impl<'a> RowAccessor<'a> { map } - /// Returns the underlying [`Row`]. - /// - /// Useful for callers that need access to row methods not exposed - /// through this accessor (e.g. positional `Row::get`). - #[must_use] - pub fn row(&self) -> &Row { - self.row - } - /// Returns the named column's value, decoded as `T`. /// /// # Errors @@ -171,8 +162,10 @@ impl<'a> RowAccessor<'a> { /// /// - [`Error::ColumnIndexOutOfBounds`] if `idx` is past the row's /// column count. - /// - [`Error::Conversion`] if the cell is `NULL` or cannot be - /// decoded as `T`. (Wraps [`Row::try_get`].) + /// - [`Error::Column`] with [`ColumnErrorKind::Null`] if the cell + /// is SQL `NULL`. The synthesized name is `col[{idx}]`. + /// - [`Error::Column`] with [`ColumnErrorKind::TypeMismatch`] if + /// the cell value cannot be decoded as `T`. Same synthesized name. pub fn position(&self, idx: usize) -> Result { if idx >= self.row.column_count() { return Err(Error::column_index_out_of_bounds( @@ -180,10 +173,26 @@ impl<'a> RowAccessor<'a> { self.row.column_count(), )); } - // Reuse Row::try_get's NULL/decode-error path. Synthesize a - // column-name label for the error message. - let label = format!("col[{idx}]"); - self.row.try_get::(idx, &label) + // Mirror the `get`/`get_opt` error shape so callers can match + // on `Error::Column { kind, .. }` uniformly across named and + // positional access. Synthesize a name for the error label. + if let Some(v) = self.row.get::(idx) { + Ok(v) + } else if self.row.is_null(idx) { + Err(Error::column(format!("col[{idx}]"), ColumnErrorKind::Null)) + } else { + let actual = self + .row + .sql_type(idx) + .map_or_else(|| "".to_string(), |t| format!("{t:?}")); + Err(Error::column( + format!("col[{idx}]"), + ColumnErrorKind::TypeMismatch { + expected: std::any::type_name::().to_string(), + actual, + }, + )) + } } } @@ -310,4 +319,24 @@ mod tests { let id: i32 = accessor.position(0).expect("position 0"); assert_eq!(id, 42); } + + #[test] + fn position_null_errors_with_kind_null() { + // NULL at a positional access path should surface as + // Error::Column { kind: Null }, mirroring the named `get` + // error shape rather than the older Error::Conversion form. + let (row, schema) = user_row(Some(1), None); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + // Column 1 is `name`, which is NULL. + let err = accessor.position::(1).unwrap_err(); + match err { + Error::Column { name, kind } => { + assert_eq!(name, "col[1]"); + assert!(matches!(kind, ColumnErrorKind::Null)); + } + other => panic!("expected Error::Column {{ kind: Null }}, got {other:?}"), + } + } } From 378c0a62bb9d725fb54cae088d58fbd361b4712e Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 09:14:50 -0700 Subject: [PATCH 5/6] chore(api): add #[hyperdb(index = N)] derive attribute + RowAccessor::position_opt Extends #[derive(FromRow)] with a positional access mode for queries where columns have no stable name (e.g. SELECT id, COUNT(*) FROM ... GROUP BY id). Mutually exclusive with #[hyperdb(rename = "...")]. Adds RowAccessor::position_opt as the symmetric Option counterpart to position, mirroring the get/get_opt naming pair. NULL becomes None; out-of-bounds and type-mismatch still error. The macro emits position(N)? for non-Option fields and position_opt(N)? for Option fields, matching the existing get/get_opt dispatch on name-based access. Updates MIGRATING-0.3.md and the derive crate README. Doc-warning count back at baseline 6. Verification: - cargo build/clippy/test/fmt/doc all clean on workspace - new RowAccessor::position_opt unit tests (NULL, happy path, OOB) - new integration test test_derive_from_row_with_index runs against a real query with COUNT(*) (unnamed column) --- MIGRATING-0.3.md | 16 +++- hyperdb-api-derive/README.md | 73 ++++++++++++++++- hyperdb-api-derive/src/lib.rs | 81 ++++++++++++++----- hyperdb-api/src/row_accessor.rs | 77 ++++++++++++++++++ hyperdb-api/tests/remaining_features_tests.rs | 30 +++++++ 5 files changed, 249 insertions(+), 28 deletions(-) diff --git a/MIGRATING-0.3.md b/MIGRATING-0.3.md index bab565e..a054eb8 100644 --- a/MIGRATING-0.3.md +++ b/MIGRATING-0.3.md @@ -262,7 +262,7 @@ The `FromRow` trait was redesigned around a new [`RowAccessor`] type and a new [ ### What's new -- **`#[derive(FromRow)]`** generates the `impl FromRow` for you. Field names match column names by default; `#[hyperdb(rename = "...")]` on a field overrides. `Option` fields use `get_opt` (NULL → `None`); other fields use `get` (NULL → error). +- **`#[derive(FromRow)]`** generates the `impl FromRow` for you. Field names match column names by default; `#[hyperdb(rename = "...")]` overrides the column name; `#[hyperdb(index = N)]` switches to positional access at column `N`. `Option` fields use `get_opt` / `position_opt` (NULL → `None`); other fields use `get` / `position` (NULL → error). `rename` and `index` are mutually exclusive. ```rust use hyperdb_api::FromRow; @@ -274,13 +274,23 @@ The `FromRow` trait was redesigned around a new [`RowAccessor`] type and a new [ #[hyperdb(rename = "email_address")] email: Option, } + + // Useful for queries with computed/unnamed columns, e.g. + // `SELECT id, COUNT(*) FROM ... GROUP BY id`. + #[derive(FromRow)] + struct Aggregate { + #[hyperdb(index = 0)] + id: i32, + #[hyperdb(index = 1)] + total: Option, + } ``` - **`RowAccessor<'a>`** is the parameter type of the new `FromRow::from_row`. It exposes: - `get(name: &str) -> Result` — required field; missing/NULL/type-mismatch return `Error::Column`. - `get_opt(name: &str) -> Result>` — optional field; NULL becomes `None`. - - `position(idx: usize) -> Result` — positional escape hatch. - - `row()` — access to the underlying `Row` for callers that need other methods. + - `position(idx: usize) -> Result` — positional access; out-of-range returns `Error::ColumnIndexOutOfBounds`. + - `position_opt(idx: usize) -> Result>` — positional access; NULL becomes `None`. - **`Row::get_by_name(name)`** does the same name-based lookup but on a single `Row` (no cached lookup map). Convenient for hand-coded paths that don't go through `FromRow`. Doc warns that it's a linear scan; recommends `#[derive(FromRow)]` or `fetch_*_as` for hot paths. diff --git a/hyperdb-api-derive/README.md b/hyperdb-api-derive/README.md index 344de83..8b99f54 100644 --- a/hyperdb-api-derive/README.md +++ b/hyperdb-api-derive/README.md @@ -5,20 +5,87 @@ Use `hyperdb-api` directly; don't add `hyperdb-api-derive` to your dependencies. This crate provides the procedural macros that `hyperdb-api` re-exports -(currently just `#[derive(FromRow)]`). Use them through `hyperdb-api`: +(currently just `#[derive(FromRow)]`). Use them through `hyperdb-api`. + +## Quick example ```rust -use hyperdb_api::FromRow; +use hyperdb_api::{Connection, ConnectionBuilder, FromRow, Result}; -#[derive(FromRow)] +#[derive(Debug, FromRow)] struct User { id: i32, name: String, #[hyperdb(rename = "email_address")] email: Option, } + +fn main() -> Result<()> { + let conn: Connection = ConnectionBuilder::new("localhost:7483").connect()?; + + let alice: User = conn.fetch_one_as("SELECT * FROM users WHERE id = 1")?; + println!("{alice:?}"); + + let everyone: Vec = conn.fetch_all_as("SELECT * FROM users ORDER BY id")?; + for u in &everyone { + println!("{u:?}"); + } + Ok(()) +} ``` +## Field-to-column mapping rules + +- **Field name = column name** by default. A field `name: String` reads + the column called `name`. +- **`#[hyperdb(rename = "...")]`** overrides the column name. Use this + when the SQL column doesn't match the Rust field — snake_case + mismatches, reserved words, columns named after Rust keywords, etc. +- **`#[hyperdb(index = N)]`** switches that field to positional access + at column `N` (zero-based). Useful for queries with computed/unnamed + columns where there's no stable name to match — e.g. `SELECT id, + COUNT(*) FROM ... GROUP BY id`. Mutually exclusive with `rename`. +- **`Option` fields tolerate SQL NULL** (become `None`). Non-`Option` + fields error with `Error::Column { kind: Null, .. }` if the cell is + NULL. +- **Missing columns** (the column isn't in the result schema) error + with `Error::Column { kind: Missing, .. }` at fetch time. + +```rust +#[derive(FromRow)] +struct Aggregate { + #[hyperdb(index = 0)] + id: i32, + #[hyperdb(index = 1)] + total: Option, +} +// Works against `SELECT id, COUNT(*) FROM ... GROUP BY id` +``` + +## When to hand-write `FromRow` instead + +The derive emits a straightforward mapping. If you need transformation +in the mapping — parsing a string column into a Rust enum, splitting a +single column into multiple fields, etc. — write the impl directly: + +```rust +impl FromRow for User { + fn from_row(row: hyperdb_api::RowAccessor<'_>) -> Result { + Ok(User { + id: row.get("id")?, + name: row.get("full_name")?, // SQL column "full_name" + email: row.get_opt("email_address")?, + }) + } +} +``` + +In a hand-written impl, the string passed to `row.get(...)` / +`row.get_opt(...)` *is* the column name — no `#[hyperdb(rename)]` is +needed, since you're spelling the column out yourself. Your `SELECT` +just needs to actually return that column (use `AS full_name` if the +underlying table column has a different name). + See the [`hyperdb-api` docs](https://docs.rs/hyperdb-api) for full usage. This crate has no stable API. Breaking changes land here without a major diff --git a/hyperdb-api-derive/src/lib.rs b/hyperdb-api-derive/src/lib.rs index 95e8d0b..01709d9 100644 --- a/hyperdb-api-derive/src/lib.rs +++ b/hyperdb-api-derive/src/lib.rs @@ -30,22 +30,37 @@ //! //! - `#[hyperdb(rename = "...")]` on a field uses the given column //! name instead of the field name. -//! - Field types of `Option` use [`RowAccessor::get_opt`] -//! (NULL → `None`); other field types use [`RowAccessor::get`] -//! (NULL → error). +//! - `#[hyperdb(index = N)]` on a field uses positional access +//! ([`RowAccessor::position`] / [`RowAccessor::position_opt`]) at +//! column index `N` instead of name-based lookup. Mutually exclusive +//! with `rename`. +//! - Field types of `Option` use [`RowAccessor::get_opt`] / +//! [`RowAccessor::position_opt`] (NULL → `None`); other field types +//! use [`RowAccessor::get`] / [`RowAccessor::position`] (NULL → +//! error). //! //! [`hyperdb_api::FromRow`]: https://docs.rs/hyperdb-api //! [`RowAccessor::get_opt`]: https://docs.rs/hyperdb-api //! [`RowAccessor::get`]: https://docs.rs/hyperdb-api +//! [`RowAccessor::position`]: https://docs.rs/hyperdb-api +//! [`RowAccessor::position_opt`]: https://docs.rs/hyperdb-api use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; use quote::quote; use syn::{ - parse_macro_input, Data, DataStruct, DeriveInput, Field, Fields, GenericArgument, LitStr, - PathArguments, Type, TypePath, + parse_macro_input, spanned::Spanned, Data, DataStruct, DeriveInput, Field, Fields, + GenericArgument, LitInt, LitStr, PathArguments, Type, TypePath, }; +/// How a field maps to a column. Either by name (the default or +/// `#[hyperdb(rename = "...")]`) or by ordinal position +/// (`#[hyperdb(index = N)]`). +enum FieldSource { + Name(String), + Index(usize), +} + /// Derives `hyperdb_api::FromRow` for a struct. /// /// See the crate-level documentation for the full feature list. @@ -106,51 +121,73 @@ fn expand(input: &DeriveInput) -> syn::Result { }) } -/// Generates `field_name: row.get("col")?` (or `get_opt` for `Option` fields). +/// Generates `field_name: row.get("col")?` (or `get_opt`/`position`/`position_opt` +/// for `Option` fields and/or `#[hyperdb(index = N)]`). fn field_assignment(field: &Field) -> syn::Result { let ident = field .ident .as_ref() .ok_or_else(|| syn::Error::new_spanned(field, "tuple-struct fields are not supported"))?; - let column_name = column_name_for(field, ident)?; - let column_lit = LitStr::new(&column_name, ident.span()); + let source = field_source_for(field, ident)?; + let is_opt = is_option_type(&field.ty); - let getter = if is_option_type(&field.ty) { - quote!(row.get_opt(#column_lit)?) - } else { - quote!(row.get(#column_lit)?) + let getter = match (source, is_opt) { + (FieldSource::Name(name), true) => { + let lit = LitStr::new(&name, ident.span()); + quote!(row.get_opt(#lit)?) + } + (FieldSource::Name(name), false) => { + let lit = LitStr::new(&name, ident.span()); + quote!(row.get(#lit)?) + } + (FieldSource::Index(idx), true) => quote!(row.position_opt(#idx)?), + (FieldSource::Index(idx), false) => quote!(row.position(#idx)?), }; Ok(quote! { #ident: #getter }) } -/// Reads `#[hyperdb(rename = "...")]` from a field's attributes; falls back -/// to the field's identifier as the column name. -fn column_name_for(field: &Field, default: &syn::Ident) -> syn::Result { +/// Reads `#[hyperdb(rename = "...")]` or `#[hyperdb(index = N)]` from a field's +/// attributes. Falls back to a name-based source using the field's identifier. +/// `rename` and `index` are mutually exclusive. +fn field_source_for(field: &Field, default: &syn::Ident) -> syn::Result { + let mut rename: Option<(String, proc_macro2::Span)> = None; + let mut index: Option<(usize, proc_macro2::Span)> = None; + for attr in &field.attrs { if !attr.path().is_ident("hyperdb") { continue; } - let mut rename: Option = None; attr.parse_nested_meta(|meta| { if meta.path.is_ident("rename") { let s: LitStr = meta.value()?.parse()?; - rename = Some(s.value()); + rename = Some((s.value(), meta.path.span())); + Ok(()) + } else if meta.path.is_ident("index") { + let n: LitInt = meta.value()?.parse()?; + let parsed: usize = n.base10_parse()?; + index = Some((parsed, meta.path.span())); Ok(()) } else { Err(meta.error(format!( - "unrecognized hyperdb attribute `{}`; supported attributes: rename", + "unrecognized hyperdb attribute `{}`; supported attributes: rename, index", meta.path .get_ident() .map_or_else(|| "?".to_string(), ToString::to_string) ))) } })?; - if let Some(name) = rename { - return Ok(name); - } } - Ok(default.to_string()) + + match (rename, index) { + (Some(_), Some((_, idx_span))) => Err(syn::Error::new( + idx_span, + "`#[hyperdb(rename = ...)]` and `#[hyperdb(index = N)]` are mutually exclusive", + )), + (Some((name, _)), None) => Ok(FieldSource::Name(name)), + (None, Some((idx, _))) => Ok(FieldSource::Index(idx)), + (None, None) => Ok(FieldSource::Name(default.to_string())), + } } /// Detects `Option` (any path ending in `Option`). diff --git a/hyperdb-api/src/row_accessor.rs b/hyperdb-api/src/row_accessor.rs index 5c07c2a..fa012dd 100644 --- a/hyperdb-api/src/row_accessor.rs +++ b/hyperdb-api/src/row_accessor.rs @@ -194,6 +194,45 @@ impl<'a> RowAccessor<'a> { )) } } + + /// Positional optional access: returns `Option` for the cell at + /// `idx`. SQL `NULL` becomes `None`; out-of-bounds and type + /// mismatches still error. Mirrors [`get_opt`](Self::get_opt) for + /// positional access. + /// + /// # Errors + /// + /// - [`Error::ColumnIndexOutOfBounds`] if `idx` is past the row's + /// column count. + /// - [`Error::Column`] with [`ColumnErrorKind::TypeMismatch`] if + /// the cell is non-NULL but cannot be decoded as `T`. The + /// synthesized name is `col[{idx}]`. + pub fn position_opt(&self, idx: usize) -> Result> { + if idx >= self.row.column_count() { + return Err(Error::column_index_out_of_bounds( + idx, + self.row.column_count(), + )); + } + if self.row.is_null(idx) { + return Ok(None); + } + if let Some(v) = self.row.get::(idx) { + Ok(Some(v)) + } else { + let actual = self + .row + .sql_type(idx) + .map_or_else(|| "".to_string(), |t| format!("{t:?}")); + Err(Error::column( + format!("col[{idx}]"), + ColumnErrorKind::TypeMismatch { + expected: std::any::type_name::().to_string(), + actual, + }, + )) + } + } } #[cfg(test)] @@ -320,6 +359,44 @@ mod tests { assert_eq!(id, 42); } + #[test] + fn position_opt_null_returns_none() { + let (row, schema) = user_row(Some(1), None); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let v: Option = accessor.position_opt(1).expect("position_opt for NULL"); + assert_eq!(v, None); + } + + #[test] + fn position_opt_value_returns_some() { + let (row, schema) = user_row(Some(42), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let id: Option = accessor.position_opt(0).expect("position_opt id"); + let name: Option = accessor.position_opt(1).expect("position_opt name"); + assert_eq!(id, Some(42)); + assert_eq!(name, Some("alice".to_string())); + } + + #[test] + fn position_opt_out_of_range_errors_with_index_oob() { + let (row, schema) = user_row(Some(1), Some("alice")); + let indices = RowAccessor::build_indices(&schema); + let accessor = RowAccessor::new(&row, &indices); + + let err = accessor.position_opt::(5).unwrap_err(); + match err { + Error::ColumnIndexOutOfBounds { idx, column_count } => { + assert_eq!(idx, 5); + assert_eq!(column_count, 2); + } + other => panic!("expected Error::ColumnIndexOutOfBounds, got {other:?}"), + } + } + #[test] fn position_null_errors_with_kind_null() { // NULL at a positional access path should surface as diff --git a/hyperdb-api/tests/remaining_features_tests.rs b/hyperdb-api/tests/remaining_features_tests.rs index c4e891b..d867e45 100644 --- a/hyperdb-api/tests/remaining_features_tests.rs +++ b/hyperdb-api/tests/remaining_features_tests.rs @@ -406,6 +406,36 @@ fn test_derive_from_row_with_rename() { assert_eq!(user.rating, Some(99.9)); } +#[test] +fn test_derive_from_row_with_index() { + // Verify `#[hyperdb(index = N)]` uses positional access. Useful + // for queries with computed/unnamed columns where the column has + // no stable name (e.g. `SELECT id, COUNT(*) FROM ...`). + #[derive(Debug, PartialEq, FromRow)] + struct PositionalRow { + #[hyperdb(index = 0)] + id: i32, + #[hyperdb(index = 1)] + total: Option, + } + + let test = TestConnection::new().expect("Failed to create test connection"); + test.execute_command("CREATE TABLE positional_test (id INT NOT NULL, value INT)") + .expect("create"); + test.execute_command("INSERT INTO positional_test VALUES (1, 10), (1, 20), (2, 30)") + .expect("insert"); + + // COUNT(*) produces an unnamed/computed column — name-based lookup + // would be brittle, but positional access works. + let row: PositionalRow = test + .connection + .fetch_one_as("SELECT id, COUNT(*) FROM positional_test WHERE id = 1 GROUP BY id") + .expect("fetch_one_as PositionalRow"); + + assert_eq!(row.id, 1); + assert_eq!(row.total, Some(2)); +} + #[test] fn test_derive_from_row_missing_column_errors() { // Asking for a column that isn't in the SELECT list should From a50b97a92a9a9c1d94db25aabc593439c135d0d6 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Thu, 28 May 2026 09:36:32 -0700 Subject: [PATCH 6/6] docs(derive): clarify RowAccessor accessor pairs and zero-based indexing in README --- hyperdb-api-derive/README.md | 41 +++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/hyperdb-api-derive/README.md b/hyperdb-api-derive/README.md index 8b99f54..2cc1701 100644 --- a/hyperdb-api-derive/README.md +++ b/hyperdb-api-derive/README.md @@ -66,7 +66,8 @@ struct Aggregate { The derive emits a straightforward mapping. If you need transformation in the mapping — parsing a string column into a Rust enum, splitting a -single column into multiple fields, etc. — write the impl directly: +single column into multiple fields, defaulting NULLs to a non-`Option` +value, etc. — write the impl directly: ```rust impl FromRow for User { @@ -86,6 +87,44 @@ needed, since you're spelling the column out yourself. Your `SELECT` just needs to actually return that column (use `AS full_name` if the underlying table column has a different name). +### `RowAccessor` accessor cheat sheet + +`RowAccessor` exposes four accessors. Pick by access mode (name vs. +index) and required vs. optional. Indices are **zero-based**. + +| | Required (`T`) | Optional (`Option`) | +|---|---|---| +| **By name** | `row.get(name)?` | `row.get_opt(name)?` | +| **By index** | `row.position(idx)?` | `row.position_opt(idx)?` | + +NULL handling differs between the two columns of the table: + +- **`get` / `position`** — NULL errors with `Error::Column { kind: Null, .. }`. + Use these for required fields where NULL is a problem. +- **`get_opt` / `position_opt`** — NULL becomes `Ok(None)`. Use these for + fields whose Rust type is `Option`. + +The Rust field type and the accessor must agree: `position` returns +`Result`, `position_opt` returns `Result>`. Mixing them +across types is a compile error, not a runtime mismatch: + +```rust +// ✅ field type matches accessor return +let email: Option = row.position_opt(2)?; +let id: i32 = row.position(0)?; + +// ❌ compile errors +let email: Option = row.position(2)?; // returns T, not Option +let id: i32 = row.position_opt(0)?; // returns Option +``` + +If you want to silently default a NULL on a non-`Option` field, opt in +explicitly: + +```rust +name: row.position_opt(1)?.unwrap_or_default(), // NULL → "" +``` + See the [`hyperdb-api` docs](https://docs.rs/hyperdb-api) for full usage. This crate has no stable API. Breaking changes land here without a major