xinlifoobar commented on code in PR #11812:
URL: https://github.com/apache/datafusion/pull/11812#discussion_r1709404834
##########
datafusion/core/tests/dataframe/describe.rs:
##########
@@ -102,8 +102,8 @@ async fn describe_null() -> Result<()> {
"| null_count | 0 | 1 |",
"| mean | null | null |",
"| std | null | null |",
- "| min | null | null |",
- "| max | null | null |",
+ "| min | a | null |",
Review Comment:
The result of 'a' seems correct compared to the test case just above it.
https://github.com/apache/datafusion/blob/2521043ddcb3895a2010b8e328f3fa10f77fc094/datafusion/core/tests/dataframe/describe.rs#L54C1-L83C1
There are at least 2 issues here,
1. In the previous, when null is not supported in the min/max accumulator,
the min, max field in the record batches are [Err] instead of [{'a', Err}] and
that's where the 'null' 'null' comes from. Below is the log when I debugged on
the master branch.
``` bash
...
batchs: Ok([RecordBatch { schema: Schema { fields: [Field { name: "a",
data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata:
{} }, Field { name: "b", data_type: Int64, nullable: true, dict_id: 0,
dict_is_ordered: false, metadata: {} }], metadata: {} }, columns:
[PrimitiveArray<Int64>
[
0,
], PrimitiveArray<Int64>
[
1,
]], row_count: 1 }])
batchs: Err(Internal("Min/Max accumulator not implemented for type
Null\n\nbacktrace: 0:
datafusion_common::error::DataFusionError::get_back_trace\n at
/home/xinli/source/repos/datafusion/datafusion/common/src/error.rs:392:30\n
1: datafusion_functions_aggregate::min_max::min_batch\n at
/home/xinli/source/repos/datafusion/datafusion/functions-aggregate/src/min_max.rs:422:24\n
2: <datafusion_functions_aggregate::min_max::MinAccumulator as
datafusion_expr::accumulator::Accumulator>::update_batch\n at
/home/xinli/source/repos/datafusion/datafusion/functions-aggregate/src/min_max.rs:1064:22\n
3:
datafusion_physical_plan::aggregates::no_grouping::aggregate_batch::{{closure}}\n
at
/home/xinli/source/repos/datafusion/datafusion/physical-plan/src/aggregates/no_grouping.rs:236:55\n
4: core::iter::traits::iterator::Iterator::try_for_each::call::{{closure}}\n
at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/
src/iter/traits/iterator.rs:2470:26\n 5:
core::iter::traits::iterator::Iterator::try_fold\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/iter/traits/iterator.rs:2411:21\n
6: core::iter::traits::iterator::Iterator::try_for_each\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/iter/traits/iterator.rs:2473:9\n
7: datafusion_physical_plan::aggregates::no_grouping::aggregate_batch\n
at
/home/xinli/source/repos/datafusion/datafusion/physical-plan/src/aggregates/no_grouping.rs:211:5\n
8:
datafusion_physical_plan::aggregates::no_grouping::AggregateStream::new::{{closure}}::{{closure}}\n
at
/home/xinli/source/repos/datafusion/datafusion/physical-plan/src/aggregates/no_grouping.rs:116:38\n
9: <futures_util::stream::unfold::Unfold<T,F,Fut> as
futures_core::stream::Stream>::poll_next\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream
/unfold.rs:107:33\n 10: <futures_util::stream::stream::fuse::Fuse<S> as
futures_core::stream::Stream>::poll_next\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/stream/fuse.rs:53:27\n
11: <core::pin::Pin<P> as futures_core::stream::Stream>::poll_next\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.30/src/stream.rs:120:9\n
12: futures_util::stream::stream::StreamExt::poll_next_unpin\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/stream/mod.rs:1638:9\n
13: <datafusion_physical_plan::aggregates::no_grouping::AggregateStream as
futures_core::stream::Stream>::poll_next\n at
/home/xinli/source/repos/datafusion/datafusion/physical-plan/src/aggregates/no_grouping.rs:181:9\n
14: <core::pin::Pin<P> as futures_core::stream::Stream>::poll_next\n
at /home/xinli/.cargo/registry/src/index.c
rates.io-6f17d22bba15001f/futures-core-0.3.30/src/stream.rs:120:9\n 15: <S as
futures_core::stream::TryStream>::try_poll_next\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-core-0.3.30/src/stream.rs:196:9\n
16: <futures_util::stream::try_stream::try_collect::TryCollect<St,C> as
core::future::future::Future>::poll\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/futures-util-0.3.30/src/stream/try_stream/try_collect.rs:46:26\n
17: datafusion_physical_plan::common::collect::{{closure}}\n at
/home/xinli/source/repos/datafusion/datafusion/physical-plan/src/common.rs:45:36\n
18: datafusion_physical_plan::execution_plan::collect::{{closure}}\n
at
/home/xinli/source/repos/datafusion/datafusion/physical-plan/src/execution_plan.rs:680:36\n
19: datafusion::dataframe::DataFrame::collect::{{closure}}\n at
./src/dataframe/mod.rs:994:33\n 20: datafusion::dataframe::DataFrame::descr
ibe::{{closure}}\n at ./src/dataframe/mod.rs:710:59\n 21:
core_integration::dataframe::describe::describe_null::{{closure}}\n
at ./tests/dataframe/describe.rs:93:10\n 22: <core::pin::Pin<P> as
core::future::future::Future>::poll\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/future/future.rs:123:9\n
23: <core::pin::Pin<P> as core::future::future::Future>::poll\n
at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/future/future.rs:123:9\n
24:
tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:659:57\n
25: tokio::runtime::coop::with_budget\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/coop.rs:107:5\n
26: tokio::runtime::coop::budget\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/coop.rs:73:5\n
27:
tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}::{{closure}}\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:659:25\n
28: tokio::runtime::scheduler::current_thread::Context::enter\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:404:19\n
29:
tokio::runtime::scheduler::current_thread::CoreGuard::block_on::{{closure}}\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:658:36\n
30:
tokio::runtime::scheduler::current_thread::CoreGuard::enter::{{closure}}\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thre
ad/mod.rs:737:68\n 31: tokio::runtime::context::scoped::Scoped<T>::set\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/context/scoped.rs:40:9\n
32: tokio::runtime::context::set_scheduler::{{closure}}\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/context.rs:180:26\n
33: std::thread::local::LocalKey<T>::try_with\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/thread/local.rs:283:12\n
34: std::thread::local::LocalKey<T>::with\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/thread/local.rs:260:9\n
35: tokio::runtime::context::set_scheduler\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/context.rs:180:9\n
36: tokio::runtime::scheduler::current_thread::CoreGuard::enter\n
at /home/xinli/.cargo/registry/src/index.crates.io-6f17d22b
ba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:737:27\n
37: tokio::runtime::scheduler::current_thread::CoreGuard::block_on\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:646:19\n
38:
tokio::runtime::scheduler::current_thread::CurrentThread::block_on::{{closure}}\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:175:28\n
39: tokio::runtime::context::runtime::enter_runtime\n at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/context/runtime.rs:65:16\n
40: tokio::runtime::scheduler::current_thread::CurrentThread::block_on\n
at
/home/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/scheduler/current_thread/mod.rs:167:9\n
41: tokio::runtime::runtime::Runtime::block_on\n at /h
ome/xinli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/runtime/runtime.rs:347:47\n
42: core_integration::dataframe::describe::describe_null\n at
./tests/dataframe/describe.rs:111:5\n 43:
core_integration::dataframe::describe::describe_null::{{closure}}\n
at ./tests/dataframe/describe.rs:85:29\n 44:
core::ops::function::FnOnce::call_once\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs:250:5\n
45: core::ops::function::FnOnce::call_once\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs:250:5\n
46: test::__rust_begin_short_backtrace\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/test/src/lib.rs:625:18\n
47: test::run_test_in_process::{{closure}}\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/test/src/lib.rs:648:60\n
48: <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::func
tion::FnOnce<()>>::call_once\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panic/unwind_safe.rs:272:9\n
49: std::panicking::try::do_call\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:559:40\n
50: std::panicking::try\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:523:19\n
51: std::panic::catch_unwind\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panic.rs:149:14\n
52: test::run_test_in_process\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/test/src/lib.rs:648:27\n
53: test::run_test::{{closure}}\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/test/src/lib.rs:569:43\n
54: test::run_test::{{closure}}\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/test/src/lib.rs:599:41\n
55: std::sys_common::backtrace::__rust_begin_short_backtrace\n
at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/sys_common/backtrace.rs:155:18\n
56: std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}\n
at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/thread/mod.rs:542:17\n
57: <core::panic::unwind_safe::AssertUnwindSafe<F> as
core::ops::function::FnOnce<()>>::call_once\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panic/unwind_safe.rs:272:9\n
58: std::panicking::try::do_call\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:559:40\n
59: std::panicking::try\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:523:19\n
60: std::panic::catch_unwind\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panic.rs:149:14\n
61: std::thread::Builder::spawn_unchecked_::{{closure}}\n at
/rustc/051478957371ee0084a7c0913941d2a8c4
757bb9/library/std/src/thread/mod.rs:541:30\n 62:
core::ops::function::FnOnce::call_once{{vtable.shim}}\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs:250:5\n
63: <alloc::boxed::Box<F,A> as
core::ops::function::FnOnce<Args>>::call_once\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/alloc/src/boxed.rs:2063:9\n
64: <alloc::boxed::Box<F,A> as
core::ops::function::FnOnce<Args>>::call_once\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/alloc/src/boxed.rs:2063:9\n
65: std::sys::pal::unix::thread::Thread::new::thread_start\n at
/rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/sys/pal/unix/thread.rs:108:17\n
66: <unknown>\n 67: <unknown>\n"))
```
2. The comment is mislead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
