adriangb commented on code in PR #18309:
URL: https://github.com/apache/datafusion/pull/18309#discussion_r2467035635


##########
datafusion/datasource/src/file.rs:
##########
@@ -129,6 +134,47 @@ pub trait FileSource: Send + Sync {
         ))
     }
 
+    fn try_pushdown_projections(

Review Comment:
   A docstring would be great



##########
datafusion/datasource/src/file.rs:
##########
@@ -155,3 +201,12 @@ pub trait FileSource: Send + Sync {
         None
     }
 }
+
+pub enum ProjectionPushdownResult {
+    None,
+    Partial {
+        new_file_source: Option<Arc<dyn FileSource>>,
+        remaining_projections: Option<ProjectionExprs>,
+        new_projection_indices: Option<Vec<usize>>,

Review Comment:
   Hmm something seems off here to me. In my mind this should be more like:
   
   ```rust
   pub struct ProjectionPushdown {
       new_file_source: Arc<dyn FileSource>,
       remaining_projections: Option<ProjectionExprs>,
   }
   
   pub type ProjectionPushdownResult = Option<ProjectionPushdown>;        
   ```
   
   I don't see how it could make sense to have a remaining projection if the 
source wasn't updated.
   
   File sources like Parquet will absorb the entire projection.
   File sources like CSV will push down indexes and create a remainder 
expression that handles anything more complex (aliases, operators, etc.). We 
can make helpers that those file sources call to handle splitting up the 
projection.
   If no projection can be handled (the default) this returns `None`.



##########
datafusion/datasource/src/file.rs:
##########
@@ -155,3 +201,12 @@ pub trait FileSource: Send + Sync {
         None
     }
 }
+
+pub enum ProjectionPushdownResult {

Review Comment:
   doctorings please



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