Skip to content

Commit 6feee8e

Browse files
author
Zoran Cvetkov
committed
do not deserialize VID
1 parent 640e7d1 commit 6feee8e

File tree

11 files changed

+56
-52
lines changed

11 files changed

+56
-52
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,15 @@ impl EntityCache {
209209
// Always test the cache consistency in debug mode. The test only
210210
// makes sense when we were actually asked to read from the store
211211
debug_assert!(match scope {
212-
GetScope::Store => entity == self.store.get(key).unwrap().map(Arc::new),
212+
GetScope::Store => {
213+
let entity2 = entity.as_ref().map(|e| {
214+
let mut e1 = (**e).clone();
215+
// make sure the VID exist and then remove it for the comparison
216+
e1.remove("vid").unwrap();
217+
e1
218+
});
219+
entity2 == self.store.get(key).unwrap()
220+
}
213221
GetScope::InBlock => true,
214222
});
215223

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -926,10 +926,10 @@ impl Entity {
926926

927927
/// Sets the VID if it's not already set. Should be used only for tests.
928928
#[cfg(debug_assertions)]
929-
pub fn set_vid_if_empty(&mut self) {
930-
let vid = self.get("vid");
931-
if vid.is_none() {
932-
let _ = self.set_vid(100).expect("the vid should be set");
929+
pub fn set_vid_if_empty(&mut self, vid: i64) {
930+
let old_vid = self.get("vid");
931+
if old_vid.is_none() {
932+
let _ = self.set_vid(vid).expect("the vid should be set");
933933
}
934934
}
935935

‎store/postgres/src/relational_queries.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -539,14 +539,7 @@ 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 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)) {
542+
if let Some(column) = table.column(&SqlName::verbatim(key)) {
550543
match T::Value::from_column_value(&column.column_type, json) {
551544
Ok(value) if value.is_null() => None,
552545
Ok(value) => Some(Ok((Word::from(column.field.to_string()), value))),

‎store/test-store/src/store.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ lazy_static! {
103103
.into();
104104
[GENESIS_PTR.clone(), BLOCK_ONE.clone(), two, three]
105105
};
106+
static ref vid_seq: Mutex<i64> = Mutex::new(200i64);
106107
}
107108

108109
/// Run the `test` after performing `setup`. The result of `setup` is passed
@@ -423,7 +424,8 @@ pub async fn insert_entities(
423424
entities: Vec<(EntityType, Entity)>,
424425
) -> Result<(), StoreError> {
425426
let insert_ops = entities.into_iter().map(|(entity_type, mut data)| {
426-
data.set_vid_if_empty();
427+
let _ = data.set_vid(*vid_seq.lock().unwrap());
428+
*vid_seq.lock().unwrap() += 1;
427429
EntityOperation::Set {
428430
key: entity_type.key(data.id()),
429431
data,

‎store/test-store/tests/graph/entity_cache.rs

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -507,9 +507,9 @@ fn check_for_account_with_multiple_wallets() {
507507
causality_region: CausalityRegion::ONCHAIN,
508508
};
509509
let result = cache.load_related(&request).unwrap();
510-
let wallet_1 = create_wallet_entity("1", &account_id, 67_i32, 1);
511-
let wallet_2 = create_wallet_entity("2", &account_id, 92_i32, 2);
512-
let wallet_3 = create_wallet_entity("3", &account_id, 192_i32, 3);
510+
let wallet_1 = create_wallet_entity_no_vid("1", &account_id, 67_i32);
511+
let wallet_2 = create_wallet_entity_no_vid("2", &account_id, 92_i32);
512+
let wallet_3 = create_wallet_entity_no_vid("3", &account_id, 192_i32);
513513
let expeted_vec = vec![wallet_1, wallet_2, wallet_3];
514514

515515
assert_eq!(result, expeted_vec);
@@ -527,7 +527,7 @@ fn check_for_account_with_single_wallet() {
527527
causality_region: CausalityRegion::ONCHAIN,
528528
};
529529
let result = cache.load_related(&request).unwrap();
530-
let wallet_1 = create_wallet_entity("4", &account_id, 32_i32, 4);
530+
let wallet_1 = create_wallet_entity_no_vid("4", &account_id, 32_i32);
531531
let expeted_vec = vec![wallet_1];
532532

533533
assert_eq!(result, expeted_vec);
@@ -611,7 +611,7 @@ fn check_for_insert_async_store() {
611611
causality_region: CausalityRegion::ONCHAIN,
612612
};
613613
let result = cache.load_related(&request).unwrap();
614-
let wallet_1 = create_wallet_entity("4", &account_id, 32_i32, 4);
614+
let wallet_1 = create_wallet_entity_no_vid("4", &account_id, 32_i32);
615615
let wallet_2 = create_wallet_entity("5", &account_id, 79_i32, 12);
616616
let wallet_3 = create_wallet_entity("6", &account_id, 200_i32, 13);
617617
let expeted_vec = vec![wallet_1, wallet_2, wallet_3];
@@ -643,9 +643,9 @@ fn check_for_insert_async_not_related() {
643643
causality_region: CausalityRegion::ONCHAIN,
644644
};
645645
let result = cache.load_related(&request).unwrap();
646-
let wallet_1 = create_wallet_entity("1", &account_id, 67_i32, 1);
647-
let wallet_2 = create_wallet_entity("2", &account_id, 92_i32, 2);
648-
let wallet_3 = create_wallet_entity("3", &account_id, 192_i32, 3);
646+
let wallet_1 = create_wallet_entity_no_vid("1", &account_id, 67_i32);
647+
let wallet_2 = create_wallet_entity_no_vid("2", &account_id, 92_i32);
648+
let wallet_3 = create_wallet_entity_no_vid("3", &account_id, 192_i32);
649649
let expeted_vec = vec![wallet_1, wallet_2, wallet_3];
650650

651651
assert_eq!(result, expeted_vec);
@@ -681,8 +681,8 @@ fn check_for_update_async_related() {
681681
causality_region: CausalityRegion::ONCHAIN,
682682
};
683683
let result = cache.load_related(&request).unwrap();
684-
let wallet_2 = create_wallet_entity("2", &account_id, 92_i32, 2);
685-
let wallet_3 = create_wallet_entity("3", &account_id, 192_i32, 3);
684+
let wallet_2 = create_wallet_entity_no_vid("2", &account_id, 92_i32);
685+
let wallet_3 = create_wallet_entity_no_vid("3", &account_id, 192_i32);
686686
let expeted_vec = vec![new_data, wallet_2, wallet_3];
687687

688688
assert_eq!(result, expeted_vec);
@@ -711,8 +711,8 @@ fn check_for_delete_async_related() {
711711
causality_region: CausalityRegion::ONCHAIN,
712712
};
713713
let result = cache.load_related(&request).unwrap();
714-
let wallet_2 = create_wallet_entity("2", &account_id, 92_i32, 2);
715-
let wallet_3 = create_wallet_entity("3", &account_id, 192_i32, 3);
714+
let wallet_2 = create_wallet_entity_no_vid("2", &account_id, 92_i32);
715+
let wallet_3 = create_wallet_entity_no_vid("3", &account_id, 192_i32);
716716
let expeted_vec = vec![wallet_2, wallet_3];
717717

718718
assert_eq!(result, expeted_vec);
@@ -739,14 +739,12 @@ fn scoped_get() {
739739
let act5 = cache.get(&key5, GetScope::Store).unwrap();
740740
assert_eq!(Some(&wallet5), act5.as_ref().map(|e| e.as_ref()));
741741

742-
let mut wallet1a = wallet1.clone();
743-
wallet1a.set_vid(1).unwrap();
744742
// For an entity in the store, we can not get it `InBlock` but with
745743
// `Store`
746744
let act1 = cache.get(&key1, GetScope::InBlock).unwrap();
747745
assert_eq!(None, act1);
748746
let act1 = cache.get(&key1, GetScope::Store).unwrap();
749-
assert_eq!(Some(&wallet1a), act1.as_ref().map(|e| e.as_ref()));
747+
assert_eq!(Some(&wallet1), act1.as_ref().map(|e| e.as_ref()));
750748

751749
// Even after reading from the store, the entity is not visible with
752750
// `InBlock`
@@ -756,12 +754,11 @@ fn scoped_get() {
756754
let mut wallet1 = wallet1;
757755
wallet1.set("balance", 70).unwrap();
758756
cache.set(key1.clone(), wallet1.clone(), 0).unwrap();
759-
wallet1a = wallet1;
760-
wallet1a.set_vid(101).unwrap();
757+
wallet1.set_vid(101).unwrap();
761758
let act1 = cache.get(&key1, GetScope::InBlock).unwrap();
762-
assert_eq!(Some(&wallet1a), act1.as_ref().map(|e| e.as_ref()));
759+
assert_eq!(Some(&wallet1), act1.as_ref().map(|e| e.as_ref()));
763760
let act1 = cache.get(&key1, GetScope::Store).unwrap();
764-
assert_eq!(Some(&wallet1a), act1.as_ref().map(|e| e.as_ref()));
761+
assert_eq!(Some(&wallet1), act1.as_ref().map(|e| e.as_ref()));
765762
})
766763
}
767764

‎store/test-store/tests/graphql/query.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ async fn insert_test_entities(
425425
.map(|(typename, entities)| {
426426
let entity_type = schema.entity_type(typename).unwrap();
427427
entities.into_iter().map(move |mut data| {
428-
data.set_vid_if_empty();
428+
data.set_vid_if_empty(100);
429429
EntityOperation::Set {
430430
key: entity_type.key(data.id()),
431431
data,

‎store/test-store/tests/postgres/aggregation.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ use graph::{
2424
use graph_store_postgres::{Store as DieselStore, SubgraphStore};
2525
use test_store::{create_test_subgraph, run_test_sequentially, BLOCKS, LOGGER, METRICS_REGISTRY};
2626

27+
use crate::postgres::relational::scrub;
28+
2729
const SCHEMA: &str = r#"
2830
type Data @entity(timeseries: true) {
2931
id: Int8!
@@ -82,7 +84,7 @@ pub async fn insert(
8284
.map(|mut data| {
8385
let data_type = schema.entity_type("Data").unwrap();
8486
let key = data_type.key(data.id());
85-
data.set_vid_if_empty();
87+
data.set_vid_if_empty(100);
8688
EntityOperation::Set { data, key }
8789
})
8890
.collect();
@@ -294,11 +296,12 @@ fn simple() {
294296
let exp = stats_hour(&env.writable.input_schema());
295297
for i in 0..4 {
296298
let act = env.all_entities("Stats_hour", BLOCKS[i].number);
297-
let diff = entity_diff(&exp[i], &act).unwrap();
299+
let scrubbed = exp[i].iter().map(|it| scrub(it)).collect::<Vec<Entity>>();
300+
let diff = entity_diff(&scrubbed, &act).unwrap();
298301
if !diff.is_empty() {
299302
panic!("entities for BLOCKS[{}] differ:\n{}", i, diff);
300303
}
301-
assert_eq!(exp[i], act, "entities for BLOCKS[{}] are the same", i);
304+
assert_eq!(scrubbed, act, "entities for BLOCKS[{}] are the same", i);
302305
}
303306
})
304307
}

‎store/test-store/tests/postgres/relational.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,9 +494,10 @@ fn create_schema(conn: &mut PgConnection) -> Layout {
494494
.expect("Failed to create relational schema")
495495
}
496496

497-
fn scrub(entity: &Entity) -> Entity {
497+
pub fn scrub(entity: &Entity) -> Entity {
498498
let mut scrubbed = entity.clone();
499499
scrubbed.remove_null_fields();
500+
scrubbed.remove("vid");
500501
scrubbed
501502
}
502503

@@ -757,7 +758,7 @@ fn serialize_bigdecimal() {
757758
)
758759
.expect("Failed to read Scalar[one]")
759760
.unwrap();
760-
assert_entity_eq!(entity, actual);
761+
assert_entity_eq!(scrub(&entity), actual);
761762
}
762763
});
763764
}

‎store/test-store/tests/postgres/relational_bytes.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ use graph_store_postgres::{
2727

2828
use test_store::*;
2929

30+
use crate::postgres::relational::scrub;
31+
3032
const THINGS_GQL: &str = "
3133
type Thing @entity {
3234
id: Bytes!
@@ -265,7 +267,7 @@ fn find() {
265267

266268
// Happy path: find existing entity
267269
let entity = find_entity(conn, layout, ID).unwrap();
268-
assert_entity_eq!(BEEF_ENTITY.clone(), entity);
270+
assert_entity_eq!(scrub(&BEEF_ENTITY), entity);
269271
assert!(CausalityRegion::from_entity(&entity) == CausalityRegion::ONCHAIN);
270272

271273
// Find non-existing entity
@@ -330,7 +332,7 @@ fn update() {
330332
.expect("Failed to read Thing[deadbeef]")
331333
.unwrap();
332334

333-
assert_entity_eq!(entity, actual);
335+
assert_entity_eq!(scrub(&entity), actual);
334336
});
335337
}
336338

‎store/test-store/tests/postgres/store.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,7 @@ fn get_entity_1() {
357357
age: 67_i32,
358358
seconds_age: Value::BigInt(BigInt::from(2114359200)),
359359
weight: Value::BigDecimal(184.4.into()),
360-
coffee: false,
361-
vid: 0i64
360+
coffee: false
362361
};
363362
// "favorite_color" was set to `Null` earlier and should be absent
364363

@@ -384,7 +383,6 @@ fn get_entity_3() {
384383
seconds_age: Value::BigInt(BigInt::from(883612800)),
385384
weight: Value::BigDecimal(111.7.into()),
386385
coffee: false,
387-
vid: 3_i64,
388386
};
389387
// "favorite_color" was set to `Null` earlier and should be absent
390388

@@ -1319,7 +1317,7 @@ fn entity_changes_are_fired_and_forwarded_to_subscriptions() {
13191317
.iter()
13201318
.map(|(id, data)| {
13211319
let mut data = data.clone();
1322-
data.set_vid_if_empty();
1320+
data.set_vid_if_empty(100);
13231321
EntityOperation::Set {
13241322
key: USER_TYPE.parse_key(id.as_str()).unwrap(),
13251323
data,

0 commit comments

Comments
 (0)