Skip to content

FromRow redesign (breaking): typed RowAccessor, structured errors, enforced NULL semantics #62

@StefanSteiner

Description

@StefanSteiner

Summary

Alternative to #61 that takes a breaking change to FromRow in exchange for substantially cleaner struct mapping. Filed for visibility — the additive path in #61 is the safer default; this issue documents what becomes possible if a 1.0-style cleanup is acceptable.

Motivation

The additive proposal in #61 gets ~70% of the way to a clean experience: a #[derive(FromRow)] macro removes per-struct boilerplate, and a new row.get_by_name::<T>(...) method removes most ordering bugs. The remaining 30% is locked behind the current trait signature:

  • NULL semantics are decided per impl, not enforced by the trait.
  • Errors are stringly-typed (Error::new("NULL id")) rather than structured.
  • The positional tuple impls at result.rs:758-787 keep positional access on the happy path.

Fixing those requires changing what from_row receives.

Proposed breaking changes

1. Trait signature carries column metadata

Replace:

pub trait FromRow: Sized {
    fn from_row(row: &Row) -> Result<Self>;
}

with:

pub trait FromRow: Sized {
    fn from_row(row: RowAccessor<'_>) -> Result<Self>;
}

RowAccessor is a thin wrapper around &Row plus a &[usize] lookup table (cached column indices, resolved once per query — see the performance note in #61). It exposes:

  • get<T>(&self, name: &str) -> Result<T> — typed, name-based, structured error on miss.
  • get_opt<T>(&self, name: &str) -> Result<Option<T>> — same but None on NULL instead of error.
  • position<T>(&self, idx: usize) -> Result<T> — escape hatch for hand-written positional impls.

2. Type-system-enforced NULL semantics

The macro and accessor own NULL handling uniformly:

  • Option<T> field → NULL becomes None.
  • Non-Option field → NULL is Err(Error::Column { name, kind: NullColumnError }).

No more per-impl unwrap_or_default() divergence.

3. Structured column errors

Replace ad-hoc Error::new("NULL id") strings with:

Error::Column {
    name: String,           // column name from the schema
    kind: ColumnErrorKind,  // Missing | Null | TypeMismatch { expected, actual }
}

fetch_one_as / fetch_all_as can then wrap these with the SQL text automatically.

4. Positional tuple impls become opt-in

Move the (Option<A>, …) impls at result.rs:758-787 behind a positional-tuples cargo feature (default-off, or default-on for one release with a deprecation warning). Pushes new code toward named access; keeps the escape hatch for ad-hoc tuple destructuring.

Migration burden (concrete)

In-tree impls that need updating:

Each hand-written impl becomes shorter and clearer:

impl FromRow for TestUser {
    fn from_row(row: RowAccessor<'_>) -> Result<Self> {
        Ok(TestUser {
            id: row.get("id")?,
            name: row.get_opt("name")?.unwrap_or_default(),
            score: row.get_opt("score")?.unwrap_or(0.0),
        })
    }
}

For most downstream users the recommended path will be #[derive(FromRow)] — the trait change is invisible to them.

Performance note (same as #61)

Name resolution must be cached once per query. ResultSchema::column_index at result.rs:893 is a linear scan; calling it per field per row would be a regression on wide rows. The RowAccessor indirection exists specifically so fetch_all_as can resolve names once from the Rowset's ResultSchema and pass cached &[usize] indices into every from_row call.

Decision

Pursuing this issue is gated on the project being willing to take a breaking change in hyperdb-api. If that's not on the table for the foreseeable future, close this and let #61 carry the work.

Related

Metadata

Metadata

Assignees

Labels

breaking-changePublic API change that is not backwards-compatibleenhancementNew feature or request

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions