Jefffrey commented on code in PR #18695:
URL: https://github.com/apache/datafusion/pull/18695#discussion_r2529741325
##########
datafusion/datasource-parquet/src/file_format.rs:
##########
@@ -1500,9 +1500,10 @@ fn spawn_parquet_parallel_serialization_task(
serialize_tx: Sender<SpawnedTask<RBStreamSerializeResult>>,
schema: Arc<Schema>,
writer_props: Arc<WriterProperties>,
- parallel_options: ParallelParquetWriterOptions,
+ parallel_options: &ParallelParquetWriterOptions,
pool: Arc<dyn MemoryPool>,
) -> SpawnedTask<Result<(), DataFusionError>> {
+ let parallel_options = parallel_options.clone();
Review Comment:
I'm a bit confused by this; it feels pointless to pass by reference if we
clone it anyway 🤔
##########
datafusion/datasource-parquet/src/opener.rs:
##########
@@ -685,7 +685,7 @@ pub(crate) fn build_page_pruning_predicate(
) -> Arc<PagePruningAccessPlanFilter> {
Arc::new(PagePruningAccessPlanFilter::new(
predicate,
- Arc::clone(file_schema),
+ &Arc::clone(file_schema),
Review Comment:
Can we just pass `file_schema` directly?
##########
datafusion/datasource-parquet/src/source.rs:
##########
@@ -334,9 +334,9 @@ impl ParquetSource {
}
/// Set predicate information
- pub fn with_predicate(&self, predicate: Arc<dyn PhysicalExpr>) -> Self {
+ pub fn with_predicate(&self, predicate: &Arc<dyn PhysicalExpr>) -> Self {
Review Comment:
Ditto
##########
datafusion/datasource-parquet/src/page_filter.rs:
##########
@@ -118,14 +118,14 @@ pub struct PagePruningAccessPlanFilter {
impl PagePruningAccessPlanFilter {
/// Create a new [`PagePruningAccessPlanFilter`] from a physical
/// expression.
- pub fn new(expr: &Arc<dyn PhysicalExpr>, schema: SchemaRef) -> Self {
+ pub fn new(expr: &Arc<dyn PhysicalExpr>, schema: &SchemaRef) -> Self {
Review Comment:
Breaking API I believe?
##########
datafusion/datasource-parquet/src/file_format.rs:
##########
@@ -1074,9 +1074,9 @@ pub async fn fetch_statistics(
)]
pub fn statistics_from_parquet_meta_calc(
metadata: &ParquetMetaData,
- table_schema: SchemaRef,
+ table_schema: &SchemaRef,
Review Comment:
Is this a public API break?
--
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]
