Skip to content

refactor(req): clarify value comparison contract - #243

Merged
tisonkun merged 3 commits into
apache:mainfrom
tisonkun:codex/canonical-float-ordering
Aug 29, 2026
Merged

refactor(req): clarify value comparison contract#243
tisonkun merged 3 commits into
apache:mainfrom
tisonkun:codex/canonical-float-ordering

Conversation

@tisonkun

@tisonkun tisonkun commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

  • rename ReqValue::total_cmp to ReqValue::compare and keep the trait contract implementation-neutral
  • implement primitive ReqValue types explicitly so each ordering and wire-codec mapping is visible without a macro
  • compare f32 and f64 numerically with partial_cmp(...).unwrap(), reject NaN split points, and preserve numeric equality for signed zeros

Rationale

REQ already skips NaN updates and rejects NaN query and deserialized values, so its internal comparator does not need to define an ordering for NaN. For accepted floating-point values, partial_cmp(...).unwrap() matches the Java and C++ numeric comparison behavior, including treating signed zeros as equal, and fails immediately if the NaN invariant is violated.

The six built-in primitive implementations are small and stable enough to keep explicit. They reuse the existing codec error adapter but do not introduce another helper abstraction. The earlier common::float extraction and canonical-hash refactor have been removed; canonical floating-point hashing remains unchanged.

No changelog entry is added because REQ is still unreleased.

Testing

  • cargo x check
  • cargo x test
  • cargo x lint

@tisonkun tisonkun changed the title fix(req): use canonical floating-point ordering refactor(req): clarify value comparison contract Aug 29, 2026
@tisonkun
tisonkun marked this pull request as ready for review August 29, 2026 02:02
@tisonkun
tisonkun merged commit 6f3f09b into apache:main Aug 29, 2026
10 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

Development

Successfully merging this pull request may close these issues.

1 participant