Require full entity path in `alter model`

server/src/engine/core/model/alt.rs

@@ -36,6 +36,7 @@ use {
             error::{DatabaseError, DatabaseResult},
             idx::{IndexST, IndexSTSeqCns, STIndex},
             ql::{
+                ast::Entity,
                 ddl::{
                     alt::{AlterKind, AlterModel},
                     syn::{ExpandedField, LayerSpec},
@@ -50,7 +51,7 @@ use {
 
 #[derive(Debug, PartialEq)]
 pub(in crate::engine::core) struct AlterPlan<'a> {
-    pub(in crate::engine::core) model: Ident<'a>,
+    pub(in crate::engine::core) model: Entity<'a>,
     pub(in crate::engine::core) no_lock: bool,
     pub(in crate::engine::core) action: AlterAction<'a>,
 }
@@ -246,13 +247,16 @@ impl<'a> AlterPlan<'a> {
 }
 
 impl ModelView {
-    pub fn exec_alter(gns: &GlobalNS, space: &[u8], alter: AlterModel) -> DatabaseResult<()> {
+    pub fn exec_alter(gns: &GlobalNS, alter: AlterModel) -> DatabaseResult<()> {
+        let Some((space, model)) = alter.model.into_full() else {
+            return Err(DatabaseError::ExpectedEntity);
+        };
         let gns = gns.spaces().read();
-        let Some(space) = gns.st_get(space) else {
+        let Some(space) = gns.st_get(space.as_bytes()) else {
             return Err(DatabaseError::DdlSpaceNotFound)
         };
         let space = space.models().read();
-        let Some(model) = space.st_get(alter.model.as_bytes()) else {
+        let Some(model) = space.st_get(model.as_bytes()) else {
             return Err(DatabaseError::DdlModelNotFound);
         };
         // make intent

server/src/engine/core/tests/model/alt.rs

@@ -56,8 +56,8 @@ fn exec_plan(
     exec_create(gns, model, "myspace", new_space)?;
     let tok = lex_insecure(plan.as_bytes()).unwrap();
     let alter = parse_ast_node_full::<AlterModel>(&tok[2..]).unwrap();
-    let model_name = alter.model;
-    ModelView::exec_alter(gns, "myspace".as_bytes(), alter)?;
+    let (_space, model_name) = alter.model.into_full().unwrap();
+    ModelView::exec_alter(gns, alter)?;
     let gns_read = gns.spaces().read();
     let space = gns_read.st_get("myspace".as_bytes()).unwrap();
     let model = space.models().read();
@@ -80,9 +80,9 @@ mod plan {
     fn simple_add() {
         super::plan(
             "create model mymodel(username: string, password: binary)",
-            "alter model mymodel add myfield { type: string, nullable: true }",
+            "alter model myspace.mymodel add myfield { type: string, nullable: true }",
             |plan| {
-                assert_eq!(plan.model.as_str(), "mymodel");
+                assert_eq!(plan.model.into_full().unwrap().1.as_str(), "mymodel");
                 assert!(plan.no_lock);
                 assert_eq!(
                     plan.action,
@@ -97,9 +97,9 @@ mod plan {
     fn simple_remove() {
         super::plan(
             "create model mymodel(username: string, password: binary, useless_field: uint8)",
-            "alter model mymodel remove useless_field",
+            "alter model myspace.mymodel remove useless_field",
             |plan| {
-                assert_eq!(plan.model.as_str(), "mymodel");
+                assert_eq!(plan.model.into_full().unwrap().1.as_str(), "mymodel");
                 assert!(plan.no_lock);
                 assert_eq!(
                     plan.action,
@@ -113,9 +113,9 @@ mod plan {
         // FREEDOM! DAMN THE PASSWORD!
         super::plan(
             "create model mymodel(username: string, password: binary)",
-            "alter model mymodel update password { nullable: true }",
+            "alter model myspace.mymodel update password { nullable: true }",
             |plan| {
-                assert_eq!(plan.model.as_str(), "mymodel");
+                assert_eq!(plan.model.into_full().unwrap().1.as_str(), "mymodel");
                 assert!(plan.no_lock);
                 assert_eq!(
                     plan.action,
@@ -131,9 +131,9 @@ mod plan {
         // FIGHT THE NULL
         super::plan(
             "create model mymodel(username: string, null password: binary)",
-            "alter model mymodel update password { nullable: false }",
+            "alter model myspace.mymodel update password { nullable: false }",
             |plan| {
-                assert_eq!(plan.model.as_str(), "mymodel");
+                assert_eq!(plan.model.into_full().unwrap().1.as_str(), "mymodel");
                 assert!(!plan.no_lock);
                 assert_eq!(
                     plan.action,
@@ -152,7 +152,7 @@ mod plan {
         assert_eq!(
             super::with_plan(
                 "create model mymodel(username: string, password: binary)",
-                "alter model mymodel remove password_e2e",
+                "alter model myspace.mymodel remove password_e2e",
                 |_| {}
             )
             .unwrap_err(),
@@ -164,7 +164,7 @@ mod plan {
         assert_eq!(
             super::with_plan(
                 "create model mymodel(username: string, password: binary)",
-                "alter model mymodel remove username",
+                "alter model myspace.mymodel remove username",
                 |_| {}
             )
             .unwrap_err(),
@@ -176,7 +176,7 @@ mod plan {
         assert_eq!(
             super::with_plan(
                 "create model mymodel(username: string, password: binary)",
-                "alter model mymodel add username { type: string }",
+                "alter model myspace.mymodel add username { type: string }",
                 |_| {}
             )
             .unwrap_err(),
@@ -188,7 +188,7 @@ mod plan {
         assert_eq!(
             super::with_plan(
                 "create model mymodel(username: string, password: binary)",
-                "alter model mymodel add password { type: string }",
+                "alter model myspace.mymodel add password { type: string }",
                 |_| {}
             )
             .unwrap_err(),
@@ -200,7 +200,7 @@ mod plan {
         assert_eq!(
             super::with_plan(
                 "create model mymodel(username: string, password: binary)",
-                "alter model mymodel update username { type: string }",
+                "alter model myspace.mymodel update username { type: string }",
                 |_| {}
             )
             .unwrap_err(),
@@ -212,7 +212,7 @@ mod plan {
         assert_eq!(
             super::with_plan(
                 "create model mymodel(username: string, password: binary)",
-                "alter model mymodel update username_secret { type: string }",
+                "alter model myspace.mymodel update username_secret { type: string }",
                 |_| {}
             )
             .unwrap_err(),
@@ -221,7 +221,7 @@ mod plan {
     }
     fn bad_type_cast(orig_ty: &str, new_ty: &str) {
         let create = format!("create model mymodel(username: string, silly_field: {orig_ty})");
-        let alter = format!("alter model mymodel update silly_field {{ type: {new_ty} }}");
+        let alter = format!("alter model myspace.mymodel update silly_field {{ type: {new_ty} }}");
         assert_eq!(
             super::with_plan(&create, &alter, |_| {}).expect_err(&format!(
                 "found no error in transformation: {orig_ty} -> {new_ty}"
@@ -350,7 +350,7 @@ mod exec {
             &gns,
             true,
             "create model mymodel(username: string, password: binary)",
-            "alter model mymodel update password { nullable: true }",
+            "alter model myspace.mymodel update password { nullable: true }",
             |model| {
                 let schema = model.intent_read_model();
                 assert!(schema.fields().st_get("password").unwrap().is_nullable());

server/src/engine/error.rs

@@ -88,6 +88,8 @@ pub enum DatabaseError {
     // query generic
     /// this needs an explicit lock
     NeedLock,
+    /// expected a full entity, but found a single implicit entity
+    ExpectedEntity,
     // ddl: create space
     /// unknown property or bad type for property
     DdlSpaceBadProperty,

server/src/engine/ql/ast/mod.rs

@@ -354,7 +354,7 @@ impl<'a> QueryData<'a> for SubstitutedData<'a> {
     AST
 */
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 /// An [`Entity`] represents the location for a specific structure, such as a model
 pub enum Entity<'a> {
     /// A single entity is used when switching to a model wrt the currently set space (commonly used
@@ -376,6 +376,20 @@ pub enum Entity<'a> {
 }
 
 impl<'a> Entity<'a> {
+    pub fn into_full(self) -> Option<(Ident<'a>, Ident<'a>)> {
+        if let Self::Full(a, b) = self {
+            Some((a, b))
+        } else {
+            None
+        }
+    }
+    pub fn into_single(self) -> Option<Ident<'a>> {
+        if let Self::Single(a) = self {
+            Some(a)
+        } else {
+            None
+        }
+    }
     #[inline(always)]
     /// Parse a full entity from the given slice
     ///

server/src/engine/ql/ddl/alt.rs

@@ -31,7 +31,7 @@ use {
             data::DictGeneric,
             error::{LangError, LangResult},
             ql::{
-                ast::{QueryData, State},
+                ast::{Entity, QueryData, State},
                 lex::{Ident, Token},
             },
         },
@@ -89,13 +89,13 @@ impl<'a> AlterSpace<'a> {
 
 #[derive(Debug, PartialEq)]
 pub struct AlterModel<'a> {
-    pub(in crate::engine) model: Ident<'a>,
+    pub(in crate::engine) model: Entity<'a>,
     pub(in crate::engine) kind: AlterKind<'a>,
 }
 
 impl<'a> AlterModel<'a> {
     #[inline(always)]
-    pub fn new(model: Ident<'a>, kind: AlterKind<'a>) -> Self {
+    pub fn new(model: Entity<'a>, kind: AlterKind<'a>) -> Self {
         Self { model, kind }
     }
 }
@@ -117,10 +117,7 @@ impl<'a> AlterModel<'a> {
             return compiler::cold_rerr(LangError::BadSyntax);
             // FIXME(@ohsayan): bad because no specificity
         }
-        let model_name = unsafe {
-            // UNSAFE(@ohsayan): did rounded check for ident in the above branch
-            state.fw_read().uck_read_ident()
-        };
+        let model_name = Entity::parse_from_state_rounded_result(state)?;
         let kind = match state.fw_read() {
             Token![add] => AlterKind::alter_add(state),
             Token![remove] => AlterKind::alter_remove(state),

server/src/engine/ql/tests/schema_tests.rs

@@ -765,7 +765,7 @@ mod dict_field_syntax {
 mod alter_model_remove {
     use super::*;
     use crate::engine::ql::{
-        ast::parse_ast_node_full,
+        ast::{parse_ast_node_full, Entity},
         ddl::alt::{AlterKind, AlterModel},
         lex::Ident,
     };
@@ -776,7 +776,7 @@ mod alter_model_remove {
         assert_eq!(
             remove,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Remove(Box::from([Ident::from("myfield")]))
             )
         );
@@ -788,7 +788,7 @@ mod alter_model_remove {
         assert_eq!(
             remove,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Remove(Box::from([Ident::from("myfield")]))
             )
         );
@@ -802,7 +802,7 @@ mod alter_model_remove {
         assert_eq!(
             remove,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Remove(Box::from([
                     Ident::from("myfield1"),
                     Ident::from("myfield2"),
@@ -816,7 +816,7 @@ mod alter_model_remove {
 mod alter_model_add {
     use super::*;
     use crate::engine::ql::{
-        ast::parse_ast_node_full,
+        ast::{parse_ast_node_full, Entity},
         ddl::{
             alt::{AlterKind, AlterModel},
             syn::{ExpandedField, LayerSpec},
@@ -833,7 +833,7 @@ mod alter_model_add {
         assert_eq!(
             parse_ast_node_full::<AlterModel>(&tok[2..]).unwrap(),
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Add(
                     [ExpandedField::new(
                         Ident::from("myfield"),
@@ -857,7 +857,7 @@ mod alter_model_add {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Add(
                     [ExpandedField::new(
                         Ident::from("myfield"),
@@ -883,7 +883,7 @@ mod alter_model_add {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Add(
                     [ExpandedField::new(
                         Ident::from("myfield"),
@@ -923,7 +923,7 @@ mod alter_model_add {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Add(
                     [
                         ExpandedField::new(
@@ -964,7 +964,7 @@ mod alter_model_add {
 mod alter_model_update {
     use super::*;
     use crate::engine::ql::{
-        ast::parse_ast_node_full,
+        ast::{parse_ast_node_full, Entity},
         ddl::{
             alt::{AlterKind, AlterModel},
             syn::{ExpandedField, LayerSpec},
@@ -983,7 +983,7 @@ mod alter_model_update {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Update(
                     [ExpandedField::new(
                         Ident::from("myfield"),
@@ -1007,7 +1007,7 @@ mod alter_model_update {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Update(
                     [ExpandedField::new(
                         Ident::from("myfield"),
@@ -1036,7 +1036,7 @@ mod alter_model_update {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Update(
                     [ExpandedField::new(
                         Ident::from("myfield"),
@@ -1070,7 +1070,7 @@ mod alter_model_update {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Update(
                     [
                         ExpandedField::new(
@@ -1113,7 +1113,7 @@ mod alter_model_update {
         assert_eq!(
             r,
             AlterModel::new(
-                Ident::from("mymodel"),
+                Entity::Single(Ident::from("mymodel")),
                 AlterKind::Update(
                     [
                         ExpandedField::new(