Repository navigation
HLL cross-language test: add union and updatable CL files - #790
Merged
Merged
Conversation
The existing fixtures are update sketches only, serialized compact, at a single lgK. That leaves two gaps, and both produced real bugs this month that these files would not have caught: - the deferred KxQ rebuild is reachable only through a union gadget, and the generator creates no unions at all; - the flags-byte bit 32 collision (REBUILD_CURMIN_NUM_KXQ here, FULL_SIZE in C++) lives in the flags byte, which nothing inspected, and only the compact form was ever emitted. Generates six more files from a union whose inputs span all three curModes, so the coupon-update path into a gadget with a pending rebuild is exercised, in both compact and updatable form for HLL_4/6/8. One lgK is deliberate: neither defect class is lgK-dependent, and the files are generated on demand into a gitignored directory, so this adds no bytes to the repository. The check is one body reused by all four languages, matching the existing pattern, and asserts three things the September fixes established: bit 32 is reserved and must never be written by any implementation, the compact flag must match the form actually produced (HLL_6 and HLL_8 used to omit it), and the estimate must be identical across target types for the same input, since an incorrect KxQ rebuild shifts it. Verified by corrupting a fixture: setting bit 32 and clearing the compact flag each fail with the intended message, and the suite is 4/4 green otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BKM9nrFVMhH2gmFaag2JuC
proost
reviewed
Nov 2, 2026
| assertEquals(sketch.getLgConfigK(), LG_K); | ||
| assertTrue(!sketch.isEmpty()); | ||
| assertEquals(sketch.getEstimate(), UNION_DISTINCT, UNION_DISTINCT * 0.02); | ||
|
|
Member
There was a problem hiding this comment.
For more robust compatibility test, how about adding comparing "curMin", "numAtCurMin"?
Member
Author
There was a problem hiding this comment.
Agreed, those are exactly the fields the deferred rebuild updates, so checking them is more direct than relying on the estimate. Added (b466491): each union CL file's curMin and numAtCurMin are compared with the values in Java's own result for the same target type and form. Only the Java CL files exist today; the C++/Go/Rust checks will start comparing once those languages write union CL files.
The deferred KxQ rebuild also updates curMin and numAtCurMin, so check them directly instead of only through the estimate. Each union CL file is compared with Java's own result for the same target type and form. Also calls these cross-language (CL) files in the javadoc. As suggested in review. Co-Authored-By: Claude Opus 5.5 <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
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.
Adds union-result cross-language (CL) files to the HLL cross-language test. Test-only; no library code changes.
Why
The existing HLL CL files are update sketches only, serialized compact, at a single lgK. That misses two classes of bugs, and both turned up in September 2026:
REBUILD_CURMIN_NUM_KXQin Java,FULL_SIZEin C++). It lives in the flags byte, which nothing inspected, and only the compact form was ever written.What it adds
generateUnionBinariesForCompatibilityTestingwrites six files,hll{4,6,8}_union_java.sk(compact) andhll{4,6,8}_unionupd_java.sk(updatable). They come from a union whose inputs span all three curModes, so coupon updates into a gadget with a pending KxQ rebuild are exercised. One lgK is enough, because neither defect class depends on lgK. The files go to the gitignored generated-files directory, so the repository doesn't grow.checkUnionJava,checkUnionCpp,checkUnionGoandcheckUnionRustshare one check body. For each file it asserts that:No C++, Go or Rust generator writes these files yet. Until one does, those checks warn about the missing files and skip them. Matching generators in the other repos would be follow-ups.
Testing
-Pcheck_java_files: generates the files and reads them back, 4/4HllSketchCrossLanguageTesttests pass.mvn verifypasses (2320 tests). The default test run is unaffected, because these groups are excluded by default.Supersedes the never-opened branch
hll-xlang-fixtures. This is the same commit, rebased onto main after #771.🤖 Generated with Claude Code