comphead commented on PR #7256:
URL: 
https://github.com/apache/datafusion-comet/pull/7256#issuecomment-6453341533

   Thanks @andygrove, addressed in 94aaa194dc.
   
   1. The cache test now also runs `sum(v) OVER (PARTITION BY k ORDER BY v DESC 
ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING)`, which lands on 
`WindowAggExec`. It runs as a separate query from `dense_rank`, since Spark 
would merge the two into one window. The planner test covers `row_number` and 
`percent_rank` too. This turned up a mistake in my description: `WindowAggExec` 
doesn't fail in `partition_by_sort_keys()`. It fails when its stream starts, 
with "All partition by columns should have an ordering". So the test now checks 
the input's order through `get_ordered_partition_by_indices`, which both 
operators use. I've corrected the description.
   2. Done. The partition key is NULL for a twentieth of the rows, and the 
cache and both windows order `v` descending. The rows are checked against 
computed values.
   3. Added a Spark 4.x test over a cache sorted by a `UTF8_LCASE` key. It 
checks that the window stays in Spark with the new fallback reason and returns 
Spark's rows. A Spark `SortExec` directly under a native window doesn't come 
up. Comet converts only leaf operators (cache, range, scans) into a native 
operator's JVM input. With `spark.comet.exec.sort.enabled=false` the window 
stays in Spark, so the order is never sent there.
   4. Updated `debugging.md` to pass `input`.
   5. Applied `run-all-spark-profiles` and `run-spark-4.1-tests`.
   


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

Reply via email to