From 603a5ff84b771da56f1805f1f4b72c0d43167880 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 1 Jul 2026 15:32:52 -0400 Subject: [PATCH 1/3] feat(tools): updated experiment infra to run experiemnts against Clickhouse using different strategies such as using materialized views, or using sketches --- asap-tools/experiments/constants.py | 5 + .../experiments/experiment_run_clickhouse.py | 15 ++- .../experiments/experiment_utils/config.py | 100 +++++++++++++----- 3 files changed, 90 insertions(+), 30 deletions(-) diff --git a/asap-tools/experiments/constants.py b/asap-tools/experiments/constants.py index 2358a3ae..f740084e 100644 --- a/asap-tools/experiments/constants.py +++ b/asap-tools/experiments/constants.py @@ -25,6 +25,11 @@ SKETCHDB_EXPERIMENT_NAME = "sketchdb" BASELINE_EXPERIMENT_NAME = "baseline" +# ClickHouse precompute-strategy arms (issue #491): same raw table/queries as +# baseline, but querying via ClickHouse's own precompute mechanisms. +BASELINE_SKETCH_EXPERIMENT_NAME = "baseline_sketch" +BASELINE_MV_EXPERIMENT_NAME = "baseline_mv" +BASELINE_MV_SKETCH_EXPERIMENT_NAME = "baseline_mv_sketch" AVOID_REMOTE_MONITOR_LONG_SSH = True AVOID_RUN_ARROYOSKETCH_LONG_SSH = True diff --git a/asap-tools/experiments/experiment_run_clickhouse.py b/asap-tools/experiments/experiment_run_clickhouse.py index 7bdd370d..2cdf8d99 100644 --- a/asap-tools/experiments/experiment_run_clickhouse.py +++ b/asap-tools/experiments/experiment_run_clickhouse.py @@ -135,10 +135,17 @@ def _expand_groups(groups): return for group in groups: sql_file = group.get("sql_file") - if sql_file and "queries" not in group: - with open(sql_file) as fq: - content = fq.read() - group["queries"] = [s.strip() for s in content.split(";") if s.strip()] + if not sql_file or "queries" in group: + continue + # sql_file may be per-mode (issue #491 precompute arms); label with + # baseline's text, matching the query text sketchdb also plans against. + if isinstance(sql_file, dict): + sql_file = sql_file.get(constants.BASELINE_EXPERIMENT_NAME) + if not sql_file: + continue + with open(sql_file) as fq: + content = fq.read() + group["queries"] = [s.strip() for s in content.split(";") if s.strip()] _expand_groups(data.get("query_groups")) diff --git a/asap-tools/experiments/experiment_utils/config.py b/asap-tools/experiments/experiment_utils/config.py index af0d59d6..25d5924f 100644 --- a/asap-tools/experiments/experiment_utils/config.py +++ b/asap-tools/experiments/experiment_utils/config.py @@ -80,6 +80,31 @@ def _is_clickhouse_experiment(experiment_params: DictConfig) -> bool: return "dataset" in experiment_params +def _resolve_sql_file(sql_file: Any, mode: str, idx: int, group_name: str) -> str: + """Resolve the sql_file path for one query group and mode. + + ``sql_file`` is either a single path (legacy, shared by every mode) or a + dict keyed by mode name (e.g. baseline/baseline_sketch/baseline_mv/...). + sketchdb always resolves through baseline's entry — ASAP's SQL-mode query + engine must see the exact same SQL text as baseline to pattern-match it + against available sketches, so it never gets its own dict entry. + """ + if isinstance(sql_file, str): + return sql_file + lookup_mode = ( + constants.BASELINE_EXPERIMENT_NAME + if mode == constants.SKETCHDB_EXPERIMENT_NAME + else mode + ) + path = sql_file.get(lookup_mode) + if not path: + raise ValueError( + f"Query group {idx} ({group_name!r}) missing sql_file entry for " + f"mode {lookup_mode!r}" + ) + return path + + def _validate_clickhouse_experiment_config(experiment_params: DictConfig) -> None: """Validate experiment_params for a ClickHouse experiment.""" # Validate dataset section @@ -118,6 +143,11 @@ def _validate_clickhouse_experiment_config(experiment_params: DictConfig) -> Non raise ValueError( "At least one query group must be defined in experiment config." ) + experiment_modes = {m["mode"] for m in experiment_params.get("experiment") or []} + experiment_modes.discard(constants.SKETCHDB_EXPERIMENT_NAME) + if not experiment_modes: + experiment_modes = {constants.BASELINE_EXPERIMENT_NAME} + for i, group in enumerate(experiment_params.query_groups): sql_file = group.get("sql_file") if not sql_file or sql_file == "???": @@ -125,8 +155,14 @@ def _validate_clickhouse_experiment_config(experiment_params: DictConfig) -> Non f"Query group {i} missing 'sql_file'. " "Generate SQL files with benchmark/generate_queries.py first." ) - if not os.path.exists(sql_file): - raise ValueError(f"Query group {i} sql_file={sql_file!r} does not exist.") + group_name = group.get("name", str(i)) + for mode in experiment_modes: + path = _resolve_sql_file(sql_file, mode, i, group_name) + if not os.path.exists(path): + raise ValueError( + f"Query group {i} sql_file for mode {mode!r} " + f"({path!r}) does not exist." + ) def validate_experiment_config( @@ -717,34 +753,39 @@ def generate_clickhouse_client_configs( else: query_groups_list = list(query_groups) - # Build query groups with SQL inlined from files - built_groups = [] - for idx, group in enumerate(query_groups_list): - sql_file = group.get("sql_file") - if not sql_file: - name = group.get("name", str(idx)) - raise ValueError(f"Query group {idx!r} ({name!r}) missing 'sql_file'") + modes = list(mode_server_urls.keys()) + for mode, url in mode_server_urls.items(): + # Each mode may target a different sql_file (e.g. baseline_mv queries a + # materialized view, baseline_sketch swaps in a sketch function) — see + # _resolve_sql_file for the sketchdb-falls-back-to-baseline rule. + built_groups = [] + for idx, group in enumerate(query_groups_list): + sql_file = group.get("sql_file") + if not sql_file: + name = group.get("name", str(idx)) + raise ValueError(f"Query group {idx!r} ({name!r}) missing 'sql_file'") + sql_file = _resolve_sql_file( + sql_file, mode, idx, group.get("name", str(idx)) + ) - queries = _load_sql_queries(sql_file) - if not queries: - raise ValueError(f"No SQL statements found in {sql_file!r}") + queries = _load_sql_queries(sql_file) + if not queries: + raise ValueError(f"No SQL statements found in {sql_file!r}") - client_opts = dict(group.get("client_options") or {}) - client_opts.setdefault("starting_delay", 0) - client_opts.setdefault("repetitions", 1) + client_opts = dict(group.get("client_options") or {}) + client_opts.setdefault("starting_delay", 0) + client_opts.setdefault("repetitions", 1) - built_groups.append( - { - "id": idx, - "queries": queries, - "repetition_delay_ms": group.get("repetition_delay_ms", 0), - "client_options": client_opts, - "time_window_seconds": group.get("time_window_seconds"), - } - ) + built_groups.append( + { + "id": idx, + "queries": queries, + "repetition_delay_ms": group.get("repetition_delay_ms", 0), + "client_options": client_opts, + "time_window_seconds": group.get("time_window_seconds"), + } + ) - modes = list(mode_server_urls.keys()) - for mode, url in mode_server_urls.items(): config: Dict[str, Any] = { "servers": [ { @@ -808,6 +849,13 @@ def generate_sql_planner_input(query_groups: Any, dataset_cfg: Any) -> str: sql_file = group.get("sql_file") if not sql_file: raise ValueError(f"query_groups[{idx}] missing 'sql_file'") + # sketchdb always plans against baseline's SQL text (see _resolve_sql_file). + sql_file = _resolve_sql_file( + sql_file, + constants.SKETCHDB_EXPERIMENT_NAME, + idx, + group.get("name", str(idx)), + ) queries = _load_sql_queries(sql_file) if not queries: raise ValueError(f"No SQL statements found in {sql_file!r}") From 2e7d25e28529389a266e0d1b73392ec2aaf47157 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 1 Jul 2026 18:42:47 -0400 Subject: [PATCH 2/3] removed fallback --- asap-tools/experiments/experiment_run_clickhouse.py | 5 ++--- asap-tools/experiments/experiment_utils/config.py | 12 +++++------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/asap-tools/experiments/experiment_run_clickhouse.py b/asap-tools/experiments/experiment_run_clickhouse.py index 2cdf8d99..1a7c7779 100644 --- a/asap-tools/experiments/experiment_run_clickhouse.py +++ b/asap-tools/experiments/experiment_run_clickhouse.py @@ -137,10 +137,9 @@ def _expand_groups(groups): sql_file = group.get("sql_file") if not sql_file or "queries" in group: continue - # sql_file may be per-mode (issue #491 precompute arms); label with + # sql_file is per-mode (issue #491 precompute arms); label with # baseline's text, matching the query text sketchdb also plans against. - if isinstance(sql_file, dict): - sql_file = sql_file.get(constants.BASELINE_EXPERIMENT_NAME) + sql_file = sql_file.get(constants.BASELINE_EXPERIMENT_NAME) if not sql_file: continue with open(sql_file) as fq: diff --git a/asap-tools/experiments/experiment_utils/config.py b/asap-tools/experiments/experiment_utils/config.py index 25d5924f..55d9cfdb 100644 --- a/asap-tools/experiments/experiment_utils/config.py +++ b/asap-tools/experiments/experiment_utils/config.py @@ -83,14 +83,12 @@ def _is_clickhouse_experiment(experiment_params: DictConfig) -> bool: def _resolve_sql_file(sql_file: Any, mode: str, idx: int, group_name: str) -> str: """Resolve the sql_file path for one query group and mode. - ``sql_file`` is either a single path (legacy, shared by every mode) or a - dict keyed by mode name (e.g. baseline/baseline_sketch/baseline_mv/...). - sketchdb always resolves through baseline's entry — ASAP's SQL-mode query - engine must see the exact same SQL text as baseline to pattern-match it - against available sketches, so it never gets its own dict entry. + ``sql_file`` is a dict keyed by mode name (e.g. + baseline/baseline_sketch/baseline_mv/...). sketchdb always resolves + through baseline's entry — ASAP's SQL-mode query engine must see the + exact same SQL text as baseline to pattern-match it against available + sketches, so it never gets its own dict entry. """ - if isinstance(sql_file, str): - return sql_file lookup_mode = ( constants.BASELINE_EXPERIMENT_NAME if mode == constants.SKETCHDB_EXPERIMENT_NAME From d49fa42d3ac7a4af20f9f25d868267b060b20fd7 Mon Sep 17 00:00:00 2001 From: Milind Srivastava Date: Wed, 1 Jul 2026 18:43:26 -0400 Subject: [PATCH 3/3] remove unnecessary code --- asap-tools/experiments/experiment_utils/config.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/asap-tools/experiments/experiment_utils/config.py b/asap-tools/experiments/experiment_utils/config.py index 55d9cfdb..64bcba49 100644 --- a/asap-tools/experiments/experiment_utils/config.py +++ b/asap-tools/experiments/experiment_utils/config.py @@ -141,10 +141,12 @@ def _validate_clickhouse_experiment_config(experiment_params: DictConfig) -> Non raise ValueError( "At least one query group must be defined in experiment config." ) - experiment_modes = {m["mode"] for m in experiment_params.get("experiment") or []} + if not experiment_params.get("experiment"): + raise ValueError( + "At least one mode must be defined in experiment_params.experiment." + ) + experiment_modes = {m["mode"] for m in experiment_params.experiment} experiment_modes.discard(constants.SKETCHDB_EXPERIMENT_NAME) - if not experiment_modes: - experiment_modes = {constants.BASELINE_EXPERIMENT_NAME} for i, group in enumerate(experiment_params.query_groups): sql_file = group.get("sql_file")