-
Notifications
You must be signed in to change notification settings - Fork 1.5k
multiply overflow in stats.rs #13775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Hi @Omega359 , I'm a newbie of datafusion and would like to try to make some contribution. 😊 Could this issue be assigned to me? Best Regards, |
Hi Edward - Thanks for taking a look at this! I do not have the ability to do that however if you add a comment here with just the word 'take' a github action will assign it to you |
Got it! Thanks very much! 😊 Best Regards, |
take |
Hi, Sorry for delay on this issue. I will try to work on it now. 😊 Best Regards, |
Hi @Omega359 , Sorry to delay this issue and bother you again. 😊 I find that the sqllogictest_add_sqlite branch seems to have been merged in #13936. Does this issue still exist, or my reproducing steps are incorrect? Best Regards, |
I think the bug is still there but I haven't seen it in a while tbh so
whatever changes were made to the sqllogictests seem to no longer trigger
it.
…On Thu, Feb 13, 2025 at 9:40 AM Edward Xu ***@***.***> wrote:
Hi @Omega359 <https://github.com/Omega359> ,
Sorry to delay this issue and bother you again. 😊
I find that the sqllogictest_add_sqlite branch seems to have been merged
in #13936 <#13936>.
So, I ran cargo test --features postgres --test sqllogictests --
--complete --postgres-runner --include-sqlite in current master branch
and find that it didn't print any error message.
Does this issue still exist, or my reproducing steps are incorrect?
Best Regards,
Edward
—
Reply to this email directly, view it on GitHub
<#13775 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABXHUUM36QPRVCRI4IB5HL2PSVFXAVCNFSM6AAAAABTS4JXMOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJWHAYDGNBVGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: LindaSummer]*LindaSummer* left a comment (apache/datafusion#13775)
<#13775 (comment)>
Hi @Omega359 <https://github.com/Omega359> ,
Sorry to delay this issue and bother you again. 😊
I find that the sqllogictest_add_sqlite branch seems to have been merged
in #13936 <#13936>.
So, I ran cargo test --features postgres --test sqllogictests --
--complete --postgres-runner --include-sqlite in current master branch
and find that it didn't print any error message.
Does this issue still exist, or my reproducing steps are incorrect?
Best Regards,
Edward
—
Reply to this email directly, view it on GitHub
<#13775 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABXHUUM36QPRVCRI4IB5HL2PSVFXAVCNFSM6AAAAABTS4JXMOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJWHAYDGNBVGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Bruce Ritchie
Principal Software Engineer
Veeva Systems
***@***.*** <https://profiles.google.com/?hl=en&tab=mX>
*Customer Master Simplified. Data, Software, and Services All-in-one with
Veeva Network
<http://www.veeva.com/customer-master/?utm_source=google&utm_medium=email%20signature&utm_campaign=Network%20Email%20Signature%20Link>*
|
Hi @Omega359 , Sorry to interrupt you. 😊 I used the commit in https://github.com/Omega359/arrow-datafusion/tree/sqllogictest_with_sqlite in Nov 2024. Would you mind give me a commit that can trigger this problem? Thanks very much. Best Regards, |
I don't think there was a commit where I saw it - it happened during my local dev of the sqlite testing. |
Got it! Thanks very much for your warm help! I will try to solve it. 😄 |
I'm reproducing this multiplication overflow panic as well. Details
Trying to execute Join Order Benchmark query 16B If it matters, I've loaded the relevant tables from parquet files converted from the Join Order Benchmark's original data dump. Code let state = ctx.state();
let logical_plan = state.create_logical_plan(JOB_16B).await?;
let planner = DefaultPhysicalPlanner::default();
let physical_plan = planner.create_physical_plan(&logical_plan, &state).await?; Notably, this panic does not reproduce when simply calling InvestigationLooks like it comes from calculating stats for one side of a join. This query joins many large-ish tables. Since this plan isn't optimized, it's a bunch of cross joins of big tables. Backtrace
|
Hi @Speculative , Thanks very much for your investigation! ❤ I'm sorry that in last month due to my personal circumstances, I didn't have enough time to follow up on this issue. I will try to follow your reproducing steps for this problem. I will try to update in this thread or reach out to you when I have any new progress or problem on reproducing if you don't mind. Best Regards, |
In my case, after comparing my implementation to what's happening in let state = ctx.state();
let logical_plan = state.create_logical_plan(JOB_16B).await?;
let planner = DefaultPhysicalPlanner::default();
let physical_plan = planner.create_physical_plan(&logical_plan, &state).await?; But this does not: let state = ctx.state();
let logical_plan = state.create_logical_plan(JOB_16B).await?;
let opt_logical_plan = state.optimize(&logical_plan);
let planner = DefaultPhysicalPlanner::default();
let physical_plan = planner.create_physical_plan(&logical_plan, &state).await?; Which is also equivalent to: let state = ctx.state();
let logical_plan = state.create_logical_plan(JOB_16B).await?;
let physical_plan = state.create_physical_plan(&logical_plan).await?; |
Hi @Speculative , Thanks so much for your investigation and example code! ❤ With the reproduction case, it will be very helpful for root cause analysis. I will try my best to solve this issue. Best Regards, |
Describe the bug
Seeing this when testing the new sqlite tests in sqllogictest against main. It occurs with the select4.slt test file only. backtrace below:
To Reproduce
I can reproduce repeatable in my branch @ https://github.com/Omega359/arrow-datafusion/tree/feature/sqllogictest_add_sqlite when running the sqlite complete
It does seem to be any particular sql in that file that is causing the issue but rather the complete number of them.Correction - it's always failing on a particular sql however running the equivalent sql in datafusion-cli does not cause the issue. That is possibly because of a difference in how things are run - in sqllogictests the DF queries are run via:
Expected behavior
No overflow :)
Additional context
No response
The text was updated successfully, but these errors were encountered: