Skip to content

Commit 11e1410

Browse files
committed
WIP: Enforce topk output schema column names properly
1 parent a865555 commit 11e1410

File tree

2 files changed

+9
-9
lines changed

2 files changed

+9
-9
lines changed

rust/cubestore/cubestore/src/queryplanner/planning.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2045,7 +2045,6 @@ pub mod tests {
20452045
);
20462046
let plan = choose_index(plan, &indices).await.unwrap().0;
20472047

2048-
// TODO upgrade DF: Isn't this projection wrong?
20492048
assert_eq!(
20502049
pretty_printers::pp_plan(&plan),
20512050
"Projection, [s.orders.order_customer:order_customer, sum(s.orders.order_amount)]\

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,20 +134,21 @@ fn materialize_topk_under_limit_sort(fetch: usize, sort_expr: &Vec<SortExpr>, so
134134
snapshots: cs.snapshots.clone(),
135135
}),
136136
});
137+
// TODO upgrade DF: Projection should always be Some, now. We are also in some
138+
// cases (such as when there was no projection node) creating a spurious
139+
// projection.
137140
if let Some(p) = projection {
138-
let in_schema = topk.schema();
139141
let out_schema = p.schema;
140142
let mut expr = Vec::with_capacity(p.input_columns.len());
141143
for out_i in 0..p.input_columns.len() {
142-
let in_field = in_schema.field(p.input_columns[out_i]);
143-
let out_name: &str = out_schema.field(out_i).name();
144+
let (out_tr, out_field) = out_schema.qualified_field(out_i);
144145

145-
//let mut e = Expr::Column(f.qualified_column());
146146
let mut e =
147147
p.post_projection[p.input_columns[out_i]].clone();
148-
if out_name != in_field.name() {
149-
// TODO upgrade DF: Check if we might need relation -- there is a commented line f.qualified_column() above, too.
150-
e = Expr::Alias(Alias { expr: Box::new(e), relation: None, name: out_name.to_owned() });
148+
let (e_tr, e_name) = e.qualified_name();
149+
150+
if out_tr != e_tr.as_ref() || out_field.name() != &e_name {
151+
e = Expr::Alias(Alias { expr: Box::new(e), relation: out_tr.cloned(), name: out_field.name().clone() });
151152
}
152153
expr.push(e);
153154
}
@@ -239,7 +240,7 @@ fn extract_aggregate_fun(e: &Expr) -> Option<(TopKAggregateFunction, &Vec<Expr>)
239240

240241
#[derive(Debug)]
241242
struct ColumnProjection<'a> {
242-
// The (sole) column index within `input.schema()` that the post_projection expr uses.
243+
// The (sole) column indexes within `input.schema()` that the post_projection expr uses.
243244
input_columns: Vec<usize>,
244245
input: &'a Arc<LogicalPlan>,
245246
// Output schema (after applying `having_expr` and then `post_projection` and then aliases). In

0 commit comments

Comments
 (0)