You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(planner): millisecond precision for SQL/Elastic data_ingestion_interval #427
#398 adds millisecond precision to the PromQL planner path (prometheus_scrape_interval, step, range_duration, repetition_delay, window_size, slide_interval). The SQL and Elastic controllers have an analogous seconds-based field, data_ingestion_interval (asap-planner-rs/src/sql/generator.rs, src/elastic_dsl/generator.rs), with the same underlying problem — sub-second ingestion intervals can't be expressed.
The original issue (#398) doesn't mention this field, so it was explicitly left out of scope there and flagged as a known follow-up rather than silently expanded into.
Work
Same fix shape as #398: rename data_ingestion_interval to data_ingestion_interval_ms across the SQL/Elastic generator path, delete now-redundant * 1000 conversions at consumption sites, update fixtures/tests.
See .design_docs/issue-398-millisecond-precision-plan.md ("Scope boundary" and "Follow-up Issues" sections) for context.
PR #437 also surfaces a related boundary: SQLPatternMatcher::new in the vendored sql_utilities crate (asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs) takes scrape_interval as seconds. Both call sites currently work around this with a /1000.0 conversion:
asap-planner-rs/src/planner/sql.rs:75 — passes data_ingestion_interval as f64 (seconds, pre-rename)
When this issue renames data_ingestion_interval → data_ingestion_interval_ms, the planner call site will need updating. At that point, consider whether to also rename SQLPatternMatcher's scrape_interval field to scrape_interval_ms and remove both /1000.0 conversions, or keep the boundary conversion as-is. Flagging here so it doesn't get missed.
Activity
milindsrivastava1997 commentedon Jun 25, 2026
PR #437 also surfaces a related boundary:
SQLPatternMatcher::newin the vendoredsql_utilitiescrate (asap-common/dependencies/rs/sql_utilities/src/ast_matching/sqlpattern_matcher.rs) takesscrape_intervalas seconds. Both call sites currently work around this with a/1000.0conversion:asap-query-engine/src/engines/simple_engine/sql.rs:404— passesprometheus_scrape_interval_ms / 1000.0asap-planner-rs/src/planner/sql.rs:75— passesdata_ingestion_interval as f64(seconds, pre-rename)When this issue renames
data_ingestion_interval→data_ingestion_interval_ms, the planner call site will need updating. At that point, consider whether to also renameSQLPatternMatcher'sscrape_intervalfield toscrape_interval_msand remove both/1000.0conversions, or keep the boundary conversion as-is. Flagging here so it doesn't get missed.docs(query-engine,planner): clarify unit ambiguities raised in PR #437 …
feat: add millisecond precision support for planner and query engine (#…