Support for INTERVAL inside window frames

[mustafasrepo]

Support for INTERVAL Inside Window Frames

Which issue does this PR close?
Closes #631 by adding INTERVAL support inside window frames. Now we can parse queries such as

SELECT 
COUNT(*) OVER (ORDER BY ts RANGE BETWEEN  INTERVAL '1 DAY' PRECEDING AND INTERVAL '1 DAY' FOLLOWING) 
FROM t;

Rationale for this change

Datafusion now supports window frame queries with PR #3570. However, since window frame bound only emits u64 values, we cannot run queries like the one above in Datafusion. If this PR merges, after some minor changes we would be able to run RANGE queries involving INTERVAL in Datafusion. The roadmap of Datafusion includes a plan to support different types for WindowFrameBound.

[alamb]

It seems like this in general can be any Expression (not just a Number of Interval):

https://www.postgresql.org/docs/current/sql-expressions.html

In the offset PRECEDING and offset FOLLOWING frame options, the offset must be an expression not containing any variables, aggregate functions, or window functions. The meaning of the offset depends on the frame mode:

What do you think about just parsing it as an Expression rather than special casing Number / Interval?

[alamb]

Thank you @mustafasrepo for the contribution

[coveralls]

Pull Request Test Coverage Report for Build 3250493085

• 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
• 846 unchanged lines in 6 files lost coverage.
• Overall coverage increased (+0.2%) to 85.799%

---

Files with Coverage Reduction	New Missed Lines	%
src/ast/value.rs	7	87.32%
src/ast/data_type.rs	14	87.5%
tests/sqlparser_postgres.rs	22	97.74%
tests/sqlparser_common.rs	44	97.05%
src/ast/mod.rs	278	77.4%
src/parser.rs	481	83.33%

Totals
Change from base Build 3181450573:	0.2%
Covered Lines:	10519
Relevant Lines:	12260

---

💛 - Coveralls

[ozankabak]

Hi Andrew! The SQL:2016 reference seems to expect an unsigned literal, that's why we opted for this. But you are right, PostgreSQL seems to allow expressions -- it only requires such expressions to evaluate to appropriate unsigned values. We can certainly follow the latter approach, but doing that will require both sqlparser and datafusion to accommodate that logic.

It seems to me a good approach could be two-tiered: We can implement the unsigned-value-only design first (both here and in datafusion), and then subsequently work towards PSQL compatibility. What do you think?

[alamb]

It seems to me a good approach could be two-tiered: We can implement the unsigned-value-only design first (both here and in datafusion), and then subsequently work towards PSQL compatibility. What do you think?

That is fine with me -- I was thinking that using a general Expr in the parser at first would actually make the process easier overall -- the parser / parsed structs would be simpler and DataFusion would have to special case what types it handled. Then over time datafusion could be extended to handle more and more types, but the parser wouldn't need to be changed again

[mustafasrepo]

Hi Andrew, we have changed the implementation as you suggested. It improves readability and decreases the changes required. Thanks for the suggestion. In this PR https://github.com/synnada-ai/arrow-datafusion/pull/5 you can see the corresponding datafusion change as POC implementation to support non-u64 values inside window frames.

[alamb]

Love it -- thank you @mustafasrepo