Skip to content

REQ: check each compactor's lgWeight against its level - #788

Merged
leerho merged 1 commit into
mainfrom
req-lgweight-check
Nov 1, 2026
Merged

leerho merged 1 commit into
mainfrom
req-lgweight-check

Conversation

@leerho

@leerho leerho commented Nov 1, 2026

Copy link
Copy Markdown
Member

Problem

ReqSketch.heapify() reads each compactor's lgWeight byte without checking it. The sketch always creates the compactor at level h with lgWeight == h, so any other value means the image is corrupt.

A wrong value doesn't cause an error later. The sorted view weights each compactor's items by 1L << lgWeight (since #787), so a corrupt byte silently changes every rank and quantile the sketch reports.

Change

ReqSerDe now checks that compactor i has lgWeight == i, in both exact and estimation mode, and throws SketchesArgumentException otherwise. Valid images, including the C++ images in the cross-language tests, are unaffected.

Tests

The new ReqSerDeTest:

  • confirms that valid images of 0 to 1,000,000 items still heapify;
  • rejects a wrong lgWeight in the first compactor (70 and 1), in the second compactor, and in the single compactor of an exact-mode image.

mvn verify passes.

🤖 Generated with Claude Code

ReqSketch.heapify() accepted any lgWeight byte. The sketch creates the
compactor at level h with lgWeight h, so any other value is corrupt.
It does not fail later: the sorted view weights the compactor's items by
1L << lgWeight, so a wrong value silently changes ranks and quantiles.

Check that compactor i has lgWeight i, in exact and estimation mode.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@leerho
leerho requested review from proost and tisonkun November 1, 2026 00:38
@leerho
leerho merged commit 5526d1a into main Nov 1, 2026
6 checks passed
@leerho
leerho deleted the req-lgweight-check branch November 1, 2026 16:33
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.

2 participants