tisonkun opened a new issue, #186:
URL: https://github.com/apache/datasketches-rust/issues/186

   ## Problem
   
   `CompactThetaSketch::deserialize_v2` reads the retained entries from a 
version 2 exact compact image (`preamble_longs == 2`) but always constructs the 
sketch with `empty: true`.
   
   A valid v2 exact image with one retained hash therefore deserializes with 
all of the following contradictory observations:
   
   - `num_retained() == 1`
   - `is_empty() == true`
   - `estimate() == 0.0`
   
   Besides returning the wrong estimate, the false logical-empty state can 
cause set operations to skip a real input.
   
   Reviewed revision: `4266ad9cad21e0660c2ae9506174e01665bd100d`.
   
   ## Expected behavior
   
   For a v2 exact image (`preamble_longs == 2`), logical emptiness is 
determined by `num_entries == 0`. An image with one or more retained entries 
must be non-empty and estimate its exact retained count.
   
   ## Relationship to other implementations
   
   The current C++ v2 reader implements this rule by returning empty only when 
`num_entries == 0`, and otherwise constructs a non-empty compact sketch:
   
   
https://github.com/apache/datasketches-cpp/blob/c22888581964fc490feee835766cc8c3adb722e0/theta/include/theta_sketch_impl.hpp#L605-L631
   
   The current Go v2 decoder uses the same rule:
   
   
https://github.com/apache/datasketches-go/blob/c558cc2d64f9a307c196a7adb0067eadd1b776f7/theta/decoder.go#L295-L343
   
   Go also retains compatibility fixtures explicitly identified as 
Java-generated v2 compact images. Current Java only heapifies compact 
serialization versions 3 and 4, so C++ and Go provide the active reference 
behavior for reading legacy Java v2 images:
   
   
https://github.com/apache/datasketches-go/blob/c558cc2d64f9a307c196a7adb0067eadd1b776f7/theta/sketch_serialization_test.go#L210-L228
   
   
https://github.com/apache/datasketches-java/blob/4067ffefabbcd03944dc3617ff2948ab760f3b75/src/main/java/org/apache/datasketches/theta/CompactThetaSketch.java#L97-L115
   
   ## Proposed fix
   
   Set `empty` from `num_entries == 0` in the v2 exact branch, matching the 
current C++ and Go legacy readers.
   
   ## Acceptance criteria
   
   - Add a regression test for a v2 exact image with retained entries and 
verify retained count, logical emptiness, estimate, and bounds.
   - Add or retain coverage showing a zero-entry v2 exact image remains empty.
   - Verify the decoded sketch survives a current-format round trip without 
losing its entries or estimate.
   - Run the repository check, test, and lint workflows.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to