Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Aug 16, 2024

What changes were proposed in this pull request?

TPCDSQueryBenchmark still runs on ANSI OFF mode because of

TPCDSBenchmark fails with divide by zero in q90 for ANSI

I have tested it with the latest revision on my local machine, and somehow the issue seems to have been resolved. And so do the GA results

Why are the changes needed?

improve test coverage for ANSI default

Does this PR introduce any user-facing change?

no

How was this patch tested?

benchmarking

Was this patch authored or co-authored using generative AI tooling?

no

@github-actions github-actions bot added the SQL label Aug 16, 2024
TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q50 555 599 48 5.8 171.3 1.0X
q50 784 841 51 4.1 241.8 1.0X
Copy link
Member Author

@yaooqinn yaooqinn Aug 16, 2024

Choose a reason for hiding this comment

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

- 40.40%

TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q49 542 629 63 10.4 96.6 1.0X
q49 670 761 131 8.4 119.3 1.0X
Copy link
Member Author

Choose a reason for hiding this comment

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

- 20.99%

TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q58 414 442 27 12.4 80.8 1.0X
q58 412 543 195 12.4 80.4 1.0X
Copy link
Member Author

Choose a reason for hiding this comment

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

- 22.85%

TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q72 114724 116824 2969 0.1 7474.9 1.0X
q72 136543 138257 2425 0.1 8896.5 1.0X
Copy link
Member Author

Choose a reason for hiding this comment

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

- 18.35%

TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q80 943 1188 346 6.0 167.0 1.0X
q80 1318 1555 335 4.3 233.5 1.0X
Copy link
Member Author

Choose a reason for hiding this comment

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

- 30.89%

AMD EPYC 7763 64-Core Processor
TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q72-v2.7 114303 116853 3606 0.1 7447.5 1.0X
Copy link
Member Author

Choose a reason for hiding this comment

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

- 18.15%

AMD EPYC 7763 64-Core Processor
TPCDS: Best Time(ms) Avg Time(ms) Stdev(ms) Rate(M/s) Per Row(ns) Relative
------------------------------------------------------------------------------------------------------------------------
q77a-v2.7 646 755 153 8.7 115.1 1.0X
Copy link
Member Author

Choose a reason for hiding this comment

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

- 33.38%

@yaooqinn yaooqinn changed the title [WIP][SPARK-48066][SQL][TESTS] Enable ANSI for TPCDSQueryBenchmark [SPARK-48066][SQL][TESTS] Enable ANSI for TPCDSQueryBenchmark Aug 16, 2024
@yaooqinn
Copy link
Member Author

Also cc @dongjoon-hyun from #47743

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you for the rechecking and update, @yaooqinn .

Ya, I'm also not sure what was the root cause at that time. In any way, it's a great to have this update.

For the perf difference part, we can track from now properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants