Dandandan commented on code in PR #15454:
URL: https://github.com/apache/datafusion/pull/15454#discussion_r2022366297


##########
datafusion/physical-plan/src/spill/mod.rs:
##########
@@ -35,7 +35,10 @@ use datafusion_common::{exec_datafusion_err, HashSet, 
Result};
 
 fn read_spill(sender: Sender<Result<RecordBatch>>, path: &Path) -> Result<()> {
     let file = BufReader::new(File::open(path)?);
-    let reader = StreamReader::try_new(file, None)?;
+    // SAFETY: DataFusion's spill writer strictly follows Arrow IPC 
specifications
+    // with validated schemas and buffers. Skip redundant validation during 
read
+    // to speedup read operation. This is a deliberate safety-performance 
tradeoff.

Review Comment:
   ```suggestion
       // to speedup read operation. This is safe for DataFusion as input 
guarenteed to be correct when written.
   ```
   I don't think it's really a tradeoff in the sense that we are losing safety. 



##########
datafusion/physical-plan/src/spill/mod.rs:
##########
@@ -35,7 +35,10 @@ use datafusion_common::{exec_datafusion_err, HashSet, 
Result};
 
 fn read_spill(sender: Sender<Result<RecordBatch>>, path: &Path) -> Result<()> {
     let file = BufReader::new(File::open(path)?);
-    let reader = StreamReader::try_new(file, None)?;
+    // SAFETY: DataFusion's spill writer strictly follows Arrow IPC 
specifications
+    // with validated schemas and buffers. Skip redundant validation during 
read
+    // to speedup read operation. This is a deliberate safety-performance 
tradeoff.

Review Comment:
   ```suggestion
       // to speedup read operation. This is safe for DataFusion as input 
guaranteed to be correct when written.
   ```
   I don't think it's really a tradeoff in the sense that we are losing safety. 



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to