fix(stats): restore big-endian build by giving slot fallbacks an accessible .0#3850
Merged
Conversation
The DataSketches gating in f27ded2 left `HllSlot` and `TDigestSlot` as unit structs on big-endian (`struct HllSlot;`) but six call sites in `update_modes`, `add_numeric_value`, and `to_record` still access `self.hll.0` / `self.tdigest.0`, breaking the s390x build with `E0609: no field '0' on type 'HllSlot'/'TDigestSlot'`. The merge sites at lines 5001/5016 were cfg-gated, but these six sit inside `else if let` chains where cfg-gating doesn't compose cleanly. Promote the big-endian fallbacks to tuple structs wrapping `Option<…Stub>`, where `TDigestStub` and `HllSketchStub` expose the same no-op method surface the call sites use (`update`, `quantile`, `is_empty`, `estimate`). The slots are always `None` on big-endian because `run()` rejects `--quantile-method approx` and `--cardinality-method approx` upstream, so the stub bodies are dead at runtime — they exist only so the shared call sites type-check unchanged. Verified by standalone rustc type-check of all six call shapes against the new stub layout; full little-endian build is clean and 744 stats tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
Two LOW findings from review 2085:
1. Update the t-digest/HLL merge-site skip comments to describe the
slots as "always Default" (the inner Option is always None on
big-endian) instead of "unit-like ZST" — after the previous commit
only the inner stubs are unit-like, the slot types themselves are
tuple-structs wrapping Option<…Stub>.
2. Mirror the little-endian `PartialEq` semantics on the big-endian
slot types: replace the derived `PartialEq` (which is structural
over the inner Option) with explicit `impl PartialEq { fn eq -> true }`
for symmetry with the little-endian little impls. Behavior is
observably the same (the inner Option is always None on big-endian),
but the explicit impl keeps the cross-target API uniform and matches
the documented "equality between sketches is not meaningful" intent.
Verified by little-endian `cargo check` + `cargo t stats` (744 passed)
and a standalone big-endian-shape `rustc` round-trip that exercises
`Stats::clone()` and `==`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The macOS arm64 rust-cache entry under key `qsv-macoslatestcache` contained a broken `~/.cargo/bin/cargo`: rustup's argv-dispatch multi-call binary was clobbered, so restored runs invoked `rustup-init test` (failing with "unexpected argument 'test' found") instead of cargo. Bump the key to `qsv-macoslatestcache-v2` so the next run starts from a clean install and re-populates the cache. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…argo proxy Same `rustup-init: unexpected argument 'test' found` failure as in rust-macos.yml — the `qsv-macospolarslatestcache` entry on the macOS arm64 runner has a broken cargo proxy binary. Bump the key to `qsv-macospolarslatestcache-v2` so the next run reinstalls cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
jqnatividad
added a commit
that referenced
this pull request
May 15, 2026
… defs (#3857) PR #3850's second commit (addressing roborev review 2085 by moving `PartialEq` from a derive to an explicit `impl` on `TDigestSlot`/ `HllSlot`) accidentally left the *old* stub struct + impl alongside the new one. On big-endian targets both `TDigestStub` and `HllSketchStub` were therefore defined twice — once with `#[derive(Default, Clone, PartialEq)]` and once with `#[derive(Default, Clone)]` — producing E0428, E0119, E0592, and E0034 errors and breaking the s390x build (run 25929817626). Delete the redundant `#[derive(Default, Clone, PartialEq)]` copies and keep the leaner `#[derive(Default, Clone)]` versions, matching the post-refactor intent of #3850: `PartialEq` lives on the wrapping `TDigestSlot`/`HllSlot` slot types (manual `impl PartialEq` with constant `true`), not on the inner stubs. Verified by a standalone `rustc --target s390x-unknown-linux-gnu --emit metadata` round-trip of the cfg-gated block (same shape-only verification approach #3850's commit message describes) and by running `cargo test --test tests stats -F all_features` (749 passed) on little-endian. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
E0609: no field 0 on type HllSlot/TDigestSloterrors insrc/cmd/stats.rs. Commit f27ded2 (DataSketches big-endian gating) made the big-endian fallbacks unit structs (struct HllSlot;) but six call sites inupdate_modes,add_numeric_value, andto_recordstill touchself.hll.0/self.tdigest.0— the merge sites at lines 5001/5016 were cfg-gated, but these six sit insideelse if letchains that don't gate cleanly.HllSlot/TDigestSlotto tuple structs ofOption<…Stub>. TheTDigestStubandHllSketchStubexpose the same no-op method surface the call sites use (update,quantile,is_empty,estimate), with signatures mirroring upstreamdatasketches-0.2.0.run()rejects--quantile-method approxand--cardinality-method approxupstream, so the slots are alwaysNone. The stubs exist only so the shared call sites type-check unchanged.Test plan
rustccheck of all six call shapes against the new stub layout passes.cargo build --locked --bin qsv -F all_featuresclean on macOS arm64.cargo t stats -F all_features— 744 passed, 0 failed.s390xworkflow once this lands to confirm the original CI failure is gone.🤖 Generated with Claude Code