alamb commented on code in PR #18721:
URL: https://github.com/apache/datafusion/pull/18721#discussion_r2547729684


##########
datafusion/datasource/src/file_scan_config.rs:
##########
@@ -192,6 +189,9 @@ pub struct FileScanConfig {
     /// Expression adapter used to adapt filters and projections that are 
pushed down into the scan
     /// from the logical schema to the physical schema of the file.
     pub expr_adapter_factory: Option<Arc<dyn PhysicalExprAdapterFactory>>,
+    /// Unprojected statistics for the table (file schema + partition columns).
+    /// These are projected on-demand via `projected_stats()`.
+    pub(crate) statistics: Statistics,

Review Comment:
   nit is that this field has a different visibility (`pub(crate)`) than all 
the other fields 



##########
datafusion/datasource/src/file_scan_config.rs:
##########
@@ -736,8 +736,21 @@ impl FileScanConfig {
         }
     }
 
-    pub fn projected_stats(&self) -> Statistics {

Review Comment:
   I think this is a (small) public API change, so we should flag this PR 
thusly (I did) and add a note to the upgrade guide



##########
datafusion/datasource/src/file_scan_config.rs:
##########
@@ -1064,11 +1077,7 @@ impl Debug for FileScanConfig {
         write!(f, "FileScanConfig {{")?;
         write!(f, "object_store_url={:?}, ", self.object_store_url)?;
 
-        write!(
-            f,
-            "statistics={:?}, ",
-            self.file_source.statistics().unwrap()

Review Comment:
   removing an unwrap in the display certainly seems like an improvement to me



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