Address PR review feedback

- Fix rustfmt formatting issues
- Remove unused imports (TableReference, DFSchema) in test module
- Change pub fields to pub(crate) with public accessors:
  - SQLFederationProvider.executor
  - SQLFederationPlanner.executor
  - FederatedPlanNode.plan, FederatedPlanNode.planner
- Pin arduino/setup-protoc to commit hash (v3.0.0)

Author: lukekim

.github/workflows/docs.yml

@@ -20,7 +20,7 @@ jobs:
           fetch-depth: 0
           submodules: recursive
       - uses: dtolnay/rust-toolchain@nightly
-      - uses: arduino/setup-protoc@v3
+      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - run: cargo rustdoc -p datafusion-federation -- --cfg docsrs

.github/workflows/integration-test.yml

@@ -21,7 +21,7 @@ jobs:
         uses: dtolnay/rust-toolchain@stable
 
       - name: Setup protoc
-        uses: arduino/setup-protoc@v3
+        uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
 

.github/workflows/release_plz.yml

@@ -22,7 +22,7 @@ jobs:
           fetch-depth: 0
       - name: Install Rust toolchain
         uses: dtolnay/rust-toolchain@stable
-      - uses: arduino/setup-protoc@v3
+      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - name: Run release-plz

.github/workflows/test.yml

@@ -16,7 +16,7 @@ jobs:
           fetch-depth: 0
           submodules: recursive
       - uses: dtolnay/rust-toolchain@stable
-      - uses: arduino/setup-protoc@v3
+      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - run: cargo check --all-features
@@ -30,7 +30,7 @@ jobs:
           fetch-depth: 0
           submodules: recursive
       - uses: dtolnay/rust-toolchain@stable
-      - uses: arduino/setup-protoc@v3
+      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - run: cargo test --all-features
@@ -56,7 +56,7 @@ jobs:
       - uses: dtolnay/rust-toolchain@stable
         with:
           components: clippy
-      - uses: arduino/setup-protoc@v3
+      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - run: cargo clippy -- -D warnings
@@ -70,7 +70,7 @@ jobs:
           fetch-depth: 0
           submodules: recursive
       - uses: dtolnay/rust-toolchain@stable
-      - uses: arduino/setup-protoc@v3
+      - uses: arduino/setup-protoc@c65c819552d16ad3c9b72d9dfd5ba5237b9c906b # v3.0.0
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
       - run: cargo build --all --all-features

datafusion-federation/src/plan_node.rs

@@ -16,8 +16,8 @@ use datafusion::{
 };
 
 pub struct FederatedPlanNode {
-    pub plan: LogicalPlan,
-    pub planner: Arc<dyn FederationPlanner>,
+    pub(crate) plan: LogicalPlan,
+    pub(crate) planner: Arc<dyn FederationPlanner>,
 }
 
 impl FederatedPlanNode {
@@ -28,6 +28,10 @@ impl FederatedPlanNode {
     pub fn plan(&self) -> &LogicalPlan {
         &self.plan
     }
+
+    pub fn planner(&self) -> &Arc<dyn FederationPlanner> {
+        &self.planner
+    }
 }
 
 impl Debug for FederatedPlanNode {

datafusion-federation/src/sql/mod.rs

@@ -12,12 +12,9 @@ use analyzer::{collect_known_rewrites, RewriteTableScanAnalyzer};
 use ast_analyzer::RewriteMultiTableReference;
 use async_trait::async_trait;
 use datafusion::{
-    common::DFSchema,
     arrow::datatypes::{Schema, SchemaRef},
-    common::{
-        tree_node::TreeNode,
-        Statistics,
-    },
+    common::DFSchema,
+    common::{tree_node::TreeNode, Statistics},
     config::ConfigOptions,
     error::{DataFusionError, Result},
     execution::{context::SessionState, TaskContext},
@@ -37,11 +34,11 @@ use datafusion::{
 };
 use optimizer::{OptimizeProjectionsFederation, PushDownFilterFederation};
 
+pub use ast_analyzer::{AstAnalyzer, AstAnalyzerRule};
 pub use executor::{LogicalOptimizer, SQLExecutor, SQLExecutorRef, SqlQueryRewriter};
 pub use schema::{MultiSchemaProvider, SQLSchemaProvider};
 pub use table::{RemoteTable, SQLTable, SQLTableSource};
 pub use table_reference::{MultiPartTableReference, RemoteTableRef};
-pub use ast_analyzer::{AstAnalyzer, AstAnalyzerRule};
 
 use crate::{
     get_table_source, schema_cast, FederatedPlanNode, FederationAnalyzerForLogicalPlan,
@@ -61,7 +58,7 @@ pub fn federation_analyzer_rule() -> FederationAnalyzerRule {
 #[derive(Debug)]
 pub struct SQLFederationProvider {
     analyzer: Arc<Analyzer>,
-    pub executor: Arc<dyn SQLExecutor>,
+    pub(crate) executor: Arc<dyn SQLExecutor>,
 }
 
 impl SQLFederationProvider {
@@ -73,6 +70,10 @@ impl SQLFederationProvider {
             executor,
         }
     }
+
+    pub fn executor(&self) -> &Arc<dyn SQLExecutor> {
+        &self.executor
+    }
 }
 
 impl FederationProvider for SQLFederationProvider {
@@ -134,13 +135,17 @@ impl AnalyzerRule for SQLFederationAnalyzerRule {
 
 #[derive(Debug)]
 pub struct SQLFederationPlanner {
-    pub executor: Arc<dyn SQLExecutor>,
+    pub(crate) executor: Arc<dyn SQLExecutor>,
 }
 
 impl SQLFederationPlanner {
     pub fn new(executor: Arc<dyn SQLExecutor>) -> Self {
         Self { executor }
     }
+
+    pub fn executor(&self) -> &Arc<dyn SQLExecutor> {
+        &self.executor
+    }
 }
 
 #[async_trait]
@@ -323,11 +328,11 @@ impl DisplayAs for VirtualExecutionPlan {
             write!(f, " base_sql={statement}")?;
         }
 
-        let (logical_optimizers, ast_analyzers, _sql_query_rewriters) = match gather_analyzers(&plan)
-        {
-            Ok(analyzers) => analyzers,
-            Err(_) => return Ok(()),
-        };
+        let (logical_optimizers, ast_analyzers, _sql_query_rewriters) =
+            match gather_analyzers(&plan) {
+                Ok(analyzers) => analyzers,
+                Err(_) => return Ok(()),
+            };
 
         let old_plan = plan.clone();
 
@@ -471,9 +476,7 @@ mod tests {
     use datafusion::prelude::Expr;
     use datafusion::sql::unparser::dialect::Dialect;
     use datafusion::sql::unparser::{self};
-    use datafusion::sql::TableReference;
     use datafusion::{
-    common::DFSchema,
         arrow::datatypes::{DataType, Field},
         datasource::TableProvider,
         execution::context::SessionContext,