Skip to content

Fix KllItemsSketch sorted-view corruption after query-before-serialize - #767

Merged
leerho merged 1 commit into
apache:mainfrom
jaideeppyne:fix-kll-items-query-serialize-sorted-view-756
Sep 17, 2026
Merged

leerho merged 1 commit into
apache:mainfrom
jaideeppyne:fix-kll-items-query-serialize-sorted-view-756

Conversation

@jaideeppyne

Copy link
Copy Markdown
Contributor

Fixes #756.

Problem

Querying a heap KllItemsSketch before serializing it makes heapify / wrap return a corrupted sorted view (wrong quantiles, duplicated min/max, wrong retained count in the view).

Root cause

KllItemsSketch.CreateSortedView.getSV() sorts level-0 and then records that it did:

final T[] srcQuantiles = getTotalItemsArray();
...
if (!isLevelZeroSorted()) {
  Arrays.sort(srcQuantiles, srcLevelsArr[0], srcLevelsArr[1], comparator);
  if (!hasMemorySegment()) { setLevelZeroSorted(true); }
}

For the heap Items variant, KllHeapItemsSketch.getTotalItemsArray() returns a defensive copy, so the sort lands on the copy while setLevelZeroSorted(true) is set on the live sketch. Serialization writes that flag; heapify / wrap trust it and skip sorting.

The Doubles path is fine because KllHeapDoublesSketch.getDoubleItemsArray() returns the live array, which is what makes the comment at KllDoublesSketch CreateSortedView true.

Fix

Minimal change: do not call setLevelZeroSorted(true) in Items CreateSortedView. Only a copy was sorted, so the live level-0 remains unsorted and the serialized flag must stay false. An alternative would be returning the live array from getTotalItemsArray() to match Doubles; this PR prefers not claiming the live sketch was sorted when it was not.

Tests

Added regression tests in KllItemsSketchSerDeTest:

  • query then heapify — sorted view quantiles/weights and probed ranks match
  • query then wrap — same
  • query after enough updates to force compaction (k=8, n=8) — heapify and wrap match

Also asserts isLevelZeroSorted() stays false after the query on the heap sketch.

AI disclosure

I used Cursor / Grok to help draft the patch and open this PR via the GitHub API (no local clone). I verified the root cause against the sources cited in #756 before editing.

CreateSortedView sorted a defensive copy from getTotalItemsArray() but
still called setLevelZeroSorted(true) on the heap sketch. Serialization
then claimed level-0 was sorted, so heapify/wrap skipped sorting and
returned a corrupted sorted view.

Do not set the flag when only the copy was sorted. Doubles are unaffected
because getDoubleItemsArray() returns the live array.

Fixes apache#756.

@leerho leerho left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for finding and fixing this!

@leerho
leerho merged commit 7b40688 into apache:main Sep 17, 2026
6 checks passed
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.

KllItemsSketch: querying before serialization corrupts the round-tripped sorted view

2 participants