Skip to content

Commit 6e5b0bc

Browse files
author
Zoran Cvetkov
committed
remove VID wrapping
1 parent 6da9287 commit 6e5b0bc

File tree

17 files changed

+166
-173
lines changed

17 files changed

+166
-173
lines changed

‎graph/src/components/store/entity_cache.rs

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::sync::Arc;
77
use crate::cheap_clone::CheapClone;
88
use crate::components::store::write::EntityModification;
99
use crate::components::store::{self as s, Entity, EntityOperation};
10-
use crate::data::store::{EntityV, EntityValidationError, Id, IdType, IntoEntityIterator};
10+
use crate::data::store::{EntityValidationError, Id, IdType, IntoEntityIterator};
1111
use crate::prelude::ENV_VARS;
1212
use crate::schema::{EntityKey, InputSchema};
1313
use crate::util::intern::Error as InternError;
@@ -33,8 +33,8 @@ pub enum GetScope {
3333
#[derive(Debug, Clone)]
3434
enum EntityOp {
3535
Remove,
36-
Update(EntityV),
37-
Overwrite(EntityV),
36+
Update(Entity),
37+
Overwrite(Entity),
3838
}
3939

4040
impl EntityOp {
@@ -45,7 +45,7 @@ impl EntityOp {
4545
use EntityOp::*;
4646
match (self, entity) {
4747
(Remove, _) => Ok(None),
48-
(Overwrite(new), _) | (Update(new), None) => Ok(Some(new.e)),
48+
(Overwrite(new), _) | (Update(new), None) => Ok(Some(new)),
4949
(Update(updates), Some(entity)) => {
5050
let mut e = entity.borrow().clone();
5151
e.merge_remove_null_fields(updates)?;
@@ -69,7 +69,7 @@ impl EntityOp {
6969
match self {
7070
// This is how `Overwrite` is constructed, by accumulating `Update` onto `Remove`.
7171
Remove => *self = Overwrite(update),
72-
Update(current) | Overwrite(current) => current.e.merge(update.e),
72+
Update(current) | Overwrite(current) => current.merge(update),
7373
}
7474
}
7575
}
@@ -288,9 +288,9 @@ impl EntityCache {
288288
) -> Result<Option<Entity>, anyhow::Error> {
289289
match op {
290290
EntityOp::Update(entity) | EntityOp::Overwrite(entity)
291-
if query.matches(key, &entity.e) =>
291+
if query.matches(key, &entity) =>
292292
{
293-
Ok(Some(entity.e.clone()))
293+
Ok(Some(entity.clone()))
294294
}
295295
EntityOp::Remove => Ok(None),
296296
_ => Ok(None),
@@ -371,7 +371,8 @@ impl EntityCache {
371371
// The next VID is based on a block number and a sequence within the block
372372
let vid = ((block as i64) << 32) + self.vid_seq as i64;
373373
self.vid_seq += 1;
374-
let entity = EntityV::new(entity, vid);
374+
let mut entity = entity;
375+
let _ = entity.set_vid(vid).expect("the vid should be set");
375376

376377
self.entity_op(key.clone(), EntityOp::Update(entity));
377378

@@ -478,22 +479,19 @@ impl EntityCache {
478479
// Entity was created
479480
(None, EntityOp::Update(mut updates))
480481
| (None, EntityOp::Overwrite(mut updates)) => {
481-
let vid = updates.vid;
482-
updates.e.remove_null_fields();
483-
let data = Arc::new(updates.e.clone());
482+
updates.remove_null_fields();
483+
let data = Arc::new(updates);
484484
self.current.insert(key.clone(), Some(data.cheap_clone()));
485485
Some(Insert {
486486
key,
487487
data,
488488
block,
489489
end: None,
490-
vid,
491490
})
492491
}
493492
// Entity may have been changed
494493
(Some(current), EntityOp::Update(updates)) => {
495494
let mut data = current.as_ref().clone();
496-
let vid = updates.vid;
497495
data.merge_remove_null_fields(updates)
498496
.map_err(|e| key.unknown_attribute(e))?;
499497
let data = Arc::new(data);
@@ -504,24 +502,21 @@ impl EntityCache {
504502
data,
505503
block,
506504
end: None,
507-
vid,
508505
})
509506
} else {
510507
None
511508
}
512509
}
513510
// Entity was removed and then updated, so it will be overwritten
514511
(Some(current), EntityOp::Overwrite(data)) => {
515-
let vid = data.vid;
516-
let data = Arc::new(data.e.clone());
512+
let data = Arc::new(data);
517513
self.current.insert(key.clone(), Some(data.cheap_clone()));
518514
if current != data {
519515
Some(Overwrite {
520516
key,
521517
data,
522518
block,
523519
end: None,
524-
vid,
525520
})
526521
} else {
527522
None

‎graph/src/components/store/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use crate::cheap_clone::CheapClone;
3030
use crate::components::store::write::EntityModification;
3131
use crate::constraint_violation;
3232
use crate::data::store::scalar::Bytes;
33-
use crate::data::store::{EntityV, Id, IdList, Value};
33+
use crate::data::store::{Id, IdList, Value};
3434
use crate::data::value::Word;
3535
use crate::data_source::CausalityRegion;
3636
use crate::derive::CheapClone;
@@ -829,7 +829,7 @@ where
829829
pub enum EntityOperation {
830830
/// Locates the entity specified by `key` and sets its attributes according to the contents of
831831
/// `data`. If no entity exists with this key, creates a new entity.
832-
Set { key: EntityKey, data: EntityV },
832+
Set { key: EntityKey, data: Entity },
833833

834834
/// Removes an entity with the specified key, if one exists.
835835
Remove { key: EntityKey },

‎graph/src/components/store/write.rs

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,13 @@ pub enum EntityModification {
4545
data: Arc<Entity>,
4646
block: BlockNumber,
4747
end: Option<BlockNumber>,
48-
vid: i64,
4948
},
5049
/// Update the entity by overwriting it
5150
Overwrite {
5251
key: EntityKey,
5352
data: Arc<Entity>,
5453
block: BlockNumber,
5554
end: Option<BlockNumber>,
56-
vid: i64,
5755
},
5856
/// Remove the entity
5957
Remove { key: EntityKey, block: BlockNumber },
@@ -69,7 +67,6 @@ pub struct EntityWrite<'a> {
6967
// The end of the block range for which this write is valid. The value
7068
// of `end` itself is not included in the range
7169
pub end: Option<BlockNumber>,
72-
pub vid: i64,
7370
}
7471

7572
impl std::fmt::Display for EntityWrite<'_> {
@@ -92,28 +89,24 @@ impl<'a> TryFrom<&'a EntityModification> for EntityWrite<'a> {
9289
data,
9390
block,
9491
end,
95-
vid,
9692
} => Ok(EntityWrite {
9793
id: &key.entity_id,
9894
entity: data,
9995
causality_region: key.causality_region,
10096
block: *block,
10197
end: *end,
102-
vid: *vid,
10398
}),
10499
EntityModification::Overwrite {
105100
key,
106101
data,
107102
block,
108103
end,
109-
vid,
110104
} => Ok(EntityWrite {
111105
id: &key.entity_id,
112106
entity: &data,
113107
causality_region: key.causality_region,
114108
block: *block,
115109
end: *end,
116-
vid: *vid,
117110
}),
118111

119112
EntityModification::Remove { .. } => Err(()),
@@ -220,13 +213,11 @@ impl EntityModification {
220213
data,
221214
block,
222215
end,
223-
vid,
224216
} => Ok(Insert {
225217
key,
226218
data,
227219
block,
228220
end,
229-
vid,
230221
}),
231222
Remove { key, .. } => {
232223
return Err(constraint_violation!(
@@ -280,23 +271,21 @@ impl EntityModification {
280271
}
281272

282273
impl EntityModification {
283-
pub fn insert(key: EntityKey, data: Entity, block: BlockNumber, vid: i64) -> Self {
274+
pub fn insert(key: EntityKey, data: Entity, block: BlockNumber) -> Self {
284275
EntityModification::Insert {
285276
key,
286277
data: Arc::new(data),
287278
block,
288279
end: None,
289-
vid,
290280
}
291281
}
292282

293-
pub fn overwrite(key: EntityKey, data: Entity, block: BlockNumber, vid: i64) -> Self {
283+
pub fn overwrite(key: EntityKey, data: Entity, block: BlockNumber) -> Self {
294284
EntityModification::Overwrite {
295285
key,
296286
data: Arc::new(data),
297287
block,
298288
end: None,
299-
vid,
300289
}
301290
}
302291

@@ -1028,36 +1017,32 @@ mod test {
10281017

10291018
let value = value.clone();
10301019
let key = THING_TYPE.parse_key("one").unwrap();
1031-
let vid = 0;
1020+
let vid = 0i64;
10321021
match value {
10331022
Ins(block) => EntityModification::Insert {
10341023
key,
1035-
data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
1024+
data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
10361025
block,
10371026
end: None,
1038-
vid,
10391027
},
10401028
Ovw(block) => EntityModification::Overwrite {
10411029
key,
1042-
data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
1030+
data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
10431031
block,
10441032
end: None,
1045-
vid,
10461033
},
10471034
Rem(block) => EntityModification::Remove { key, block },
10481035
InsC(block, end) => EntityModification::Insert {
10491036
key,
1050-
data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
1037+
data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
10511038
block,
10521039
end: Some(end),
1053-
vid,
10541040
},
10551041
OvwC(block, end) => EntityModification::Overwrite {
10561042
key,
1057-
data: Arc::new(entity! { SCHEMA => id: "one", count: block }),
1043+
data: Arc::new(entity! { SCHEMA => id: "one", count: block, vid: vid }),
10581044
block,
10591045
end: Some(end),
1060-
vid,
10611046
},
10621047
}
10631048
}

‎graph/src/data/store/mod.rs

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -922,14 +922,26 @@ impl Entity {
922922
.expect("the vid is set to a valid value")
923923
}
924924

925+
pub fn set_vid(&mut self, value: i64) -> Result<Option<Value>, InternError> {
926+
self.0.insert("vid", value.into())
927+
}
928+
925929
/// This version of the function returns 0 if the VID is not set. It should be
926930
/// used only in the testing code for more lenient definition of entities.
931+
// #[cfg(debug_assertions)]
932+
// pub fn vid_or_default(&self) -> i64 {
933+
// self.get("vid")
934+
// .unwrap_or(&Value::Int8(100))
935+
// .as_int8()
936+
// .expect("the vid is set to a valid value")
937+
// }
938+
927939
#[cfg(debug_assertions)]
928-
pub fn vid_or_default(&self) -> i64 {
929-
self.get("vid")
930-
.unwrap_or(&Value::Int8(100))
931-
.as_int8()
932-
.expect("the vid is set to a valid value")
940+
pub fn set_vid_if_empty(&mut self) {
941+
let vid = self.get("vid");
942+
if vid.is_none() {
943+
let _ = self.set_vid(100).expect("the vid should be set");
944+
}
933945
}
934946

935947
/// Merges an entity update `update` into this entity.
@@ -946,8 +958,8 @@ impl Entity {
946958
/// If a key exists in both entities, the value from `update` is chosen.
947959
/// If a key only exists on one entity, the value from that entity is chosen.
948960
/// If a key is set to `Value::Null` in `update`, the key/value pair is removed.
949-
pub fn merge_remove_null_fields(&mut self, update: EntityV) -> Result<(), InternError> {
950-
for (key, value) in update.e.0.into_iter() {
961+
pub fn merge_remove_null_fields(&mut self, update: Entity) -> Result<(), InternError> {
962+
for (key, value) in update.into_iter() {
951963
match value {
952964
Value::Null => self.0.remove(&key),
953965
_ => self.0.insert(&key, value)?,
@@ -1098,19 +1110,6 @@ impl std::fmt::Debug for Entity {
10981110
}
10991111
}
11001112

1101-
/// An entity wrapper that has VID too.
1102-
#[derive(Debug, Clone, CacheWeight, PartialEq, Eq, Serialize)]
1103-
pub struct EntityV {
1104-
pub e: Entity,
1105-
pub vid: i64,
1106-
}
1107-
1108-
impl EntityV {
1109-
pub fn new(e: Entity, vid: i64) -> Self {
1110-
Self { e, vid }
1111-
}
1112-
}
1113-
11141113
/// An object that is returned from a query. It's a an `r::Value` which
11151114
/// carries the attributes of the object (`__typename`, `id` etc.) and
11161115
/// possibly a pointer to its parent if the query that constructed it is one

‎runtime/test/src/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -482,11 +482,11 @@ fn make_thing(id: &str, value: &str, vid: i64) -> (String, EntityModification) {
482482
static ref SCHEMA: InputSchema = InputSchema::raw(DOCUMENT, "doesntmatter");
483483
static ref THING_TYPE: EntityType = SCHEMA.entity_type("Thing").unwrap();
484484
}
485-
let data = entity! { SCHEMA => id: id, value: value, extra: USER_DATA};
485+
let data = entity! { SCHEMA => id: id, value: value, extra: USER_DATA, vid: vid };
486486
let key = THING_TYPE.parse_key(id).unwrap();
487487
(
488488
format!("{{ \"id\": \"{}\", \"value\": \"{}\"}}", id, value),
489-
EntityModification::insert(key, data, 0, vid),
489+
EntityModification::insert(key, data, 0),
490490
)
491491
}
492492

‎server/index-node/src/resolver.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,7 @@ fn entity_changes_to_graphql(entity_changes: Vec<EntityOperation>) -> r::Value {
768768
.push(key.entity_id);
769769
}
770770
EntityOperation::Set { key, data } => {
771-
updates.entry(key.entity_type).or_default().push(data.e);
771+
updates.entry(key.entity_type).or_default().push(data);
772772
}
773773
}
774774
}

‎store/postgres/src/relational.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ use crate::{
6464
},
6565
};
6666
use graph::components::store::DerivedEntityQuery;
67-
use graph::data::store::{EntityV, Id, IdList, IdType, BYTES_SCALAR};
67+
use graph::data::store::{Id, IdList, IdType, BYTES_SCALAR};
6868
use graph::data::subgraph::schema::POI_TABLE;
6969
use graph::prelude::{
7070
anyhow, info, BlockNumber, DeploymentHash, Entity, EntityChange, EntityOperation, Logger,
@@ -697,10 +697,9 @@ impl Layout {
697697
let entity_id = data.id();
698698
processed_entities.insert((entity_type.clone(), entity_id.clone()));
699699

700-
let vid = data.vid();
701700
changes.push(EntityOperation::Set {
702701
key: entity_type.key_in(entity_id, CausalityRegion::from_entity(&data)),
703-
data: EntityV::new(data, vid),
702+
data,
704703
});
705704
}
706705

‎store/postgres/src/relational_queries.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,14 @@ impl EntityData {
539539
// table column; those will be things like the
540540
// block_range that `select *` pulls in but that we
541541
// don't care about here
542-
if let Some(column) = table.column(&SqlName::verbatim(key)) {
542+
if key == "vid" {
543+
// VID is not in the input schema but we need it so deserialize it too
544+
match T::Value::from_column_value(&ColumnType::Int8, json) {
545+
Ok(value) if value.is_null() => None,
546+
Ok(value) => Some(Ok((Word::from("vid"), value))),
547+
Err(e) => Some(Err(e)),
548+
}
549+
} else if let Some(column) = table.column(&SqlName::verbatim(key)) {
543550
match T::Value::from_column_value(&column.column_type, json) {
544551
Ok(value) if value.is_null() => None,
545552
Ok(value) => Some(Ok((Word::from(column.field.to_string()), value))),
@@ -2450,7 +2457,8 @@ impl<'a> InsertRow<'a> {
24502457
}
24512458
let br_value = BlockRangeValue::new(table, row.block, row.end);
24522459
let causality_region = row.causality_region;
2453-
let vid = row.vid;
2460+
// println!("ROW: {:?}", row.entity);
2461+
let vid = row.entity.vid();
24542462
Ok(Self {
24552463
values,
24562464
br_value,

0 commit comments

Comments
 (0)