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/MIGRATING-0.3.md b/MIGRATING-0.3.md index 75265c2..a054eb8 100644 --- a/MIGRATING-0.3.md +++ b/MIGRATING-0.3.md @@ -243,6 +243,136 @@ 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 = "...")]` 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; + + #[derive(FromRow)] + struct User { + id: i32, + name: String, + #[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 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. + +### 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-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..2cc1701 --- /dev/null +++ b/hyperdb-api-derive/README.md @@ -0,0 +1,132 @@ +# 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`. + +## Quick example + +```rust +use hyperdb_api::{Connection, ConnectionBuilder, FromRow, Result}; + +#[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, defaulting NULLs to a non-`Option` +value, 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). + +### `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 +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..01709d9 --- /dev/null +++ b/hyperdb-api-derive/src/lib.rs @@ -0,0 +1,209 @@ +// 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. +//! - `#[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, 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. +#[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`/`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 source = field_source_for(field, ident)?; + let is_opt = is_option_type(&field.ty); + + 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 = "...")]` 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; + } + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("rename") { + let s: LitStr = meta.value()?.parse()?; + 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, index", + meta.path + .get_ident() + .map_or_else(|| "?".to_string(), ToString::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`). +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/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..b4fd2e4 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,12 @@ 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; + +// 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/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..fa012dd --- /dev/null +++ b/hyperdb-api/src/row_accessor.rs @@ -0,0 +1,419 @@ +// 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 [`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). +/// +/// # 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 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::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( + idx, + self.row.column_count(), + )); + } + // 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, + }, + )) + } + } + + /// 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)] +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); + } + + #[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 + // 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:?}"), + } + } +} 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..d867e45 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,24 +306,173 @@ fn test_fetch_all_as() { assert_eq!(users[2].name, "Carol"); } +// `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)]`. + +// ============================================================================= +// #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_from_row_tuple() { +fn test_derive_from_row_parity_with_handwritten() { let test = TestConnection::new().expect("Failed to create test connection"); - test.execute_command("CREATE TABLE tuple_test (id INT NOT NULL, name TEXT)") + 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_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 tuple_test VALUES (1, 'Alice')") + test.execute_command("INSERT INTO positional_test VALUES (1, 10), (1, 20), (2, 30)") .expect("insert"); - let row = test + // COUNT(*) produces an unnamed/computed column — name-based lookup + // would be brittle, but positional access works. + let row: PositionalRow = test .connection - .fetch_one("SELECT id, name FROM tuple_test") - .expect("fetch"); + .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 + // 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 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())); + 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:?}"), + } } // ============================================================================= 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" } ] }