mustafasrepo commented on code in PR #7040:
URL: https://github.com/apache/arrow-datafusion/pull/7040#discussion_r1276074747
##########
datafusion/sql/src/select.rs:
##########
@@ -431,6 +432,37 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
group_by_exprs: Vec<Expr>,
aggr_exprs: Vec<Expr>,
) -> Result<(LogicalPlan, Vec<Expr>, Option<Expr>)> {
+ let schema = input.schema();
Review Comment:
In here, we update group by exprs by their expressions that are in target
indices (if they are in the select exprs). With this change, we can use
depedents in the functional dependence in the select expression, when their
determinant is inside group by expression. We can do so inside
`Aggregate::try_new_with_schema` if we pass `select_expr` argument to it. I
thought it is a bit weird for aggregate to receive `select_expr` argument.
After this PR merges, I can file another PR, that implements this change. Then
we can decide which implementation is better. In any case, I have moved groupby
update logic to its own function to not clutter this method.
##########
datafusion/sql/src/select.rs:
##########
@@ -431,6 +432,37 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
group_by_exprs: Vec<Expr>,
aggr_exprs: Vec<Expr>,
) -> Result<(LogicalPlan, Vec<Expr>, Option<Expr>)> {
+ let schema = input.schema();
Review Comment:
In here, we update group by exprs by the expressions that are in target
indices (if they are in the select exprs). With this change, we can use
depedents in the functional dependence in the select expression, when their
determinant is inside group by expression. We can do so inside
`Aggregate::try_new_with_schema` if we pass `select_expr` argument to it. I
thought it is a bit weird for aggregate to receive `select_expr` argument.
After this PR merges, I can file another PR, that implements this change. Then
we can decide which implementation is better. In any case, I have moved groupby
update logic to its own function to not clutter this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
