Remove tests from sql_integration that were ported to sqllogictest #4836
Conversation
github-actions
bot
added
core
matthewwillian
changed the title
|
Thanks @matthewwillian and @jackwener -- I plan to review this PR tomorrow as well. |
|
Thanks @jackwener. One of the aggregates tests and all of the arrow_typeof tests are duplicates of tests that already exist. |
There was a problem hiding this comment.
Thank you @matthewwillian -- this looks great to me. I double checked the tests and 👍
Deleting code is a wonderful first contribution 🎉
| @@ -20,24 +20,6 @@ use datafusion::scalar::ScalarValue; | |||
| use datafusion::test_util::scan_empty; | |||
| use datafusion_common::cast::as_float64_array; | |||
|
|
|||
| #[tokio::test] | |||
| async fn csv_query_avg_multi_batch() -> Result<()> { | |||
There was a problem hiding this comment.
Here is the corresponding test in .slt
👍
| @@ -846,26 +846,23 @@ SELECT count(1 + 1) | |||
| ---- | |||
| 1 | |||
|
|
|||
| # FIX: "CSV Writer does not support List(Field { name: \"item\", data_type: Utf8, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: None }) data type") | |||
| @@ -976,6 +973,7 @@ select max(c1) from d_table | |||
| ---- | |||
| 110.009 | |||
|
|
|||
| # FIX: doesn't check datatype | |||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn arrow_typeof_boolean() -> Result<()> { |
There was a problem hiding this comment.
I double checked -- here is the corresponding .slt file:
|
It appears there are some CI checks failing on this PR |
There was a problem hiding this comment.
Thanks @jackwener. One of the aggregates tests and all of the arrow_typeof tests are duplicates of tests that already exist.
Looks great to me, cleanup the useless code is great to me.
I have checked the tests. Thanks @matthewwillian.
|
Benchmark runs are scheduled for baseline = c4f4dff and contender = 42f7dd5. 42f7dd5 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?
Closes #4498
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Unit-tests pass
Are there any user-facing changes?
No