From 0e802ec0f21887e5bdb5ed9758c6956707038710 Mon Sep 17 00:00:00 2001 From: zz_y Date: Thu, 28 May 2026 08:58:40 -0600 Subject: [PATCH 1/3] docs: clarify Scan.schema is a binding schema, not a complete output schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Scan.schema`'s docstring called it "the authoritative, self-contained output schema", which overclaims for PromQL and caused confusion: a metric's label set is open and known only at runtime, so the PromQL schema is usage-derived (the `(ts, value)` floor + referenced labels), not the metric's full label set. Reframe it as the **binding schema** — the resolved column set positional `ColumnId`s index into: complete when catalog-backed (SQL), usage-derived for schemaless PromQL. Also note on `SchemaCatalog` that it is the column *source* (the "catalog"), distinct from the resolved `Scan.schema` it feeds, and is the extension point for a future registry-backed PromQL catalog. Docs only — no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/core/src/intent_algebra/binder.rs | 5 +++++ crates/core/src/intent_algebra/query_expr.rs | 11 ++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/crates/core/src/intent_algebra/binder.rs b/crates/core/src/intent_algebra/binder.rs index 0004e327..6111ba1b 100644 --- a/crates/core/src/intent_algebra/binder.rs +++ b/crates/core/src/intent_algebra/binder.rs @@ -18,6 +18,11 @@ use crate::intent_algebra::schema::{Column, DataType, Schema}; /// The DB / source-schema metadata source — resolves a source (metric / /// table) name to its known columns. +/// Source of truth for a source's columns — the "catalog". `SqlCatalog` backs +/// it for SQL; PromQL uses [`UsageDerivedCatalog`] (returns `None`) until a +/// registry-backed impl (returning a metric's known label set) drops in here. +/// Distinct from `Scan.schema`, which is the *resolved* binding schema this +/// feeds — the catalog is the input, the schema is the result. pub trait SchemaCatalog { /// Columns known for `source`. `None` when unknown — the [`Binder`] then /// falls back to a usage-derived column set. diff --git a/crates/core/src/intent_algebra/query_expr.rs b/crates/core/src/intent_algebra/query_expr.rs index 2fc76f78..3703fd12 100644 --- a/crates/core/src/intent_algebra/query_expr.rs +++ b/crates/core/src/intent_algebra/query_expr.rs @@ -208,9 +208,14 @@ pub struct ProjectItem { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum QueryExpr { - /// Outermost leaf. `schema` is the authoritative, self-contained output - /// schema (Binder-built); `predicates` are leaf-level row filters - /// (PromQL label matchers, pushed-down `WHERE` conjuncts). + /// Outermost leaf. `schema` is the **binding schema** — the resolved column + /// set every positional `ColumnId` in the tree indexes into, *not* a full + /// description of the runtime row. Complete when catalog-backed (SQL); for + /// schemaless PromQL it is usage-derived by the [`Binder`](super::binder) + /// (the `(ts, value)` floor + the labels the query references), since a + /// metric's label set is open and known only at runtime. `predicates` are + /// leaf-level row filters (PromQL label matchers, pushed-down `WHERE` + /// conjuncts). Scan { source: Source, #[serde(default)] From 4cef9d9ed9fd39418b49d53fc1093a57414fc37d Mon Sep 17 00:00:00 2001 From: zz_y Date: Thu, 28 May 2026 09:56:21 -0600 Subject: [PATCH 2/3] feat(core): model schema completeness with Schema.closed (open/closed) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the SQL/PromQL schema difference an explicit, checkable property instead of implicit. `Schema` gains `closed: bool` (`#[serde(default)]` → false = open): - **closed** — the schema completely enumerates the columns here (no more at runtime): a catalog-backed SQL leaf, or an output fully determined by an `Aggregate`/`Project`. - **open** — a dynamic / superset schema (a schemaless PromQL leaf lists only the `(ts, value)` floor + referenced labels; the runtime row may carry more). This is Apache Calcite's `DynamicRecordType` pattern: a schema starts open at a schemaless leaf and is **frozen to closed** by the first operator that fully determines its output columns. `output_schema_in` propagates accordingly — passthrough nodes (Filter/Sort/Limit/Distinct/Window/TimeRange/SetOp/Merge/ WindowFunc/BinaryOp) inherit their child's `closed`; cross-series `Aggregate` and `Project` set `true` (the freeze); per-series reductions (rate/increase, *_over_time) and joins propagate (`input.closed` / `left && right`). Leaves: SQL `scan_source` → true, PromQL `Binder` → false. Decouples completeness from the data model (`Source::Table` vs `TimeSeries`), so a future registry-backed PromQL catalog stays **open** (per-series + time-varying labels = superset hint, not a per-row contract). No current consumer reads it — it's a passive marker reserved for label validation / full-output enumeration / cardinality (cost model, #6). **Invariant: open ⇒ no closed-world validation** (PromQL tolerates unknown labels — unlike Ibis/SQL's eager rejection). `#[serde(default)]` keeps it backward-compatible (absent ⇒ open). Tests: the open→closed freeze, serde default, SQL-closed / PromQL-open leaves. Full suite green (141), clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/core/src/intent_algebra/binder.rs | 9 ++- .../src/intent_algebra/column_resolution.rs | 3 + crates/core/src/intent_algebra/query_expr.rs | 69 ++++++++++++++++++- crates/core/src/intent_algebra/schema.rs | 31 +++++++++ crates/e2e/src/lib.rs | 3 + crates/lower/src/sql/mod.rs | 2 + crates/lower/tests/promql_conformance.rs | 15 ++++ crates/lower/tests/sql_lowering.rs | 8 ++- 8 files changed, 135 insertions(+), 5 deletions(-) diff --git a/crates/core/src/intent_algebra/binder.rs b/crates/core/src/intent_algebra/binder.rs index 6111ba1b..b9e6ec91 100644 --- a/crates/core/src/intent_algebra/binder.rs +++ b/crates/core/src/intent_algebra/binder.rs @@ -22,7 +22,11 @@ use crate::intent_algebra::schema::{Column, DataType, Schema}; /// it for SQL; PromQL uses [`UsageDerivedCatalog`] (returns `None`) until a /// registry-backed impl (returning a metric's known label set) drops in here. /// Distinct from `Scan.schema`, which is the *resolved* binding schema this -/// feeds — the catalog is the input, the schema is the result. +/// feeds — the catalog is the input, the schema is the result. Even a +/// registry-backed PromQL catalog yields an **open** schema +/// ([`Schema::closed`](super::schema::Schema::closed) `= false`): a metric's +/// labels are per-series and time-varying, so the registry is a superset hint, +/// not a per-row contract. pub trait SchemaCatalog { /// Columns known for `source`. `None` when unknown — the [`Binder`] then /// falls back to a usage-derived column set. @@ -93,6 +97,9 @@ impl Binder { columns, time_index, unique_keys: Vec::new(), + // Usage-derived (schemaless PromQL): the metric's full label set is + // open and runtime-only, so this lists only what the query references. + closed: false, } } } diff --git a/crates/core/src/intent_algebra/column_resolution.rs b/crates/core/src/intent_algebra/column_resolution.rs index 7fc87fa5..2b5177a3 100644 --- a/crates/core/src/intent_algebra/column_resolution.rs +++ b/crates/core/src/intent_algebra/column_resolution.rs @@ -194,6 +194,9 @@ pub fn output_schema_for_aggregate( columns: out_cols, time_index: None, unique_keys, + // A cross-series aggregate fully determines its output columns, so the + // result is closed even over an open input (mirrors `output_schema_in`). + closed: true, } } diff --git a/crates/core/src/intent_algebra/query_expr.rs b/crates/core/src/intent_algebra/query_expr.rs index 3703fd12..1d105f03 100644 --- a/crates/core/src/intent_algebra/query_expr.rs +++ b/crates/core/src/intent_algebra/query_expr.rs @@ -213,9 +213,10 @@ pub enum QueryExpr { /// description of the runtime row. Complete when catalog-backed (SQL); for /// schemaless PromQL it is usage-derived by the [`Binder`](super::binder) /// (the `(ts, value)` floor + the labels the query references), since a - /// metric's label set is open and known only at runtime. `predicates` are - /// leaf-level row filters (PromQL label matchers, pushed-down `WHERE` - /// conjuncts). + /// metric's label set is open and known only at runtime. That distinction is + /// carried explicitly by [`Schema::closed`](super::schema::Schema::closed) + /// (SQL leaf → `true`, PromQL leaf → `false`). `predicates` are leaf-level + /// row filters (PromQL label matchers, pushed-down `WHERE` conjuncts). Scan { source: Source, #[serde(default)] @@ -436,6 +437,10 @@ impl QueryExpr { columns: out_cols, time_index: None, unique_keys, + // A cross-series aggregate enumerates exactly `by ++ aggs`, + // so its output is closed even over an open input — this is + // the "freeze" point (cf. Calcite resolving a dynamic type). + closed: true, }) } @@ -480,6 +485,8 @@ impl QueryExpr { columns, time_index, unique_keys: Vec::new(), + // Projection enumerates exactly its items → closed. + closed: true, }) } @@ -533,6 +540,8 @@ impl QueryExpr { columns, time_index, unique_keys: Vec::new(), + // The concatenation is complete only if both sides are. + closed: l.closed && r.closed, }) } // ψ-analytic — child schema + one appended window-output column. @@ -596,6 +605,9 @@ fn per_series_reduction_schema(input: &Schema, agg: &AggIntent) -> Schema { columns, time_index: input.time_index, unique_keys: input.unique_keys.clone(), + // Per-series reduction is label-preserving: it inherits its input's + // completeness (an open scan stays open; a closed one stays closed). + closed: input.closed, } } @@ -709,6 +721,7 @@ mod tests { columns, time_index, unique_keys: uk, + closed: true, }, } } @@ -840,6 +853,56 @@ mod tests { ); } + #[test] + fn completeness_open_leaf_freezes_to_closed_at_cross_series_aggregate() { + // A schemaless (PromQL-style) leaf is *open*; it stays open through a + // per-series reduction (`rate`), then is **frozen to closed** by a + // cross-series aggregate — mirroring Calcite's DynamicRecordType being + // resolved to a fixed RelRecordType. + let open_leaf = QueryExpr::Scan { + source: Source::TimeSeries { metric: "m".into() }, + predicates: vec![], + // `with_time_index` defaults to `closed: false` (open). + schema: Schema::with_time_index( + vec![ + col("ts", DataType::Timestamp, false), + col("value", DataType::Float64, false), + col("job", DataType::Utf8, true), + ], + 0, + vec![], + ), + }; + assert!( + !open_leaf.output_schema().unwrap().closed, + "schemaless leaf is open" + ); + + let rate = QueryExpr::Aggregate { + by: vec![], + aggs: vec![AggIntent::Rate], + output_names: vec![], + having: None, + child: Box::new(open_leaf), + }; + assert!( + !rate.output_schema().unwrap().closed, + "per-series rate is label-preserving → stays open" + ); + + let sum_by_job = QueryExpr::Aggregate { + by: vec![2], // `job` + aggs: vec![AggIntent::Sum { col: None }], + output_names: vec![], + having: None, + child: Box::new(rate), + }; + assert!( + sum_by_job.output_schema().unwrap().closed, + "cross-series aggregate enumerates `by ++ aggs` → frozen to closed" + ); + } + #[test] fn project_keeps_time_index_when_ts_passed_through() { let child = scan( diff --git a/crates/core/src/intent_algebra/schema.rs b/crates/core/src/intent_algebra/schema.rs index 235afb72..afb83b0a 100644 --- a/crates/core/src/intent_algebra/schema.rs +++ b/crates/core/src/intent_algebra/schema.rs @@ -113,6 +113,24 @@ pub struct Schema { /// "no provable unique constraint" (the conservative default). #[serde(default)] pub unique_keys: Vec>, + /// Whether this schema **completely enumerates** the columns at this point. + /// + /// - `true` (**closed**): there are no columns beyond these — a catalog-backed + /// SQL source, or an output fully determined by an `Aggregate`/`Project`. + /// - `false` (**open**): a dynamic / superset schema — the runtime row may + /// carry more columns than are listed (a schemaless PromQL leaf lists only + /// the `(ts, value)` floor + the labels the query references). + /// + /// This mirrors Apache Calcite's `DynamicRecordType` (schema-on-read): the + /// schema starts open at a schemaless leaf and is **frozen to closed** by the + /// first operator that fully determines its output columns (`Aggregate` / + /// `Project`). Consumers needing completeness (label validation, full-output + /// enumeration, cardinality for the cost model) must check this; positional + /// resolution does not care. **Invariant: open ⇒ do not apply closed-world + /// validation** (PromQL tolerates unknown labels). Defaults to `false` (open) + /// — the conservative choice when completeness is unknown. + #[serde(default)] + pub closed: bool, } impl Schema { @@ -124,6 +142,7 @@ impl Schema { columns, time_index: None, unique_keys: Vec::new(), + closed: false, } } @@ -138,6 +157,7 @@ impl Schema { columns, time_index: Some(time_index), unique_keys, + closed: false, } } @@ -359,6 +379,17 @@ mod tests { assert_eq!(s, back); } + #[test] + fn schema_closed_defaults_to_open_when_absent() { + // `closed` is `#[serde(default)]` so schemas serialized before the field + // existed deserialize to `closed: false` (open) — the conservative + // default (don't claim completeness you can't prove). + let mut v = serde_json::to_value(Schema::new(vec![col("a", DataType::Utf8)])).unwrap(); + assert!(v.as_object_mut().unwrap().remove("closed").is_some()); + let back: Schema = serde_json::from_value(v).unwrap(); + assert!(!back.closed, "absent `closed` ⇒ open"); + } + #[test] fn column_table_defaults_to_none_when_absent() { // `Column.table` is `#[serde(default)]` so schemas serialized before the diff --git a/crates/e2e/src/lib.rs b/crates/e2e/src/lib.rs index aee3779f..9a05e817 100644 --- a/crates/e2e/src/lib.rs +++ b/crates/e2e/src/lib.rs @@ -33,6 +33,9 @@ pub mod fixtures { columns: cols, time_index: Some(0), unique_keys: vec![], + // Schemaless PromQL leaf: open (the metric's full label set is + // runtime-only; this lists just the referenced labels). + closed: false, } } } diff --git a/crates/lower/src/sql/mod.rs b/crates/lower/src/sql/mod.rs index a06353ec..f2122adc 100644 --- a/crates/lower/src/sql/mod.rs +++ b/crates/lower/src/sql/mod.rs @@ -151,6 +151,8 @@ impl<'a> SqlLowerer<'a> { .collect(), time_index: schema.time_index, unique_keys: schema.unique_keys.clone(), + // Catalog-backed: the table's columns are fully declared → closed. + closed: true, }; Ok(L2::Source(SourceSpec::with_schema( table.to_string(), diff --git a/crates/lower/tests/promql_conformance.rs b/crates/lower/tests/promql_conformance.rs index 35fe869b..f29e53c6 100644 --- a/crates/lower/tests/promql_conformance.rs +++ b/crates/lower/tests/promql_conformance.rs @@ -137,6 +137,21 @@ fn instant_vector_selector() { assert_eq!(preds, 0, "no label matchers → no predicates"); } +#[test] +fn promql_scan_schema_is_open() { + // A schemaless PromQL leaf is *open*: the metric's full label set is + // runtime-only, so the binding schema lists only the (ts, value) floor + + // referenced labels and may be a subset of the runtime row. + let qe = ok("node_cpu_seconds_total"); + let QueryExpr::Scan { schema, .. } = &qe else { + panic!("expected a Scan for a bare selector, got {qe:?}"); + }; + assert!( + !schema.closed, + "a schemaless PromQL scan has an open schema" + ); +} + #[test] fn label_matchers_become_scan_predicates() { // SEMANTICS: `=`, `!=`, `=~`, `!~` filter series; one conjunct per matcher. diff --git a/crates/lower/tests/sql_lowering.rs b/crates/lower/tests/sql_lowering.rs index e37e81c4..91596de5 100644 --- a/crates/lower/tests/sql_lowering.rs +++ b/crates/lower/tests/sql_lowering.rs @@ -100,13 +100,19 @@ async fn select_star_with_where_folds_predicate_onto_scan() { // SELECT * elides the projection; WHERE folds onto the Scan predicates. let qe = lower("SELECT * FROM metrics WHERE service = 'api'").await; let QueryExpr::Scan { - source, predicates, .. + source, + predicates, + schema, } = &qe else { panic!("expected Scan at root, got {qe:?}"); }; assert!(matches!(source, Source::Table { table_ref } if table_ref == "metrics")); assert_eq!(predicates.len(), 1, "WHERE clause folded onto the scan"); + assert!( + schema.closed, + "a catalog-backed SQL scan has a closed schema" + ); } #[tokio::test] From d42ca7eecdd85734bb5f4d6c9d2847379616dba5 Mon Sep 17 00:00:00 2001 From: zz_y Date: Thu, 28 May 2026 10:07:06 -0600 Subject: [PATCH 3/3] docs: keep the Calcite anchor on Schema.closed only; de-decorate the two call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The external pattern reference earns its place at the definition site (`Schema.closed` docstring, as a lookup-able anchor) but is noise repeated at the Aggregate arm + the test — reword those to describe the behavior (freeze to closed) directly. The full design survey stays in the PR description. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/core/src/intent_algebra/query_expr.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/core/src/intent_algebra/query_expr.rs b/crates/core/src/intent_algebra/query_expr.rs index 1d105f03..7bcd5d1a 100644 --- a/crates/core/src/intent_algebra/query_expr.rs +++ b/crates/core/src/intent_algebra/query_expr.rs @@ -439,7 +439,7 @@ impl QueryExpr { unique_keys, // A cross-series aggregate enumerates exactly `by ++ aggs`, // so its output is closed even over an open input — this is - // the "freeze" point (cf. Calcite resolving a dynamic type). + // where an open schema freezes to closed. closed: true, }) } @@ -857,8 +857,7 @@ mod tests { fn completeness_open_leaf_freezes_to_closed_at_cross_series_aggregate() { // A schemaless (PromQL-style) leaf is *open*; it stays open through a // per-series reduction (`rate`), then is **frozen to closed** by a - // cross-series aggregate — mirroring Calcite's DynamicRecordType being - // resolved to a fixed RelRecordType. + // cross-series aggregate (which enumerates exactly its output columns). let open_leaf = QueryExpr::Scan { source: Source::TimeSeries { metric: "m".into() }, predicates: vec![],