alamb commented on code in PR #11648:
URL: https://github.com/apache/datafusion/pull/11648#discussion_r1691297827
##########
datafusion/expr/src/window_frame.rs:
##########
@@ -246,59 +246,55 @@ impl WindowFrame {
causal,
}
}
-}
-/// Regularizes ORDER BY clause for window definition for implicit corner
cases.
-pub fn regularize_window_order_by(
- frame: &WindowFrame,
- order_by: &mut Vec<Expr>,
-) -> Result<()> {
- if frame.units == WindowFrameUnits::Range && order_by.len() != 1 {
- // Normally, RANGE frames require an ORDER BY clause with exactly one
- // column. However, an ORDER BY clause may be absent or present but
with
- // more than one column in two edge cases:
- // 1. start bound is UNBOUNDED or CURRENT ROW
- // 2. end bound is CURRENT ROW or UNBOUNDED.
- // In these cases, we regularize the ORDER BY clause if the ORDER BY
clause
- // is absent. If an ORDER BY clause is present but has more than one
column,
- // the ORDER BY clause is unchanged. Note that this follows Postgres
behavior.
- if (frame.start_bound.is_unbounded()
- || frame.start_bound == WindowFrameBound::CurrentRow)
- && (frame.end_bound == WindowFrameBound::CurrentRow
- || frame.end_bound.is_unbounded())
- {
- // If an ORDER BY clause is absent, it is equivalent to a ORDER BY
clause
- // with constant value as sort key.
- // If an ORDER BY clause is present but has more than one column,
it is
- // unchanged.
- if order_by.is_empty() {
- order_by.push(Expr::Sort(Sort::new(
- Box::new(Expr::Literal(ScalarValue::UInt64(Some(1)))),
- true,
- false,
- )));
+ /// Regularizes the ORDER BY clause of the window frame.
+ pub fn regularize_order_bys(&self, order_by: &mut Vec<Expr>) -> Result<()>
{
+ match self.units {
+ // Normally, RANGE frames require an ORDER BY clause with exactly
+ // one column. However, an ORDER BY clause may be absent or have
+ // more than one column when the start/end bounds are UNBOUNDED or
+ // CURRENT ROW.
+ WindowFrameUnits::Range if self.free_range() => {
+ // If an ORDER BY clause is absent, it is equivalent to an
+ // ORDER BY clause with constant value as sort key. If an
+ // ORDER BY clause is present but has more than one column,
+ // it is unchanged. Note that this follows PostgreSQL behavior.
+ if order_by.is_empty() {
+ order_by.push(Expr::Sort(Sort::new(
+ Box::new(Expr::Literal(ScalarValue::UInt64(Some(1)))),
+ true,
+ false,
+ )));
Review Comment:
FWIW you can write this more succinctly with the expr API like this I think
```suggestion
order_by.push(lit(1u64).sort(true, false));
```
--
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]
