logan-keede commented on code in PR #14951:
URL: https://github.com/apache/datafusion/pull/14951#discussion_r1976405859
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -832,17 +836,16 @@ impl SessionState {
self.config.options()
}
- /// return the TableOptions options with its extensions
- pub fn default_table_options(&self) -> TableOptions {
- self.table_options
- .combine_with_session_config(self.config_options())
- }
-
/// Return the table options
pub fn table_options(&self) -> &TableOptions {
&self.table_options
}
+ /// return the TableOptions options with its extensions
+ pub fn default_table_options(&self) -> TableOptions {
+ Session::default_table_options(self)
+ }
+
Review Comment:
I think `default_table_options` and `table_options` only need to be part of
`impl Session` rather than
`SessionState`
##########
datafusion/datasource/src/file_scan_config.rs:
##########
@@ -501,7 +501,7 @@ impl FileScanConfig {
(schema, constraints, stats, output_ordering)
}
- #[cfg_attr(not(feature = "avro"), allow(unused))] // Only used by avro
Review Comment:
It is my understanding that this is being removed to avoid dependency on
avro.
I am not sure what is better between this and empty feature.
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -832,17 +836,16 @@ impl SessionState {
self.config.options()
}
- /// return the TableOptions options with its extensions
- pub fn default_table_options(&self) -> TableOptions {
- self.table_options
- .combine_with_session_config(self.config_options())
- }
-
/// Return the table options
pub fn table_options(&self) -> &TableOptions {
&self.table_options
}
+ /// return the TableOptions options with its extensions
+ pub fn default_table_options(&self) -> TableOptions {
+ Session::default_table_options(self)
+ }
+
/// Return mutable table options
pub fn table_options_mut(&mut self) -> &mut TableOptions {
&mut self.table_options
Review Comment:
perhaps this should also be a part of Session
##########
datafusion/core/Cargo.toml:
##########
@@ -54,14 +60,15 @@ default = [
"string_expressions",
"unicode_expressions",
"compression",
+ "avro",
Review Comment:
should avro be a default feature?
--
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]
