Ted-Jiang commented on code in PR #4643:
URL: https://github.com/apache/arrow-datafusion/pull/4643#discussion_r1050424599


##########
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:
   I think here `normalized_order_by_keys`, using the  `order_by_keys` do sort 
partition inner each hashed partition.  Only care about same value  are 
adjacent. So no need call about the sort order ? am i right 🤔



-- 
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]

Reply via email to