martin-g commented on code in PR #22951:
URL: https://github.com/apache/datafusion/pull/22951#discussion_r3414100087
##########
datafusion/ffi/src/session/mod.rs:
##########
Review Comment:
Are there any downsides of not doing the changes below when the `parquet`
feature is not enabled ?
Since there is no usage of Parquet types I think it would be better to still
support `ConfigFileType::PARQUET` as before.
##########
datafusion/ffi/src/session/mod.rs:
##########
@@ -473,16 +476,19 @@ fn table_options_from_rhashmap(options: SVec<(SString,
SString)>) -> TableOption
let current_format = options.remove("datafusion_ffi.table_current_format");
let mut table_options = TableOptions::default();
- let formats = [
- ConfigFileType::CSV,
- ConfigFileType::JSON,
- ConfigFileType::PARQUET,
- ];
+ let formats = vec![ConfigFileType::CSV, ConfigFileType::JSON];
+ #[cfg(feature = "parquet")]
+ let formats = {
+ let mut formats = formats;
+ formats.push(ConfigFileType::PARQUET);
+ formats
+ };
Review Comment:
```suggestion
let formats = [
ConfigFileType::CSV,
ConfigFileType::JSON,
#[cfg(feature = "parquet")]
ConfigFileType::PARQUET,
];
```
##########
benchmarks/Cargo.toml:
##########
@@ -63,7 +63,7 @@ tokio = { workspace = true, features = ["rt-multi-thread",
"parking_lot"] }
tokio-util = { version = "0.7.17" }
[dev-dependencies]
-datafusion-proto = { workspace = true }
+datafusion-proto = { workspace = true, default-features = true }
Review Comment:
This is redundant since `default-features = true` is the default behavior
anyway.
Maybe list the used features explicitly:
```suggestion
datafusion-proto = { workspace = true, features = ["parquet"] }
```
##########
datafusion-examples/Cargo.toml:
##########
@@ -53,7 +53,7 @@ dashmap = { workspace = true }
base64 = "0.22.1"
datafusion-expr = { workspace = true }
datafusion-physical-expr-adapter = { workspace = true }
-datafusion-proto = { workspace = true }
+datafusion-proto = { workspace = true, default-features = true }
Review Comment:
Same as for -benchmarks
--
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]
