chore(cubestore): Upgrade DF: fix aggregate index hll tests

cube-js · Dec 2, 2024 · 5369396 · 5369396
1 parent b4d8607
commit 5369396
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 3 deletions.
diff --git a/rust/cubestore/cubestore/src/metastore/table.rs b/rust/cubestore/cubestore/src/metastore/table.rs
@@ -93,8 +93,19 @@ impl AggregateColumn {
             .build()?,
             AggregateFunction::MERGE => {
                 let fun = aggregate_udf_by_kind(CubeAggregateUDFKind::MergeHll);
-                // TODO upgrade DF: cleanup: don't wrap fun in Arc::new
-                AggregateExprBuilder::new(Arc::new(fun), vec![col]).build()?
+
+                // TODO upgrade DF: Understand what effect the choice of alias value has.
+                // TODO upgrade DF: We probably want .schema and .alias on other cases.
+                // TODO upgrade DF: schema.clone() is wasteful; pass an &Arc<ArrowSchema> to this function.
+                // TODO upgrade DF: Do we want more than .alias and .schema?  It seems some stuff is mandatory, in general
+
+                // A comment in DF downstream name() fn suggests 'Human readable name such as
+                // `"MIN(c2)"`.'  It is mandatory that a .alias be supplied.
+                let alias = format!("MERGE({})", col.name());
+                AggregateExprBuilder::new(Arc::new(fun), vec![col])
+                    .schema(Arc::new(schema.clone()))
+                    .alias(alias)
+                    .build()?
             }
         };
         Ok(res)

diff --git a/rust/cubestore/cubestore/src/store/mod.rs b/rust/cubestore/cubestore/src/store/mod.rs
@@ -1329,12 +1329,15 @@ impl ChunkStore {
                 //     .map(|x| x as usize)
                 //     .collect();
 
+                // TODO upgrade DF:  this is probably correct, but find out if we now need to supply some filter_expr from some loose end.
+                let filter_expr: Vec<Option<Arc<dyn PhysicalExpr>>> = vec![None; aggregates.len()];
+
                 // TODO merge sort
                 let aggregate = Arc::new(AggregateExec::try_new(
                     AggregateMode::Single,
                     PhysicalGroupBy::new_single(groups),
                     aggregates,
-                    Vec::new(),
+                    filter_expr,
                     input,
                     schema.clone(),
                 )?);