Skip to content

refactor(scalars): generalize the test harness (catalog-driven shape, temporal_values!, unified scalar_matrix!)#261

Open
tobyhede wants to merge 12 commits into
eql_v3from
v3-scalar-harness-reduction
Open

refactor(scalars): generalize the test harness (catalog-driven shape, temporal_values!, unified scalar_matrix!)#261
tobyhede wants to merge 12 commits into
eql_v3from
v3-scalar-harness-reduction

Conversation

@tobyhede
Copy link
Copy Markdown
Contributor

@tobyhede tobyhede commented Jun 5, 2026

What

Generalize the scalar test harness so a type's shape — temporal vs integer, ordered vs equality-only — is catalog-driven, not hand-declared. Adding a scalar drops from ~hundreds of lines to ~one catalog row.

Four levers

1. Catalog is the source of truth — drop the [temporal]/[eq_only] dispatch markers; shape is read from eql-scalars::CATALOG.

 scalar_types! {
     int4 => i32,
-    date => chrono::NaiveDate [temporal],
+    date => chrono::NaiveDate,
 }

2. temporal_values! — one macro generates a chrono-backed scalar's ScalarType wiring, so the next temporal type is a one-liner.

temporal_values! {
    cell = DATE_VALUES_CELL, accessor = date_values,
    rust_type = chrono::NaiveDate, spec = eql_scalars::DATE, variant = Date,
    pg_type = "date",
    parse = |s| NaiveDate::parse_from_str(s, "%Y-%m-%d").unwrap(),
    min_pivot = .., max_pivot = .., sql_lit = |v| format!("'{v}'"),
}

3. Unified scalar_matrix! — one wrapper replaces ordered_numeric_matrix! + eq_only_scalar_matrix!, selected by a caps marker the emitter derives from the catalog.

scalar_matrix! { suite = int4, scalar = i32,  eql_type = "eql_v2_int4", caps = [eq, ord] }  // ordered
scalar_matrix! { suite = ts,   scalar = ..,   eql_type = "eql_v2_ts",   caps = [eq] }        // eq-only

4. Single snapshot — the inventory derives the eq-only name set from the one ordered baseline (matrix_tests.txt minus _ord/order_by/routes_through_ob); no second committed file.

Verification

  • cargo test -p eql-scalars -p eql-tests-macros40 + 13 green
  • matrix inventory → 4 types match the canonical snapshot (names byte-identical)
  • scalar matrix → 844 passed, 0 failed
  • clean && buildrelease/*.sql unchanged (harness/metadata-only)

Test-harness-only; no CHANGELOG entry. The payoff is #257 (timestamptz), which rebases on top and collapses from +671 to +259.

Summary by CodeRabbit

  • Refactor
    • Migrated scalar property classification from explicit markers to catalog-driven properties. Temporal and equality-only scalar behaviors are now determined centrally from a catalog instead of inline annotations, reducing code duplication and improving maintainability. Public APIs and user-facing functionality remain unchanged.

tobyhede added 7 commits June 5, 2026 15:57
…a is_eq_only_token

The ordered_numeric_matrix! suite exercises </>/min/max; an equality-only
scalar (no _ord domain in eql-scalars::CATALOG) does not support those. Route
the matrix emitter through matrix_suite_for_entry, which reads eq-only-ness from
the catalog (is_eq_only_token) and emits a compile_error! for an eq-only token
instead of silently generating ordering tests it cannot pass. Makes the
catalog-derived is_eq_only accessor load-bearing and leaves a clean seam for an
equality-only matrix path. Both arms unit-tested.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f6ed58fe-9521-4545-afa8-467a1f9ae03a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main refactoring objective: deriving scalar harness shape from the catalog while introducing the temporal_values! and unified scalar_matrix! mechanisms.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v3-scalar-harness-reduction

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

tobyhede added 5 commits June 5, 2026 16:44
… ord])

Single entry point selected by a capability marker, replacing the two
parallel wrappers. caps = [eq, ord] is the ordered-numeric body (verbatim
from ordered_numeric_matrix!); caps = [eq] is the equality-only body with
pivots derived from the ScalarType impl (min/max/Default), matching the
proven timestamptz-era eq-only form so the eq-only name set stays a clean
subset of the ordered snapshot. Old wrappers still present; removed next.
…rappers

The matrix-suite emitter now routes every type through scalar_matrix! with
a caps marker derived from the catalog (is_eq_only_token): ordered types get
caps = [eq, ord], equality-only types caps = [eq]. This replaces the prior
ordered-only emission + eq-only compile_error! seam with a real eq-only path,
and deletes ordered_numeric_matrix! / eq_only_scalar_matrix! from matrix.rs.

Generated test names are byte-identical (inventory OK, 4 types match the
canonical snapshot) and the scalar matrix is green at the baseline
(844 passed, 0 failed). release/*.sql unchanged. Doc comments naming the old
wrappers updated to scalar_matrix!.
Each discovered scalar type now matches EITHER the full canonical snapshot
(ordered shape) OR a subset derived from it on the fly — the ordered names
minus the ord-only lines (_ord / order_by / routes_through_ob). An
equality-only scalar (timestamptz, next) is validated against that derivation,
so it needs no second committed snapshot. The per-type shape (ordered/eq_only)
is now printed. All four current types are ordered; the eq-only branch is
dormant but unit-proven (51-line strict subset of the 211-line baseline).
Document that there is ONE committed snapshot (the ordered shape) and that
equality-only types are validated against a subset derived from it on the fly
(baseline minus _ord/order_by/routes_through_ob), so an eq-only scalar needs no
second snapshot. Update the stale ordered_numeric_matrix! references to
scalar_matrix! and describe the printed per-type shape.
@tobyhede tobyhede changed the title refactor(tests): derive scalar harness shape (temporal/eq-only) from the catalog refactor(scalars): generalize the test harness (catalog-driven shape, temporal_values!, unified scalar_matrix!) Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant