refactor(scalars): generalize the test harness (catalog-driven shape, temporal_values!, unified scalar_matrix!) by tobyhede · Pull Request #261 · cipherstash/encrypt-query-language

tobyhede · 2026-06-05T06:22:08Z

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-macros → 40 + 13 green
matrix inventory → 4 types match the canonical snapshot (names byte-identical)
scalar matrix → 844 passed, 0 failed
clean && build → release/*.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
- Simplified scalar domain classification by centralizing temporal and equality-only detection through the catalog, eliminating manual markers.
- Unified scalar matrix test generation into a single configurable macro with capability flags instead of separate ordered/equality-only variants.
- Enhanced index scan validation to verify exact index matching via execution plan analysis.
Tests
- Improved encrypted domain matrix test snapshot validation to support derived equality-only subsets alongside the canonical baseline.

…e wiring

…p [temporal] marker

…ived)

…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 · 2026-06-05T06:22:16Z

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: 6d657ea4-0939-4eb7-817f-c9abf87259e9

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.}

… 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.

Copilot

Pull request overview

This PR refactors the SQLx encrypted-domain scalar test harness so scalar “shape” (temporal vs non-temporal, ordered vs equality-only) is derived from eql-scalars::CATALOG, eliminating per-type shape markers and consolidating matrix generation behind a single scalar_matrix! wrapper.

Changes:

Removed [temporal] dispatch markers from the scalar harness list; temporal / eq-only classification now comes from catalog helpers (ScalarKind::is_temporal, ScalarSpec::is_eq_only).
Introduced temporal_values! in the SQLx harness to generate chrono-backed scalar wiring (fixtures parsing + ScalarType impl + invariants).
Unified the matrix wrapper macros into scalar_matrix! { caps = [...] } and updated snapshot inventory tooling/docs to validate eq-only types via a derived subset from the single canonical snapshot.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/sqlx/tests/encrypted_domain/scalars/mod.rs	Updates generated-suite docs to reference `scalar_matrix!`.
tests/sqlx/src/scalar_types.rs	Removes temporal marker usage and documents catalog-driven shape.
tests/sqlx/src/scalar_domains.rs	Adds `temporal_values!` macro and migrates `date` wiring to it.
tests/sqlx/src/matrix.rs	Replaces dual wrappers with unified `scalar_matrix!` using `caps = [eq]` vs `[eq, ord]`.
tests/sqlx/snapshots/README.md	Documents single snapshot + derived eq-only subset approach.
mise.toml	Updates matrix inventory task to accept either ordered baseline or derived eq-only subset.
crates/eql-tests-macros/src/lib.rs	Removes marker parsing; consults `eql-scalars::CATALOG` to derive temporal/eq-only and emit `scalar_matrix!` with caps.
crates/eql-tests-macros/Cargo.toml	Adds `eql-scalars` dependency for catalog-driven emission.
crates/eql-scalars/src/lib.rs	Adds `ScalarKind::is_temporal` and `ScalarSpec::is_eq_only` helpers + tests.
Cargo.lock	Records the new `eql-tests-macros -> eql-scalars` dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+  if cmp -s "/tmp/matrix-norm-${t}.txt" snapshots/matrix_tests.txt; then
+    shape="ordered"
+  elif [ "$(cat "/tmp/matrix-norm-${t}.txt")" = "$eq_only_expected" ]; then
+    shape="eq_only"


The eq-only inventory check captured the derived subset into a shell variable and compared with [ string equality, which strips trailing newlines on capture and forced the failure diff to reconstruct the string via printf. Materialise the subset to a file so both shapes compare through the same cmp/diff path as the ordered snapshot.

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

tobyhede added 7 commits June 5, 2026 15:57

test(scalars): add temporal_values! macro for chrono-backed ScalarTyp…

06aec21

…e wiring

test(scalars): generate date ScalarType via temporal_values!

c34b2c3

feat(eql-scalars): add is_temporal()/is_eq_only() capability accessors

4026f3a

build(eql-tests-macros): depend on eql-scalars catalog

dc72afb

refactor(eql-tests-macros): derive temporal/eq_only from catalog, dro…

4eb40cd

…p [temporal] marker

test(scalars): drop [temporal] marker from dispatch list (catalog-der…

0c4a55b

…ived)

tobyhede added 5 commits June 5, 2026 16:44

style(eql-tests-macros): apply rustfmt to scalar matrix emitters

4e82b47

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

tobyhede mentioned this pull request Jun 5, 2026

feat(scalars): add eql_v3.timestamptz encrypted-domain type (equality-only) #257

Merged

coderdan requested a review from Copilot June 8, 2026 03:22

Copilot started reviewing on behalf of coderdan June 8, 2026 03:22 View session

Copilot AI reviewed Jun 8, 2026

View reviewed changes

Comment thread mise.toml

Comment on lines +192 to +195

if cmp -s "/tmp/matrix-norm-${t}.txt" snapshots/matrix_tests.txt; then

shape="ordered"

elif [ "$(cat "/tmp/matrix-norm-${t}.txt")" = "$eq_only_expected" ]; then

shape="eq_only"

coderdan approved these changes Jun 8, 2026

View reviewed changes

tobyhede merged commit 8b6b7a9 into eql_v3 Jun 8, 2026
11 checks passed

tobyhede deleted the v3-scalar-harness-reduction branch June 8, 2026 22:44

This was referenced Jun 9, 2026

refactor(eql-scalars): split lib.rs into definitions + impl/test modules #266

Merged

test(v3): property-based tests for the eql_v3 encrypted scalar domains (CIP-3141) #275

Merged

feat(v3): N-block ORE comparator + ordered numeric & timestamptz #276

Merged

tobyhede added a commit that referenced this pull request Jun 20, 2026

Merge pull request #261 from cipherstash/v3-scalar-harness-reduction

accc36b

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tobyhede commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tobyhede commented Jun 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Four levers

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tobyhede commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading