Skip to content

Commit 09375f7

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 b9b5650 commit 09375f7

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
@@ -160,14 +160,20 @@ def _mode_sql_files(query_groups: Any, mode: str) -> List[Tuple[int, Any, str]]:
160160
sketchdb). Every mode but sketchdb needs an entry in every group.
161161
sketchdb skips the groups without one (e.g. queries ASAPQuery has no SQL
162162
for), but only trailing groups: results are matched across modes by
163-
global query index, so sketchdb's queries must be a prefix of baseline's.
163+
global query index, so sketchdb's queries must be a prefix of baseline's,
164+
and each file it runs must hold as many statements as baseline's.
164165
"""
165166
runs = []
166167
for idx, group in enumerate(query_groups):
167168
name = group.get("name", str(idx))
168169
sql_file = group.get("sql_file")
169170
if not sql_file:
170171
raise ValueError(f"Query group {idx} ({name!r}) missing 'sql_file'")
172+
if not hasattr(sql_file, "get"):
173+
raise ValueError(
174+
f"Query group {idx} ({name!r}) sql_file must map mode name to "
175+
f"SQL file, e.g. {{baseline: {sql_file}}}; got {sql_file!r}"
176+
)
171177
path = sql_file.get(mode)
172178
if path:
173179
if len(runs) != idx:
@@ -176,6 +182,20 @@ def _mode_sql_files(query_groups: Any, mode: str) -> List[Tuple[int, Any, str]]:
176182
f"{mode!r} after a group without one; a mode may skip "
177183
"only trailing groups, so its query indices match baseline's"
178184
)
185+
baseline_path = sql_file.get(constants.BASELINE_EXPERIMENT_NAME)
186+
if (
187+
mode != constants.BASELINE_EXPERIMENT_NAME
188+
and baseline_path
189+
and os.path.exists(path)
190+
and os.path.exists(baseline_path)
191+
and len(_load_sql_queries(path))
192+
!= len(_load_sql_queries(baseline_path))
193+
):
194+
raise ValueError(
195+
f"Query group {idx} ({name!r}): mode {mode!r}'s {path!r} and "
196+
f"baseline's {baseline_path!r} hold different numbers of "
197+
"statements, so their query indices would not match"
198+
)
179199
runs.append((idx, group, path))
180200
elif mode != constants.SKETCHDB_EXPERIMENT_NAME:
181201
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)