Derive Debug for SessionStateBuilder, adding Debug requirements to fields (#12632)

* Require Debug for PhysicalOptimizerRule

* Add reference to meet &JoinType type required

* Revert "Add reference to meet &JoinType type required" as clippy lint informs this is unnecessary

This reverts commit f69d73c.

* Require `Debug` for `CatalogProvider`, `CatalogProviderList`, UrlTableFactory

* Add derive Debug to meet api-change

* Add derive Debug in datafusion-cli to support api-change of CatalogProviderList

* Require Debug for ExprPlanner

* Require Debug for QueryPlanner

* Require Debug for TableFunctionImpl

* Require Debug for SerializerRegistry

* Require Debug for FunctionFactory

* Derive `Debug` on `SessionStateBuilder`

* Implement `Debug` for `SessionStateBuilder` to reorder output fields, keep consistent Debug field order with `SessionState`

* Settle TODO for displaying `Debug` of `InformationSchemaConfig` after `CatalogProviderList` requires `Debug`

Author: AnthonyZhOon

datafusion-cli/src/catalog.rs

@@ -34,6 +34,7 @@ use dirs::home_dir;
 use parking_lot::RwLock;
 
 /// Wraps another catalog, automatically register require object stores for the file locations
+#[derive(Debug)]
 pub struct DynamicObjectStoreCatalog {
     inner: Arc<dyn CatalogProviderList>,
     state: Weak<RwLock<SessionState>>,
@@ -74,6 +75,7 @@ impl CatalogProviderList for DynamicObjectStoreCatalog {
 }
 
 /// Wraps another catalog provider
+#[derive(Debug)]
 struct DynamicObjectStoreCatalogProvider {
     inner: Arc<dyn CatalogProvider>,
     state: Weak<RwLock<SessionState>>,
@@ -115,6 +117,7 @@ impl CatalogProvider for DynamicObjectStoreCatalogProvider {
 
 /// Wraps another schema provider. [DynamicObjectStoreSchemaProvider] is responsible for registering the required
 /// object stores for the file locations.
+#[derive(Debug)]
 struct DynamicObjectStoreSchemaProvider {
     inner: Arc<dyn SchemaProvider>,
     state: Weak<RwLock<SessionState>>,

datafusion-cli/src/functions.rs

@@ -315,6 +315,7 @@ fn fixed_len_byte_array_to_string(val: Option<&FixedLenByteArray>) -> Option<Str
     })
 }
 
+#[derive(Debug)]
 pub struct ParquetMetadataFunc {}
 
 impl TableFunctionImpl for ParquetMetadataFunc {

datafusion-examples/examples/catalog.rs

@@ -135,6 +135,7 @@ struct DirSchemaOpts<'a> {
     format: Arc<dyn FileFormat>,
 }
 /// Schema where every file with extension `ext` in a given `dir` is a table.
+#[derive(Debug)]
 struct DirSchema {
     ext: String,
     tables: RwLock<HashMap<String, Arc<dyn TableProvider>>>,
@@ -218,6 +219,7 @@ impl SchemaProvider for DirSchema {
     }
 }
 /// Catalog holds multiple schemas
+#[derive(Debug)]
 struct DirCatalog {
     schemas: RwLock<HashMap<String, Arc<dyn SchemaProvider>>>,
 }
@@ -259,6 +261,7 @@ impl CatalogProvider for DirCatalog {
     }
 }
 /// Catalog lists holds multiple catalog providers. Each context has a single catalog list.
+#[derive(Debug)]
 struct CustomCatalogProviderList {
     catalogs: RwLock<HashMap<String, Arc<dyn CatalogProvider>>>,
 }

datafusion-examples/examples/simple_udtf.rs

@@ -128,6 +128,7 @@ impl TableProvider for LocalCsvTable {
     }
 }
 
+#[derive(Debug)]
 struct LocalCsvTableFunc {}
 
 impl TableFunctionImpl for LocalCsvTableFunc {

datafusion/catalog/src/catalog.rs

@@ -16,6 +16,7 @@
 // under the License.
 
 use std::any::Any;
+use std::fmt::Debug;
 use std::sync::Arc;
 
 pub use crate::schema::SchemaProvider;
@@ -101,7 +102,7 @@ use datafusion_common::Result;
 ///
 /// [`TableProvider`]: crate::TableProvider
 
-pub trait CatalogProvider: Sync + Send {
+pub trait CatalogProvider: Debug + Sync + Send {
     /// Returns the catalog provider as [`Any`]
     /// so that it can be downcast to a specific implementation.
     fn as_any(&self) -> &dyn Any;
@@ -152,7 +153,7 @@ pub trait CatalogProvider: Sync + Send {
 ///
 /// Please see the documentation on `CatalogProvider` for details of
 /// implementing a custom catalog.
-pub trait CatalogProviderList: Sync + Send {
+pub trait CatalogProviderList: Debug + Sync + Send {
     /// Returns the catalog list as [`Any`]
     /// so that it can be downcast to a specific implementation.
     fn as_any(&self) -> &dyn Any;

datafusion/catalog/src/dynamic_file/catalog.rs

@@ -20,9 +20,11 @@
 use crate::{CatalogProvider, CatalogProviderList, SchemaProvider, TableProvider};
 use async_trait::async_trait;
 use std::any::Any;
+use std::fmt::Debug;
 use std::sync::Arc;
 
 /// Wrap another catalog provider list
+#[derive(Debug)]
 pub struct DynamicFileCatalog {
     /// The inner catalog provider list
     inner: Arc<dyn CatalogProviderList>,
@@ -67,6 +69,7 @@ impl CatalogProviderList for DynamicFileCatalog {
 }
 
 /// Wraps another catalog provider
+#[derive(Debug)]
 struct DynamicFileCatalogProvider {
     /// The inner catalog provider
     inner: Arc<dyn CatalogProvider>,
@@ -114,6 +117,7 @@ impl CatalogProvider for DynamicFileCatalogProvider {
 ///
 /// The provider will try to create a table provider from the file path if the table provider
 /// isn't exist in the inner schema provider.
+#[derive(Debug)]
 pub struct DynamicFileSchemaProvider {
     /// The inner schema provider
     inner: Arc<dyn SchemaProvider>,
@@ -174,7 +178,7 @@ impl SchemaProvider for DynamicFileSchemaProvider {
 
 /// [UrlTableFactory] is a factory that can create a table provider from the given url.
 #[async_trait]
-pub trait UrlTableFactory: Sync + Send {
+pub trait UrlTableFactory: Debug + Sync + Send {
     /// create a new table provider from the provided url
     async fn try_new(
         &self,

datafusion/catalog/src/schema.rs

@@ -21,6 +21,7 @@
 use async_trait::async_trait;
 use datafusion_common::{exec_err, DataFusionError};
 use std::any::Any;
+use std::fmt::Debug;
 use std::sync::Arc;
 
 use crate::table::TableProvider;
@@ -32,7 +33,7 @@ use datafusion_common::Result;
 ///
 /// [`CatalogProvider`]: super::CatalogProvider
 #[async_trait]
-pub trait SchemaProvider: Sync + Send {
+pub trait SchemaProvider: Debug + Sync + Send {
     /// Returns the owner of the Schema, default is None. This value is reported
     /// as part of `information_tables.schemata
     fn owner_name(&self) -> Option<&str> {

datafusion/catalog/src/session.rs

@@ -139,6 +139,7 @@ impl From<&dyn Session> for TaskContext {
 }
 type SessionRefLock = Arc<Mutex<Option<Weak<RwLock<dyn Session>>>>>;
 /// The state store that stores the reference of the runtime session state.
+#[derive(Debug)]
 pub struct SessionStore {
     session: SessionRefLock,
 }

datafusion/core/src/catalog_common/information_schema.rs

@@ -26,7 +26,7 @@ use arrow::{
 };
 use async_trait::async_trait;
 use datafusion_common::DataFusionError;
-use std::fmt::{Debug, Formatter};
+use std::fmt::Debug;
 use std::{any::Any, sync::Arc};
 
 use crate::catalog::{CatalogProviderList, SchemaProvider, TableProvider};
@@ -57,6 +57,7 @@ pub const INFORMATION_SCHEMA_TABLES: &[&str] =
 /// demand. This means that if more tables are added to the underlying
 /// providers, they will appear the next time the `information_schema`
 /// table is queried.
+#[derive(Debug)]
 pub struct InformationSchemaProvider {
     config: InformationSchemaConfig,
 }
@@ -70,20 +71,11 @@ impl InformationSchemaProvider {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 struct InformationSchemaConfig {
     catalog_list: Arc<dyn CatalogProviderList>,
 }
 
-impl Debug for InformationSchemaConfig {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("InformationSchemaConfig")
-            // TODO it would be great to print the catalog list here
-            // but that would require CatalogProviderList to implement Debug
-            .finish_non_exhaustive()
-    }
-}
-
 impl InformationSchemaConfig {
     /// Construct the `information_schema.tables` virtual table
     async fn make_tables(

datafusion/core/src/catalog_common/listing_schema.rs

@@ -48,6 +48,7 @@ use object_store::ObjectStore;
 /// - `s3://host.example.com:3000/data/tpch/customer/_delta_log/`
 ///
 /// [`ObjectStore`]: object_store::ObjectStore
+#[derive(Debug)]
 pub struct ListingSchemaProvider {
     authority: String,
     path: object_store::path::Path,

datafusion/core/src/catalog_common/memory.rs

@@ -28,6 +28,7 @@ use std::any::Any;
 use std::sync::Arc;
 
 /// Simple in-memory list of catalogs
+#[derive(Debug)]
 pub struct MemoryCatalogProviderList {
     /// Collection of catalogs containing schemas and ultimately TableProviders
     pub catalogs: DashMap<String, Arc<dyn CatalogProvider>>,
@@ -71,6 +72,7 @@ impl CatalogProviderList for MemoryCatalogProviderList {
 }
 
 /// Simple in-memory implementation of a catalog.
+#[derive(Debug)]
 pub struct MemoryCatalogProvider {
     schemas: DashMap<String, Arc<dyn SchemaProvider>>,
 }
@@ -136,6 +138,7 @@ impl CatalogProvider for MemoryCatalogProvider {
 }
 
 /// Simple in-memory implementation of a schema.
+#[derive(Debug)]
 pub struct MemorySchemaProvider {
     tables: DashMap<String, Arc<dyn TableProvider>>,
 }
@@ -248,6 +251,7 @@ mod test {
     #[test]
     fn default_register_schema_not_supported() {
         // mimic a new CatalogProvider and ensure it does not support registering schemas
+        #[derive(Debug)]
         struct TestProvider {}
         impl CatalogProvider for TestProvider {
             fn as_any(&self) -> &dyn Any {

datafusion/core/src/datasource/dynamic_file.rs

@@ -30,7 +30,7 @@ use crate::error::Result;
 use crate::execution::context::SessionState;
 
 /// [DynamicListTableFactory] is a factory that can create a [ListingTable] from the given url.
-#[derive(Default)]
+#[derive(Default, Debug)]
 pub struct DynamicListTableFactory {
     /// The session store that contains the current session.
     session_store: SessionStore,

datafusion/core/src/datasource/function.rs

@@ -22,15 +22,17 @@ use super::TableProvider;
 use datafusion_common::Result;
 use datafusion_expr::Expr;
 
+use std::fmt::Debug;
 use std::sync::Arc;
 
 /// A trait for table function implementations
-pub trait TableFunctionImpl: Sync + Send {
+pub trait TableFunctionImpl: Debug + Sync + Send {
     /// Create a table provider
     fn call(&self, args: &[Expr]) -> Result<Arc<dyn TableProvider>>;
 }
 
 /// A table that uses a function to generate data
+#[derive(Debug)]
 pub struct TableFunction {
     /// Name of the table function
     name: String,

datafusion/core/src/execution/context/mod.rs

@@ -1547,7 +1547,7 @@ impl From<SessionContext> for SessionStateBuilder {
 
 /// A planner used to add extensions to DataFusion logical and physical plans.
 #[async_trait]
-pub trait QueryPlanner {
+pub trait QueryPlanner: Debug {
     /// Given a `LogicalPlan`, create an [`ExecutionPlan`] suitable for execution
     async fn create_physical_plan(
         &self,
@@ -1560,7 +1560,7 @@ pub trait QueryPlanner {
 /// and interact with [SessionState] to registers new udf, udaf or udwf.
 
 #[async_trait]
-pub trait FunctionFactory: Sync + Send {
+pub trait FunctionFactory: Debug + Sync + Send {
     /// Handles creation of user defined function specified in [CreateFunction] statement
     async fn create(
         &self,
@@ -1583,6 +1583,7 @@ pub enum RegisterFunction {
 
 /// Default implementation of [SerializerRegistry] that throws unimplemented error
 /// for all requests.
+#[derive(Debug)]
 pub struct EmptySerializerRegistry;
 
 impl SerializerRegistry for EmptySerializerRegistry {
@@ -2129,6 +2130,7 @@ mod tests {
         }
     }
 
+    #[derive(Debug)]
     struct MyQueryPlanner {}
 
     #[async_trait]

datafusion/core/src/execution/session_state.rs

@@ -177,23 +177,23 @@ impl Debug for SessionState {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("SessionState")
             .field("session_id", &self.session_id)
-            .field("analyzer", &"...")
+            .field("config", &self.config)
+            .field("runtime_env", &self.runtime_env)
+            .field("catalog_list", &"...")
+            .field("serializer_registry", &"...")
+            .field("execution_props", &self.execution_props)
+            .field("table_options", &self.table_options)
+            .field("table_factories", &"...")
+            .field("function_factory", &"...")
             .field("expr_planners", &"...")
+            .field("query_planner", &"...")
+            .field("analyzer", &"...")
             .field("optimizer", &"...")
             .field("physical_optimizers", &"...")
-            .field("query_planner", &"...")
-            .field("catalog_list", &"...")
             .field("table_functions", &"...")
             .field("scalar_functions", &self.scalar_functions)
             .field("aggregate_functions", &self.aggregate_functions)
             .field("window_functions", &self.window_functions)
-            .field("serializer_registry", &"...")
-            .field("config", &self.config)
-            .field("table_options", &self.table_options)
-            .field("execution_props", &self.execution_props)
-            .field("table_factories", &"...")
-            .field("runtime_env", &self.runtime_env)
-            .field("function_factory", &"...")
             .finish_non_exhaustive()
     }
 }
@@ -1519,6 +1519,37 @@ impl SessionStateBuilder {
     }
 }
 
+impl Debug for SessionStateBuilder {
+    /// Prefer having short fields at the top and long vector fields near the end
+    /// Group fields by
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("SessionStateBuilder")
+            .field("session_id", &self.session_id)
+            .field("config", &self.config)
+            .field("runtime_env", &self.runtime_env)
+            .field("catalog_list", &self.catalog_list)
+            .field("serializer_registry", &self.serializer_registry)
+            .field("file_formats", &self.file_formats)
+            .field("execution_props", &self.execution_props)
+            .field("table_options", &self.table_options)
+            .field("table_factories", &self.table_factories)
+            .field("function_factory", &self.function_factory)
+            .field("expr_planners", &self.expr_planners)
+            .field("query_planners", &self.query_planner)
+            .field("analyzer_rules", &self.analyzer_rules)
+            .field("analyzer", &self.analyzer)
+            .field("optimizer_rules", &self.optimizer_rules)
+            .field("optimizer", &self.optimizer)
+            .field("physical_optimizer_rules", &self.physical_optimizer_rules)
+            .field("physical_optimizers", &self.physical_optimizers)
+            .field("table_functions", &self.table_functions)
+            .field("scalar_functions", &self.scalar_functions)
+            .field("aggregate_functions", &self.aggregate_functions)
+            .field("window_functions", &self.window_functions)
+            .finish()
+    }
+}
+
 impl Default for SessionStateBuilder {
     fn default() -> Self {
         Self::new()
@@ -1795,6 +1826,7 @@ impl From<&SessionState> for TaskContext {
 }
 
 /// The query planner used if no user defined planner is provided
+#[derive(Debug)]
 struct DefaultQueryPlanner {}
 
 #[async_trait]

datafusion/core/tests/user_defined/expr_planner.rs

@@ -29,6 +29,7 @@ use datafusion_expr::expr::Alias;
 use datafusion_expr::planner::{ExprPlanner, PlannerResult, RawBinaryExpr};
 use datafusion_expr::BinaryExpr;
 
+#[derive(Debug)]
 struct MyCustomPlanner;
 
 impl ExprPlanner for MyCustomPlanner {

datafusion/core/tests/user_defined/user_defined_plan.rs

@@ -312,6 +312,7 @@ fn make_topk_context() -> SessionContext {
 
 // ------ The implementation of the TopK code follows -----
 
+#[derive(Debug)]
 struct TopKQueryPlanner {}
 
 #[async_trait]

datafusion/core/tests/user_defined/user_defined_table_functions.rs

@@ -192,6 +192,7 @@ impl SimpleCsvTable {
     }
 }
 
+#[derive(Debug)]
 struct SimpleCsvTableFunc {}
 
 impl TableFunctionImpl for SimpleCsvTableFunc {