Skip to content

feat: #215 resolve aliases for group by exprs - #485

Merged
alamb merged 2 commits into
apache:masterfrom
jychen7:215-group-by-alias
Jun 4, 2021
Merged

feat: #215 resolve aliases for group by exprs#485
alamb merged 2 commits into
apache:masterfrom
jychen7:215-group-by-alias

Conversation

@jychen7

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes#215 .

What changes are included in this PR?

support alias in group by expression

Are there any user-facing changes?

no

@jychen7

Copy link
Copy Markdown
ContributorAuthor

cc @alamb
it would be nice if you have time to help review

select_exprs: &[Expr],
having_expr_opt: &Option<Expr>,
group_by: &[SQLExpr],
group_by_exprs: Vec<Expr>,

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.

👍

format!("{:?}", err)
fn select_simple_aggregate_with_groupby_can_use_alias() {
quick_test(
"SELECT state AS a, MIN(age) AS b FROM person GROUP BY a",

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.

👍

@DandandanDandandan 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.

Looks great to me @jychen7 !!

@alamb

alamb commented Jun 3, 2021

Copy link
Copy Markdown
Contributor

Thank you for the contribution @jychen7 !

@alamb
alamb merged commit ae15a51 into apache:masterJun 4, 2021
@jychen7
jychen7 deleted the 215-group-by-alias branch June 6, 2021 15:41
@houqphouqp added the enhancement New feature or request label Jul 31, 2021
@jychen7jychen7 changed the title #215 resolve aliases for group by exprsfeat: #215 resolve aliases for group by exprsMar 27, 2022
unkloud pushed a commit to unkloud/datafusion that referenced this pull request Mar 23, 2025
…#506)
## Which issue does this PR close?
Closesapache#485 ## Rationale for this change
Compatibility with how Spark handles logarithms of values <=0. ## What changes are included in this PR?
Use IfExpr to check when input to log2 is <=0 and return null. This is done to match Spark's behavior, which in turn is implemented to match Hive's behavior.
## How are these changes tested?
The existing test for `ln`, `log2` and `log10` was modified so that it includes negative numbers as part of the inputs being tested.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Group by Alias

4 participants

@jychen7@alamb@Dandandan@houqp