feat(flow): support eval schedule offsets (#8878)

* feat(flow): support eval schedule offsets Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(flow): remove redundant schedule assertion Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * refactor(flow): trim eval offset compatibility scope Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(flow): trim eval offset edge coverage Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * docs(flow): trim eval offset comment noise Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(flow): address eval offset review feedback Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(compat): cover Flow eval offset persistence Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
2026-10-15 16:32:34 +00:00 · 2026-09-04 02:39:59 +00:00
parent 84bd993131
commit ad7b0ace64
22 changed files with 1702 additions and 151 deletions
@@ -1098,6 +1098,7 @@ pub fn to_create_flow_task_expr(

    let eval_interval = create_flow.eval_interval;

+    let flow_options = stringify_flow_options(create_flow.flow_options)?;
    Ok(CreateFlowExpr {
        catalog_name: query_ctx.current_catalog().to_string(),
        flow_name: sanitize_flow_name(create_flow.flow_name)?,
@@ -1109,7 +1110,7 @@ pub fn to_create_flow_task_expr(
        eval_interval: eval_interval.map(|seconds| api::v1::EvalInterval { seconds }),
        comment: create_flow.comment.unwrap_or_default(),
        sql: create_flow.query.to_string(),
-        flow_options: stringify_flow_options(create_flow.flow_options)?,
+        flow_options,
    })
 }

@@ -42,6 +42,7 @@ use common_meta::cache_invalidator::CacheInvalidatorRef;
 use common_meta::cache_invalidator::Context;
 use common_meta::ddl::create_flow::{
    DEFER_ON_MISSING_SOURCE_KEY, FLOW_EXPERIMENTAL_ENABLE_INCREMENTAL_READ_KEY, FlowType,
+    INTERNAL_EVAL_OFFSET_KEY, INTERNAL_EVAL_SCHEDULE_KEY,
 };
 use common_meta::instruction::CacheIdent;
 #[cfg(feature = "enterprise")]
@@ -205,7 +206,10 @@ fn validate_and_normalize_flow_options(
    options
        .into_iter()
        .map(|(key, value)| {
-            if key == FlowType::FLOW_TYPE_KEY {
+            if matches!(
+                key.as_str(),
+                FlowType::FLOW_TYPE_KEY | INTERNAL_EVAL_OFFSET_KEY | INTERNAL_EVAL_SCHEDULE_KEY
+            ) {
                return InvalidSqlSnafu {
                    err_msg: format!("flow option '{key}' is reserved for internal use"),
                }
@@ -232,12 +236,51 @@ fn validate_and_normalize_flow_options(
        .collect()
 }

+/// Validates `EVAL OFFSET` semantics at the operator boundary: an offset is
+/// only legal together with `EVAL INTERVAL` and must lie in
+/// `[0, eval_interval)`. Never modulo-normalized.
+fn validate_eval_offset(
+    eval_offset_secs: Option<i64>,
+    eval_interval_secs: Option<i64>,
+) -> Result<()> {
+    if let Some(offset_secs) = eval_offset_secs {
+        let Some(eval_interval_secs) = eval_interval_secs else {
+            return InvalidSqlSnafu {
+                err_msg: "EVAL OFFSET requires EVAL INTERVAL to be specified".to_string(),
+            }
+            .fail();
+        };
+        if !(0..eval_interval_secs).contains(&offset_secs) {
+            return InvalidSqlSnafu {
+                err_msg: format!(
+                    "EVAL OFFSET must be in range [0, EVAL INTERVAL), got {offset_secs} seconds with EVAL INTERVAL {eval_interval_secs} seconds"
+                ),
+            }
+            .fail();
+        }
+    }
+    Ok(())
+}
+
 fn determine_flow_type_for_source_state(
    flow_name: &str,
    flow_options: &HashMap<String, String>,
    has_missing_source_table: bool,
    has_instant_ttl_source_table: bool,
+    force_batching: bool,
 ) -> Result<Option<FlowType>> {
+    if has_instant_ttl_source_table && force_batching {
+        // The batching scheduler cannot read instant-TTL source tables, so
+        // reject this combination even when another source table is missing.
+        return InvalidSqlSnafu {
+            err_msg: format!(
+                "flow '{}' with EVAL INTERVAL requires the batching scheduler, but source tables with ttl=instant are not supported under batching mode; use a TTL longer than the flush interval",
+                flow_name
+            ),
+        }
+        .fail();
+    }
+
    if has_missing_source_table {
        let defer_on_missing_source = flow_options
            .get(DEFER_ON_MISSING_SOURCE_KEY)
@@ -741,23 +784,31 @@ impl StatementExecutor {
        query_context: QueryContextRef,
    ) -> Result<Output> {
        // TODO(ruihang): do some verification
+        let eval_offset_secs = stmt.eval_offset;
        let expr = expr_helper::to_create_flow_task_expr(stmt, &query_context)?;

-        self.create_flow_inner(expr, query_context).await
+        // The typed `EVAL OFFSET` comes straight from the parsed SQL AST; it is
+        // the only trusted source of the offset at this boundary.
+        self.create_flow_procedure(expr, eval_offset_secs, query_context)
+            .await?;
+        Ok(Output::new_with_affected_rows(0))
    }

+    /// Direct gRPC entry point for `CREATE FLOW`.
    pub async fn create_flow_inner(
        &self,
        expr: CreateFlowExpr,
        query_context: QueryContextRef,
    ) -> Result<Output> {
-        self.create_flow_procedure(expr, query_context).await?;
+        self.create_flow_procedure(expr, None, query_context)
+            .await?;
        Ok(Output::new_with_affected_rows(0))
    }

    async fn create_flow_procedure(
        &self,
        mut expr: CreateFlowExpr,
+        eval_offset_secs: Option<i64>,
        query_context: QueryContextRef,
    ) -> Result<SubmitDdlTaskResponse> {
        let eval_interval_secs = expr.eval_interval.as_ref().map(|e| e.seconds);
@@ -772,6 +823,14 @@ impl StatementExecutor {
            .fail();
        }

+        // Validate EVAL OFFSET semantics at the operator boundary (defense in
+        // depth; the parser already enforces them deterministically). The
+        // offset arrives as a typed parameter, never from `flow_options`.
+        validate_eval_offset(eval_offset_secs, eval_interval_secs)?;
+
+        // Validate user options. `validate_and_normalize_flow_options` also
+        // rejects the internal transport keys, so a spoofed key smuggled into
+        // the expr can never be honored here.
        expr.flow_options =
            validate_and_normalize_flow_options(expr.flow_options, eval_interval_secs)?;

@@ -783,10 +842,11 @@ impl StatementExecutor {
        expr.flow_options
            .insert(FlowType::FLOW_TYPE_KEY.to_string(), flow_type.to_string());

-        let task = CreateFlowTask::try_from(PbCreateFlowTask {
+        let mut task = CreateFlowTask::try_from(PbCreateFlowTask {
            create_flow: Some(expr),
        })
        .context(error::InvalidExprSnafu)?;
+        task.eval_offset_secs = eval_offset_secs;
        let executor_context = to_executor_context(query_context, TriggerReason::Manual);
        let request = SubmitDdlTaskRequest::new(DdlTask::new_create_flow(task));

@@ -796,14 +856,15 @@ impl StatementExecutor {
            .context(error::ExecuteDdlSnafu)
    }

-    /// Determine the flow type based on the SQL query
-    ///
-    /// If it contains aggregation or distinct, then it is a batch flow, otherwise it is a streaming flow
+    /// Determines the flow type from source-table state, schedule requirements,
+    /// and SQL shape.
    async fn determine_flow_type(
        &self,
        expr: &CreateFlowExpr,
        query_ctx: QueryContextRef,
    ) -> Result<FlowType> {
+        let has_eval_interval = expr.eval_interval.is_some();
+
        let mut has_missing_source_table = false;
        let mut has_instant_ttl_source_table = false;

@@ -827,13 +888,18 @@ impl StatementExecutor {

            if table.table_info().meta.options.ttl == Some(common_time::TimeToLive::Instant) {
                warn!(
-                    "Source table `{}` for flow `{}`'s ttl=instant, fallback to streaming mode",
+                    "Source table `{}` for flow `{}`'s ttl=instant, {}",
                    format_full_table_name(
                        &src_table_name.catalog_name,
                        &src_table_name.schema_name,
                        &src_table_name.table_name
                    ),
-                    expr.flow_name
+                    expr.flow_name,
+                    if has_eval_interval {
+                        "rejecting flow because EVAL INTERVAL requires the batching scheduler which does not support instant TTL"
+                    } else {
+                        "fallback to streaming mode"
+                    }
                );
                has_instant_ttl_source_table = true;
            }
@@ -844,10 +910,20 @@ impl StatementExecutor {
            &expr.flow_options,
            has_missing_source_table,
            has_instant_ttl_source_table,
+            has_eval_interval,
        )? {
            return Ok(flow_type);
        }

+        // A flow with `EVAL INTERVAL` (and therefore possibly `EVAL OFFSET`)
+        // always uses the batching scheduler: the fixed epoch-phase schedule is
+        // only honored by the batching scheduler, regardless of the SQL shape.
+        // Non-aggregate SQL is supported as an explicit full-query flow (the
+        // batching engine requires a time-window expression or EVAL INTERVAL).
+        if has_eval_interval {
+            return Ok(FlowType::Batching);
+        }
+
        let engine = &self.query_engine;
        let stmts = ParserContext::create_with_dialect(
            &expr.sql,
@@ -3229,7 +3305,6 @@ SELECT max(c1), min(c2) FROM schema_2.table_2;";
            "eval_interval_missed_tick_policy",
            "eval_interval_catchup_max_runs",
            "eval_interval_catchup_max_lag",
-            "__greptime_internal_eval_schedule",
        ] {
            let err = validate_and_normalize_flow_options(
                HashMap::from([(key.to_string(), "value".to_string())]),
@@ -3246,10 +3321,31 @@ SELECT max(c1), min(c2) FROM schema_2.table_2;";
    }

    #[test]
-    fn test_determine_flow_type_for_source_state_missing_sources_require_opt_in() {
-        let err = determine_flow_type_for_source_state("my_flow", &HashMap::new(), true, false)
+    fn test_internal_transport_keys_rejected_as_reserved() {
+        for key in [
+            "__greptime_internal_eval_schedule",
+            "__greptime_internal_eval_offset_secs",
+        ] {
+            let err = validate_and_normalize_flow_options(
+                HashMap::from([(key.to_string(), "120".to_string())]),
+                Some(300),
+            )
            .unwrap_err();

+            assert!(
+                err.to_string()
+                    .contains(&format!("flow option '{key}' is reserved for internal use")),
+                "unexpected error for {key}: {err}"
+            );
+        }
+    }
+
+    #[test]
+    fn test_determine_flow_type_for_source_state_missing_sources_require_opt_in() {
+        let err =
+            determine_flow_type_for_source_state("my_flow", &HashMap::new(), true, false, false)
+                .unwrap_err();
+
        assert!(err.to_string().contains(
            "missing source tables for flow 'my_flow'; use WITH (defer_on_missing_source = true) to create a pending flow"
        ));
@@ -3261,19 +3357,43 @@ SELECT max(c1), min(c2) FROM schema_2.table_2;";
            HashMap::from([(DEFER_ON_MISSING_SOURCE_KEY.to_string(), "true".to_string())]);

        assert_eq!(
-            determine_flow_type_for_source_state("my_flow", &flow_options, true, true).unwrap(),
+            determine_flow_type_for_source_state("my_flow", &flow_options, true, true, false)
+                .unwrap(),
            Some(FlowType::Batching)
        );
+        let err = determine_flow_type_for_source_state("my_flow", &flow_options, true, true, true)
+            .unwrap_err();
+        assert!(err.to_string().contains(
+            "flow 'my_flow' with EVAL INTERVAL requires the batching scheduler, but source tables with ttl=instant are not supported under batching mode"
+        ));
    }

    #[test]
    fn test_determine_flow_type_for_source_state_instant_ttl_without_missing_sources() {
        assert_eq!(
-            determine_flow_type_for_source_state("my_flow", &HashMap::new(), false, true).unwrap(),
+            determine_flow_type_for_source_state("my_flow", &HashMap::new(), false, true, false)
+                .unwrap(),
            Some(FlowType::Streaming)
        );
    }

+    #[test]
+    fn test_determine_flow_type_for_source_state_instant_ttl_with_eval_interval_is_rejected() {
+        // A flow with `EVAL INTERVAL` is forced onto the batching scheduler,
+        // which cannot read instant-TTL source tables: reject instead of
+        // silently falling back to streaming (where the schedule/offset would
+        // be ignored).
+        let err =
+            determine_flow_type_for_source_state("my_flow", &HashMap::new(), false, true, true)
+                .unwrap_err();
+        assert!(
+            err.to_string().contains(
+                "flow 'my_flow' with EVAL INTERVAL requires the batching scheduler, but source tables with ttl=instant are not supported under batching mode"
+            ),
+            "unexpected error: {err}"
+        );
+    }
+
    #[test]
    fn test_name_is_match() {
        assert!(!NAME_PATTERN_REG.is_match("/adaf"));