mustafasrepo commented on code in PR #9625:
URL: https://github.com/apache/arrow-datafusion/pull/9625#discussion_r1527899224
##########
datafusion/physical-plan/src/windows/mod.rs:
##########
@@ -249,7 +249,6 @@ fn create_built_in_window_expr(
.clone()
.try_into()
.map_err(|e| DataFusionError::Execution(format!("{e:?}")))?;
- let n: u32 = n as u32;
Review Comment:
Currently negative index is not supported from an sql query. This means that
we reject following kind of queries
```
SELECT nth_VALUE(column1, -2) OVER(ORDER BY column2 DESC rows between
unbounded preceding and current row) FROM t;
```
This behaviour is similar to Postgres behaviour. However, internally we
support negative index. Internal support enables us to rewrite some window
expressions by taking their reverse if it helps to remove a `SortEXec` from the
plan.
Consider following query
```
SELECT nth_VALUE(column1, 2) OVER(ORDER BY column2 DESC rows between
unbounded preceding and current row) FROM t;
```
where it is known that table `t` is ordered by `column2 ASC`. We can treat
query above as below internally
```
SELECT nth_VALUE(column1, -2) OVER(ORDER BY column2 ASC rows between
current row and unbounded following) FROM t;
```
which is the same in terms of end result.
In the second version, we ordering requirement for the window function is
already satisfied.
In short, we have internal mechanism, to handle negative index. However, it
is not exposed to outside users since it is not standart. However, I think
there is no harm in having this support. If others agree, we can continue with
the changes in this PR.
--
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]
