Repository navigation
feat(asap-tools): sketchdb arm for sketch-bench ClickHouse workloads - #901
Merged
Merged
Conversation
milindsrivastava1997
left a comment
milindsrivastava1997
left a comment
Contributor
Author
There was a problem hiding this comment.
Code review findings (Claude Code).
| outputs[os.path.join(sql_dir, "asapquery.sql")] = statements_text(asapquery) | ||
| files[BASELINE_MV].append(comment + mv_sql(q, views, labels)) | ||
| if structured: | ||
| files[SKETCHDB].append(comment + asapquery_sql(entry)) |
Contributor
Author
There was a problem hiding this comment.
Without --prebuilt-output-dir, structured groups still get a sketchdb SQL file and a sketchdb key in the sql_file of the baseline and baseline_mv configs, even though no config runs sketchdb. Every run writes unused interval_<T>s_structured_sketchdb.sql files, and the configs suggest a sketchdb arm that doesn't exist. Suggest adding SKETCHDB to the group's modes only when prebuilt_output_dir is set.
Contributor
Author
There was a problem hiding this comment.
Fixed in 828d6aa: sketchdb SQL files and sql_file entries are written only with --prebuilt-output-dir. The ASAPQuery SQL is still rendered, so a bad table entry fails either way.
milindsrivastava1997
force-pushed
the
874-pr3-sketchdb-arm-generator
branch
from
October 15, 2026 04:05
724f04c to
189911f
Compare
milindsrivastava1997
force-pushed
the
874-pr2-feed-sketchdb-arm
branch
from
October 15, 2026 04:05
8d9502f to
7866734
Compare
milindsrivastava1997
force-pushed
the
874-pr3-sketchdb-arm-generator
branch
from
October 15, 2026 04:10
189911f to
828d6aa
Compare
This was referenced Oct 15, 2026
milindsrivastava1997
force-pushed
the
874-pr3-sketchdb-arm-generator
branch
from
October 15, 2026 14:51
bdf7f76 to
f9f9846
Compare
milindsrivastava1997
force-pushed
the
874-pr2-feed-sketchdb-arm
branch
2 times, most recently
from
October 15, 2026 16:28
eee2cec to
7202240
Compare
Structured entries get their own query groups, ordered before the PromQL ones, each with a sketchdb SQL file (the entry's ASAPQuery SQL) in place of asapquery.sql. With --prebuilt-output-dir the generator writes <stem>_clickhouse_sketchdb.yaml: baseline and sketchdb in one experiment on those planner configs, so sketchdb's query indices are baseline's first and its results can be scored against baseline's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without --prebuilt-output-dir no config runs sketchdb, yet structured groups got sketchdb SQL files and sql_file entries. They are now written only with the flag; the ASAPQuery SQL is still rendered, so a bad entry fails either way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The engine's ClickHouse adapter serves SQL at /clickhouse/query; the query client posted to the server URL's root, so every sketchdb query got a 404. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
milindsrivastava1997
force-pushed
the
874-pr3-sketchdb-arm-generator
branch
from
October 15, 2026 16:32
f9f9846 to
a098c70
Compare
milindsrivastava1997
changed the base branch from
874-pr2-feed-sketchdb-arm
to
main
October 15, 2026 16:32
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
1 participant
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.
Closes #874 (3 of 3). Stacked on #900; review only its commits.
Problem:
sketch_bench_workloads.pywroteasapquery.sqlfor an ASAPQuery arm, but no config read it.Fix:
interval_<T>s_structured), ordered before the PromQL groups (interval_<T>s). With--prebuilt-output-dir, each gets a sketchdb SQL file (the entry'ssqlwithNOW()replaced by the floored eval time), in place ofasapquery.sql; without it no sketchdb SQL is written.--prebuilt-output-dir(e.g. fromasap-planner --planner saved) writes<stem>_clickhouse_sketchdb.yaml: baseline and sketchdb in one experiment, socalculate_fidelitycan score sketchdb against baseline, withcontroller.prebuilt_output_dirset (hence@package _global_).The group split renames files and reorders groups in every arm's config.
Tests:
tests/test_sketch_bench_workloads.py, including an end-to-end check: the generated sketchdb client config gets only the structured group and passes the runner's prebuilt-plan check against a saved plan onNOW().🤖 Generated with Claude Code