rishvin commented on issue #1820: URL: https://github.com/apache/datafusion-comet/issues/1820#issuecomment-3002947426
> > [@andygrove](https://github.com/andygrove) can I backport [SHA2-fix](https://github.com/apache/datafusion/pull/16350) to branch-48 of datafusion ? I tried updating with datafusion-main branch until my commit, however, looks like there are many changes in between and they are causing different test failures in Comet. > > Looks like the test-cases are failing in Comet after updating the Datafusion dependency with main-branch due to this [commit](https://github.com/apache/datafusion/commit/0c3037404929fc3a3c4fbf6b9b7325d422ce10bd). This commit was revert in the branch-48 [here](https://github.com/apache/datafusion/commit/c76c1f076cca6f1922de8ba86b98c05b6a27e7ac), however it wasn't in the main branch. And the failures that I encountered were also related to Window functions tests, > > For eg, I ran this: `./mvnw test -Dtest=None -Dsuites="org.apache.comet.exec.CometExecSuite Windows support"` and it failed with subraction underflow, > > ``` > Windows support *** FAILED *** (7 seconds, 210 milliseconds) > org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 7.0 failed 1 times, most recent failure: Lost task 0.0 in stage 7.0 (TID 17) (pop-os executor driver): org.apache.comet.CometNativeException: attempt to subtract with overflow > at comet::errors::init::{{closure}}(/home/rishabjoshi/Projects/datafusion-comet/native/core/src/errors.rs:151) > at <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/alloc/src/boxed.rs:2007) > at std::panicking::rust_panic_with_hook(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/panicking.rs:836) > at std::panicking::begin_panic_handler::{{closure}}(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/panicking.rs:694) > at std::sys::backtrace::__rust_end_short_backtrace(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/sys/backtrace.rs:168) > at rust_begin_unwind(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/panicking.rs:692) > at core::panicking::panic_fmt(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/core/src/panicking.rs:75) > at core::panicking::panic_const::panic_const_sub_overflow(/rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/core/src/panicking.rs:178) > at datafusion_expr::window_state::WindowAggState::update(/home/rishabjoshi/Projects/datafusion/datafusion/expr/src/window_state.rs:95) > at datafusion_physical_expr::window::window_expr::AggregateWindowExpr::aggregate_evaluate_stateful(/home/rishabjoshi/Projects/datafusion/datafusion/physical-expr/src/window/window_expr.rs:260) > at <datafusion_physical_expr::window::aggregate::PlainAggregateWindowExpr as datafusion_physical_expr::window::window_expr::WindowExpr>::evaluate_stateful(/home/rishabjoshi/Projects/datafusion/datafusion/physical-expr/src/window/aggregate.rs:148) > at datafusion_physical_plan::windows::bounded_window_agg_exec::BoundedWindowAggStream::compute_aggregates(/home/rishabjoshi/Projects/datafusion/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs:983) > at datafusion_physical_plan::windows::bounded_window_agg_exec::BoundedWindowAggStream::poll_next_inner(/home/rishabjoshi/Projects/datafusion/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs:1033) > at <datafusion_physical_plan::windows::bounded_window_agg_exec::BoundedWindowAggStream as futures_core::stream::Stream>::poll_next(/home/rishabjoshi/Projects/datafusion/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs:949) > ``` > > After applying the revert I see the test case passing in main also. > > [@andygrove](https://github.com/andygrove) would you prefer if I apply this revert in the main also? Looks like applying the revert in main is no longer clean because there were many conflicting patches in between now. However, if you prefer, rather than revert I might disable that pieces of code within some property/feature-flag that will be remain turned off. I can do this piece of work as part of this ticket itself. Ok, this issue is fixed here: https://github.com/apache/datafusion/pull/16430 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org
