remove VID wrapping

Author: Zoran Cvetkov

graph/src/components/store/entity_cache.rs

@@ -7,7 +7,7 @@ use std::sync::Arc;
 use crate::cheap_clone::CheapClone;
 use crate::components::store::write::EntityModification;
 use crate::components::store::{self as s, Entity, EntityOperation};
-use crate::data::store::{EntityV, EntityValidationError, Id, IdType, IntoEntityIterator};
+use crate::data::store::{EntityValidationError, Id, IdType, IntoEntityIterator};
 use crate::prelude::ENV_VARS;
 use crate::schema::{EntityKey, InputSchema};
 use crate::util::intern::Error as InternError;
@@ -33,8 +33,8 @@ pub enum GetScope {
 #[derive(Debug, Clone)]
 enum EntityOp {
     Remove,
-    Update(EntityV),
-    Overwrite(EntityV),
+    Update(Entity),
+    Overwrite(Entity),
 }
 
 impl EntityOp {
@@ -45,7 +45,7 @@ impl EntityOp {
         use EntityOp::*;
         match (self, entity) {
             (Remove, _) => Ok(None),
-            (Overwrite(new), _) | (Update(new), None) => Ok(Some(new.e)),
+            (Overwrite(new), _) | (Update(new), None) => Ok(Some(new)),
             (Update(updates), Some(entity)) => {
                 let mut e = entity.borrow().clone();
                 e.merge_remove_null_fields(updates)?;
@@ -69,7 +69,7 @@ impl EntityOp {
         match self {
             // This is how `Overwrite` is constructed, by accumulating `Update` onto `Remove`.
             Remove => *self = Overwrite(update),
-            Update(current) | Overwrite(current) => current.e.merge(update.e),
+            Update(current) | Overwrite(current) => current.merge(update),
         }
     }
 }
@@ -288,9 +288,9 @@ impl EntityCache {
         ) -> Result<Option<Entity>, anyhow::Error> {
             match op {
                 EntityOp::Update(entity) | EntityOp::Overwrite(entity)
-                    if query.matches(key, &entity.e) =>
+                    if query.matches(key, &entity) =>
                 {
-                    Ok(Some(entity.e.clone()))
+                    Ok(Some(entity.clone()))
                 }
                 EntityOp::Remove => Ok(None),
                 _ => Ok(None),
@@ -371,7 +371,10 @@ impl EntityCache {
         // The next VID is based on a block number and a sequence within the block
         let vid = ((block as i64) << 32) + self.vid_seq as i64;
         self.vid_seq += 1;
-        let entity = EntityV::new(entity, vid);
+        let mut entity = entity;
+        let old_vid = entity.set_vid(vid).expect("the vid should be set");
+        // Make sure that there was no VID previously set for this entity.
+        assert!(old_vid.is_none());
 
         self.entity_op(key.clone(), EntityOp::Update(entity));
 
@@ -478,22 +481,19 @@ impl EntityCache {
                 // Entity was created
                 (None, EntityOp::Update(mut updates))
                 | (None, EntityOp::Overwrite(mut updates)) => {
-                    let vid = updates.vid;
-                    updates.e.remove_null_fields();
-                    let data = Arc::new(updates.e.clone());
+                    updates.remove_null_fields();
+                    let data = Arc::new(updates);
                     self.current.insert(key.clone(), Some(data.cheap_clone()));
                     Some(Insert {
                         key,
                         data,
                         block,
                         end: None,
-                        vid,
                     })
                 }
                 // Entity may have been changed
                 (Some(current), EntityOp::Update(updates)) => {
                     let mut data = current.as_ref().clone();
-                    let vid = updates.vid;
                     data.merge_remove_null_fields(updates)
                         .map_err(|e| key.unknown_attribute(e))?;
                     let data = Arc::new(data);
@@ -504,24 +504,21 @@ impl EntityCache {
                             data,
                             block,
                             end: None,
-                            vid,
                         })
                     } else {
                         None
                     }
                 }
                 // Entity was removed and then updated, so it will be overwritten
                 (Some(current), EntityOp::Overwrite(data)) => {
-                    let vid = data.vid;
-                    let data = Arc::new(data.e.clone());
+                    let data = Arc::new(data);
                     self.current.insert(key.clone(), Some(data.cheap_clone()));
                     if current != data {
                         Some(Overwrite {
                             key,
                             data,
                             block,
                             end: None,
-                            vid,
                         })
                     } else {
                         None

graph/src/components/store/mod.rs

@@ -30,7 +30,7 @@ use crate::cheap_clone::CheapClone;
 use crate::components::store::write::EntityModification;
 use crate::constraint_violation;
 use crate::data::store::scalar::Bytes;
-use crate::data::store::{EntityV, Id, IdList, Value};
+use crate::data::store::{Id, IdList, Value};
 use crate::data::value::Word;
 use crate::data_source::CausalityRegion;
 use crate::derive::CheapClone;
@@ -829,7 +829,7 @@ where
 pub enum EntityOperation {
     /// Locates the entity specified by `key` and sets its attributes according to the contents of
     /// `data`.  If no entity exists with this key, creates a new entity.
-    Set { key: EntityKey, data: EntityV },
+    Set { key: EntityKey, data: Entity },
 
     /// Removes an entity with the specified key, if one exists.
     Remove { key: EntityKey },

graph/src/components/store/write.rs

@@ -45,15 +45,13 @@ pub enum EntityModification {
         data: Arc<Entity>,
         block: BlockNumber,
         end: Option<BlockNumber>,
-        vid: i64,
     },
     /// Update the entity by overwriting it
     Overwrite {
         key: EntityKey,
         data: Arc<Entity>,
         block: BlockNumber,
         end: Option<BlockNumber>,
-        vid: i64,
     },
     /// Remove the entity
     Remove { key: EntityKey, block: BlockNumber },
@@ -69,7 +67,6 @@ pub struct EntityWrite<'a> {
     // The end of the block range for which this write is valid. The value
     // of `end` itself is not included in the range
     pub end: Option<BlockNumber>,
-    pub vid: i64,
 }
 
 impl std::fmt::Display for EntityWrite<'_> {
@@ -92,28 +89,24 @@ impl<'a> TryFrom<&'a EntityModification> for EntityWrite<'a> {
                 data,
                 block,
                 end,
-                vid,
             } => Ok(EntityWrite {
                 id: &key.entity_id,
                 entity: data,
                 causality_region: key.causality_region,
                 block: *block,
                 end: *end,
-                vid: *vid,
             }),
             EntityModification::Overwrite {
                 key,
                 data,
                 block,
                 end,
-                vid,
             } => Ok(EntityWrite {
                 id: &key.entity_id,
                 entity: &data,
                 causality_region: key.causality_region,
                 block: *block,
                 end: *end,
-                vid: *vid,
             }),
 
             EntityModification::Remove { .. } => Err(()),
@@ -220,13 +213,11 @@ impl EntityModification {
                 data,
                 block,
                 end,
-                vid,
             } => Ok(Insert {
                 key,
                 data,
                 block,
                 end,
-                vid,
             }),
             Remove { key, .. } => {
                 return Err(constraint_violation!(
@@ -280,23 +271,21 @@ impl EntityModification {
 }
 
 impl EntityModification {
-    pub fn insert(key: EntityKey, data: Entity, block: BlockNumber, vid: i64) -> Self {
+    pub fn insert(key: EntityKey, data: Entity, block: BlockNumber) -> Self {
         EntityModification::Insert {
             key,
             data: Arc::new(data),
             block,
             end: None,
-            vid,
         }
     }
 
-    pub fn overwrite(key: EntityKey, data: Entity, block: BlockNumber, vid: i64) -> Self {
+    pub fn overwrite(key: EntityKey, data: Entity, block: BlockNumber) -> Self {
         EntityModification::Overwrite {
             key,
             data: Arc::new(data),
             block,
             end: None,
-            vid,
         }
     }
 
@@ -1028,36 +1017,32 @@ mod test {
 
             let value = value.clone();
             let key = THING_TYPE.parse_key("one").unwrap();
-            let vid = 0;
+            let vid = 0i64;
             match value {
                 Ins(block) => EntityModification::Insert {
                     key,
-                    data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
+                    data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
                     block,
                     end: None,
-                    vid,
                 },
                 Ovw(block) => EntityModification::Overwrite {
                     key,
-                    data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
+                    data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
                     block,
                     end: None,
-                    vid,
                 },
                 Rem(block) => EntityModification::Remove { key, block },
                 InsC(block, end) => EntityModification::Insert {
                     key,
-                    data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
+                    data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
                     block,
                     end: Some(end),
-                    vid,
                 },
                 OvwC(block, end) => EntityModification::Overwrite {
                     key,
-                    data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
+                    data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
                     block,
                     end: Some(end),
-                    vid,
                 },
             }
         }

graph/src/data/store/mod.rs

@@ -735,9 +735,6 @@ where
 lazy_static! {
     /// The name of the id attribute, `"id"`
     pub static ref ID: Word = Word::from("id");
-
-    /// The name of the vid attribute, `"vid"`
-    pub static ref VID: Word = Word::from("vid");
 }
 
 /// An entity is represented as a map of attribute names to values.
@@ -922,14 +919,18 @@ impl Entity {
             .expect("the vid is set to a valid value")
     }
 
-    /// This version of the function returns 0 if the VID is not set. It should be
-    /// used only in the testing code for more lenient definition of entities.
+    /// Sets the VID of the entity. The previous one is returned.
+    pub fn set_vid(&mut self, value: i64) -> Result<Option<Value>, InternError> {
+        self.0.insert("vid", value.into())
+    }
+
+    /// Sets the VID if not set. Should be used only for tests.
     #[cfg(debug_assertions)]
-    pub fn vid_or_default(&self) -> i64 {
-        self.get("vid")
-            .unwrap_or(&Value::Int8(100))
-            .as_int8()
-            .expect("the vid is set to a valid value")
+    pub fn set_vid_if_empty(&mut self) {
+        let vid = self.get("vid");
+        if vid.is_none() {
+            let _ = self.set_vid(100).expect("the vid should be set");
+        }
     }
 
     /// Merges an entity update `update` into this entity.
@@ -946,8 +947,8 @@ impl Entity {
     /// If a key exists in both entities, the value from `update` is chosen.
     /// If a key only exists on one entity, the value from that entity is chosen.
     /// If a key is set to `Value::Null` in `update`, the key/value pair is removed.
-    pub fn merge_remove_null_fields(&mut self, update: EntityV) -> Result<(), InternError> {
-        for (key, value) in update.e.0.into_iter() {
+    pub fn merge_remove_null_fields(&mut self, update: Entity) -> Result<(), InternError> {
+        for (key, value) in update.into_iter() {
             match value {
                 Value::Null => self.0.remove(&key),
                 _ => self.0.insert(&key, value)?,
@@ -1098,19 +1099,6 @@ impl std::fmt::Debug for Entity {
     }
 }
 
-/// An entity wrapper that has VID too.
-#[derive(Debug, Clone, CacheWeight, PartialEq, Eq, Serialize)]
-pub struct EntityV {
-    pub e: Entity,
-    pub vid: i64,
-}
-
-impl EntityV {
-    pub fn new(e: Entity, vid: i64) -> Self {
-        Self { e, vid }
-    }
-}
-
 /// An object that is returned from a query. It's a an `r::Value` which
 /// carries the attributes of the object (`__typename`, `id` etc.) and
 /// possibly a pointer to its parent if the query that constructed it is one

runtime/test/src/test.rs

@@ -482,11 +482,11 @@ fn make_thing(id: &str, value: &str, vid: i64) -> (String, EntityModification) {
         static ref SCHEMA: InputSchema = InputSchema::raw(DOCUMENT, "doesntmatter");
         static ref THING_TYPE: EntityType = SCHEMA.entity_type("Thing").unwrap();
     }
-    let data = entity! { SCHEMA => id: id, value: value, extra: USER_DATA};
+    let data = entity! { SCHEMA => id: id, value: value, extra: USER_DATA, vid: vid };
     let key = THING_TYPE.parse_key(id).unwrap();
     (
         format!("{{ \"id\": \"{}\", \"value\": \"{}\"}}", id, value),
-        EntityModification::insert(key, data, 0, vid),
+        EntityModification::insert(key, data, 0),
     )
 }
 

server/index-node/src/resolver.rs

@@ -768,7 +768,7 @@ fn entity_changes_to_graphql(entity_changes: Vec<EntityOperation>) -> r::Value {
                     .push(key.entity_id);
             }
             EntityOperation::Set { key, data } => {
-                updates.entry(key.entity_type).or_default().push(data.e);
+                updates.entry(key.entity_type).or_default().push(data);
             }
         }
     }

store/postgres/src/relational.rs

@@ -64,7 +64,7 @@ use crate::{
     },
 };
 use graph::components::store::DerivedEntityQuery;
-use graph::data::store::{EntityV, Id, IdList, IdType, BYTES_SCALAR};
+use graph::data::store::{Id, IdList, IdType, BYTES_SCALAR};
 use graph::data::subgraph::schema::POI_TABLE;
 use graph::prelude::{
     anyhow, info, BlockNumber, DeploymentHash, Entity, EntityChange, EntityOperation, Logger,
@@ -697,10 +697,9 @@ impl Layout {
             let entity_id = data.id();
             processed_entities.insert((entity_type.clone(), entity_id.clone()));
 
-            let vid = data.vid();
             changes.push(EntityOperation::Set {
                 key: entity_type.key_in(entity_id, CausalityRegion::from_entity(&data)),
-                data: EntityV::new(data, vid),
+                data,
             });
         }