Skip to content

chore(api): modernize FromRow with #[derive] + RowAccessor (#61, #62)#74

Merged
StefanSteiner merged 6 commits into
tableau:mainfrom
StefanSteiner:ssteiner/issue-61-62-fromrow-modernization
May 28, 2026
Merged

chore(api): modernize FromRow with #[derive] + RowAccessor (#61, #62)#74
StefanSteiner merged 6 commits into
tableau:mainfrom
StefanSteiner:ssteiner/issue-61-62-fromrow-modernization

Conversation

@StefanSteiner
Copy link
Copy Markdown
Contributor

Summary

Modernizes FromRow for the v0.3.0 bundle (closes #61, #62):

  • Introduces RowAccessor<'a> — name-based column access with a per-query cached HashMap<&str, usize> lookup. Replaces the &Row parameter on FromRow::from_row (breaking).
  • Adds #[derive(FromRow)] in a new workspace crate hyperdb-api-derive, re-exported from hyperdb-api (same pattern as serde / thiserror / tokio). Supports #[hyperdb(rename = "...")] for column-name overrides and #[hyperdb(index = N)] for positional access (useful with computed/unnamed columns like SELECT id, COUNT(*) FROM ... GROUP BY id).
  • Adds Row::get_by_name<T>(name) for one-off named access outside fetch_*_as (linear scan; rustdoc steers hot paths to the derive).
  • Deletes the four blanket tuple FromRow impls — define a struct with #[derive(FromRow)] instead.
  • Migrates the three hand-written FromRow impls in tests/examples; documents migration in MIGRATING-0.3.md.

Bundle policy: chore: prefix to defer release-please version bump until the v0.3.0 rollup feat!: commit. PR is breaking — landed under the bundle umbrella.

Commits

  1. e705aa5 — Foundation: RowAccessor, trait signature change, Row::get_by_name, fetch_*_as cache, hand-written impl migration, 8 unit tests.
  2. 70356c4hyperdb-api-derive crate; #[derive(FromRow)] with #[hyperdb(rename = "...")]; integration tests (parity, NULL, rename, missing-column).
  3. ac3166eMIGRATING-0.3.md #61+#62 section.
  4. 676a4c2 — Final-review fixes: RowAccessor::position returns structured Error::Column { kind: Null | TypeMismatch }; removes leak-vector RowAccessor::row(); clearer attribute error message.
  5. 378c0a6 — Adds #[hyperdb(index = N)] and RowAccessor::position_opt. Mutually exclusive with rename. Integration test against a COUNT(*) query.
  6. a50b97a — Derive crate README: 2×2 accessor cheat sheet, zero-based indexing note, type-system rules for the position / position_opt pair.

Why a separate hyperdb-api-derive crate

Rust requires #[proc_macro_derive] to live in a crate with proc-macro = true. hyperdb-api-core is not a proc-macro crate and cannot host the derive. Re-export from hyperdb-api means downstream callers do use hyperdb_api::FromRow; and never depend on hyperdb-api-derive directly — same ergonomic shape as serde / thiserror.

Performance

Ran cargo run --release --example benchmark_suite -- 200000 against this branch and against main (origin/main = 5c7e78b). N=1 at 200K rows; deltas are within typical run-to-run noise on this M3 Max. No regression observed.

Workload Variant main rows/s PR rows/s
insert.bulk Inserter (HyperBinary) 3.37 M 4.68 M
insert.bulk ChunkSender × 4 3.63 M 5.10 M
insert.bulk AsyncArrowInserter 5.24 M 6.59 M
query.full_scan sync single 9.79 M 7.24 M
query.full_scan async single 8.85 M 11.80 M
query.filtered sync single 3.81 M 4.56 M
query.aggregation sync single 1.43 K 1.94 K

The fetch_*_as cached-index path is hit by the FromRow tests rather than the bulk-insert/query benchmarks — the lookup is O(N) once per query plus O(1) per field per row, strictly better than the previous "no cache, hand-coded positions" pattern.

Migration

MIGRATING-0.3.md covers the recipes; the short version:

// Before
impl FromRow for User {
    fn from_row(row: &Row) -> Result<Self> { /* ... */ }
}

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

// Or just derive it
#[derive(FromRow)]
struct User { id: i32, name: Option<String> }

Tuple FromRow impls are gone; define a struct.

Verification

Test plan

  • Workspace build/clippy/test/fmt/doc all green on macOS aarch64
  • RowAccessor unit tests cover Missing, Null, TypeMismatch, position-OOB, position-Null
  • Derive integration tests cover parity-with-handwritten, NULL → None on Option<T>, #[hyperdb(rename)], #[hyperdb(index)] (against SELECT id, COUNT(*) FROM ... GROUP BY id), missing column → Error::Column { kind: Missing }
  • async_parity_smoke example exercises the new fetch_*_as cached-index path end-to-end
  • Linux CI (will run on push)

Closes #61
Closes #62

…ableau#61, tableau#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<T>(name)`,
  `get_opt<T>(name)`, `position<T>(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<T>(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<ResultSchema>, 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
…leau#61, tableau#62 part 2)

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<T>` 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
….3.md

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 tableau#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-tableau#70 baseline)
- cargo fmt --check — clean
…ableau#61, tableau#62 part 4)

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
…: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<T> 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<T> 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)
@StefanSteiner StefanSteiner merged commit c1f4d84 into tableau:main May 28, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant