Problem:
The SQL logic currently parses and plans queries with nested aggregations but executes them incorrectly. It just discards the outer aggregation. This could be fixed, but is architecturally messy. The reason the query engine has become unwieldly is because earlier all queries were getting translated to a struct which would have “metric_name, timestamps, statistic, labels to group by” and this becomes a pain to extend for nested queries or even other queries that may need things like pre-filtering based on some label or a HAVING clause, etc
Approach:
I am thinking of removing any planning/execution support for nested that exists today (since its anyways incorrect), and then tackle this using the ASAPController’s IR abstraction. If we remove support for this, everything can be folded into the SpatioTemporal query type, and thus be query type-agnostic.
For this purpose, I think we should do the following:
The last one needs atleast:
- Update
asap-planner-rs SQL planner, window-size related logic (depends on above)
- Rewrite
sql_utilities ast_matching tests for SpatioTemporal-only classification
Related to #487, #488, and #489
Problem:
The SQL logic currently parses and plans queries with nested aggregations but executes them incorrectly. It just discards the outer aggregation. This could be fixed, but is architecturally messy. The reason the query engine has become unwieldly is because earlier all queries were getting translated to a struct which would have “metric_name, timestamps, statistic, labels to group by” and this becomes a pain to extend for nested queries or even other queries that may need things like pre-filtering based on some label or a HAVING clause, etc
Approach:
I am thinking of removing any planning/execution support for nested that exists today (since its anyways incorrect), and then tackle this using the ASAPController’s IR abstraction. If we remove support for this, everything can be folded into the
SpatioTemporalquery type, and thus be query type-agnostic.For this purpose, I think we should do the following:
topkdetection inbuild_spatiotemporal_context(Add topk detection in build_spatiotemporal_context #498)agg(fn(m[w]))queries #496 talks about similar issue for PromQL) - can addQueryError::NestedQueryUnsupportedSpatial,TemporalGeneric,TemporalQuantile, andOneTemporalOneSpatialqueries (Collapse Spatial/TemporalGeneric/TemporalQuantile/OneTemporalOneSpatial into SpatioTemporal-only classification #500)The last one needs atleast:
asap-planner-rsSQL planner, window-size related logic (depends on above)sql_utilitiesast_matching tests forSpatioTemporal-only classificationRelated to #487, #488, and #489