Update datafusion/tests/sql.rs tests to use assert_batches_eq #1126
Conversation
|
optimizers.rs doesn't seem related? |
@houqp |
| "+--------------------------+", | ||
| "| totimestampmillis(t1.ts) |", | ||
| "+--------------------------+", | ||
| "| 2009-03-01 00:00:00 |", | ||
| "| 2009-03-01 00:01:00 |", | ||
| "| 2009-04-01 00:00:00 |", | ||
| "+--------------------------+", |
There was a problem hiding this comment.
@alamb this test seems to be testing for milliseconds but the results dont have that much precision. should the test be updated to get to the appropriate precision? similar question for the microsecond test beneath this one.
There was a problem hiding this comment.
The idea of adding some data that actually has millisecond precision sounds like a good idea to me. I think it could also be done as another PR (perhaps file at ticket) as the tests are no worse than before
There was a problem hiding this comment.
Thanks @matthewmturner this is looking good / on the right track 👍
Thank you
| "+--------------------------+", | ||
| "| totimestampmillis(t1.ts) |", | ||
| "+--------------------------+", | ||
| "| 2009-03-01 00:00:00 |", | ||
| "| 2009-03-01 00:01:00 |", | ||
| "| 2009-04-01 00:00:00 |", | ||
| "+--------------------------+", |
There was a problem hiding this comment.
The idea of adding some data that actually has millisecond precision sounds like a good idea to me. I think it could also be done as another PR (perhaps file at ticket) as the tests are no worse than before
|
This looks like it is getting close... Would you like some help with this one @matthewmturner ? |
@alamb Maybe im missing something but I still see a bunch of |
|
@matthewmturner I was thinking that we could polish up the work you have so far and get it committed and then work on the remaining tests as a follow on PR(s). Rationale for merging sooner rather than later is to minimize potential conflicts (but also so that we are able to keep chipping away at this backlog on master rather than a branch which might go stale / never get entirely completed due to other priorities). I guess I jealously want to get these changes in so as to take advantage of all your work so far :) We did one chunk in #760 and this would be a nice chunk |
|
@alamb okay makes sense! I will start cleaning it up / getting CI to pass. |
|
❤️ |
| vec!["2020-09-08 12:42:29.190"], | ||
| vec!["2020-09-08 11:42:29.190"], | ||
| "+---------------------------------+", | ||
| "| totimestampmillis(ts_micros.ts) |", |
There was a problem hiding this comment.
this column name is fascinating -- it seem like it should be to_timestamp_millis(..) but I'll let someone else file a ticket if it is important to them
…#1126) * First tests updated * More tests * More tests * More tests * More tests * Cleanup format * Cleanup for CI
Which issue does this PR close?
Related to #743 task 2
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?