Minor: Port more aggregate tests to sqllogictests #5574
Conversation
github-actions
bot
added
core
| statement ok | ||
| create table t as | ||
| select | ||
| arrow_cast(column1, 'Timestamp(Nanosecond, None)') as nanos, |
There was a problem hiding this comment.
I added arrow_cast in #5166 to help port this kind of test (to control the arrow types explicitly)
| 2021-01-01T05:11:10.432 2021-01-01T05:11:10.432 2021-01-01T05:11:10.432 2021-01-01T05:11:10 Row 3 | ||
|
|
||
|
|
||
| # aggregate_timestamps_sum() -> Result<()> { |
There was a problem hiding this comment.
I guess the part with function signature is excessive here
There was a problem hiding this comment.
Agreed -- removed in 9f59a70
| statement error DataFusion error: Error during planning: The function Sum does not support inputs of type Time64\(Nanosecond\). | ||
| SELECT sum(nanos), sum(micros), sum(millis), sum(secs) FROM t | ||
|
|
||
| # aggregate_times_count() -> Result<()> |
There was a problem hiding this comment.
Same here regarding function signature.
| 21:06:28.247821084 21:06:28.247821 21:06:28.247 21:06:28 | ||
|
|
||
|
|
||
| # aggregate_times_avg |
| statement error Error during planning: The function Sum does not support inputs of type Timestamp\(Nanosecond, None\) | ||
| SELECT sum(nanos), sum(micros), sum(millis), sum(secs) FROM t; | ||
|
|
||
| query IIII |
There was a problem hiding this comment.
Maybe it's wort adding test name for this case
There was a problem hiding this comment.
Good idea -- added in 5f77bb0
|
Thanks @alamb! Looks great overall. I've only left couple of comments regarding typos (I guess) -- they don't affect logs readability, but still. |
|
Benchmark runs are scheduled for baseline = 35a3acf and contender = e6d7106. e6d7106 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #4495
Follow on to #5434
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 aggregate tests to aggregate.slt
Are these changes tested?
Yes all tests
Are there any user-facing changes?
No