Revert to a85804b: Restore dictionary coercion in min_max.rs

Author: kosiew

datafusion-cli/tests/snapshots/cli_explain_environment_overrides@explain_plan_environment_overrides.snap

@@ -18,19 +18,19 @@ exit_code: 0
 | logical_plan | [                                       |
 |              |   {                                     |
 |              |     "Plan": {                           |
-|              |       "Node Type": "Projection",        |
 |              |       "Expressions": [                  |
 |              |         "Int64(123)"                    |
 |              |       ],                                |
+|              |       "Node Type": "Projection",        |
+|              |       "Output": [                       |
+|              |         "Int64(123)"                    |
+|              |       ],                                |
 |              |       "Plans": [                        |
 |              |         {                               |
 |              |           "Node Type": "EmptyRelation", |
-|              |           "Plans": [],                  |
-|              |           "Output": []                  |
+|              |           "Output": [],                 |
+|              |           "Plans": []                   |
 |              |         }                               |
-|              |       ],                                |
-|              |       "Output": [                       |
-|              |         "Int64(123)"                    |
 |              |       ]                                 |
 |              |     }                                   |
 |              |   }                                     |

datafusion/core/tests/sql/aggregates/basic.rs

@@ -448,35 +448,28 @@ async fn min_max_dictionary_uses_planned_dictionary_path() -> Result<()> {
     let ctx =
         SessionContext::new_with_config(SessionConfig::new().with_target_partitions(2));
 
-    fn dictionary_schema() -> Arc<Schema> {
-        let dict_type =
-            DataType::Dictionary(Box::new(DataType::Int32), Box::new(DataType::Utf8));
-        Arc::new(Schema::new(vec![Field::new("dict", dict_type, true)]))
-    }
-
-    fn dictionary_record_batch(
-        schema: Arc<Schema>,
-        values: &[&str],
-        keys: &[Option<i32>],
-    ) -> Result<RecordBatch> {
-        Ok(RecordBatch::try_new(
-            schema,
-            vec![Arc::new(DictionaryArray::new(
-                Int32Array::from(keys.to_vec()),
-                Arc::new(StringArray::from(values.to_vec())),
-            ))],
-        )?)
-    }
-
-    let schema = dictionary_schema();
-
-    let batch1 = dictionary_record_batch(
+    let dict_type =
+        DataType::Dictionary(Box::new(DataType::Int32), Box::new(DataType::Utf8));
+    let schema = Arc::new(Schema::new(vec![Field::new(
+        "dict",
+        dict_type.clone(),
+        true,
+    )]));
+
+    let batch1 = RecordBatch::try_new(
+        schema.clone(),
+        vec![Arc::new(DictionaryArray::new(
+            Int32Array::from(vec![Some(1), Some(1), None]),
+            Arc::new(StringArray::from(vec!["a", "z", "zz_unused"])),
+        ))],
+    )?;
+    let batch2 = RecordBatch::try_new(
         schema.clone(),
-        &["a", "z", "zz_unused"],
-        &[Some(1), Some(1), None],
+        vec![Arc::new(DictionaryArray::new(
+            Int32Array::from(vec![Some(0), Some(1)]),
+            Arc::new(StringArray::from(vec!["a", "d"])),
+        ))],
     )?;
-    let batch2 =
-        dictionary_record_batch(schema.clone(), &["a", "d"], &[Some(0), Some(1)])?;
     let provider = MemTable::try_new(schema, vec![vec![batch1], vec![batch2]])?;
     ctx.register_table("t", Arc::new(provider))?;
 

datafusion/expr/src/logical_plan/display.rs

@@ -283,116 +283,6 @@ pub struct PgJsonVisitor<'a, 'b> {
     parent_ids: Vec<u32>,
 }
 
-const NODE_TYPE_KEY: &str = "Node Type";
-const PLANS_KEY: &str = "Plans";
-const OUTPUT_KEY: &str = "Output";
-
-fn ordered_keys(map: &serde_json::Map<String, serde_json::Value>) -> Vec<&str> {
-    let mut keys = map.keys().map(String::as_str).collect::<Vec<_>>();
-    keys.sort_unstable();
-
-    if !map.contains_key(NODE_TYPE_KEY) {
-        return keys;
-    }
-
-    let mut remaining_keys = Vec::with_capacity(keys.len().saturating_sub(1));
-    let mut has_plans_key = false;
-    let mut has_output_key = false;
-    for key in keys {
-        match key {
-            NODE_TYPE_KEY => {}
-            PLANS_KEY => has_plans_key = true,
-            OUTPUT_KEY => has_output_key = true,
-            _ => remaining_keys.push(key),
-        }
-    }
-
-    let mut ordered_keys = Vec::with_capacity(map.len());
-    ordered_keys.push(NODE_TYPE_KEY);
-    ordered_keys.extend(remaining_keys);
-    if has_plans_key {
-        ordered_keys.push(PLANS_KEY);
-    }
-    if has_output_key {
-        ordered_keys.push(OUTPUT_KEY);
-    }
-    ordered_keys
-}
-
-macro_rules! to_json_string {
-    ($value:expr) => {
-        serde_json::to_string($value).map_err(|e| DataFusionError::External(Box::new(e)))
-    };
-}
-
-fn push_indent(buf: &mut String, indent: usize) {
-    buf.push_str(&" ".repeat(indent * 2));
-}
-
-fn write_delimited_entries(
-    buf: &mut String,
-    indent: usize,
-    open: char,
-    close: char,
-    len: usize,
-    mut write_entry: impl FnMut(usize, &mut String) -> datafusion_common::Result<()>,
-) -> datafusion_common::Result<()> {
-    if len == 0 {
-        buf.push(open);
-        buf.push(close);
-        return Ok(());
-    }
-
-    buf.push(open);
-    buf.push('\n');
-    for idx in 0..len {
-        push_indent(buf, indent + 1);
-        write_entry(idx, buf)?;
-        if idx + 1 != len {
-            buf.push(',');
-        }
-        buf.push('\n');
-    }
-    push_indent(buf, indent);
-    buf.push(close);
-    Ok(())
-}
-
-fn write_ordered_json(
-    value: &serde_json::Value,
-    buf: &mut String,
-    indent: usize,
-) -> datafusion_common::Result<()> {
-    match value {
-        serde_json::Value::Null
-        | serde_json::Value::Bool(_)
-        | serde_json::Value::Number(_)
-        | serde_json::Value::String(_) => {
-            buf.push_str(&to_json_string!(value)?);
-        }
-        serde_json::Value::Array(values) => {
-            write_delimited_entries(buf, indent, '[', ']', values.len(), |idx, buf| {
-                write_ordered_json(&values[idx], buf, indent + 1)
-            })?;
-        }
-        serde_json::Value::Object(map) => {
-            let keys = ordered_keys(map);
-            write_delimited_entries(buf, indent, '{', '}', keys.len(), |idx, buf| {
-                let key = keys[idx];
-                let value = map
-                    .get(key)
-                    .ok_or_else(|| internal_datafusion_err!("Missing key in object!"))?;
-
-                buf.push_str(&to_json_string!(key)?);
-                buf.push_str(": ");
-                write_ordered_json(value, buf, indent + 1)
-            })?;
-        }
-    }
-
-    Ok(())
-}
-
 impl<'a, 'b> PgJsonVisitor<'a, 'b> {
     pub fn new(f: &'a mut fmt::Formatter<'b>) -> Self {
         Self {
@@ -802,9 +692,12 @@ impl<'n> TreeNodeVisitor<'n> for PgJsonVisitor<'_, '_> {
         } else {
             // This is the root node
             let plan = serde_json::json!([{"Plan": current_node}]);
-            let mut plan_str = String::new();
-            write_ordered_json(&plan, &mut plan_str, 0)?;
-            write!(self.f, "{plan_str}")?;
+            write!(
+                self.f,
+                "{}",
+                serde_json::to_string_pretty(&plan)
+                    .map_err(|e| DataFusionError::External(Box::new(e)))?
+            )?;
         }
 
         Ok(TreeNodeRecursion::Continue)

datafusion/expr/src/logical_plan/plan.rs

@@ -4593,30 +4593,6 @@ mod tests {
         Ok(())
     }
 
-    #[test]
-    fn test_display_pg_json_values_field_order() -> Result<()> {
-        let plan = LogicalPlanBuilder::values(vec![vec![lit(1_i64)]])?.build()?;
-
-        let output = plan.display_pg_json().to_string();
-        let node_type_pos = output
-            .find("\"Node Type\": \"Values\"")
-            .expect("expected Node Type key in pgjson output");
-        let values_pos = output
-            .find("\"Values\": \"(Int64(1))\"")
-            .expect("expected Values key in pgjson output");
-        let plans_pos = output
-            .find("\"Plans\": []")
-            .expect("expected Plans key in pgjson output");
-        let output_pos = output
-            .find("\"Output\": [")
-            .expect("expected Output key in pgjson output");
-
-        assert!(node_type_pos < values_pos);
-        assert!(values_pos < plans_pos);
-        assert!(plans_pos < output_pos);
-        Ok(())
-    }
-
     /// Tests for the Visitor trait and walking logical plan nodes
     #[derive(Debug, Default)]
     struct OkVisitor {