Skip to content

Commit bbcdac9

Browse files
fix(asap-tools): refuse misaligned or plain-string sql_file
A mode whose SQL file holds a different number of statements than baseline's for the same group would pair later queries with the wrong baseline queries; a plain-string sql_file failed with an AttributeError. Both now fail with a clear error, and the docs' template uses the dict form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b7cea02 commit bbcdac9

3 files changed

Lines changed: 41 additions & 2 deletions

File tree

‎asap-tools/experiments/HYDRA_CONFIG_USAGE.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,8 @@ experiment_params:
174174
max_rows: 0 # 0 = load all rows
175175
query_groups:
176176
- name: default_queries
177-
sql_file: ??? # path to SQL file generated by benchmark/generate_queries.py
177+
sql_file: # mode -> SQL file generated by benchmark/generate_queries.py
178+
baseline: ??? # a sketchdb mode needs its own entry (may be the same file)
178179
repetition_delay: 0
179180
client_options:
180181
starting_delay: 0
@@ -202,6 +203,9 @@ python experiment_run_clickhouse.py \
202203
'experiment_params.query_groups[0].sql_file={baseline:/path/to/queries.sql}'
203204
```
204205

206+
With a `sketchdb` mode in `experiment_params.experiment`, give it its own
207+
entry, e.g. `sql_file={baseline:/path/to/queries.sql,sketchdb:/path/to/queries.sql}`.
208+
205209
Optional overrides:
206210
```bash
207211
# Skip data loading step, keep ClickHouse running after experiment

‎asap-tools/experiments/experiment_utils/config.py‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,14 +95,20 @@ def _mode_sql_files(query_groups: Any, mode: str) -> List[Tuple[int, Any, str]]:
9595
sketchdb). Every mode but sketchdb needs an entry in every group.
9696
sketchdb skips the groups without one (e.g. queries ASAPQuery has no SQL
9797
for), but only trailing groups: results are matched across modes by
98-
global query index, so sketchdb's queries must be a prefix of baseline's.
98+
global query index, so sketchdb's queries must be a prefix of baseline's,
99+
and each file it runs must hold as many statements as baseline's.
99100
"""
100101
runs = []
101102
for idx, group in enumerate(query_groups):
102103
name = group.get("name", str(idx))
103104
sql_file = group.get("sql_file")
104105
if not sql_file:
105106
raise ValueError(f"Query group {idx} ({name!r}) missing 'sql_file'")
107+
if not hasattr(sql_file, "get"):
108+
raise ValueError(
109+
f"Query group {idx} ({name!r}) sql_file must map mode name to "
110+
f"SQL file, e.g. {{baseline: {sql_file}}}; got {sql_file!r}"
111+
)
106112
path = sql_file.get(mode)
107113
if path:
108114
if len(runs) != idx:
@@ -111,6 +117,20 @@ def _mode_sql_files(query_groups: Any, mode: str) -> List[Tuple[int, Any, str]]:
111117
f"{mode!r} after a group without one; a mode may skip "
112118
"only trailing groups, so its query indices match baseline's"
113119
)
120+
baseline_path = sql_file.get(constants.BASELINE_EXPERIMENT_NAME)
121+
if (
122+
mode != constants.BASELINE_EXPERIMENT_NAME
123+
and baseline_path
124+
and os.path.exists(path)
125+
and os.path.exists(baseline_path)
126+
and len(_load_sql_queries(path))
127+
!= len(_load_sql_queries(baseline_path))
128+
):
129+
raise ValueError(
130+
f"Query group {idx} ({name!r}): mode {mode!r}'s {path!r} and "
131+
f"baseline's {baseline_path!r} hold different numbers of "
132+
"statements, so their query indices would not match"
133+
)
114134
runs.append((idx, group, path))
115135
elif mode != constants.SKETCHDB_EXPERIMENT_NAME:
116136
raise ValueError(

‎asap-tools/experiments/tests/test_clickhouse_client_configs.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,21 @@ def test_sketchdb_without_any_sql_is_refused(self):
6868
with self.assertRaisesRegex(ValueError, "No query group .* 'sketchdb'"):
6969
self.client_queries(groups)
7070

71+
def test_statement_count_mismatch_is_refused(self):
72+
# sketchdb's later queries would be scored against the wrong
73+
# baseline queries.
74+
two = os.path.join(self.tmp.name, "two.sql")
75+
with open(two, "w") as f:
76+
f.write("SELECT 1;\nSELECT 2;\n")
77+
groups = [{"sql_file": {"baseline": two, "sketchdb": self.sql("s")}}]
78+
with self.assertRaisesRegex(ValueError, "different numbers of statements"):
79+
self.client_queries(groups)
80+
81+
def test_plain_string_sql_file_is_refused_clearly(self):
82+
groups = [{"sql_file": self.sql("b")}]
83+
with self.assertRaisesRegex(ValueError, "must map mode name"):
84+
self.client_queries(groups, {"baseline": URLS["baseline"]})
85+
7186
def test_other_modes_need_every_group(self):
7287
groups = [
7388
{"sql_file": {"baseline": self.sql("b0")}},

0 commit comments

Comments
 (0)