From 02f9666fba022c59e08bff8c029380c92cf825b5 Mon Sep 17 00:00:00 2001 From: Aaron Gibson Date: Thu, 14 May 2026 20:23:19 +0200 Subject: [PATCH 1/3] Add _resolve_time_grain to validate/resolve time_grain (handles "auto"). Refactor group_by_date and add where_by_date to not repeat Make WHERE use gmap functions (e.g. toDate, toStartOfHour) on both column and params so ClickHouse aggregate projection can match. Replace raw measurement_start_time >= / < clauses with where_by_date call and preserve :since/:until params._ --- .../routers/v1/aggregation.py | 49 ++++++++++++------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py b/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py index fe0f292a4..c89718e37 100644 --- a/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py +++ b/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py @@ -33,13 +33,20 @@ def set_dload(resp, fname: str): resp.headers["Content-Disposition"] = f"attachment; filename={fname}" -def group_by_date(since, until, time_grain, cols, colnames, group_by): +gmap = dict( + hour="toStartOfHour", + day="toDate", + week="toStartOfWeek", + month="toStartOfMonth", +) + + +def _resolve_time_grain(since, until, time_grain): if since and until: delta = until - since else: delta = None - # on time_grain = "auto" or empty the smallest allowed gran. is used ranges = ( (7, ("hour", "day", "auto")), (30, ("day", "week", "auto")), @@ -54,28 +61,34 @@ def group_by_date(since, until, time_grain, cols, colnames, group_by): continue if time_grain not in allowed: a = ", ".join(allowed) - msg = f"Choose time_grain between {a} for the given time range" - raise Exception(msg) + raise Exception(f"Choose time_grain between {a} for the given time range") if time_grain == "auto": time_grain = allowed[0] - break - - # TODO: check around query weight / response size. - # Also add support in CSV format. - gmap = dict( - hour="toStartOfHour", - day="toDate", - week="toStartOfWeek", - month="toStartOfMonth", - ) + return time_grain + + raise Exception("Unable to resolve time_grain") + + +def group_by_date(since, until, time_grain, cols, colnames, group_by): + time_grain = _resolve_time_grain(since, until, time_grain) fun = gmap[time_grain] - tcol = "measurement_start_day" # TODO: support dynamic axis names + tcol = "measurement_start_day" cols.append(sql_text(f"{fun}(measurement_start_time) AS {tcol}")) colnames.append(tcol) group_by.append(column(tcol)) return time_grain +def where_by_date(since, until, time_grain, cols, colnames, where_by): + time_grain = _resolve_time_grain(since, until, time_grain) + fun = gmap[time_grain] + if since: + where_by.append(sql_text(f"{fun}(measurement_start_time) >= {fun}(:since)")) + if until: + where_by.append(sql_text(f"{fun}(measurement_start_time) < {fun}(:until)")) + return time_grain + + def validate_axis_name(axis): valid = ( "blocking_type", @@ -349,12 +362,12 @@ async def get_measurements( where.append(sql_text("ooni_run_link_id IN :ooni_run_link_id_s")) query_params["ooni_run_link_id_s"] = ooni_run_link_id_s + time_grain = where_by_date(since, until, time_grain, cols, colnames, where) + + # still need to set the query params (where_by_date uses :since / :until) if since: - where.append(sql_text("measurement_start_time >= :since")) query_params["since"] = since - if until: - where.append(sql_text("measurement_start_time < :until")) query_params["until"] = until if test_name_s: From 50441572588f84f822469d5e59762b77d9b020dd Mon Sep 17 00:00:00 2001 From: Aaron Gibson Date: Thu, 14 May 2026 20:34:28 +0200 Subject: [PATCH 2/3] fix type after round --- .../oonimeasurements/routers/v1/aggregation.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py b/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py index c89718e37..130db8046 100644 --- a/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py +++ b/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py @@ -79,13 +79,24 @@ def group_by_date(since, until, time_grain, cols, colnames, group_by): return time_grain +_param_cast = { + "hour": "toDateTime", + "day": "toDate", + "week": "toDateTime", + "month": "toDateTime", +} + + def where_by_date(since, until, time_grain, cols, colnames, where_by): time_grain = _resolve_time_grain(since, until, time_grain) fun = gmap[time_grain] + cast = _param_cast[time_grain] + if since: - where_by.append(sql_text(f"{fun}(measurement_start_time) >= {fun}(:since)")) + where_by.append(sql_text(f"{fun}(measurement_start_time) >= {fun}({cast}(:since))")) if until: - where_by.append(sql_text(f"{fun}(measurement_start_time) < {fun}(:until)")) + where_by.append(sql_text(f"{fun}(measurement_start_time) < {fun}({cast}(:until))")) + return time_grain From 706feaea495b2bea952a5781be27c87cf5a7f1d0 Mon Sep 17 00:00:00 2001 From: Aaron Gibson Date: Mon, 18 May 2026 14:53:33 +0200 Subject: [PATCH 3/3] catch exception and return HTTPError for invalid date range --- .../src/oonimeasurements/routers/v1/aggregation.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py b/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py index 130db8046..3ed091c3c 100644 --- a/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py +++ b/ooniapi/services/oonimeasurements/src/oonimeasurements/routers/v1/aggregation.py @@ -373,7 +373,10 @@ async def get_measurements( where.append(sql_text("ooni_run_link_id IN :ooni_run_link_id_s")) query_params["ooni_run_link_id_s"] = ooni_run_link_id_s - time_grain = where_by_date(since, until, time_grain, cols, colnames, where) + try: + time_grain = where_by_date(since, until, time_grain, cols, colnames, where) + except Exception as e: + return jerror(str(e), v=0) # still need to set the query params (where_by_date uses :since / :until) if since: