-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add benchmark derived from TPC-H #355
Conversation
@kylebrooks-8451 fyi |
dev/release/rat_exclude_files.txt
Outdated
.github/* | ||
benchmarks/tpch/queries/q*.sql |
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.
The queries are copyright TPC so we can't claim ASF copyright
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.
LGTM. Had a single question/comment about if a disclaimer should have been included in the DDL file. I don't know the answer so approving and letting you use your judgement there.
benchmarks/tpch/create_tables.sql
Outdated
@@ -0,0 +1,133 @@ | |||
-- Licensed to the Apache Software Foundation (ASF) under one |
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.
Should this also contain the disclaimer about being derived from TPC-H since it is performing DDL? Don't know the answer there?
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.
Good point. I will go ahead and add this.
Which issue does this PR close?
N/A
Rationale for this change
Make it easier to run benchmarks so we can track performance between releases and catch regressions earlier.
I had to add a workaround for apache/datafusion#6147 by renaming the generated
.tbl
files to.csv
files.What changes are included in this PR?
Are there any user-facing changes?