alamb commented on code in PR #3363:
URL: https://github.com/apache/arrow-datafusion/pull/3363#discussion_r963634557
##########
datafusion/core/src/avro_to_arrow/arrow_array_reader.rs:
##########
@@ -258,7 +258,7 @@ impl<'a, R: Read> AvroArrowArrayReader<'a, R> {
{
let mut builder: Box<dyn ArrayBuilder> = match data_type {
DataType::Utf8 => {
- let values_builder = StringBuilder::new(rows.len() * 5);
+ let values_builder = StringBuilder::with_capacity(rows.len(),
5);
Review Comment:
👍
##########
datafusion/core/src/physical_plan/common.rs:
##########
@@ -393,7 +393,10 @@ mod tests {
]),
};
- assert_eq!(result, expected);
+ // Prevent test flakiness due to undefined / changing implementation
details
Review Comment:
This makes sense (and is something we see in IOx as well -- that the exact
size of the parquet file changes from release to release). I think at least
part of the discrepancy is that the version and gitsha of the arrow-rs library
are encoded in the parquet metadata
##########
datafusion/core/src/physical_plan/explain.rs:
##########
@@ -121,8 +121,8 @@ impl ExecutionPlan for ExplainExec {
)));
}
- let mut type_builder =
StringBuilder::new(self.stringified_plans.len());
- let mut plan_builder =
StringBuilder::new(self.stringified_plans.len());
+ let mut type_builder = StringBuilder::new();
Review Comment:
FWIW I don't think the performance of explain plan is particularly important
-- a few more allocations likely won't hurt either way
--
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]
