Repository navigation
fix!: port the v2.0 sketch fixes to v1.5 - #15
Merged
Merged
Conversation
Ports the fixes from #13 and #14 (the codegen refactor stays on v2.0): - K is validated against the range each library accepts. Quantiles needs a power of 2 from 2 to 32768, KLL 8 to 65535, TDigest 10 to 65535 (K above 65535 was truncated), REQ 4 to 1024. REQ's library kept K in 8 bits, so K of 256 or more wrapped around (256 became 4); the vcpkg overlay now patches that (apache/datasketches-cpp#552). Theta's lg_k was cast to uint8_t unchecked; it is now validated. - CPC keeps its HIP estimator when combining an empty partial state. - HLL, CPC and Theta bounds validate num_std_devs (1, 2 or 3) instead of failing with a raw library error. - Theta and Frequent Items report corrupt sketches as "Failed to deserialize ... sketch" instead of a raw library error. - datasketch_frequent_items_get_frequent's error type is case-insensitive and rejects other values (anything but the exact 'NO_FALSE_NEGATIVES' silently meant NO_FALSE_POSITIVES); a NULL lg_max_k is an error. - Casts to sketch_hll, sketch_cpc, sketch_theta and sketch_frequent_items have the sketch type instead of BLOB. - Every sketch aggregate returns NULL when it has no input rows. BREAKING CHANGE: datasketch_theta, datasketch_frequent_items and datasketch_frequent_items_weighted over no rows return NULL instead of an empty sketch (use coalesce(..., 0) on an estimate for the old result), and K values that used to be accepted but misbehaved are now bind errors. 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.
Ports the behaviour fixes from #13 and #14 to v1.5. The codegen refactor stays on v2.0, because v1.5's codegen has diverged too far to share it.
lg_kwas cast touint8_tunchecked and is now validated.num_std_devs.get_frequenterror type is case-insensitive and rejects invalid values; anything but the exact'NO_FALSE_NEGATIVES'used to silently mean NO_FALSE_POSITIVES.lg_max_kis now an error.sketch_hll,sketch_cpc,sketch_thetaandsketch_frequent_itemsgive the sketch type instead of BLOB.Built on linux_arm64 against DuckDB 1.5. The full suite passes (866 assertions), and the UUID/HUGEINT parallel checks from #11 pass 30/30.
🤖 Generated with Claude Code