Set up basic CI, added tests, and implemented the mode function. #2
Conversation
.gitignore
Outdated
| Cargo.lock | ||
| !datafusion-cli/Cargo.lock |
There was a problem hiding this comment.
some of this looks like it may only be relevant for the datafusion project and not this one
| <!-- [](https://github.com/datafusion-contrib/datafusion-functions-json/actions/workflows/ci.yml?query=branch%3Amain) --> | ||
| <!-- [](https://crates.io/crates/datafusion-functions-json) --> | ||
|
|
||
| **Note:** This is not an official Apache Software Foundation release. |
README.md
Outdated
| @@ -0,0 +1,64 @@ | |||
| # datafusion-functions-extra | |||
|
|
|||
| <!-- [](https://github.com/datafusion-contrib/datafusion-functions-json/actions/workflows/ci.yml?query=branch%3Amain) --> | |||
There was a problem hiding this comment.
These still refer to datafusion-functions-json which may be ok
tests/main.rs
Outdated
| async fn test_mode_date64() { | ||
| let sql = "SELECT MODE(date64_col) FROM test_table"; | ||
| let batches = run_query(sql).await.unwrap(); | ||
| assert_eq!( |
There was a problem hiding this comment.
Some potential ideas to make testing easier:
- use assert_batches_eq
- integrate https://crates.io/crates/sqllogictest (we can avoid most of the stuff in the DataFusion runner, and it could be much simpler here I think) to assist in testing
- Use insta.rs (my personal suggestion) to compare expected and actual (it automates the capture and update of expected output). Here is an example
There was a problem hiding this comment.
@alamb thanks for the review!
I’ve refactored the tests using insta, and I must say, it looks great now.
Regarding the sqllogictest, am I correct in understanding that we won’t be reusing the setup from DataFusion core and will instead implement it in place?
There was a problem hiding this comment.
am I correct in understanding that we won’t be reusing the setup from DataFusion core and will instead implement it in place?
that is one proposal -- but I think you should consider if that is worth while or if Insta is ok for this repo.
|
@alamb and @austin362667 thanks for the review! |
| let actual = execution.run_and_format("SELECT MODE(utf8_col) FROM test_table").await; | ||
|
|
||
| insta::assert_yaml_snapshot!(actual, @r###" | ||
| - +---------------------------+ |
Closes: #1