minor: port more window tests to sqllogictests#5330
Conversation
alamb
changed the title
alamb
changed the title
| #// } | ||
|
|
||
| # async fn window_frame_ranges_preceding_following_desc() -> Result<()> { | ||
| query error DataFusion error: Internal error: Operator + is not implemented for types Int8(5) and Utf8("1"). This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker |
There was a problem hiding this comment.
This one is strange to me -- it seems like a real bug -- if you agree I will file a ticket for it.
However, the corresponding query ran just fine in .rs rust tests 🤔
There was a problem hiding this comment.
I think it's a bug, I also met it in other code when disable skip_failed
| } else { | ||
| return Err(DataFusionError::Plan(format!( | ||
| "With window frame of type RANGE, the ORDER BY expression must be of length 1, got {}", order_bys))); | ||
| "With window frame of type RANGE, the ORDER BY expression must be of length 1, got {order_bys}"))); |
There was a problem hiding this comment.
clippy was complaining about this for me locally
|
cc @mustafasrepo and @ozankabak |
github-actions
bot
added
core
jackwener
left a comment
jackwener
left a comment
There was a problem hiding this comment.
LGTM.
I prepare to merge this PR in one day in case of new conflict.
|
Thanks everyone! |
|
Benchmark runs are scheduled for baseline = a79f9c8 and contender = 3882eea. 3882eea is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
* minor: port more window tests * Update comments and add link to ticket * escape
4 participants
Which issue does this PR close?
Part of #4495
Follow on to #5199
Rationale for this change
I am trying to keep the testability of DataFusion reasonable. Sqllogictests are easier to add / update so we are trying to move stuff over there.
What changes are included in this PR?
Port more window tests to window.slt
Are these changes tested?
Yes all tests
Are there any user-facing changes?
No