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

Remove deprecated feature group-by-uses-equal #22888

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

rschlussel
Copy link
Contributor

@rschlussel rschlussel commented May 31, 2024

Description

Remove deprecated legacy behavior for group by to use equal to rather than distinct. Remove the corresponding config
deprecated.group-by-uses-equal

Motivation and Context

Clean up of deprecated/not maintained feature.

Impact

see above

Test Plan

CI

Contributor checklist

  • Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Remove deprecated feature and configuration property ``deprecated.group-by-uses-equal``, which allowed group by to use equal to rather than distinct semantics.

@rschlussel rschlussel mentioned this pull request May 31, 2024
6 tasks
jaystarshot
jaystarshot previously approved these changes May 31, 2024
@rschlussel rschlussel force-pushed the remove-group-by-uses-equal branch 6 times, most recently from 22e3fbf to a060870 Compare May 31, 2024 21:08
elharo
elharo previously approved these changes May 31, 2024
Copy link
Contributor

@elharo elharo left a comment

Choose a reason for hiding this comment

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

[INFO] There is 1 error reported by Checkstyle 8.16 with src/checkstyle/presto-checks.xml ruleset.
Error: src/test/java/com/facebook/presto/hive/TestHivePageSink.java:[36,8] (imports) UnusedImports: Unused import - com.facebook.presto.sql.analyzer.FeaturesConfig.

Remove deprecated legacy behavior for group by to use equal to rather than
distinct.  Remove the corresponding config
`deprecated.group-by-uses-equal`
@feilong-liu
Copy link
Contributor

Can you give a brief context on what this feature is and why we are removing it?

@rschlussel
Copy link
Contributor Author

Can you give a brief context on what this feature is and why we are removing it?

The original implementation of group by in Presto was incorrectly using equal to rather than is distinct from. When that was fixed, this config was added to use the legacy behavior baad26e. It's been deprecated a long time, and I think we can safely remove it now.

@rschlussel rschlussel merged commit 9c57e73 into prestodb:master Jun 3, 2024
56 checks passed
@wanglinsong wanglinsong mentioned this pull request Jun 25, 2024
36 tasks
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.

5 participants