[GitHub] [arrow-datafusion] alamb commented on a change in pull request #984: Update DataFusion to arrow 6.0 (WIP)

[GitBox]

alamb commented on a change in pull request #984:
URL: https://github.com/apache/arrow-datafusion/pull/984#discussion_r705572422



##########
File path: datafusion/src/physical_plan/sort_preserving_merge.rs
##########
@@ -1156,10 +1156,24 @@ mod tests {
     #[tokio::test]
     async fn test_async() {
         let schema = test::aggr_test_schema();
-        let sort = vec![PhysicalSortExpr {
-            expr: col("c7", &schema).unwrap(),
-            options: SortOptions::default(),
-        }];
+        let sort = vec![

Review comment:
       FYI @tustvold 

##########
File path: datafusion/tests/sql.rs
##########
@@ -1027,9 +1027,9 @@ async fn csv_query_window_with_partition_by() -> 
Result<()> {
                count(cast(c4 as Int)) over (partition by c3), \
                max(cast(c4 as Int)) over (partition by c3), \
                min(cast(c4 as Int)) over (partition by c3), \
-               first_value(cast(c4 as Int)) over (partition by c3), \
-               last_value(cast(c4 as Int)) over (partition by c3), \
-               nth_value(cast(c4 as Int), 2) over (partition by c3) \
+               first_value(cast(c4 as Int)) over (partition by c3 order by c3, 
c4), \

Review comment:
       This change is also required due to the change to use unstable sorting 
in https://github.com/apache/arrow-rs/pull/552 but the test is non 
deterministic according to the sql spec (the query output depends on 
implementation details). 
   
   Specifically, it is computing `first_value` `last_value` and `nth_value` for 
partitions that have more than one value of  the partition by value `c3` but 
does not specify an order by clause to determine how those values should be 
sorted. :
   
   ![Screen Shot 2021-09-09 at 1 45 06 
PM](https://user-images.githubusercontent.com/490673/132737723-036b38f8-dbdb-4d6c-99a2-78dc42471384.png)
   

##########
File path: datafusion/tests/sql.rs
##########
@@ -1027,9 +1027,9 @@ async fn csv_query_window_with_partition_by() -> 
Result<()> {
                count(cast(c4 as Int)) over (partition by c3), \
                max(cast(c4 as Int)) over (partition by c3), \
                min(cast(c4 as Int)) over (partition by c3), \
-               first_value(cast(c4 as Int)) over (partition by c3), \
-               last_value(cast(c4 as Int)) over (partition by c3), \
-               nth_value(cast(c4 as Int), 2) over (partition by c3) \
+               first_value(cast(c4 as Int)) over (partition by c3 order by c3, 
c4), \

Review comment:
       fyi @Jimexist 




-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org