Jefffrey commented on code in PR #9936:
URL: https://github.com/apache/arrow-rs/pull/9936#discussion_r3295722019


##########
parquet/benches/arrow_writer.rs:
##########
@@ -363,8 +384,10 @@ fn write_batch_with_option(
 
     bench.iter(|| {
         let mut file = Empty::default();
-        let mut writer =
-            ArrowWriter::try_new(&mut file, batch.schema(), 
Some(props.clone())).unwrap();
+        let Ok(mut writer) = ArrowWriter::try_new(&mut file, batch.schema(), 
Some(props.clone()))

Review Comment:
   Generally its easier to have benchmarks merged to main since it lets us use 
the bot to run the comparison; given we don't have a baseline anyway since its 
not really supported, maybe we can just comment out the lines with the ree 
benchmarks in this PR, e.g.
   
   ```rust
       // let batch = create_ree_bench_batch(DataType::Utf8, BATCH_SIZE, 0.25, 
0.75).unwrap();
       // batches.push(("string_ree", batch));
   
       // let batch = create_ree_bench_batch(DataType::Int32, BATCH_SIZE, 0.25, 
0.75).unwrap();
       // batches.push(("int32_ree", batch));
   ```
   
   So we don't need to have this handling for potentially unsupported datatypes 
in the benchmarks which can be confusing.
   
   This way we can still have this benchmark code ready and the main PR later 
won't get bogged down (other than uncommenting the lines)



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

Reply via email to