Fix metadata merge in alter space exec for `null` prop cases

server/src/engine/core/space.rs

@@ -24,8 +24,6 @@
  *
 */
 
-use crate::engine::data::md_dict::DictGeneric;
-
 use {
     crate::engine::{
         core::{model::ModelView, ItemID, RWLIdx},
@@ -58,15 +56,6 @@ impl SpaceMeta {
             env: RWLIdx::new(env),
         }
     }
-    fn read_from_props(props: &mut DictGeneric) -> DatabaseResult<DictGeneric> {
-        match props.remove(SpaceMeta::KEY_ENV) {
-            Some(Some(DictEntryGeneric::Map(m))) if props.is_empty() => Ok(m),
-            None | Some(None) if props.is_empty() => Ok(IndexST::default()),
-            _ => {
-                return Err(DatabaseError::DdlSpaceBadProperty);
-            }
-        }
-    }
 }
 
 #[derive(Debug)]
@@ -86,6 +75,9 @@ impl ProcedureCreate {
 }
 
 impl Space {
+    pub fn empty() -> Self {
+        Space::new(Default::default(), SpaceMeta::with_env(into_dict! {}))
+    }
     #[inline(always)]
     pub fn new(mns: IndexST<ItemID, Arc<ModelView>>, meta: SpaceMeta) -> Self {
         Self {
@@ -103,7 +95,13 @@ impl Space {
     ) -> DatabaseResult<ProcedureCreate> {
         let space_name = ItemID::try_new(&space_name).ok_or(DatabaseError::SysBadItemID)?;
         // check env
-        let env = SpaceMeta::read_from_props(&mut props)?;
+        let env = match props.remove(SpaceMeta::KEY_ENV) {
+            Some(Some(DictEntryGeneric::Map(m))) if props.is_empty() => m,
+            Some(None) | None if props.is_empty() => IndexST::default(),
+            _ => {
+                return Err(DatabaseError::DdlSpaceBadProperty);
+            }
+        };
         Ok(ProcedureCreate {
             space_name,
             space: Self::new(
@@ -133,15 +131,20 @@ impl Space {
             mut updated_props,
         }: AlterSpace,
     ) -> DatabaseResult<()> {
-        let env = SpaceMeta::read_from_props(&mut updated_props)?;
         match gns._spaces().read().st_get_cloned(space_name.as_bytes()) {
             Some(space) => {
-                let mut space_meta = space.meta.env.write();
-                if md_dict::rmerge_metadata(&mut space_meta, env) {
-                    Ok(())
-                } else {
-                    Err(DatabaseError::DdlSpaceBadProperty)
+                let mut space_env = space.meta.env.write();
+                match updated_props.remove(SpaceMeta::KEY_ENV) {
+                    Some(Some(DictEntryGeneric::Map(env))) if updated_props.is_empty() => {
+                        if !md_dict::rmerge_metadata(&mut space_env, env) {
+                            return Err(DatabaseError::DdlSpaceBadProperty);
+                        }
+                    }
+                    Some(None) if updated_props.is_empty() => space_env.clear(),
+                    None => {}
+                    _ => return Err(DatabaseError::DdlSpaceBadProperty),
                 }
+                Ok(())
             }
             None => Err(DatabaseError::DdlSpaceNotFound),
         }

server/src/engine/core/tests/space/alter.rs

@@ -34,7 +34,7 @@ use crate::engine::{
 };
 
 #[test]
-fn alter_add_props() {
+fn alter_add_prop_env_var() {
     let gns = GlobalNS::empty();
     super::exec_create_empty_verify(&gns, "create space myspace");
     super::exec_alter_and_verify(
@@ -53,7 +53,7 @@ fn alter_add_props() {
 }
 
 #[test]
-fn alter_update_props() {
+fn alter_update_prop_env_var() {
     let gns = GlobalNS::empty();
     super::exec_create_and_verify(
         &gns,
@@ -81,7 +81,7 @@ fn alter_update_props() {
 }
 
 #[test]
-fn alter_remove_props() {
+fn alter_remove_prop_env_var() {
     let gns = GlobalNS::empty();
     super::exec_create_and_verify(
         &gns,
@@ -114,3 +114,21 @@ fn alter_nx() {
         |space| assert_eq!(space.unwrap_err(), DatabaseError::DdlSpaceNotFound),
     )
 }
+
+#[test]
+fn alter_remove_all_env() {
+    let gns = GlobalNS::empty();
+    super::exec_create_and_verify(
+        &gns,
+        "create space myspace with { env: { MY_NEW_PROP: 100 } }",
+        |space| {
+            assert_eq!(
+                space.unwrap().meta.env.read().get("MY_NEW_PROP").unwrap(),
+                &(HSData::UnsignedInt(100).into())
+            )
+        },
+    );
+    super::exec_alter_and_verify(&gns, "alter space myspace with { env: null }", |space| {
+        assert_eq!(space.unwrap(), &Space::empty())
+    })
+}

server/src/engine/core/tests/space/mod.rs

@@ -28,10 +28,7 @@ mod alter;
 mod create;
 
 use crate::engine::{
-    core::{
-        space::{Space, SpaceMeta},
-        GlobalNS,
-    },
+    core::{space::Space, GlobalNS},
     error::DatabaseResult,
     idx::STIndex,
     ql::{
@@ -87,9 +84,6 @@ fn exec_create_and_verify(gns: &GlobalNS, tok: &str, verify: impl Fn(DatabaseRes
 /// Creates an empty space with the given tokens
 fn exec_create_empty_verify(gns: &GlobalNS, tok: &str) {
     self::exec_create_and_verify(gns, tok, |space| {
-        assert_eq!(
-            space.unwrap(),
-            &Space::new(Default::default(), SpaceMeta::with_env(into_dict! {}))
-        );
+        assert_eq!(space.unwrap(), &Space::empty());
     });
 }

server/src/engine/data/tests/md_dict_tests.rs

@@ -25,7 +25,7 @@
 */
 
 use crate::engine::data::{
-    md_dict::{self, DictEntryGeneric, DictGeneric, MetaDict},
+    md_dict::{self, DictEntryGeneric, DictGeneric, MetaDict, MetaDictEntry},
     HSData,
 };
 
@@ -78,3 +78,26 @@ fn t_bad_patch() {
     assert!(!md_dict::rmerge_metadata(&mut current, new));
     assert_eq!(current, backup);
 }
+
+#[test]
+fn patch_null_out_dict() {
+    let mut current: MetaDict = into_dict! {
+        "a" => HSData::UnsignedInt(2),
+        "b" => HSData::UnsignedInt(3),
+        "z" => MetaDictEntry::Map(into_dict!(
+            "c" => HSData::UnsignedInt(1),
+            "d" => HSData::UnsignedInt(2)
+        )),
+    };
+    let expected: MetaDict = into_dict! {
+        "a" => HSData::UnsignedInt(2),
+        "b" => HSData::UnsignedInt(3),
+    };
+    let new: DictGeneric = into_dict! {
+        "a" => Some(HSData::UnsignedInt(2).into()),
+        "b" => Some(HSData::UnsignedInt(3).into()),
+        "z" => None,
+    };
+    assert!(md_dict::rmerge_metadata(&mut current, new));
+    assert_eq!(current, expected);
+}