alamb commented on code in PR #4441:
URL: https://github.com/apache/arrow-datafusion/pull/4441#discussion_r1037270435


##########
datafusion/physical-expr/src/window/window_expr.rs:
##########
@@ -99,15 +99,32 @@ pub trait WindowExpr: Send + Sync + Debug {
             .collect()
     }
 
+    /// get order by columns, empty if absent
+    fn order_by_columns(&self, batch: &RecordBatch) -> Result<Vec<SortColumn>> 
{
+        self.order_by()
+            .iter()
+            .map(|e| e.evaluate_to_sort_column(batch))
+            .collect::<Result<Vec<SortColumn>>>()
+    }
+
     /// get sort columns that can be used for peer evaluation, empty if absent
     fn sort_columns(&self, batch: &RecordBatch) -> Result<Vec<SortColumn>> {
         let mut sort_columns = self.partition_columns(batch)?;
-        let order_by_columns = self
-            .order_by()
-            .iter()
-            .map(|e| e.evaluate_to_sort_column(batch))
-            .collect::<Result<Vec<SortColumn>>>()?;
+        let order_by_columns = self.order_by_columns(batch)?;
         sort_columns.extend(order_by_columns);
         Ok(sort_columns)
     }
+
+    /// Get values columns(argument of Window Function)
+    /// and order by columns (columns of the ORDER BY expression)used in 
evaluators
+    fn get_values_orderbys(

Review Comment:
   👍  this is a nice refactor of common functionality



##########
datafusion/physical-expr/src/window/row_number.rs:
##########
@@ -61,10 +60,7 @@ impl BuiltInWindowFunctionExpr for RowNumber {
         &self.name
     }
 
-    fn create_evaluator(
-        &self,
-        _batch: &RecordBatch,
-    ) -> Result<Box<dyn PartitionEvaluator>> {
+    fn create_evaluator(&self) -> Result<Box<dyn PartitionEvaluator>> {

Review Comment:
   👍 



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