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


##########
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 think it is better to have an example in Datafusion as well. Having a 
method but not used at all is quite confusing to maintain -- what is it used 
for and whether it could be deprecated. With the example, we could also evolve 
with the better idea / interface gradually and could also avoid ending up with 
plenty of specialized method that is only used in specific downstream project 
but not generally useful to most of the implementation.
   
   Btw, If you need to mutate plan concurrently, I think what you need is 
something like `fn update_logical_plan()` instead of returning a cloned 🤔 



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