mustafasrepo commented on issue #8296:
URL: 
https://github.com/apache/arrow-datafusion/issues/8296#issuecomment-1822969187

   > Thank you @mustafasrepo I have tested on [this 
branch](https://github.com/synnada-ai/arrow-datafusion/commits/enhance/aggregate_pk),
 and it looks really nice.
   > 
   > ```
   > ❯ explain  verbose select a/2, a/2+1 from t;
   > | logical_plan after common_sub_expression_eliminate         | Projection: 
t.a / Int64(2)Int64(2)t.a AS t.a / Int64(2), t.a / Int64(2)Int64(2)t.a AS t.a / 
Int64(2) + Int64(1)                                                             
                           |
   > |                                                            |   
Projection: t.a / Int64(2) AS t.a / Int64(2)Int64(2)t.a, t.a                    
                                                                                
                                     |
   > |                                                            |     
TableScan: t
   > | logical_plan after optimize_projections                    | Projection: 
t.a / Int64(2)Int64(2)t.a AS t.a / Int64(2), t.a / Int64(2)Int64(2)t.a AS t.a / 
Int64(2) + Int64(1)                                                             
                           |
   > |                                                            |   
Projection: t.a / Int64(2) AS t.a / Int64(2)Int64(2)t.a                         
                                                                                
                                     |
   > |                                                            |     
TableScan: t projection=[a]
   > | logical_plan after merge_projection                        | Projection: 
t.a / Int64(2) AS t.a / Int64(2), t.a / Int64(2) AS t.a / Int64(2) + Int64(1)   
                                                                                
                           |
   > |                                                            |   
TableScan: t projection=[a]
   > ```
   > 
   > The new `optimize_projections` rule **no longer** undo the optimization of 
`common_sub_expression_eliminate`.
   > 
   > However, the `merge_projection` rule still causes the same problem, 
because it contains the same 
[merge](https://github.com/apache/arrow-datafusion/blob/f2b03443260cade7e43eba568d94d8c09cd002f4/datafusion/optimizer/src/push_down_projection.rs#L92)
 logic as the old `push_down_projection` rule. The merge logic is the root 
cause.
   > 
   > I’m wondering if the new `OptimizeProjections` rule can also cover the 
functionality of 
[MergeProjection](https://github.com/apache/arrow-datafusion/blob/f2b03443260cade7e43eba568d94d8c09cd002f4/datafusion/optimizer/src/merge_projection.rs#L31).
 I believe that, in this way, we can solve this problem. Or wait until your PR 
is merged, and then reconsider this issue.
   
   I think we can cover merge functionality in the new rule also. I will try it 
out, then let you know about the result.


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

Reply via email to