Skip to content

Commit ba4e6b9

Browse files
committed
feat(nodedb-sql): extend bitemporal support to columnar and timeseries engines
Add `bitemporal` and `temporal` fields to `AggregatePlanParams` so that temporal scope is preserved from the outer SELECT into the inner scan when planning aggregate queries over bitemporal collections. Update `ColumnarRules` and `TimeseriesRules` to allow `FOR SYSTEM_TIME / FOR VALID_TIME` clauses when the collection was created `WITH BITEMPORAL`; non-bitemporal collections still receive a clear error message. The aggregate path receives the same guard. Propagate `temporal` into `SqlPlan::TimeseriesScan` so the conversion layer has the scope available when building the physical plan. Fix aggregate planning in the document engines to forward `temporal` into the inner scan instead of resetting it to the default scope.
1 parent 01512b3 commit ba4e6b9

8 files changed

Lines changed: 46 additions & 10 deletions

File tree

nodedb-sql/src/engine_rules/columnar.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ impl EngineRules for ColumnarRules {
3232
}
3333

3434
fn plan_scan(&self, p: ScanParams) -> Result<SqlPlan> {
35-
if p.temporal.is_temporal() {
35+
if p.temporal.is_temporal() && !p.bitemporal {
3636
return Err(SqlError::Unsupported {
3737
detail: format!(
38-
"FOR SYSTEM_TIME / FOR VALID_TIME is not supported on columnar \
39-
collection '{}'",
38+
"FOR SYSTEM_TIME / FOR VALID_TIME requires a bitemporal \
39+
collection; '{}' was not created WITH bitemporal = true",
4040
p.collection
4141
),
4242
});
@@ -87,6 +87,15 @@ impl EngineRules for ColumnarRules {
8787
}
8888

8989
fn plan_aggregate(&self, p: AggregateParams) -> Result<SqlPlan> {
90+
if p.temporal.is_temporal() && !p.bitemporal {
91+
return Err(SqlError::Unsupported {
92+
detail: format!(
93+
"FOR SYSTEM_TIME / FOR VALID_TIME requires a bitemporal \
94+
collection; '{}' was not created WITH bitemporal = true",
95+
p.collection
96+
),
97+
});
98+
}
9099
let base_scan = SqlPlan::Scan {
91100
collection: p.collection,
92101
alias: p.alias,
@@ -98,7 +107,7 @@ impl EngineRules for ColumnarRules {
98107
offset: 0,
99108
distinct: false,
100109
window_functions: Vec::new(),
101-
temporal: crate::temporal::TemporalScope::default(),
110+
temporal: p.temporal,
102111
};
103112
Ok(SqlPlan::Aggregate {
104113
input: Box::new(base_scan),

nodedb-sql/src/engine_rules/document_schemaless.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl EngineRules for SchemalessRules {
9999
offset: 0,
100100
distinct: false,
101101
window_functions: Vec::new(),
102-
temporal: crate::temporal::TemporalScope::default(),
102+
temporal: p.temporal,
103103
};
104104
Ok(SqlPlan::Aggregate {
105105
input: Box::new(base_scan),

nodedb-sql/src/engine_rules/document_strict.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ impl EngineRules for StrictRules {
9999
offset: 0,
100100
distinct: false,
101101
window_functions: Vec::new(),
102-
temporal: crate::temporal::TemporalScope::default(),
102+
temporal: p.temporal,
103103
};
104104
Ok(SqlPlan::Aggregate {
105105
input: Box::new(base_scan),

nodedb-sql/src/engine_rules/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,14 @@ pub struct AggregateParams {
9797
pub group_columns: Vec<String>,
9898
/// Whether the collection has auto-tiering enabled.
9999
pub has_auto_tier: bool,
100+
/// Whether this collection was created with bitemporal storage.
101+
/// When `true`, the base scan inside the aggregate is allowed to
102+
/// carry a non-default temporal scope.
103+
pub bitemporal: bool,
104+
/// System-time / valid-time scope to propagate into the underlying
105+
/// scan so bitemporal aggregate queries project an as-of snapshot
106+
/// before grouping.
107+
pub temporal: crate::temporal::TemporalScope,
100108
}
101109

102110
/// Engine-specific planning rules.

nodedb-sql/src/engine_rules/timeseries.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ impl EngineRules for TimeseriesRules {
2626
}
2727

2828
fn plan_scan(&self, p: ScanParams) -> Result<SqlPlan> {
29-
if p.temporal.is_temporal() {
29+
if p.temporal.is_temporal() && !p.bitemporal {
3030
return Err(SqlError::Unsupported {
3131
detail: format!(
32-
"FOR SYSTEM_TIME / FOR VALID_TIME is not supported on timeseries \
33-
collection '{}' — filter by the existing time column instead",
32+
"FOR SYSTEM_TIME / FOR VALID_TIME requires a bitemporal \
33+
timeseries collection; '{}' was not created WITH bitemporal = true",
3434
p.collection
3535
),
3636
});
@@ -48,6 +48,7 @@ impl EngineRules for TimeseriesRules {
4848
gap_fill: String::new(),
4949
limit: p.limit.unwrap_or(10000),
5050
tiered: false,
51+
temporal: p.temporal,
5152
})
5253
}
5354

@@ -78,6 +79,15 @@ impl EngineRules for TimeseriesRules {
7879
}
7980

8081
fn plan_aggregate(&self, p: AggregateParams) -> Result<SqlPlan> {
82+
if p.temporal.is_temporal() && !p.bitemporal {
83+
return Err(SqlError::Unsupported {
84+
detail: format!(
85+
"FOR SYSTEM_TIME / FOR VALID_TIME requires a bitemporal \
86+
timeseries collection; '{}' was not created WITH bitemporal = true",
87+
p.collection
88+
),
89+
});
90+
}
8191
Ok(SqlPlan::TimeseriesScan {
8292
collection: p.collection,
8393
time_range: default_time_range(),
@@ -89,6 +99,7 @@ impl EngineRules for TimeseriesRules {
8999
gap_fill: String::new(),
90100
limit: p.limit,
91101
tiered: p.has_auto_tier,
102+
temporal: p.temporal,
92103
})
93104
}
94105
}

nodedb-sql/src/planner/aggregate.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::functions::registry::{FunctionRegistry, SearchTrigger};
88
use crate::parser::normalize::normalize_ident;
99
use crate::resolver::columns::ResolvedTable;
1010
use crate::resolver::expr::convert_expr;
11+
use crate::temporal::TemporalScope;
1112
use crate::types::*;
1213

1314
/// Plan an aggregate query (GROUP BY + aggregate functions).
@@ -17,6 +18,7 @@ pub fn plan_aggregate(
1718
filters: &[Filter],
1819
_scope: &crate::resolver::columns::TableScope,
1920
functions: &FunctionRegistry,
21+
temporal: &TemporalScope,
2022
) -> Result<SqlPlan> {
2123
let group_by_exprs = convert_group_by(&select.group_by)?;
2224
let aggregates = extract_aggregates_from_projection(&select.projection, functions)?;
@@ -41,6 +43,8 @@ pub fn plan_aggregate(
4143
bucket_interval_ms,
4244
group_columns,
4345
has_auto_tier: table.info.has_auto_tier,
46+
bitemporal: table.info.bitemporal,
47+
temporal: *temporal,
4448
})
4549
}
4650

nodedb-sql/src/planner/select.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ fn plan_select(
164164
// 6. Check for GROUP BY / aggregation.
165165
if has_aggregation(select, functions) {
166166
let mut plan =
167-
super::aggregate::plan_aggregate(select, table, &filters, &scope, functions)?;
167+
super::aggregate::plan_aggregate(select, table, &filters, &scope, functions, &temporal)?;
168168

169169
// Semi/anti subquery joins belong below the aggregate so they filter
170170
// the input rows before grouping. Scalar subqueries remain above the

nodedb-sql/src/types.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,10 @@ pub enum SqlPlan {
181181
gap_fill: String,
182182
limit: usize,
183183
tiered: bool,
184+
/// Bitemporal system-time / valid-time scope. Only non-default
185+
/// on collections created `WITH BITEMPORAL`; `TimeseriesRules::plan_scan`
186+
/// rejects temporal scopes otherwise.
187+
temporal: crate::temporal::TemporalScope,
184188
},
185189
TimeseriesIngest {
186190
collection: String,

0 commit comments

Comments
 (0)