timsaucer commented on code in PR #22951:
URL: https://github.com/apache/datafusion/pull/22951#discussion_r3500270012
##########
datafusion/ffi/src/session/mod.rs:
##########
@@ -504,6 +509,15 @@ fn table_options_from_rhashmap(options: SVec<(SString,
SString)>) -> TableOption
.unwrap_or_else(|err| log::warn!("Error parsing table options:
{err}"));
}
}
+ #[cfg(not(feature = "parquet"))]
+ for (key, value) in options.iter().filter_map(|(k, v)| {
+ let (prefix, key) = k.split_once(".")?;
+ (prefix == "parquet").then(|| (key, v))
+ }) {
+ table_options.parquet.set(key, value).unwrap_or_else(|err| {
+ log::warn!("Error parsing parquet table option: {err}")
+ });
+ }
Review Comment:
I recommend removing this code snippet. I ran a test and when we do set
valid parquet options the warning does not fire because
`table_options.parquet.set()` passes. Plus there are a bunch of default table
options for parquet that do pass as well and we cannot tell a priori if they're
the default values or if the user has set the value. After all, a user could
intentionally set a value to a default and the intent of this snippet would be
to warn when setting these values.
In the end, maybe I'm being pedantic but I recommend just removing this bit.
--
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]
