Skip to content

Commit 1cddccb

Browse files
committed
refactor: name paired aggregates bivariate
1 parent f3718d2 commit 1cddccb

11 files changed

Lines changed: 53 additions & 53 deletions

File tree

‎crates/asap-aware-mapping/src/query_physical_lowering.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,8 +1187,8 @@ fn supports_hash_aggregate(
11871187
| AggIntent::Avg { .. }
11881188
| AggIntent::StdDev { .. }
11891189
| AggIntent::Variance { .. }
1190-
| AggIntent::Binary {
1191-
op: asap_types::pre_asap::BinaryAggOp::Correlation,
1190+
| AggIntent::Bivariate {
1191+
op: asap_types::pre_asap::BivariateAggOp::Correlation,
11921192
..
11931193
}
11941194
| AggIntent::Group
@@ -1451,15 +1451,15 @@ mod tests {
14511451
#[test]
14521452
fn correlation_lowers_to_physical_hash_aggregate() {
14531453
use asap_types::pre_asap::{
1454-
AggIntent, BinaryAggOp, Column, DataType, QueryExpr, Reduction, Schema, Source,
1454+
AggIntent, BivariateAggOp, Column, DataType, QueryExpr, Reduction, Schema, Source,
14551455
};
14561456
let source = Source::Table {
14571457
table_ref: "pairs".into(),
14581458
};
14591459
let root = Rc::new(QueryExpr::Aggregate {
14601460
reduction: Reduction::by(vec![]),
1461-
measures: vec![AggIntent::Binary {
1462-
op: BinaryAggOp::Correlation,
1461+
measures: vec![AggIntent::Bivariate {
1462+
op: BivariateAggOp::Correlation,
14631463
left: 0,
14641464
right: 1,
14651465
}],

‎crates/asap-aware-mapping/src/replacement.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ pub(crate) fn implementations_for_with(
822822
AggIntent::Avg { .. }
823823
| AggIntent::StdDev { .. }
824824
| AggIntent::Variance { .. }
825-
| AggIntent::Binary { .. } => {
825+
| AggIntent::Bivariate { .. } => {
826826
vec![Implementation::PassThrough]
827827
}
828828

@@ -6067,9 +6067,9 @@ mod tests {
60676067

60686068
// Correlation must never acquire a single-input sketch or scalar accumulator.
60696069
#[test]
6070-
fn binary_aggregate_keeps_exact_paired_input() {
6071-
let intent = AggIntent::Binary {
6072-
op: asap_types::pre_asap::BinaryAggOp::Correlation,
6070+
fn bivariate_aggregate_keeps_exact_paired_input() {
6071+
let intent = AggIntent::Bivariate {
6072+
op: asap_types::pre_asap::BivariateAggOp::Correlation,
60736073
left: 0,
60746074
right: 1,
60756075
};

‎crates/frontend-sql/src/sql/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1637,8 +1637,8 @@ fn lower_agg_intent(expr: &Expr) -> Result<AggIntent<ColumnRef>, LoweringError>
16371637
"corr requires two arguments".into(),
16381638
));
16391639
};
1640-
AggIntent::Binary {
1641-
op: asap_types::pre_asap::BinaryAggOp::Correlation,
1640+
AggIntent::Bivariate {
1641+
op: asap_types::pre_asap::BivariateAggOp::Correlation,
16421642
left: expr_to_group_ref(left)?,
16431643
right: expr_to_group_ref(right)?,
16441644
}

crates/frontend-sql/tests/binary_aggregates.rs renamed to crates/frontend-sql/tests/bivariate_aggregates.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
use std::rc::Rc;
33

44
use asap_frontend_sql::{lower_sql, SqlCatalog};
5-
use asap_types::pre_asap::{AggIntent, BinaryAggOp, Column, DataType, QueryExpr, Schema};
5+
use asap_types::pre_asap::{AggIntent, BivariateAggOp, Column, DataType, QueryExpr, Schema};
66
use asap_types::types::AccuracyTarget;
77

88
fn catalog() -> SqlCatalog {
@@ -47,8 +47,8 @@ async fn corr_materializes_both_arguments() {
4747
let (measures, child) = aggregate(&query);
4848
assert_eq!(
4949
measures,
50-
&[AggIntent::Binary {
51-
op: BinaryAggOp::Correlation,
50+
&[AggIntent::Bivariate {
51+
op: BivariateAggOp::Correlation,
5252
left: 0,
5353
right: 1,
5454
}]
@@ -87,7 +87,7 @@ async fn corr_coexists_with_grouping_having_and_other_measures() {
8787
let (measures, child) = aggregate(&query);
8888
let pair = measures
8989
.iter()
90-
.find(|m| matches!(m, AggIntent::Binary { .. }))
90+
.find(|m| matches!(m, AggIntent::Bivariate { .. }))
9191
.unwrap();
9292
let schema = child.output_schema().unwrap();
9393
for id in pair.input_cols() {

‎crates/sql-function-catalog/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ pub const KNOWN_UNMAPPED_NATIVE_FUNCTIONS: &[&str] = &[
353353
"bool_and",
354354
"bool_or",
355355
// Two-column covariance / regression operations not yet implemented by
356-
// `AggIntent::Binary`. Correlation is the first supported operation.
356+
// `AggIntent::Bivariate`. Correlation is the first supported operation.
357357
"covar",
358358
"covar_pop",
359359
"covar_samp",

‎crates/types/src/post_asap/execution_data_state.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,11 +1069,11 @@ mod tests {
10691069

10701070
// Both paired operands must be plain; an unrelated state column is not an input.
10711071
#[test]
1072-
fn binary_aggregate_checks_both_operand_states() {
1072+
fn bivariate_aggregate_checks_both_operand_states() {
10731073
let operation = ValueOperation::Exact(ExactOperation::Aggregate {
10741074
reduction: Reduction::by(vec![]),
1075-
measures: vec![AggIntent::Binary {
1076-
op: crate::pre_asap::BinaryAggOp::Correlation,
1075+
measures: vec![AggIntent::Bivariate {
1076+
op: crate::pre_asap::BivariateAggOp::Correlation,
10771077
left: 0,
10781078
right: 1,
10791079
}],

‎crates/types/src/pre_asap/agg_intent.rs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,10 @@ use crate::types::AccuracyTarget;
4141
#[serde(bound(serialize = "C: Serialize", deserialize = "C: Deserialize<'de>"))]
4242
pub enum AggIntent<C = ColumnId> {
4343
/// A reduction over paired values from two explicit input columns.
44-
/// Operation semantics live in `BinaryAggOp`; both references participate
44+
/// Operation semantics live in `BivariateAggOp`; both references participate
4545
/// in binding and dependency tracking, unlike an opaque extension payload.
46-
Binary {
47-
op: BinaryAggOp,
46+
Bivariate {
47+
op: BivariateAggOp,
4848
left: C,
4949
right: C,
5050
},
@@ -292,7 +292,7 @@ pub enum AggIntent<C = ColumnId> {
292292
/// must define their output type and exact/summary realization explicitly.
293293
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
294294
#[serde(rename_all = "snake_case")]
295-
pub enum BinaryAggOp {
295+
pub enum BivariateAggOp {
296296
/// Pearson correlation over pairs where both inputs are non-null.
297297
Correlation,
298298
}
@@ -489,12 +489,12 @@ impl<C: Clone> AggIntent<C> {
489489
/// An empty list retains the existing implicit sample/row-count convention.
490490
pub fn input_cols(&self) -> Vec<C> {
491491
match self {
492-
Self::Binary { left, right, .. } => vec![left.clone(), right.clone()],
492+
Self::Bivariate { left, right, .. } => vec![left.clone(), right.clone()],
493493
_ => self.input_col().into_iter().collect(),
494494
}
495495
}
496496

497-
/// The explicit input of a single-column reducer. Binary aggregates,
497+
/// The explicit input of a single-column reducer. Bivariate aggregates,
498498
/// argument-less aggregates, and implicit PromQL sample inputs return `None`.
499499
/// Use `input_cols` for dependency tracking; this accessor is for consumers
500500
/// that have already selected a single-column implementation.
@@ -529,8 +529,8 @@ impl<C: Clone> AggIntent<C> {
529529
AggIntent::Avg { .. } => col("avg", DataType::Float64, false),
530530
AggIntent::StdDev { .. } => col("stddev", DataType::Float64, false),
531531
AggIntent::Variance { .. } => col("variance", DataType::Float64, false),
532-
AggIntent::Binary { op, .. } => match op {
533-
BinaryAggOp::Correlation => col("corr", DataType::Float64, true),
532+
AggIntent::Bivariate { op, .. } => match op {
533+
BivariateAggOp::Correlation => col("corr", DataType::Float64, true),
534534
},
535535
AggIntent::Quantile { q, .. } => col(
536536
&format!("quantile_{}", quantile_suffix(*q)),
@@ -656,20 +656,20 @@ pub mod topk {
656656

657657
/// Two instances of this aggregation can be merged
658658
/// (`agg(A ∪ B) = combine(agg(A), agg(B))`). `Avg` / `StdDev` / `Variance`
659-
/// and binary correlation need richer partial state than a single value, so
659+
/// and correlation need richer partial state than a single value, so
660660
/// their finalized values are not mergeable.
661661
pub fn agg_is_mergeable(op: &AggIntent) -> bool {
662662
!matches!(
663663
op,
664664
AggIntent::Avg { .. }
665665
| AggIntent::StdDev { .. }
666666
| AggIntent::Variance { .. }
667-
| AggIntent::Binary { .. }
667+
| AggIntent::Bivariate { .. }
668668
)
669669
}
670670

671671
/// Whether this op implies `exact_required` — no sketch benefit. The exact
672-
/// intents include `Sum / Count / Avg / Min / Max` and binary correlation.
672+
/// intents include `Sum / Count / Avg / Min / Max` and correlation.
673673
pub fn agg_is_exact(op: &AggIntent) -> bool {
674674
matches!(
675675
op,
@@ -678,7 +678,7 @@ pub fn agg_is_exact(op: &AggIntent) -> bool {
678678
| AggIntent::Avg { .. }
679679
| AggIntent::Min { .. }
680680
| AggIntent::Max { .. }
681-
| AggIntent::Binary { .. }
681+
| AggIntent::Bivariate { .. }
682682
| AggIntent::Group
683683
| AggIntent::CountValues { .. }
684684
)
@@ -735,9 +735,9 @@ mod tests {
735735

736736
// Paired aggregates expose both dependencies but cannot merge final scalar results.
737737
#[test]
738-
fn binary_aggregate_contract() {
739-
let intent = AggIntent::Binary {
740-
op: BinaryAggOp::Correlation,
738+
fn bivariate_aggregate_contract() {
739+
let intent = AggIntent::Bivariate {
740+
op: BivariateAggOp::Correlation,
741741
left: 2,
742742
right: 5,
743743
};
@@ -749,7 +749,7 @@ mod tests {
749749
assert_eq!(output.dtype, DataType::Float64);
750750
assert!(output.nullable);
751751
let value = serde_json::to_value(&intent).unwrap();
752-
assert_eq!(value["kind"], "binary");
752+
assert_eq!(value["kind"], "bivariate");
753753
assert_eq!(value["op"], "correlation");
754754
assert_eq!(serde_json::from_value::<AggIntent>(value).unwrap(), intent);
755755
}

‎crates/types/src/pre_asap/binder.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -380,12 +380,12 @@ mod tests {
380380

381381
// Both binary inputs must seed a usage-derived schema before positional resolution.
382382
#[test]
383-
fn binary_inputs_seed_usage_derived_schema() {
384-
use crate::pre_asap::{AggIntent, BinaryAggOp, Reduction};
383+
fn bivariate_inputs_seed_usage_derived_schema() {
384+
use crate::pre_asap::{AggIntent, BivariateAggOp, Reduction};
385385
let tree = UnresolvedQueryExpr::Aggregate {
386386
reduction: Reduction::by(vec![]),
387-
measures: vec![AggIntent::Binary {
388-
op: BinaryAggOp::Correlation,
387+
measures: vec![AggIntent::Bivariate {
388+
op: BivariateAggOp::Correlation,
389389
left: ColumnRef::Named("x".into()),
390390
right: ColumnRef::Named("y".into()),
391391
}],

‎crates/types/src/pre_asap/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ pub mod schema;
4343

4444
pub use agg_intent::{
4545
agg_accuracy, agg_is_exact, agg_is_mergeable, default_cardinality, default_quantile, AggIntent,
46-
BinaryAggOp, MathFunc, TimeFunc,
46+
BivariateAggOp, MathFunc, TimeFunc,
4747
};
4848
pub use binder::{Binder, SchemaCatalog, UsageDerivedCatalog};
4949
pub use canonicalize::canonicalize;

‎crates/types/src/pre_asap/resolve.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ fn resolve_agg_intent(
518518
AggIntent::Count { accuracy } => AggIntent::Count {
519519
accuracy: accuracy.clone(),
520520
},
521-
AggIntent::Binary { op, left, right } => AggIntent::Binary {
521+
AggIntent::Bivariate { op, left, right } => AggIntent::Bivariate {
522522
op: *op,
523523
left: resolve_column_ref(left, schema)?,
524524
right: resolve_column_ref(right, schema)?,
@@ -616,14 +616,14 @@ mod tests {
616616

617617
// Both sides resolve with qualifiers; an unknown right input is an error.
618618
#[test]
619-
fn resolve_binary_aggregate_inputs() {
620-
use crate::pre_asap::{BinaryAggOp, Column, DataType};
619+
fn resolve_bivariate_aggregate_inputs() {
620+
use crate::pre_asap::{BivariateAggOp, Column, DataType};
621621
let schema = Schema::new(vec![
622622
Column::new("x", DataType::Float64, true).with_table("a"),
623623
Column::new("x", DataType::Float64, true).with_table("b"),
624624
]);
625-
let intent = AggIntent::Binary {
626-
op: BinaryAggOp::Correlation,
625+
let intent = AggIntent::Bivariate {
626+
op: BivariateAggOp::Correlation,
627627
left: ColumnRef::Qualified {
628628
table: "a".into(),
629629
name: "x".into(),
@@ -635,14 +635,14 @@ mod tests {
635635
};
636636
assert_eq!(
637637
resolve_agg_intent(&intent, &schema).unwrap(),
638-
AggIntent::Binary {
639-
op: BinaryAggOp::Correlation,
638+
AggIntent::Bivariate {
639+
op: BivariateAggOp::Correlation,
640640
left: 0,
641641
right: 1,
642642
}
643643
);
644-
let missing = AggIntent::Binary {
645-
op: BinaryAggOp::Correlation,
644+
let missing = AggIntent::Bivariate {
645+
op: BivariateAggOp::Correlation,
646646
left: ColumnRef::Qualified {
647647
table: "a".into(),
648648
name: "x".into(),

0 commit comments

Comments
 (0)