Skip to content

Minor: Add negative test for SORT BY#5254

Merged
xudong963 merged 1 commit intoapache:masterfrom
alamb:alamb/sort_by_test
Feb 12, 2023
Merged

Minor: Add negative test for SORT BY#5254
xudong963 merged 1 commit intoapache:masterfrom
alamb:alamb/sort_by_test

Conversation

@alamb
Copy link
Copy Markdown
Contributor

@alamb alamb commented Feb 11, 2023

Which issue does this PR close?

related to #5247

Rationale for this change

Add a small test for #5249

What changes are included in this PR?

More tests more better

Are these changes tested?

Yes

Are there any user-facing changes?

@alamb alamb mentioned this pull request Feb 11, 2023
@github-actions github-actions bot added core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Feb 11, 2023
@alamb alamb changed the title Add negative test for SORT BY Minor: Add negative test for SORT BY Feb 11, 2023
Copy link
Copy Markdown
Member

@xudong963 xudong963 left a comment

Choose a reason for hiding this comment

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

I find the file contains some tests that don't contain order by, is it expected?

@alamb
Copy link
Copy Markdown
Contributor Author

alamb commented Feb 11, 2023

I find the file contains some tests that don't contain order by, is it expected?

At least one is expected (to demonstrate that the query works the same with and without ORDER BY)

https://github.com/apache/arrow-datafusion/blob/4be56108d55ba301d1677e1a5554308a536479dd/datafusion/core/tests/sqllogictests/test_files/order.slt#L233-L238

Are there others that you had in mind?

Copy link
Copy Markdown
Contributor

@Jefffrey Jefffrey left a comment

Choose a reason for hiding this comment

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

thanks 👍

@xudong963
Copy link
Copy Markdown
Member

to demonstrate that the query works the same with and without ORDER BY

Got

@xudong963 xudong963 merged commit f75d25f into apache:master Feb 12, 2023
@ursabot
Copy link
Copy Markdown

ursabot commented Feb 12, 2023

Benchmark runs are scheduled for baseline = 4be5610 and contender = f75d25f. f75d25f 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

@alamb alamb deleted the alamb/sort_by_test branch July 26, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants