Skip to content

Commit

Permalink
Change how group bys are taken into account
Browse files Browse the repository at this point in the history
  • Loading branch information
ngrislain committed Dec 11, 2023
1 parent f3016fe commit 72657ac
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 26 deletions.
8 changes: 8 additions & 0 deletions src/expr/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,4 +985,12 @@ mod tests {
]);
println!("split = {split}");
}

#[test]
fn test_split_map_reduce_map_group_by_expr() {
let split = Split::from(("b", expr!(2*count(1 + y))));
let split = split.and(Split::group_by(expr!(x-y)).into());
let split = split.and(Split::from(("a", expr!(x-y))));
println!("split = {split}");
}
}
39 changes: 13 additions & 26 deletions src/sql/relation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
ast,
builder::{Ready, With, WithIterator, WithoutContext},
dialect::{Dialect, GenericDialect},
expr::{Expr, Identifier, Split},
expr::{Expr, Identifier, Split, Reduce},
hierarchy::{Hierarchy, Path},
namer::{self, FIELD},
parser::Parser,
Expand Down Expand Up @@ -315,25 +315,6 @@ impl<'a> VisitedQueryRelations<'a> {
.map(|e| e.with(columns).try_into())
.collect::<Result<Vec<Expr>>>()?,
};
let exprs:HashMap<Expr, String> = named_exprs
.iter()
.cloned()
.map(|(name, x)| (x, name))
.collect();
// let mut named_exprs = named_exprs.into_iter().chain(
// group_by.iter()
// .cloned()
// .map(|gx| (
// exprs
// .get(&gx)
// .unwrap_or(&namer::name_from_content(FIELD, &gx))
// .to_string(),
// gx)
// )
// .collect::<Vec<_>>()
// ).collect::<Vec<_>>();


// Add the having in named_exprs
let having = if let Some(expr) = having {
let having_name = namer::name_from_content(FIELD, &expr);
Expand All @@ -355,14 +336,20 @@ impl<'a> VisitedQueryRelations<'a> {
} else {
None
};

// Build the Map or Reduce based on the type of split
let split = group_by.into_iter()
.fold(
Split::from_iter(named_exprs),
|s, expr| s.and(Split::Reduce(Split::group_by(expr)))
// If group_by is non-empty, start with them so that aggregations can take them into account
let split = if group_by.is_empty() {
Split::from_iter(named_exprs)
} else {
let group_by = group_by.into_iter()
.fold(Split::Reduce(Reduce::default()),
|s, expr| s.and(Split::Reduce(Split::group_by(expr)))
);
println!("split = {}", split);
named_exprs.into_iter()
.fold(group_by,
|s, named_expr| s.and(named_expr.into())
)
};
// Prepare the WHERE
let filter: Option<Expr> = selection
.as_ref()
Expand Down

0 comments on commit 72657ac

Please sign in to comment.