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

Add distinct aggregate tests to sqllogictest #10158

Merged
merged 2 commits into from
Apr 21, 2024

Conversation

Jefffrey
Copy link
Contributor

@Jefffrey Jefffrey commented Apr 21, 2024

Which issue does this PR close?

Relates to #2411, #2410, #2408

Rationale for this change

Datafusion already supports these use cases. Just locking them into the test suite

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Copy link
Contributor

@jayzhan211 jayzhan211 left a comment

Choose a reason for hiding this comment

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

👍

@Jefffrey
Copy link
Contributor Author

Jefffrey commented Apr 21, 2024

FYI we don't support this

> select median(distinct "1"), median("1") from '/home/jeffrey/Downloads/data.csv';
This feature is not implemented: MEDIAN(DISTINCT) aggregations are not available

But maybe can tackle as separate issue from those original issues 🤔

See #10159 (comment)

@Weijun-H
Copy link
Member

FYI we don't support this

> select median(distinct "1"), median("1") from '/home/jeffrey/Downloads/data.csv';
This feature is not implemented: MEDIAN(DISTINCT) aggregations are not available

But maybe can tackle as separate issue from those original issues 🤔

See #10159 (comment)

If it hasn't been implemented yet, we should add them to sqllogictest and return an error first.

@Jefffrey
Copy link
Contributor Author

Done. Also I'll keep the original issues open.

Copy link
Member

@Weijun-H Weijun-H 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 @Jefffrey

@alamb alamb merged commit 70d1a5d into apache:main Apr 21, 2024
24 checks passed
@alamb
Copy link
Contributor

alamb commented Apr 21, 2024

Thanks @Jefffrey and @Weijun-H and @jayzhan211 🚀

@Jefffrey Jefffrey deleted the distinct-aggregate-tests branch April 21, 2024 11:27
ccciudatu pushed a commit to hstack/arrow-datafusion that referenced this pull request Apr 26, 2024
* Add distinct aggregate tests to sqllogictest

* Update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants