edmondop commented on code in PR #12113:
URL: https://github.com/apache/datafusion/pull/12113#discussion_r1727709430


##########
datafusion/optimizer/src/analyzer/inline_table_scan.rs:
##########
@@ -56,24 +56,22 @@ fn analyze_internal(plan: LogicalPlan) -> 
Result<Transformed<LogicalPlan>> {
         match plan {
             // Match only on scans without filter / projection / fetch
             // Views and DataFrames won't have those added
-            // during the early stage of planning
-            LogicalPlan::TableScan(TableScan {
-                table_name,
-                source,
-                projection,
-                filters,
-                ..
-            }) if filters.is_empty() && source.get_logical_plan().is_some() => 
{
-                let sub_plan = source.get_logical_plan().unwrap();
-                let projection_exprs = generate_projection_expr(&projection, 
sub_plan)?;
-                LogicalPlanBuilder::from(sub_plan.clone())
-                    .project(projection_exprs)?
-                    // Ensures that the reference to the inlined table remains 
the
-                    // same, meaning we don't have to change any of the parent 
nodes
-                    // that reference this table.
-                    .alias(table_name)?
-                    .build()
-                    .map(Transformed::yes)
+            // during the early stage of planning.
+            LogicalPlan::TableScan(table_scan) if 
table_scan.filters.is_empty() => {
+                if let Some(sub_plan) = 
table_scan.source.get_logical_plan_cloned() {

Review Comment:
   I am confused about this change, there seems to be two different things 
happening:
   
   - Using de-structuring rather than unwrap, which seems useful (but that 
should be possible even without `get_logical_plan_cloned`
   - Using an owned `LogicalPlan`.
   
   It seems in this case there is effectively an extra clone that wasn't 
present before (inside `get_logical_plan_cloned`), can you explain why this is 
necessary?



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to