-
Notifications
You must be signed in to change notification settings - Fork 608
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
refactor(tpc): add tpc-ds tests #9467
Conversation
22a07b6
to
53747a4
Compare
If this PR is too large I can split up the things into a separate PR for each backend, and disable the runs until the refactoring is done. |
Nah, I'm most of the way through reviewing it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the changeset here is good -- do we want to keep the tpch and tpc-ds snapshots around? Since we're comparing results against hand-written SQL, I don't know that we're getting much out of them
They're gone. |
The empty result sets are questionable, but I couldn't find a SF (up to sf=10) where all queries were non-empty. I believe that q17 is supposed to empty, based on the fact that it's always empty for the larger scale factors. So, basically what I did was selectively allow empty queries in the |
lol, I have an optimization for O(1) compute for query 17 at any scale factor |
snowflake is passing:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boy do I not understand waht asceding
is about, but it's clearly intentional.
This looks good, and makes it easy for other contributors to tackle individual tpc-ds queries.
First 27 TPC-DS queries running against DuckDB, Trino, Snowflake, and DataFusion, sans the ones that requires
ROLLUP
.More fail on DataFusion than the others. Those are marked with appropriate xfail marker.