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.

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

MINOR: Enable multi-statement benchmark queries #2507

Merged
merged 1 commit into from
May 11, 2022

Conversation

andygrove
Copy link
Member

Which issue does this PR close?

N/A

Rationale for this change

q15.sql contains two SQL statements and this was not supported by the benchmark runner

What changes are included in this PR?

Support multiple queries per file

Are there any user-facing changes?

No

@andygrove andygrove requested a review from Dandandan May 10, 2022 23:38
@matthewmturner
Copy link
Contributor

When running Q15 do we get a new error now? Something like CREATE VIEW isnt implemented?

@matthewmturner
Copy link
Contributor

If we get this merged first i can enable Q15 in #2279

Copy link
Member

@yjshen yjshen left a comment

Choose a reason for hiding this comment

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

LGTM.

An off-topic question: I've noticed there were instructions on how to get golden files and get results verified but removed before. Do you think it's the time to add goldens and enable verify_query here for tpc-h?

Currently, these tests are skipped TPCH_DATA environment variable not set, skipping test

@andygrove
Copy link
Member Author

When running Q15 do we get a new error now? Something like CREATE VIEW isnt implemented?

Yes, exactly that!

@andygrove
Copy link
Member Author

LGTM.

An off-topic question: I've noticed there were instructions on how to get golden files and get results verified but removed before. Do you think it's the time to add goldens and enable verify_query here for tpc-h?

Currently, these tests are skipped TPCH_DATA environment variable not set, skipping test

The current approach is for developers to use the tpch toolkit to generate the expected answers locally and then set this env var to enable those tests. It would be nice if we could enable CI to do this as well.

@andygrove andygrove merged commit 559c294 into apache:master May 11, 2022
@andygrove andygrove deleted the benchmark-multi-statement branch May 11, 2022 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants