From a8e293fd799438ea839df7e2af27fcd82e247b55 Mon Sep 17 00:00:00 2001 From: dennis zhuang Date: Sat, 10 Oct 2026 05:26:21 +0000 Subject: [PATCH] fix(promql): accept evaluation times before the Unix epoch (#9480) * fix(promql): accept evaluation times before the Unix epoch Prometheus accepts negative timestamps in query parameters; GreptimeDB rejected them while parsing and would panic converting them in the planner. Signed-off-by: Dennis Zhuang * test: update flow_tql error for a negative TQL end time A negative end now parses and fails the end >= start check instead of the timestamp conversion. Signed-off-by: Dennis Zhuang * fix(promql): reject evaluation times outside the millisecond range The signed millisecond conversion cast u128 to i64 unchecked, so a far-past time wrapped into the future. Reuse the checked conversion from label_values and return an error instead. Signed-off-by: Dennis Zhuang * chore: drop an unused import Signed-off-by: Dennis Zhuang * fix(promql): round query times to milliseconds like Prometheus Numeric times round the fraction of a second to milliseconds, and pre-epoch times floor sub-millisecond parts, matching Prometheus' parseTime and timestamp.FromTime. Signed-off-by: Dennis Zhuang * fix(promql): round the float fraction before converting to nanoseconds Signed-off-by: Dennis Zhuang * test: skip the overflow case where SystemTime cannot hold it Signed-off-by: Dennis Zhuang --------- Signed-off-by: Dennis Zhuang --- src/query/src/parser.rs | 55 +++++++++++++++++-- src/query/src/promql/label_values.rs | 15 +++-- src/query/src/promql/planner.rs | 15 ++--- src/query/src/promql/planner/test.rs | 34 +++++++++++- .../distributed/flow-tql/flow_tql.result | 2 +- .../cases/standalone/flow-tql/flow_tql.result | 2 +- 6 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/query/src/parser.rs b/src/query/src/parser.rs index 87b827e0cc1..c89c47392f6 100644 --- a/src/query/src/parser.rs +++ b/src/query/src/parser.rs @@ -224,11 +224,31 @@ impl QueryLanguageParser { // also report rfc3339 error if float parsing fails .map_err(|_| rfc3339_result.unwrap_err())?; - let duration = - Duration::try_from_secs_f64(secs).context(TryIntoDurationSnafu { raw: timestamp })?; - SystemTime::UNIX_EPOCH - .checked_add(duration) - .context(AddSystemTimeOverflowSnafu { duration }) + // Prometheus accepts timestamps before the Unix epoch. Follow the arithmetic of its + // `parseTime` and `timestamp.FromTime` step by step: the float fraction is rounded to + // milliseconds before any conversion to nanoseconds, which decides boundary values + // like 1.0005, whose fraction is just below 0.0005. + let magnitude = Duration::try_from_secs_f64(secs.abs()) + .context(TryIntoDurationSnafu { raw: timestamp })?; + let whole = secs.trunc(); + let fraction = ((secs - whole) * 1000.0).round() / 1000.0; + let nanos = whole as i128 * 1_000_000_000 + (fraction * 1e9) as i128; + let millis = nanos.div_euclid(1_000_000); + let duration = u64::try_from(millis.unsigned_abs()) + .ok() + .map(Duration::from_millis) + .context(AddSystemTimeOverflowSnafu { + duration: magnitude, + })?; + if millis < 0 { + SystemTime::UNIX_EPOCH + .checked_sub(duration) + .context(AddSystemTimeOverflowSnafu { duration }) + } else { + SystemTime::UNIX_EPOCH + .checked_add(duration) + .context(AddSystemTimeOverflowSnafu { duration }) + } } } @@ -380,6 +400,31 @@ mod test { } } + #[test] + fn parse_promql_timestamp_matches_prometheus_millis() { + // Expected values are Prometheus' `timestamp.FromTime(parseTime(input))`. + for (input, millis) in [ + ("-30000.5", -30_000_500), + ("-0.001", -1), + ("-0.0006", -1), + ("-1.2346", -1235), + ("1.2346", 1235), + ("0.0004", 0), + ("1.0005", 1000), + ("-1.0005", -1000), + ("0.0004999999", 0), + ("1969-12-31T23:59:59.9999Z", -1), + ("1970-01-01T00:00:00.0009Z", 0), + ] { + let result = QueryLanguageParser::parse_promql_timestamp(input).unwrap(); + assert_eq!( + crate::promql::label_values::signed_millis_since_epoch(result).unwrap(), + millis, + "{input}" + ); + } + } + #[test] fn parse_promql_timestamp() { let cases = vec![ diff --git a/src/query/src/promql/label_values.rs b/src/query/src/promql/label_values.rs index 841666758e4..cae53c64853 100644 --- a/src/query/src/promql/label_values.rs +++ b/src/query/src/promql/label_values.rs @@ -33,13 +33,20 @@ use crate::promql::error::{ /// a negative duration, and an RFC3339 query parameter can name one, so the sign /// is recovered here instead of unwrapping. fn millis_since_epoch(time: SystemTime) -> Result { + Ok(Timestamp::new_millisecond(signed_millis_since_epoch(time)?)) +} + +/// Converts a [`SystemTime`] to signed milliseconds since the Unix epoch, rejecting instants +/// outside the `i64` millisecond range. +/// +/// Sub-millisecond parts round down, also before the epoch, like Prometheus' +/// `timestamp.FromTime`. +pub(crate) fn signed_millis_since_epoch(time: SystemTime) -> Result { let (millis, before_epoch) = match time.duration_since(UNIX_EPOCH) { Ok(duration) => (duration.as_millis(), false), - Err(earlier) => (earlier.duration().as_millis(), true), + Err(earlier) => (earlier.duration().as_nanos().div_ceil(1_000_000), true), }; - let millis = - signed_millis(millis, before_epoch).with_context(|| SystemTimeOutOfRangeSnafu { time })?; - Ok(Timestamp::new_millisecond(millis)) + signed_millis(millis, before_epoch).with_context(|| SystemTimeOutOfRangeSnafu { time }) } /// Converts a millisecond magnitude and epoch direction without platform time limits. diff --git a/src/query/src/promql/planner.rs b/src/query/src/promql/planner.rs index a0ace9b9db6..5d8fc50edbd 100644 --- a/src/query/src/promql/planner.rs +++ b/src/query/src/promql/planner.rs @@ -21,7 +21,6 @@ mod set_operator; use std::collections::{BTreeSet, HashSet, VecDeque}; use std::sync::Arc; -use std::time::UNIX_EPOCH; use arrow::datatypes::IntervalDayTime; use async_recursion::async_recursion; @@ -112,6 +111,7 @@ use crate::promql::error::{ UnknownTableSnafu, UnsupportedExprSnafu, UnsupportedMatcherOpSnafu, ValueNotFoundSnafu, ZeroRangeSelectorSnafu, }; +use crate::promql::label_values::signed_millis_since_epoch; use crate::query_engine::QueryEngineState; /// `time()` function in PromQL. @@ -241,10 +241,11 @@ impl BinaryResultLabels { } impl PromPlannerContext { - fn from_eval_stmt(stmt: &EvalStmt) -> Self { - let start = stmt.start.duration_since(UNIX_EPOCH).unwrap().as_millis() as Millisecond; - let end = stmt.end.duration_since(UNIX_EPOCH).unwrap().as_millis() as Millisecond; - Self { + fn from_eval_stmt(stmt: &EvalStmt) -> Result { + // Evaluation times before the Unix epoch are valid in Prometheus. + let start = signed_millis_since_epoch(stmt.start)?; + let end = signed_millis_since_epoch(stmt.end)?; + Ok(Self { start, end, interval: stmt.interval.as_millis() as _, @@ -252,7 +253,7 @@ impl PromPlannerContext { stmt_start: start, stmt_end: end, ..Default::default() - } + }) } /// Reset all planner states @@ -309,7 +310,7 @@ impl PromPlanner { ) -> Result { let mut planner = Self { table_provider, - ctx: PromPlannerContext::from_eval_stmt(stmt), + ctx: PromPlannerContext::from_eval_stmt(stmt)?, promql_annotations, }; diff --git a/src/query/src/promql/planner/test.rs b/src/query/src/promql/planner/test.rs index 642255532bd..19de1098288 100644 --- a/src/query/src/promql/planner/test.rs +++ b/src/query/src/promql/planner/test.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::collections::HashMap; -use std::time::{Duration, UNIX_EPOCH}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use catalog::RegisterTableRequest; use catalog::memory::{MemoryCatalogManager, new_memory_catalog_manager}; @@ -3297,7 +3297,7 @@ async fn comparison_binary_join_uses_tsid_and_keeps_it_in_filtered_result() { .await; let mut planner = PromPlanner { table_provider, - ctx: PromPlannerContext::from_eval_stmt(&eval_stmt), + ctx: PromPlannerContext::from_eval_stmt(&eval_stmt).unwrap(), promql_annotations: None, }; let plan = planner @@ -6266,7 +6266,7 @@ async fn native_scan_bounds_preserve_zero_lookback_and_overflow() { .await; let mut planner = PromPlanner { table_provider, - ctx: PromPlannerContext::from_eval_stmt(&build_eval_stmt("some_metric")), + ctx: PromPlannerContext::from_eval_stmt(&build_eval_stmt("some_metric")).unwrap(), promql_annotations: None, }; planner.ctx.time_index_column = Some("timestamp".to_string()); @@ -8872,3 +8872,31 @@ async fn test_count_values_groups_by_formatted_value_for_bigint_input() { vec![("9007199254740992", 2.0)] ); } + +#[tokio::test] +async fn eval_times_before_epoch() { + let plan = |start: SystemTime, end: SystemTime| async move { + let mut eval_stmt = build_eval_stmt("some_metric"); + eval_stmt.start = start; + eval_stmt.end = end; + let table_provider = build_test_table_provider( + &[(DEFAULT_SCHEMA_NAME.to_string(), "some_metric".to_string())], + 1, + 1, + ) + .await; + PromPlanner::stmt_to_plan(table_provider, &eval_stmt, &build_query_engine_state()).await + }; + + plan( + UNIX_EPOCH - Duration::from_secs(30_000), + UNIX_EPOCH - Duration::from_secs(29_000), + ) + .await + .unwrap(); + // Beyond the i64 millisecond range, which must not wrap into the future. Windows cannot + // represent this SystemTime. + if let Some(too_early) = UNIX_EPOCH.checked_sub(Duration::from_secs(10_000_000_000_000_000)) { + assert!(plan(too_early, too_early).await.is_err()); + } +} diff --git a/tests/cases/distributed/flow-tql/flow_tql.result b/tests/cases/distributed/flow-tql/flow_tql.result index 30158a176f8..73321d040e1 100644 --- a/tests/cases/distributed/flow-tql/flow_tql.result +++ b/tests/cases/distributed/flow-tql/flow_tql.result @@ -155,7 +155,7 @@ Error: 2000(InvalidSyntax), Invalid TQL syntax: sql parser error: Expected expre CREATE FLOW calc_reqs SINK TO cnt_reqs EVAL INTERVAL '1m' AS TQL EVAL (now() - now(), now()-(now()+'15s'::interval), '5s') count_values("status_code", http_requests); -Error: 3001(EngineExecuteQuery), Failed to convert float seconds to duration, raw: -15 +Error: 3001(EngineExecuteQuery), The end time must be greater than start time, start: 0, end: -15000 CREATE FLOW calc_reqs SINK TO cnt_reqs EVAL INTERVAL '1m' AS TQL EVAL (now() - now(), now()-now()+'15s'::interval, '5s') count_values("status_code", http_requests); diff --git a/tests/cases/standalone/flow-tql/flow_tql.result b/tests/cases/standalone/flow-tql/flow_tql.result index 13239bf4e1f..73321d040e1 100644 --- a/tests/cases/standalone/flow-tql/flow_tql.result +++ b/tests/cases/standalone/flow-tql/flow_tql.result @@ -155,7 +155,7 @@ Error: 2000(InvalidSyntax), Invalid TQL syntax: sql parser error: Expected expre CREATE FLOW calc_reqs SINK TO cnt_reqs EVAL INTERVAL '1m' AS TQL EVAL (now() - now(), now()-(now()+'15s'::interval), '5s') count_values("status_code", http_requests); -Error: 3001(EngineExecuteQuery), Failed to convert float seconds to duration, raw: -15: cannot convert float seconds to Duration: value is negative +Error: 3001(EngineExecuteQuery), The end time must be greater than start time, start: 0, end: -15000 CREATE FLOW calc_reqs SINK TO cnt_reqs EVAL INTERVAL '1m' AS TQL EVAL (now() - now(), now()-now()+'15s'::interval, '5s') count_values("status_code", http_requests);