mingmwang commented on code in PR #4643:
URL: https://github.com/apache/arrow-datafusion/pull/4643#discussion_r1050520827
##########
datafusion/expr/src/utils.rs:
##########
@@ -202,20 +203,113 @@ pub fn expand_qualified_wildcard(
type WindowSortKey = Vec<Expr>;
/// Generate a sort key for a given window expr's partition_by and order_bu
expr
-pub fn generate_sort_key(partition_by: &[Expr], order_by: &[Expr]) ->
WindowSortKey {
- let mut sort_key = vec![];
+pub fn generate_sort_key(
+ partition_by: &[Expr],
+ order_by: &[Expr],
+) -> Result<WindowSortKey> {
+ let normalized_order_by_keys = order_by
+ .iter()
+ .map(|e| match e {
+ Expr::Sort {
+ expr,
+ asc: _,
+ nulls_first: _,
+ } => Ok(Expr::Sort {
+ expr: expr.clone(),
+ asc: true,
+ nulls_first: false,
Review Comment:
No, here the `normalized_order_by_keys` are for dedup purpose. The
generated sort keys for window expressions are partition_by_keys + sort_keys.
There might be over lap between partition_by_keys and sort_keys. The original
implement leverage the `contains()` method to do the dedup but this is not
enough.
--
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]
