logan-keede commented on code in PR #14951:
URL: https://github.com/apache/datafusion/pull/14951#discussion_r1976486683


##########
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:
   My understanding is that since `Session` trait is implemented for 
`SessionState`, you should be able to call 
`some_SessionState_struct.default_table_options()`, It seems that you are 
essentially overriding that function with itself(hence the original suggestion 
of removing it) unless I am missing something. Please let me know if that's the 
case!!
    



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to