-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
sql result discrepency with sqlite, postgres and duckdb #13780
Comments
The core issue is something related to the cast to the > SELECT - NULLIF ( + 15, - 27 + + CAST ( NULL AS DOUBLE ) + + + MIN ( 35 ) * - COUNT ( * ) * - 14 + 64 ) * 87 * + 75 * 34 * + 76 + - - 31 AS col0, - 10 * - + 33 * ( + - NULLIF ( 21, + 70 * + 71 ) ) * - 15;
+--------------+---------------------------------------------------------------------------------------+
| col0 | Int64(-10) * (- Int64(33)) * (- nullif(Int64(21),Int64(70) * Int64(71))) * Int64(-15) |
+--------------+---------------------------------------------------------------------------------------+
| -252908969.0 | 103950 |
+--------------+---------------------------------------------------------------------------------------+
|
I've spent a few hours trying to determine where DataFusion was going wrong with the math and finally came up with a condensed test case. The math is fine ... as far as Rust is concerned. You can see a boiled down test case I wrote up on the rust playground. The core issue is that the result cannot be represented as a f32 which is what the Sqlite maps real to 8 bytes.
Duckdb maps real to 4 bytes. If you force the type to be float4 it returns the same result as DataFusion: D SELECT - 15.0000::float4 * 16860600 + 31 AS col0;
┌──────────────┐
│ col0 │
│ float │
├──────────────┤
│ -252908960.0 │
└──────────────┘ So I think for these queries the resolution here should be that DataFusion should fix the typing of nullif .. and then update the results in the slt test if the type maps to f32. |
Addendum: Since the sqlite tests come from sqlite (duh) where REAL is mapped to 8 bytes (Double/f64) I would like to propose that I update the sqlite .slt files and change: 'AS REAL' -> 'AS DOUBLE' This would better match the actual types being tested and would fix many of the failing results. Along with correcting the nullif type behavior would fix almost all the remaining tests that have result mismatches with sqlite/postgresql. Thoughts? @alamb, @aweltsch, @2010YOUY01, @jayzhan-synnada |
First of all, very nice 🕵️ work !
As I understand, this means using f64 for floating point operations in the test. This sounds like a very good and pragmatic thing to do in my mind The other potential thing to do might simply be to map Thanks agian @Omega359 |
I went with using FLOAT8 vs DOUBLE as double isn't a valid type name in Postgresql and it interfered with the result comparisons. |
Resolved as fixed with change 'as REAL' to 'AS FLOAT8' in sqlite test files |
Describe the bug
External error: query result mismatch:
[SQL] SELECT - NULLIF ( + 15, - 27 + + CAST ( NULL AS REAL ) + + + MIN ( 35 ) * - COUNT ( * ) * - 14 + 64 ) * 87 * + 75 * 34 * + 76 + - - 31 AS col0, - 10 * - + 33 * ( + - NULLIF ( 21, + 70 * + 71 ) ) * - 15
at test_files/sqlite/random/expr/slt_good_11.slt:9790
df result:
postgres, duckdb and sqlite result:
Duckdb and sqlite have col0 as an integer, pg and df have it as a float.
To Reproduce
sql above.
Expected behavior
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: