Skip to content

changed TPC-H benchmark to use Decimal types#3393

Merged
andygrove merged 1 commit intoapache:masterfrom
kmitchener:166-tpc-h-query-15-1
Sep 8, 2022
Merged

changed TPC-H benchmark to use Decimal types#3393
andygrove merged 1 commit intoapache:masterfrom
kmitchener:166-tpc-h-query-15-1

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

andygrove commented Sep 7, 2022

cc @avantgardnerio

@liukun4515
Copy link
Contributor

liukun4515 commented Sep 8, 2022

@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