Skip to content

Change name of MAX/MIN udaf to lowercase max/min - #11795

Merged
jayzhan211 merged 4 commits into
apache:mainfrom
edmondop:issue-11779
Aug 5, 2024
Merged

Change name of MAX/MIN udaf to lowercase max/min #11795
jayzhan211 merged 4 commits into
apache:mainfrom
edmondop:issue-11779

Conversation

@edmondop

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Addresses #11779 . Remove also dead code in moving_min_max

@github-actionsgithub-actionsBot added sql SQL Planner logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates optimizer Optimizer rules core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) labels Aug 3, 2024
@edmondop
edmondopforce-pushed the issue-11779 branch 2 times, most recently from cdcda3f to eb92c73CompareAugust 3, 2024 16:17
@github-actionsgithub-actionsBot added the substrait Changes to the substrait crate label Aug 3, 2024
@github-actionsgithub-actionsBot added the documentation Improvements or additions to documentation label Aug 3, 2024
@edmondop
edmondop marked this pull request as ready for review August 3, 2024 18:39
impl Min {
pub fn new() -> Self {
Self {
aliases: vec!["min".to_string()],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't need uppercase alias.

To use uppercase alias the query is something like select "MAX"(a) from t;, which is uncommon.

pub fn new() -> Self {
Self {
aliases: vec!["max".to_string()],
aliases: vec!["MAX".to_string()],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

pub fn new() -> Self {
Self {
aliases: vec!["max".to_owned()],
aliases: vec!["MAX".to_owned()],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

@jayzhan211jayzhan211 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @edmondop

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

Labels

coreCore DataFusion cratedocumentationImprovements or additions to documentationlogical-exprLogical plan and expressionsoptimizerOptimizer rulesphysical-exprChanges to the physical-expr cratessqlSQL PlannersqllogictestSQL Logic Tests (.slt)substraitChanges to the substrait crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@edmondop@jayzhan211