fix: address review feedback — proto format roundtrip, use-after-move, metrics null, preserve_order

- statement.rs: match on `&format` to avoid use-after-move before
  storing the value in Analyze { format, .. }
- display.rs: omit metrics fields entirely when plan.metrics() returns
  None instead of emitting "Metrics": null
- physical-plan/Cargo.toml: promote serde_json preserve_order to main
  dep so key insertion order is guaranteed in production builds too
- proto: add ExplainFormat field to AnalyzeNode (logical) so the format
  survives logical plan proto roundtrips (complements the AnalyzeExecNode
  fix already in the previous commit for the physical plan)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: adriangb

datafusion/physical-plan/Cargo.toml

@@ -85,7 +85,7 @@ log = { workspace = true }
 num-traits = { workspace = true }
 parking_lot = { workspace = true }
 pin-project-lite = "^0.2.7"
-serde_json = { workspace = true }
+serde_json = { workspace = true, features = ["preserve_order"] }
 tokio = { workspace = true }
 
 [dev-dependencies]
@@ -97,9 +97,6 @@ insta = { workspace = true }
 rand = { workspace = true }
 rstest = { workspace = true }
 rstest_reuse = "0.7.0"
-# Ensure `pgjson_snapshot_of_sample_plan` sees insertion-order JSON output
-# regardless of feature unification with upstream consumers.
-serde_json = { workspace = true, features = ["preserve_order"] }
 tokio = { workspace = true, features = [
     "rt-multi-thread",
     "fs",

datafusion/physical-plan/src/display.rs

@@ -786,7 +786,6 @@ impl PgJsonExecutionPlanVisitor<'_> {
             return;
         }
         let Some(metrics) = plan.metrics() else {
-            object["Metrics"] = serde_json::json!(null);
             return;
         };
 

datafusion/proto-models/proto/datafusion.proto

@@ -230,6 +230,7 @@ message AnalyzeNode {
   // Statement-level override for `datafusion.explain.analyze_categories`.
   // Absent means "fall back to session config".
   optional datafusion_common.ExplainAnalyzeCategoriesNode analyze_categories = 4;
+  datafusion_common.ExplainFormat format = 5;
 }
 
 message ExplainNode {

datafusion/proto-models/src/generated/pbjson.rs

@@ -1191,6 +1191,9 @@ impl serde::Serialize for AnalyzeNode {
         if self.analyze_categories.is_some() {
             len += 1;
         }
+        if self.format != 0 {
+            len += 1;
+        }
         let mut struct_ser = serializer.serialize_struct("datafusion.AnalyzeNode", len)?;
         if let Some(v) = self.input.as_ref() {
             struct_ser.serialize_field("input", v)?;
@@ -1206,6 +1209,11 @@ impl serde::Serialize for AnalyzeNode {
         if let Some(v) = self.analyze_categories.as_ref() {
             struct_ser.serialize_field("analyzeCategories", v)?;
         }
+        if self.format != 0 {
+            let v = super::datafusion_common::ExplainFormat::try_from(self.format)
+                .map_err(|_| serde::ser::Error::custom(format!("Invalid variant {}", self.format)))?;
+            struct_ser.serialize_field("format", &v)?;
+        }
         struct_ser.end()
     }
 }
@@ -1222,6 +1230,7 @@ impl<'de> serde::Deserialize<'de> for AnalyzeNode {
             "analyzeLevel",
             "analyze_categories",
             "analyzeCategories",
+            "format",
         ];
 
         #[allow(clippy::enum_variant_names)]
@@ -1230,6 +1239,7 @@ impl<'de> serde::Deserialize<'de> for AnalyzeNode {
             Verbose,
             AnalyzeLevel,
             AnalyzeCategories,
+            Format,
         }
         impl<'de> serde::Deserialize<'de> for GeneratedField {
             fn deserialize<D>(deserializer: D) -> std::result::Result<GeneratedField, D::Error>
@@ -1255,6 +1265,7 @@ impl<'de> serde::Deserialize<'de> for AnalyzeNode {
                             "verbose" => Ok(GeneratedField::Verbose),
                             "analyzeLevel" | "analyze_level" => Ok(GeneratedField::AnalyzeLevel),
                             "analyzeCategories" | "analyze_categories" => Ok(GeneratedField::AnalyzeCategories),
+                            "format" => Ok(GeneratedField::Format),
                             _ => Err(serde::de::Error::unknown_field(value, FIELDS)),
                         }
                     }
@@ -1278,6 +1289,7 @@ impl<'de> serde::Deserialize<'de> for AnalyzeNode {
                 let mut verbose__ = None;
                 let mut analyze_level__ = None;
                 let mut analyze_categories__ = None;
+                let mut format__ = None;
                 while let Some(k) = map_.next_key()? {
                     match k {
                         GeneratedField::Input => {
@@ -1304,13 +1316,20 @@ impl<'de> serde::Deserialize<'de> for AnalyzeNode {
                             }
                             analyze_categories__ = map_.next_value()?;
                         }
+                        GeneratedField::Format => {
+                            if format__.is_some() {
+                                return Err(serde::de::Error::duplicate_field("format"));
+                            }
+                            format__ = Some(map_.next_value::<super::datafusion_common::ExplainFormat>()? as i32);
+                        }
                     }
                 }
                 Ok(AnalyzeNode {
                     input: input__,
                     verbose: verbose__.unwrap_or_default(),
                     analyze_level: analyze_level__,
                     analyze_categories: analyze_categories__,
+                    format: format__.unwrap_or_default(),
                 })
             }
         }

datafusion/proto-models/src/generated/prost.rs

@@ -354,6 +354,8 @@ pub struct AnalyzeNode {
     pub analyze_categories: ::core::option::Option<
         super::datafusion_common::ExplainAnalyzeCategoriesNode,
     >,
+    #[prost(enumeration = "super::datafusion_common::ExplainFormat", tag = "5")]
+    pub format: i32,
 }
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct ExplainNode {

datafusion/proto/src/logical_plan/mod.rs

@@ -874,12 +874,26 @@ impl AsLogicalPlan for LogicalPlanNode {
                     .as_ref()
                     .map(explain_analyze_categories_from_proto)
                     .transpose()?;
+                let pb_format = protobuf::ExplainFormat::try_from(analyze.format)
+                    .map_err(|_| {
+                        proto_error(format!(
+                            "Received an AnalyzeNode message with unknown ExplainFormat {}",
+                            analyze.format
+                        ))
+                    })?;
+                let analyze_format = match pb_format {
+                    protobuf::ExplainFormat::Indent => ExplainFormat::Indent,
+                    protobuf::ExplainFormat::Tree => ExplainFormat::Tree,
+                    protobuf::ExplainFormat::Pgjson => ExplainFormat::PostgresJSON,
+                    protobuf::ExplainFormat::Graphviz => ExplainFormat::Graphviz,
+                };
                 let explain_option =
                     datafusion_expr::logical_plan::ExplainOption::default()
                         .with_verbose(analyze.verbose)
                         .with_analyze(true)
                         .with_analyze_level(analyze_level)
-                        .with_analyze_categories(analyze_categories);
+                        .with_analyze_categories(analyze_categories)
+                        .with_format(analyze_format);
                 LogicalPlanBuilder::from(input)
                     .explain_option_format(explain_option)?
                     .build()
@@ -1878,6 +1892,16 @@ impl AsLogicalPlan for LogicalPlanNode {
                                 .analyze_categories
                                 .as_ref()
                                 .map(explain_analyze_categories_to_proto),
+                            format: match &a.format {
+                                ExplainFormat::Indent => protobuf::ExplainFormat::Indent,
+                                ExplainFormat::Tree => protobuf::ExplainFormat::Tree,
+                                ExplainFormat::PostgresJSON => {
+                                    protobuf::ExplainFormat::Pgjson
+                                }
+                                ExplainFormat::Graphviz => {
+                                    protobuf::ExplainFormat::Graphviz
+                                }
+                            } as i32,
                         },
                     ))),
                 })

datafusion/sql/src/statement.rs

@@ -2066,7 +2066,7 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
         };
 
         if analyze {
-            match format {
+            match &format {
                 ExplainFormat::Indent | ExplainFormat::PostgresJSON => {}
                 ExplainFormat::Tree | ExplainFormat::Graphviz => {
                     return plan_err!(