Skip to content
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.

Sign up for GitHub

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

Jump to bottom

changed TPC-H benchmark to use Decimal types #3393

Merged
merged 1 commit into from
Sep 8, 2022
Merged

changed TPC-H benchmark to use Decimal types #3393

merged 1 commit into from
Sep 8, 2022

Conversation

kmitchener
Copy link
Contributor

@kmitchener kmitchener commented Sep 7, 2022 •
edited
Loading

Which issue does this PR close?

Closes #3392 and closes #166.

Rationale for this change

What changes are included in this PR?

  • changed datatypes to match TPC-H definition -- where Float64 was used, using Decimal128 now
  • added special handling of q15 results, where we want to capture the results of the second of 3 statements
  • fixed up the comparison of query results against known-good answers
  • stop ignoring q15 and q21
  • update README with link to benchmark PDF for reference

Assuming you set the TPCH_DATA env, all the tests now complete successfully except:

  • q6 returns wrong results according to this test ( TPC-H Query 6 has a wrong result #192)
  • q9 has at least one row that's off by .01
  • q11 "attempt to multiply with overflow" panic
  • q14 "attempt to multiply with overflow" panic
  • q17 "attempt to multiply with overflow" panic

Are there any user-facing changes?

@kmitchener
changed datatypes to match TPC-H definition -- where Float64 was used…
165737e 
…, using Decimal128 now

added special handling of q15 results, where we want to capture the results of the second of 3 statements
fixed up the comparison of query results against known-good answers
stop ignoring q15 and q21
andygrove
andygrove approved these changes Sep 7, 2022
View reviewed changes
Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @kmitchener!

@andygrove
Copy link
Member

cc @avantgardnerio

@liukun4515
Copy link
Contributor

@kmitchener Thanks for that.
I will take a look the overflow of the decimal.

@andygrove andygrove merged commit 4258751 into apache:master Sep 8, 2022
@ursabot
Copy link

ursabot commented Sep 8, 2022

Benchmark runs are scheduled for baseline = e6378f4 and contender = 4258751. 4258751 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@kmitchener kmitchener deleted the 166-tpc-h-query-15-1 branch September 9, 2022 12:41
This was referenced Sep 10, 2022
Closed
Closed
@avantgardnerio avantgardnerio mentioned this pull request Sep 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andygrove andygrove andygrove approved these changes

@xudong963 xudong963 xudong963 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

update TPC-H benchmark to Decimal types from Float TPC-H Query 15
5 participants
@kmitchener @andygrove @liukun4515 @ursabot @xudong963