persist: fix schema stability migration (#30837)

As part of #30205 we
check if a new `arrow::DataType` is "at least as nullable" as the
previous type, and if so we update the registered data type in-place.

This check failed to account for the change we made a little while ago
where were use `arrow::DataType::List` for `ScalarType::Map`, instead of
`arrow::DataType::Map`.

### Motivation

Fixes MaterializeInc/database-issues#8834

### Tips for reviewer

<!--
Leave some tips for your reviewer, like:

    * The diff is much smaller if viewed with whitespace hidden.
    * [Some function/module/file] deserves extra attention.
* [Some function/module/file] is pure code movement and only needs a
skim.

Delete this section if no tips.
-->

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered. ([trigger-ci for additional test/nightly
runs](https://trigger-ci.dev.materialize.com/))
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
  <!-- Reference the design in the description. -->
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration or tests,
there is a companion cloud PR to account for those changes that is
tagged with the release-blocker label
([example](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [ ] If this PR includes major [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note),
I have pinged the relevant PM to schedule a changelog post.

src/persist-client/src/schema.rs

@@ -435,7 +435,12 @@ pub(crate) fn is_atleast_as_nullable(old: &DataType, new: &DataType) -> Result<(
             Ok(())
         }
         (DataType::List(old_field), DataType::List(new_field))
-        | (DataType::Map(old_field, _), DataType::Map(new_field, _)) => {
+        | (DataType::Map(old_field, _), DataType::Map(new_field, _))
+        // Note: We previously represented Maps as arrow::DataType::Map, but have since switched to
+        // using List-of-Structs since it's the same thing but better supported.
+        //
+        // See: <https://github.com/MaterializeInc/materialize/pull/28912>
+        | (DataType::Map(old_field, _), DataType::List(new_field)) => {
             if !check(old_field, new_field) {
                 anyhow::bail!("'{}' is now less nullable", old_field.name());
             }
@@ -974,4 +979,61 @@ mod tests {
             "'k': 'details': 'plan' is now less nullable"
         );
     }
+
+    #[mz_ore::test]
+    fn test_map_to_list_backwards_compatible() {
+        let map_type = DataType::Map(
+            Arc::new(Field::new(
+                "map_entries",
+                DataType::Struct(Fields::from(vec![
+                    Field::new("key", DataType::Utf8, false),
+                    Field::new("val", DataType::Utf8, true),
+                ])),
+                false,
+            )),
+            true,
+        );
+        let list_type = DataType::List(Arc::new(Field::new(
+            "map_entries",
+            DataType::Struct(Fields::from(vec![
+                Field::new("key", DataType::Utf8, false),
+                Field::new("val", DataType::Utf8, true),
+            ])),
+            false,
+        )));
+        assert_ok!(is_atleast_as_nullable(&map_type, &list_type));
+
+        let map_type_less_nullable = DataType::Map(
+            Arc::new(Field::new(
+                "map_entries",
+                DataType::Struct(Fields::from(vec![
+                    Field::new("key", DataType::Utf8, false),
+                    Field::new("val", DataType::Utf8, false),
+                ])),
+                false,
+            )),
+            true,
+        );
+        assert_ok!(is_atleast_as_nullable(&map_type_less_nullable, &list_type));
+
+        let list_type_less_nullable = DataType::List(Arc::new(Field::new(
+            "map_entries",
+            DataType::Struct(Fields::from(vec![
+                Field::new("key", DataType::Utf8, false),
+                Field::new("val", DataType::Utf8, false),
+            ])),
+            false,
+        )));
+        let result = is_atleast_as_nullable(&map_type, &list_type_less_nullable);
+        assert_err!(result);
+        assert_contains!(
+            result.unwrap_err().to_string_with_causes(),
+            "'map_entries': 'val' is now less nullable"
+        );
+
+        assert_ok!(is_atleast_as_nullable(
+            &map_type_less_nullable,
+            &list_type_less_nullable
+        ));
+    }
 }