findepi commented on code in PR #17684:
URL: https://github.com/apache/datafusion/pull/17684#discussion_r2376658224
##########
datafusion/physical-plan/src/windows/mod.rs:
##########
@@ -634,11 +681,45 @@ pub fn get_window_mode(
Ok(None)
}
-fn sort_options_resolving_constant(expr: Arc<dyn PhysicalExpr>) ->
Vec<PhysicalSortExpr> {
- vec![
- PhysicalSortExpr::new(Arc::clone(&expr), SortOptions::new(false,
false)),
- PhysicalSortExpr::new(expr, SortOptions::new(true, true)),
- ]
+/// Generates sort option variations for a given expression.
+///
+/// This function is used to handle constant columns in window operations.
Since constant
+/// columns can be considered as having any ordering, we generate multiple
sort options
+/// to explore different ordering possibilities.
+///
+/// # Parameters
+/// - `expr`: The physical expression to generate sort options for
+/// - `all_options`: If true, generates all 4 possible sort options (ASC/DESC
× NULLS FIRST/LAST).
+/// If false, generates only 2 options that preserve set monotonicity.
+///
+/// # When to use `all_options = true`:
+/// Use for PARTITION BY columns where we want to explore all possible
orderings to find
+/// one that matches the existing data ordering.
+///
+/// # When to use `all_options = false`:
+/// Use for aggregate/window function arguments where set monotonicity needs
to be preserved.
+/// Only generates ASC NULLS LAST and DESC NULLS FIRST because:
+/// - Set monotonicity is broken if data has increasing order but nulls come
first
+/// - Set monotonicity is broken if data has decreasing order but nulls come
last
Review Comment:
Sure it's not.
But "all options" is even less obvious. It well hints what gets returned
when all_options=true, but reader has no clue what gets returned when
all_options=false. The all_options=false variant is meaningless on its own.
--
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]
