Skip to content

Commit 87bf3df

Browse files
committed
WIP: Make ClusterAggregateTopK::expressions, and with_exprs_and_inputs, ignore the having_exprs
1 parent c3c9bb9 commit 87bf3df

File tree

1 file changed

+14
-8
lines changed
  • rust/cubestore/cubestore/src/queryplanner/topk

1 file changed

+14
-8
lines changed

rust/cubestore/cubestore/src/queryplanner/topk/mod.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,11 @@ impl UserDefinedLogicalNode for ClusterAggregateTopK {
166166
.chain(&self.aggregate_expr)
167167
.cloned()
168168
.collect_vec();
169-
if self.having_expr.is_some() {
169+
// TODO upgrade DF: DF's type_coercion analysis pass doesn't like these exprs (which are
170+
// defined on the aggregate's output schema instead of the input schema). Maybe we should
171+
// split ClusterAggregateTopK into separate logical nodes. Omitting having_expr is a bit of
172+
// a hack...
173+
if false && self.having_expr.is_some() {
170174
res.push(self.having_expr.clone().unwrap());
171175
}
172176
res
@@ -187,14 +191,16 @@ impl UserDefinedLogicalNode for ClusterAggregateTopK {
187191
) -> datafusion::common::Result<Arc<dyn UserDefinedLogicalNode>> {
188192
let num_groups = self.group_expr.len();
189193
let num_aggs = self.aggregate_expr.len();
190-
let num_having = if self.having_expr.is_some() { 1 } else { 0 };
194+
// TODO upgrade DF: See expressions() comment -- we make the having expressions be "invisible" because they're defined on the output schema.
195+
// let num_having = if self.having_expr.is_some() { 1 } else { 0 };
191196
assert_eq!(inputs.len(), 1);
192-
assert_eq!(exprs.len(), num_groups + num_aggs + num_having);
193-
let having_expr = if self.having_expr.is_some() {
194-
exprs.last().map(|p| p.clone())
195-
} else {
196-
None
197-
};
197+
assert_eq!(exprs.len(), num_groups + num_aggs /* + num_having */); // TODO upgrade DF
198+
// let having_expr = if self.having_expr.is_some() {
199+
// exprs.last().map(|p| p.clone())
200+
// } else {
201+
// None
202+
// };
203+
let having_expr = self.having_expr.clone(); // TODO upgrade DF
198204
Ok(Arc::new(ClusterAggregateTopK {
199205
limit: self.limit,
200206
input: Arc::new(inputs[0].clone()),

0 commit comments

Comments
 (0)