Skip to content

Conversation

@jychen7
Copy link
Contributor

@jychen7 jychen7 commented Jun 3, 2021

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
Contributor Author

jychen7 commented Jun 3, 2021

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
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
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@Dandandan Dandandan left a comment

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
Copy link
Contributor

alamb commented Jun 3, 2021

Thank you for the contribution @jychen7 !

@alamb alamb merged commit ae15a51 into apache:master Jun 4, 2021
@jychen7 jychen7 deleted the 215-group-by-alias branch June 6, 2021 15:41
@houqp houqp added the enhancement New feature or request label Jul 31, 2021
@jychen7 jychen7 changed the title #215 resolve aliases for group by exprs feat: #215 resolve aliases for group by exprs Mar 27, 2022
unkloud pushed a commit to unkloud/datafusion that referenced this pull request Mar 23, 2025
…#506)

## Which issue does this PR close?

Closes apache#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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Group by Alias

4 participants