pola-rs / polars

Dataframes powered by a multithreaded, vectorized query engine, written in Rust
https://docs.pola.rs
Other
30.61k stars 1.99k forks source link

`fill_null` + `type_coercion=False` results in panic! #19988

Closed CoderJoshDK closed 1 day ago

CoderJoshDK commented 4 days ago

Checks

Reproducible example

pl.LazyFrame({"a": [1,2, None, 3]}).select(pl.col("a").fill_null(0)).collect(type_coercion=False)

Log output

thread '<unnamed>' panicked at crates/polars-core/src/series/mod.rs:1011:9:
implementation error, cannot get ref Int64 from Int32
stack backtrace:
   0:        0x10bd87974 - <std::sys::backtrace::BacktraceLock::print::DisplayBacktrace as core::fmt::Display>::fmt::h47fda2adb1d95ca6
   1:        0x1085ef968 - core::fmt::write::hf02c740536f0830c
   2:        0x10bd588e4 - std::io::Write::write_fmt::h18cd01bac5cbac08
   3:        0x10bd88964 - std::sys::backtrace::BacktraceLock::print::hfdb3f5f734a161c5
   4:        0x10bd887cc - std::panicking::default_hook::{{closure}}::hcfb606595fc6038e
   5:        0x10bd8906c - std::panicking::rust_panic_with_hook::h5feeb8802cd3d0bd
   6:        0x10bd88a2c - std::panicking::begin_panic_handler::{{closure}}::he394d57f81ff5447
   7:        0x10bd8899c - std::sys::backtrace::__rust_end_short_backtrace::h0362d52013de77c2
   8:        0x10bd88990 - _rust_begin_unwind
   9:        0x10becd8d8 - core::panicking::panic_fmt::h0da7cadfe6cfa60b
  10:        0x108b28a1c - polars_core::series::<impl core::convert::AsRef<polars_core::chunked_array::ChunkedArray<T>> for dyn polars_core::series::series_trait::SeriesTrait>::as_ref::hc948d3bfa9b97b8f
  11:        0x108eab8e4 - <polars_core::series::implementations::SeriesWrap<polars_core::chunked_array::ChunkedArray<polars_core::datatypes::Int64Type>> as polars_core::series::series_trait::private::PrivateSeries>::zip_with_same_type::hea76a2ade4675db0
  12:        0x1093781dc - polars_core::frame::column::Column::zip_with_same_type::h52f222168e63d4e0
  13:        0x10a825ab4 - polars_plan::dsl::function_expr::fill_null::fill_null::default::hc1df105d2240898c
  14:        0x10a8257b0 - <F as polars_plan::dsl::expr_dyn_fn::ColumnsUdf>::call_udf::h0b1472032b127757
  15:        0x1094ac52c - polars_expr::expressions::apply::ApplyExpr::eval_and_flatten::hd19b26b5077e2cf4
  16:        0x1094abce8 - <polars_expr::expressions::apply::ApplyExpr as polars_expr::expressions::PhysicalExpr>::evaluate::h72a5f29bb27e6ecd
  17:        0x1098ecce4 - polars_mem_engine::executors::projection_utils::run_exprs_seq::h96c3a9aac2d2f130
  18:        0x1098ec95c - polars_mem_engine::executors::projection::ProjectionExec::execute_impl::haf4be37e98679b52
  19:        0x1098ec10c - <polars_mem_engine::executors::projection::ProjectionExec as polars_mem_engine::executors::executor::Executor>::execute::h622cc7d16607ca41
  20:        0x10981d0a0 - polars_lazy::frame::LazyFrame::collect::hbb56674e5cb6348d
  21:        0x10b0045a8 - polars_python::lazyframe::general::<impl polars_python::lazyframe::PyLazyFrame>::__pymethod_collect__::hb40c88f7b9866679
  22:        0x10a9d0368 - pyo3::impl_::trampoline::trampoline::h1fd928181dfab5bf
  23:        0x10afebed8 - polars_python::lazyframe::general::_::__INVENTORY::trampoline::h12c221bd7ffc73eb
  24:        0x1029fb7a0 - _method_vectorcall_VARARGS_KEYWORDS
  25:        0x102ae4dcc - __PyEval_EvalFrameDefault
  26:        0x102ada14c - _PyEval_EvalCode
  27:        0x102b3bbe4 - _run_eval_code_obj
  28:        0x102b39c7c - _run_mod
  29:        0x102b38a34 - _PyRun_InteractiveOneObjectEx
  30:        0x102b38320 - __PyRun_InteractiveLoopObject
  31:        0x102b38220 - __PyRun_AnyFileObject
  32:        0x102b38788 - _PyRun_AnyFileExFlags
  33:        0x102b5d6b4 - _pymain_run_stdin
  34:        0x102b5cbcc - _Py_RunMain
  35:        0x102b5cff8 - _pymain_main
  36:        0x102b5d098 - _Py_BytesMain
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/josh/work/dev/fills/.venv/lib/python3.12/site-packages/polars/lazyframe/frame.py", line 2029, in collect
    return wrap_df(ldf.collect(callback))
                   ^^^^^^^^^^^^^^^^^^^^^
pyo3_runtime.PanicException: implementation error, cannot get ref Int64 from Int32

Issue description

When disabling type_coercion in a lazy frame and also doing a fill_null, the system panics. This isn't just for ints; the same happens for decimals. The obvious answer might seem to be "well just don't disable type_coercion" ... but that doesn't work for me right now since there is another bug the type_coercion is causing and I have to disable it to get around that.

Casting observations

If I use a normal collect, the output is: ```py shape: (4, 1) ┌─────┐ │ a │ │ --- │ │ i64 │ ╞═════╡ │ 1 │ │ 2 │ │ 0 │ │ 3 │ └─────┘ ``` However, if I try to cast `a` before filling it: ```py >>> pl.LazyFrame({"a": [1,2, None, 3]}).select(pl.col("a").cast(pl.Int64).fill_null(0)).collect(type_coercion=False) pyo3_runtime.PanicException: implementation error, cannot get ref Int64 from Int32 ``` And finally, if I cast it to - specifically - Int32, it doesn't panic: ```py >>> pl.LazyFrame({"a": [1,2, None, 3]}).select(pl.col("a").cast(pl.Int32).fill_null(0)).collect(type_coercion=False) shape: (4, 1) ┌─────┐ │ a │ │ --- │ │ i32 │ ╞═════╡ │ 1 │ │ 2 │ │ 0 │ │ 3 │ └─────┘ ``` But this seems ... wrong. So it got me thinking; "maybe it is the `0` in the `fill_null` that is not aligned." And that led me to doing: ```py >>> pl.LazyFrame({"a": [1,2, None, 3]}).select(pl.col("a").cast(pl.Decimal).fill_null(0)).collect(type_coercion=False) pyo3_runtime.PanicException: implementation error, cannot get ref Decimal(None, Some(0)) from Int32 >>> pl.LazyFrame({"a": [1,2, None, 3]}).select(pl.col("a").cast(pl.Decimal).fill_null(pl.lit(0).cast(pl.Decimal))).collect(type_coercion=False) shape: (4, 1) ┌───────────────┐ │ a │ │ --- │ │ decimal[38,0] │ ╞═══════════════╡ │ 1 │ │ 2 │ │ 0 │ │ 3 │ └───────────────┘ ``` And yep, this looks to be roughly what is going on. Regardless, we shouldn't be panic'ing.

Expected behavior

The null values should be filled. It should not panic no matter what. And if a proper solution is impossible, this should be documented or mentioned somewhere.

Installed versions

``` --------Version info--------- Polars: 1.14.0 Index type: UInt32 Platform: macOS-15.1-arm64-arm-64bit Python: 3.12.7 (main, Oct 1 2024, 02:05:46) [Clang 15.0.0 (clang-1500.3.9.4)] LTS CPU: False ----Optional dependencies---- adbc_driver_manager 1.3.0 altair 5.5.0 boto3 cloudpickle 3.1.0 connectorx 0.4.0 deltalake 0.22.0 fastexcel 0.12.0 fsspec 2024.10.0 gevent 24.11.1 google.auth great_tables 0.14.0 matplotlib 3.9.2 nest_asyncio 1.6.0 numpy 2.1.3 openpyxl 3.1.5 pandas 2.2.3 pyarrow 18.0.0 pydantic 2.10.1 pyiceberg 0.8.0 sqlalchemy 2.0.36 torch xlsx2csv 0.8.4 xlsxwriter 3.2.0 ```
ritchie46 commented 4 days ago

Yes, type-coercion is required. Maybe we should make it private.

CoderJoshDK commented 3 days ago

I have seen it mentioned somewhere else that you are thinking about this. However, I have ran into a bug with the location of the decimal point in my columns, that is only fixed when I disable type-coercion. I have yet to create such a simplistic example like the one here. But please see #19871 for the bug I am talking about. Perhaps a footnote of some type could be added to that optimization, in the docs¿ Either way, even if this issue with the fill nulls is a "not going to fix," the other one is more of a real problem.

ritchie46 commented 3 days ago

Then we must fix the bug. Type coercion is needed.

CoderJoshDK commented 3 days ago

The bug I talked about above, doesn't actually seem related to type_coercion. Disabling it coincidently fixed the bug rather than being the cause. See #20013 for more details on mentioned bug.

@ritchie46 feel free to close this issue if you are not going to make any changes based on the panic. Thank you for your time