Skip to content

Commit fce0ca2

Browse files
committed
Preserve explicit derived inline aggregates
1 parent 858e1d7 commit fce0ca2

3 files changed

Lines changed: 64 additions & 13 deletions

File tree

sidemantic-rs/src/config/schema.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -606,20 +606,18 @@ impl DimensionConfig {
606606

607607
impl MetricConfig {
608608
fn into_metric(self) -> Metric {
609-
let inline_aggregation = if self.agg.is_none() {
609+
let explicit_metric_type = self.metric_type.as_deref().map(str::to_ascii_lowercase);
610+
let can_normalize_inline_aggregation =
611+
matches!(explicit_metric_type.as_deref(), None | Some("simple"));
612+
let inline_aggregation = if self.agg.is_none() && can_normalize_inline_aggregation {
610613
self.sql
611614
.as_deref()
612615
.and_then(parse_inline_metric_aggregation)
613616
} else {
614617
None
615618
};
616619

617-
let metric_type = match self
618-
.metric_type
619-
.as_deref()
620-
.map(str::to_ascii_lowercase)
621-
.as_deref()
622-
{
620+
let metric_type = match explicit_metric_type.as_deref() {
623621
Some("simple") => MetricType::Simple,
624622
Some("derived") => MetricType::Derived,
625623
Some("ratio") => MetricType::Ratio,
@@ -1795,6 +1793,9 @@ models:
17951793
sql: VARIANCE_POP(amount)
17961794
- name: revenue_per_order
17971795
sql: SUM(amount) / COUNT(*)
1796+
- name: explicit_derived_revenue
1797+
type: derived
1798+
sql: SUM(orders.amount)
17981799
"#;
17991800

18001801
let config: SidemanticConfig = serde_yaml::from_str(yaml).unwrap();
@@ -1844,6 +1845,18 @@ models:
18441845
revenue_per_order.sql.as_deref(),
18451846
Some("SUM(amount) / COUNT(*)")
18461847
);
1848+
1849+
let explicit_derived_revenue = orders
1850+
.metrics
1851+
.iter()
1852+
.find(|m| m.name == "explicit_derived_revenue")
1853+
.unwrap();
1854+
assert_eq!(explicit_derived_revenue.r#type, MetricType::Derived);
1855+
assert_eq!(explicit_derived_revenue.agg, None);
1856+
assert_eq!(
1857+
explicit_derived_revenue.sql.as_deref(),
1858+
Some("SUM(orders.amount)")
1859+
);
18471860
}
18481861

18491862
#[test]

sidemantic-rs/src/config/sql_parser.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1938,12 +1938,8 @@ fn build_metric(props: &HashMap<String, String>) -> Option<Metric> {
19381938

19391939
metric.agg = parse_metric_aggregation(props.get("agg"));
19401940

1941-
if metric.agg.is_none()
1942-
&& matches!(
1943-
metric.r#type,
1944-
MetricType::Simple | MetricType::Cumulative | MetricType::Derived
1945-
)
1946-
{
1941+
let explicit_metric_type = props.get("type").map(|value| value.to_ascii_lowercase());
1942+
if metric.agg.is_none() && matches!(explicit_metric_type.as_deref(), None | Some("simple")) {
19471943
if let Some(sql) = metric.sql.as_deref() {
19481944
if let Some((agg, inner_expr)) = extract_aggregation_with_polyglot(sql) {
19491945
metric.agg = parse_metric_aggregation(Some(&agg));
@@ -2228,6 +2224,20 @@ mod tests {
22282224
assert_eq!(revenue.sql, Some("amount".to_string()));
22292225
}
22302226

2227+
#[test]
2228+
fn test_parse_explicit_derived_metric_preserves_inline_aggregate_expression() {
2229+
let sql = r#"
2230+
MODEL (name orders, table orders);
2231+
METRIC (name revenue, type derived, sql SUM(orders.amount));
2232+
"#;
2233+
2234+
let model = parse_sql_model(sql).unwrap();
2235+
let revenue = model.get_metric("revenue").unwrap();
2236+
assert_eq!(revenue.r#type, MetricType::Derived);
2237+
assert_eq!(revenue.agg, None);
2238+
assert_eq!(revenue.sql, Some("SUM(orders.amount)".to_string()));
2239+
}
2240+
22312241
#[test]
22322242
fn test_parse_cohort_metric_preserves_outer_aggregation() {
22332243
let sql = r#"

sidemantic-rs/src/sql/generator.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4804,6 +4804,34 @@ mod tests {
48044804
);
48054805
}
48064806

4807+
#[test]
4808+
fn test_explicit_derived_inline_aggregate_from_yaml_generates_aggregate() {
4809+
let graph = crate::config::load_from_string(
4810+
r#"
4811+
models:
4812+
- name: orders
4813+
table: orders
4814+
primary_key: order_id
4815+
metrics:
4816+
- name: derived_revenue
4817+
type: derived
4818+
sql: SUM(orders.amount)
4819+
"#,
4820+
)
4821+
.unwrap();
4822+
let generator = SqlGenerator::new(&graph);
4823+
4824+
let query = SemanticQuery::new().with_metrics(vec!["orders.derived_revenue".into()]);
4825+
4826+
let sql = generator.generate(&query).unwrap();
4827+
4828+
assert!(sql.contains("amount AS amount"), "{sql}");
4829+
assert!(
4830+
sql.contains("SUM(orders_cte.amount) AS derived_revenue"),
4831+
"{sql}"
4832+
);
4833+
}
4834+
48074835
#[test]
48084836
fn test_ordered_set_aggregate_metric_is_not_treated_as_metric_reference() {
48094837
let mut graph = SemanticGraph::new();

0 commit comments

Comments
 (0)