tisonkun commented on PR #224:
URL:
https://github.com/apache/datasketches-rust/pull/224#issuecomment-5412720133
Could you please take a look at this round-trip case on a 64-bit target?
```rust
#[test]
fn round_trips_supported_large_max_map_size() {
let sketch = FrequentItemsSketch::<i64>::new(1usize << 31);
let bytes = sketch.serialize();
FrequentItemsSketch::<i64>::deserialize(&bytes).unwrap();
}
```
`new(1 << 31)` currently succeeds and only allocates the small initial map,
while serialization writes `lg_max_map_size = 31`. With this PR,
deserialization then rejects the same bytes because of the new `LG_MAX_MAP_SIZE
= 30` cap. I reproduced the test locally and it fails with `lg_max_map_size
must be at most 30, got 31`.
Is this an intended new limit? If so, the constructor/API contract may need
the same validation and the compatibility change should be documented;
otherwise the deserialize-only cap appears to break a previously supported self
round-trip.
--
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]
