xiedeyantu commented on code in PR #21362:
URL: https://github.com/apache/datafusion/pull/21362#discussion_r3058253384
##########
datafusion/optimizer/src/eliminate_duplicated_expr.rs:
##########
@@ -76,12 +76,34 @@ impl OptimizerRule for EliminateDuplicatedExpr {
.map(|wrapper| wrapper.0)
.collect();
+ let sort_expr_names = unique_exprs
+ .iter()
+ .map(|sort_expr| sort_expr.expr.schema_name().to_string())
+ .collect::<Vec<_>>();
+ let required_indices = get_required_sort_exprs_indices(
+ sort.input.schema().as_ref(),
+ &sort_expr_names,
+ );
+
+ let unique_exprs = match required_indices {
+ Some(indices) if indices.len() < unique_exprs.len() =>
indices
+ .into_iter()
+ .map(|idx| unique_exprs[idx].clone())
+ .collect(),
+ _ => unique_exprs,
+ };
+
let transformed = if len != unique_exprs.len() {
Transformed::yes
} else {
Transformed::no
};
+ assert!(
Review Comment:
I change here to `internal_err`, since `order by` clause can not be empty.
Perhaps `internal_err` is a better choice.
##########
datafusion/common/src/functional_dependencies.rs:
##########
@@ -590,6 +590,46 @@ pub fn get_required_group_by_exprs_indices(
.collect()
}
+/// Returns indices for the minimal subset of ORDER BY expressions that are
+/// functionally equivalent to the original set of ORDER BY expressions.
+pub fn get_required_sort_exprs_indices(
+ schema: &DFSchema,
+ sort_expr_names: &[String],
+) -> Option<Vec<usize>> {
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
