Skip to content

refactor: retire sketch-core mirror#307

Merged
zzylol merged 2 commits intomainfrom
refactor/retire-sketch-core-asap-sketchlib-main
May 2, 2026
Merged

refactor: retire sketch-core mirror#307
zzylol merged 2 commits intomainfrom
refactor/retire-sketch-core-asap-sketchlib-main

Conversation

@zzylol
Copy link
Copy Markdown
Contributor

@zzylol zzylol commented May 1, 2026

Summary

Retire the vendored sketch-core crate. Switch query-engine consumer imports to the reorganized asap_sketchlib (#36 — runtime sketches now embedded in the existing src/sketches/ layout, no separate asap:: namespace).

Mechanical path swaps

Old (sketch-core → first cut on PR #36) New (after PR #36 reorganization)
asap_sketchlib::asap::count_min::* asap_sketchlib::sketches::countmin::*
asap_sketchlib::asap::kll::* asap_sketchlib::sketches::kll::*
asap_sketchlib::asap::count_min_with_heap::* asap_sketchlib::sketches::cms_heap::*
asap_sketchlib::asap::hydra_kll::* asap_sketchlib::sketches::hydra_kll::*
asap_sketchlib::asap::set_aggregator::* asap_sketchlib::sketches::set_aggregator::*
asap_sketchlib::asap::delta_set_aggregator::* asap_sketchlib::sketches::delta_set_aggregator::*
asap_sketchlib::asap::config::* asap_sketchlib::asap_runtime::*

(This legacy fork doesn't have DDSketch / CountSketch / HllSketch accumulators — those live only in ASAPQuery-backend.)

Renames carried through

  • HeapItemCmsHeapItem (avoids collision with common::input::HeapItem)

main.rs aliases asap_sketchlib::asap_runtime as config so the existing clap-derive references (config::DEFAULT_CMS_IMPL, config::configure(...)) keep working without touching the rest of the bin.

Workspace cleanup

  • asap-common/sketch-core/ deleted.
  • Workspace member dropped from top-level Cargo.toml; query-engine Cargo.toml no longer pulls sketch-core.
  • Dockerfile no longer copies asap-common/sketch-core.

Tests

```
CARGO_NET_GIT_FETCH_WITH_CLI=true cargo check -p query_engine_rust → clean
CARGO_NET_GIT_FETCH_WITH_CLI=true cargo test -p query_engine_rust --lib precompute_operators → 87 passed, 0 failed
```

Depends on

ProjectASAP/asap_sketchlib#36 (force-pushed; head: e473ccc).

zzylol added a commit that referenced this pull request May 1, 2026
Update PR #307 against the reorganized asap_sketchlib (PR #36) — runtime
sketches now live in the existing src/sketches/ layout (single home per
sketch concept), not under a separate `asap::` namespace.

Path swaps in asap-query-engine (legacy fork; smaller surface than
ASAPQuery-backend — no DDSketch / CountSketch / HllSketch accumulators):
- asap_sketchlib::asap::count_min::*            → ::sketches::countmin::*
- asap_sketchlib::asap::kll::*                  → ::sketches::kll::*
- asap_sketchlib::asap::count_min_with_heap::*  → ::sketches::cms_heap::*
- asap_sketchlib::asap::hydra_kll::*            → ::sketches::hydra_kll::*
- asap_sketchlib::asap::set_aggregator::*       → ::sketches::set_aggregator::*
- asap_sketchlib::asap::delta_set_aggregator::* → ::sketches::delta_set_aggregator::*
- asap_sketchlib::asap::config::*               → ::asap_runtime::*

Rename carried through:
- HeapItem → CmsHeapItem (avoids common::input::HeapItem collision)

main.rs aliases asap_runtime as `config` so existing clap-derive references
keep working.

Tests:
- cargo check -p query_engine_rust                       → clean
- cargo test -p query_engine_rust --lib precompute_operators → 87 passed, 0 failed

Depends on ProjectASAP/asap_sketchlib#36 (force-pushed e473ccc).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 1, 2026
Update PR #307 against the reorganized asap_sketchlib (PR #36) — runtime
sketches now live in the existing src/sketches/ layout (single home per
sketch concept), not under a separate `asap::` namespace.

Path swaps in asap-query-engine (legacy fork; smaller surface than
ASAPQuery-backend — no DDSketch / CountSketch / HllSketch accumulators):
- asap_sketchlib::asap::count_min::*            → ::sketches::countmin::*
- asap_sketchlib::asap::kll::*                  → ::sketches::kll::*
- asap_sketchlib::asap::count_min_with_heap::*  → ::sketches::cms_heap::*
- asap_sketchlib::asap::hydra_kll::*            → ::sketches::hydra_kll::*
- asap_sketchlib::asap::set_aggregator::*       → ::sketches::set_aggregator::*
- asap_sketchlib::asap::delta_set_aggregator::* → ::sketches::delta_set_aggregator::*
- asap_sketchlib::asap::config::*               → ::asap_runtime::*

Rename carried through:
- HeapItem → CmsHeapItem (avoids common::input::HeapItem collision)

main.rs aliases asap_runtime as `config` so existing clap-derive references
keep working.

Tests:
- cargo check -p query_engine_rust                       → clean
- cargo test -p query_engine_rust --lib precompute_operators → 87 passed, 0 failed

Depends on ProjectASAP/asap_sketchlib#36 (force-pushed e473ccc).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol zzylol force-pushed the refactor/retire-sketch-core-asap-sketchlib-main branch from d1ed32d to 4af8953 Compare May 1, 2026 20:36
zzylol added a commit that referenced this pull request May 1, 2026
Update PR #307 against the reorganized asap_sketchlib (PR #36) — runtime
sketches now live in the existing src/sketches/ layout (single home per
sketch concept), not under a separate `asap::` namespace.

Path swaps in asap-query-engine (legacy fork; smaller surface than
ASAPQuery-backend — no DDSketch / CountSketch / HllSketch accumulators):
- asap_sketchlib::asap::count_min::*            → ::sketches::countmin::*
- asap_sketchlib::asap::kll::*                  → ::sketches::kll::*
- asap_sketchlib::asap::count_min_with_heap::*  → ::sketches::cms_heap::*
- asap_sketchlib::asap::hydra_kll::*            → ::sketches::hydra_kll::*
- asap_sketchlib::asap::set_aggregator::*       → ::sketches::set_aggregator::*
- asap_sketchlib::asap::delta_set_aggregator::* → ::sketches::delta_set_aggregator::*
- asap_sketchlib::asap::config::*               → ::asap_runtime::*

Rename carried through:
- HeapItem → CmsHeapItem (avoids common::input::HeapItem collision)

main.rs aliases asap_runtime as `config` so existing clap-derive references
keep working.

Tests:
- cargo check -p query_engine_rust                       → clean
- cargo test -p query_engine_rust --lib precompute_operators → 87 passed, 0 failed

Depends on ProjectASAP/asap_sketchlib#36 (force-pushed e473ccc).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol zzylol force-pushed the refactor/retire-sketch-core-asap-sketchlib-main branch from 4af8953 to 9c96274 Compare May 1, 2026 20:53
Update PR #307 against the reorganized asap_sketchlib (PR #36) — runtime
sketches now live in the existing src/sketches/ layout (single home per
sketch concept), not under a separate `asap::` namespace.

Path swaps in asap-query-engine (legacy fork; smaller surface than
ASAPQuery-backend — no DDSketch / CountSketch / HllSketch accumulators):
- asap_sketchlib::asap::count_min::*            → ::sketches::countmin::*
- asap_sketchlib::asap::kll::*                  → ::sketches::kll::*
- asap_sketchlib::asap::count_min_with_heap::*  → ::sketches::cms_heap::*
- asap_sketchlib::asap::hydra_kll::*            → ::sketches::hydra_kll::*
- asap_sketchlib::asap::set_aggregator::*       → ::sketches::set_aggregator::*
- asap_sketchlib::asap::delta_set_aggregator::* → ::sketches::delta_set_aggregator::*
- asap_sketchlib::asap::config::*               → ::asap_runtime::*

Rename carried through:
- HeapItem → CmsHeapItem (avoids common::input::HeapItem collision)

main.rs aliases asap_runtime as `config` so existing clap-derive references
keep working.

Tests:
- cargo check -p query_engine_rust                       → clean
- cargo test -p query_engine_rust --lib precompute_operators → 87 passed, 0 failed

Depends on ProjectASAP/asap_sketchlib#36 (force-pushed e473ccc).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zzylol zzylol force-pushed the refactor/retire-sketch-core-asap-sketchlib-main branch from 9c96274 to e2e4b67 Compare May 1, 2026 21:29
@zzylol zzylol merged commit ba0cfce into main May 2, 2026
8 of 9 checks passed
zzylol added a commit that referenced this pull request May 2, 2026
zzylol added a commit that referenced this pull request May 2, 2026
zzylol added a commit that referenced this pull request May 2, 2026
After PR #307 was admin-merged with a failing test (and then reverted in
PR #310), redo the consumer migration cleanly with two follow-up changes:

1. Drop the \`branch = "refactor/adopt-sketch-core-modules"\` pin from
   asap-query-engine/Cargo.toml. asap_sketchlib#36 is on main as
   commit \`d22a9ab\`; the consumer now tracks the default branch.

2. Loosen the \`test_esdsl_time_range_query\` assertion to ±1 tolerance.
   The test computed P90 of 200..300 from a KLL and asserted exactly 291;
   asap_sketchlib's KLL reports 290 on this distribution, which is within
   KLL's published rank-error bound but breaks an exact-match assertion
   that previously passed against the dsrs/datasketches backend (now
   retired with sketch-core).

Tests:
- \`cargo test -p query_engine_rust --lib precompute_operators\` → 87 / 0
- \`cargo test -p query_engine_rust --lib tests::elastic\` → 15 / 0
- \`cargo test -p query_engine_rust --lib tests::elastic_dsl_query_tests::tests::test_esdsl_time_range_query\` → passes

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol added a commit that referenced this pull request May 2, 2026
After PR #307 was admin-merged with a failing test (and then reverted in
PR #310), redo the consumer migration cleanly with two follow-up changes:

1. Drop the \`branch = "refactor/adopt-sketch-core-modules"\` pin from
   asap-query-engine/Cargo.toml. asap_sketchlib#36 is on main as
   commit \`d22a9ab\`; the consumer now tracks the default branch.

2. Loosen the \`test_esdsl_time_range_query\` assertion to ±1 tolerance.
   The test computed P90 of 200..300 from a KLL and asserted exactly 291;
   asap_sketchlib's KLL reports 290 on this distribution, which is within
   KLL's published rank-error bound but breaks an exact-match assertion
   that previously passed against the dsrs/datasketches backend (now
   retired with sketch-core).

Tests:
- \`cargo test -p query_engine_rust --lib precompute_operators\` → 87 / 0
- \`cargo test -p query_engine_rust --lib tests::elastic\` → 15 / 0
- \`cargo test -p query_engine_rust --lib tests::elastic_dsl_query_tests::tests::test_esdsl_time_range_query\` → passes

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
milindsrivastava1997 pushed a commit that referenced this pull request May 4, 2026
…est fix (#309)

* Revert "Revert "refactor: retire sketch-core mirror (#307)" (#310)"

This reverts commit ea723f4.

* chore: track asap_sketchlib main + fix elastic DSL quantile assert

After PR #307 was admin-merged with a failing test (and then reverted in
PR #310), redo the consumer migration cleanly with two follow-up changes:

1. Drop the \`branch = "refactor/adopt-sketch-core-modules"\` pin from
   asap-query-engine/Cargo.toml. asap_sketchlib#36 is on main as
   commit \`d22a9ab\`; the consumer now tracks the default branch.

2. Loosen the \`test_esdsl_time_range_query\` assertion to ±1 tolerance.
   The test computed P90 of 200..300 from a KLL and asserted exactly 291;
   asap_sketchlib's KLL reports 290 on this distribution, which is within
   KLL's published rank-error bound but breaks an exact-match assertion
   that previously passed against the dsrs/datasketches backend (now
   retired with sketch-core).

Tests:
- \`cargo test -p query_engine_rust --lib precompute_operators\` → 87 / 0
- \`cargo test -p query_engine_rust --lib tests::elastic\` → 15 / 0
- \`cargo test -p query_engine_rust --lib tests::elastic_dsl_query_tests::tests::test_esdsl_time_range_query\` → passes

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@milindsrivastava1997 milindsrivastava1997 deleted the refactor/retire-sketch-core-asap-sketchlib-main branch May 8, 2026 12:37
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.

1 participant