Skip to content

Allow theta sketches with lg_k = 4 to match Java - #514

Open
SavicStefan wants to merge 2 commits into
apache:masterfrom
SavicStefan:lg=4
Open

Allow theta sketches with lg_k = 4 to match Java#514
SavicStefan wants to merge 2 commits into
apache:masterfrom
SavicStefan:lg=4

Conversation

@SavicStefan

@SavicStefan SavicStefan commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Java allows theta and tuple sketches down to lg_k = 4 (16 nominal entries), but C++ rejected anything below lg_k = 5, so the two libraries disagreed on the smallest sketch you can build.

The C++ theta implementation used a single constant, theta_constants::MIN_LG_K = 5, for two distinct roles: the floor on the user-requested nominal size (K) and the floor on the internal hash table (cache) size. Java keeps these separate ThetaUtil.MIN_LG_NOM_LONGS = 4 and MIN_LG_ARR_LONGS = 5, so Java accepts lg_k = 4 (nominal 16) while C++ rejected it in theta_base_builder::set_lg_k.

Because C++ used 5 for both, theta_base_builder::set_lg_k(4) threw, and there was no way to build a theta/tuple update sketch or union at lg_k = 4.

This only affected construction. Reading was already fine: the compact serialization format does not store lg_nom, so C++ has always been able to deserialize Java-produced lg = 4 compact sketches.

Fix, split the one constant into the two that Java has:

  • MIN_LG_K -> 4, the nominal floor checked by set_lg_k.
  • new MIN_LG_ARR -> 5, the internal hash-table floor used by starting_sub_multiple().

A lg_k = 4 sketch now starts with lg_cur_size (5) > lg_nom_size (4): the hash table keeps its full 32 slots and simply rebuilds down to the nominal 16, a path the existing update/rebuild logic already handles. There is no format or behavior change for lg_k >= 5.

The fix covers tuple sketches as well as theta, with no tuple-specific change. Tuple sketches have no size constants of their own their builder inherits theta_base_builder (sharing the same set_lg_k validation and starting_lg_size()), and their internal hash table is a theta_update_sketch_base (the exact table this PR resizes). So lg_k = 4 now works for tuple update sketches and unions too.

Added new min lg_k tests in both theta_sketch_test.cpp and tuple_sketch_test.cpp. Each test:

  • builds at lg_k = 4 and confirms set_lg_k(MIN_LG_K - 1) throws;
  • verifies the new MIN_LG_ARR floor -- after many updates the peak retained count is > 16 (2^MIN_LG_K) yet <= 32 (2^MIN_LG_ARR), i.e. the table keeps its 32 slots rather than collapsing to the nominal 16;
  • confirms estimation mode, with the true count inside the 2-standard-deviation confidence bounds;
  • trims down to exactly the nominal 16 entries;
  • round-trips through compact serialize/deserialize.

Stefan Savić added 2 commits August 20, 2026 10:00
The C++ theta implementation used a single constant, theta_constants::MIN_LG_K
= 5, for two distinct roles: the floor on the user-requested nominal size (K)
and the floor on the internal hash table (cache) size. Java keeps these
separate -- ThetaUtil.MIN_LG_NOM_LONGS = 4 and MIN_LG_ARR_LONGS = 5 -- so Java
accepts lg_k = 4 (nominal 16) while C++ rejected it in theta_base_builder::
set_lg_k.

As a result, C++ could not *construct* a theta/tuple update sketch or a union
at lg_k = 4, even though Java can. (Reading was unaffected: the compact
serialization format does not carry lg_nom, so C++ already deserializes
Java-produced lg = 4 compact sketches -- this only concerns building them.)

Split the constant: MIN_LG_K becomes 4 (the nominal floor validated by
set_lg_k, shared by update/union/tuple builders), and a new MIN_LG_ARR = 5
keeps the hash table floored at 32 slots via starting_sub_multiple(). A lg_k =
4 sketch therefore starts with lg_cur_size (5) > lg_nom_size (4) and only ever
rebuilds down to the nominal 16, which the existing update/rebuild path already
handles.

Add tests: building at the minimum lg_k (and rejecting below it), estimation +
trim + serialization round trip for an update sketch, and a union built at the
minimum lg_k.

Co-authored-by: Isaac
Signed-off-by: Stefan Savić <stefan.savic@databricks.com>
@SavicStefan
SavicStefan marked this pull request as ready for review August 20, 2026 12:40
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