alamb commented on code in PR #7745:
URL: https://github.com/apache/arrow-datafusion/pull/7745#discussion_r1372248425
##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -150,6 +151,7 @@ impl ListingTableConfig {
FileType::JSON => Arc::new(
JsonFormat::default().with_file_compression_type(file_compression_type),
),
+ #[cfg(feature = "parquet")]
Review Comment:
It is strange to me that AVRO isn't `#[cfg`]d the same way as `PARQUET` 🤔
##########
datafusion/core/src/dataframe/parquet.rs:
##########
@@ -0,0 +1,162 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use datafusion_common::file_options::parquet_writer::{
Review Comment:
This is a nice refactor to isolate the parquet related functions in a
separate module. 👍
##########
datafusion/core/src/execution/context/avro.rs:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
👨🍳 👌 - very nice
##########
datafusion/core/src/dataframe/parquet.rs:
##########
@@ -0,0 +1,162 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use datafusion_common::file_options::parquet_writer::{
Review Comment:
This is a nice refactor to isolate the parquet related functions in a
separate module. 👍
##########
datafusion/core/src/datasource/physical_plan/mod.rs:
##########
@@ -23,17 +23,20 @@ mod csv;
mod file_scan_config;
mod file_stream;
mod json;
+#[cfg(feature = "parquet")]
pub mod parquet;
pub(crate) use self::csv::plan_to_csv;
pub use self::csv::{CsvConfig, CsvExec, CsvOpener};
-pub(crate) use self::file_scan_config::PartitionColumnProjector;
pub(crate) use self::json::plan_to_json;
+#[cfg(feature = "parquet")]
Review Comment:
We can probably simplify the code a little as a follow on PR by removing
this `pub(crate) use` and using it directly in the parquet module
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -16,6 +16,13 @@
// under the License.
//! [`SessionContext`] contains methods for registering data sources and
executing queries
+
+mod avro;
Review Comment:
It would be neat (again as a follow on PR) to make avro support also
conditionalized
##########
datafusion/core/src/physical_optimizer/enforce_distribution.rs:
##########
@@ -1308,16 +1310,16 @@ fn ensure_distribution(
// When `repartition_file_scans` is set, leverage source
operators
// (`ParquetExec`, `CsvExec` etc.) to increase parallelism at
the source.
if repartition_file_scans {
+ #[cfg(feature = "parquet")]
Review Comment:
It would be nice to remove this `#[cfg` as well as the special case handling
of ParquetExec in the enforce distribution rule. Not in this PR, but as a
follow on. I will file a ticket
##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -150,6 +151,7 @@ impl ListingTableConfig {
FileType::JSON => Arc::new(
JsonFormat::default().with_file_compression_type(file_compression_type),
),
+ #[cfg(feature = "parquet")]
Review Comment:
It is strange to me that AVRO isn't `#[cfg`]d the same way as `PARQUET` 🤔
##########
datafusion/core/src/execution/context/avro.rs:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
👨🍳 👌 - very nice
--
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]
