Repository navigation
Compare timestamps with a <= b + INTERVAL instead of (a - b) <= INTERVAL - #8
mbutrovich wants to merge 2 commits into
Conversation
|
The description of this PR is sort of hard to parse I think, so I would like to try a summary: There is a real DataFusion bug comparing certain types of intervals that is exposed in the benchmark. The bug is tracked by this ticket (and we will fix) This PR proposes a workaround to the DataFusion runner using a syntax that computes the correct quantity |
Thanks, I rewrote the description. This PR is not a DataFusion workaround. It proposes a change to the canonical text of nine queries based on rule 3, and I found the sites while debugging DataFusion errors. Those errors come from a real DataFusion bug (apache/datafusion#26671). The change brings the queries into line with DIALECTS.md and also works around that failure. |
Here is the actual query. q114 uses dbolympics/workload/queries/q114.sql Line 10 in 1e22405 Here is an example of the other syntax (in q086) of dbolympics/workload/queries/q086.sql Line 25 in 1e22405 |
|
@mbutrovich, thanks for finding this. We will look into it! |
|
Timestamps have been a big source of pain in cleaning up the queries and this rewrite seems like a great fix. I just checked that it works on PostgreSQL and MySQL, so it passes the criteria. The previous version only worked on PostgreSQL so this is a definite improvement. The real standard way of doing datetime subtraction is like But from what I know, most databases don't support this. So your suggestion is the best option we have right now. |
|
merged, thanks @mbutrovich |
This proposes a change to nine queries based on rule 3 of DIALECTS.md, which gives
(t - x) > interval '30 days'tox < t - INTERVAL '30' DAYas the canonical form. q114 already has it. I found the other nine sites (q086, q233, q237, q274, q288, q304, q350, q404, q408) while debugging DataFusion errors. They come from a real DataFusion bug, apache/datafusion#26671. This change brings the queries into line with DIALECTS.md and also works around that failure.The PR adds one
edits.jsonentry per site and regenerates withtools/canonical.py.numerics.jsonchanges only where its replacement text contains one of these predicates.Tests are the SF1 results of all nine queries, which match
workload/results/sf1/through-2025-Q4withdbo-bench checkon DuckDB 2.0.0 (integer_divisionon) and DataFusion 55.0.0. With theyear_monthchange from #7 applied, DataFusion SF1 mismatches go from 12 to 5. The new form returns the same values as the old one on PostgreSQL and Umbra, checked on a small table with boundary and NULL rows. Hyper, CedarDB, Firebolt and ClickHouse were not run.