Skip to content

Commit 3431f2c

Browse files
authored
store: Remove ability to recount entities (#4406)
We used to send very clever SQL to the database on every block to make it possible to force a recount of the entities in a subgraph by setting the entity_count to -1 or null. The SQL that got sent could be very complicated, and might introduce some overhead. This facility hasn't been used in a very long time, and since it might cause an overhead, it's better to remove it. If recounting entities is something that we do need, it should be added as a `graphman` command and not be something that might cause headaches on the busiest code path.
1 parent e7d1555 commit 3431f2c

File tree

2 files changed

+7
-49
lines changed

2 files changed

+7
-49
lines changed

store/postgres/src/deployment.rs

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,6 @@ pub fn transact_block(
362362
site: &Site,
363363
ptr: &BlockPtr,
364364
firehose_cursor: &FirehoseCursor,
365-
full_count_query: &str,
366365
count: i32,
367366
) -> Result<(), StoreError> {
368367
use crate::diesel::BoolExpressionMethods;
@@ -371,12 +370,7 @@ pub fn transact_block(
371370
// Work around a Diesel issue with serializing BigDecimals to numeric
372371
let number = format!("{}::numeric", ptr.number);
373372

374-
let count_sql = if count == 0 {
375-
// This amounts to a noop - the entity count does not change
376-
"entity_count".to_string()
377-
} else {
378-
entity_count_sql(full_count_query, count)
379-
};
373+
let count_sql = entity_count_sql(count);
380374

381375
let row_count = update(
382376
d::table.filter(d::id.eq(site.id)).filter(
@@ -1071,42 +1065,18 @@ pub fn create_deployment(
10711065
Ok(())
10721066
}
10731067

1074-
fn entity_count_sql(full_count_query: &str, count: i32) -> String {
1075-
// The big complication in this query is how to determine what the
1076-
// new entityCount should be. We want to make sure that if the entityCount
1077-
// is NULL or the special value `-1`, it gets recomputed. Using `-1` here
1078-
// makes it possible to manually set the `entityCount` to that value
1079-
// to force a recount; setting it to `NULL` is not desirable since
1080-
// `entityCount` on the GraphQL level is not nullable, and so setting
1081-
// `entityCount` to `NULL` could cause errors at that layer; temporarily
1082-
// returning `-1` is more palatable. To be exact, recounts have to be
1083-
// done here, from the subgraph writer.
1084-
//
1085-
// The first argument of `coalesce` will be `NULL` if the entity count
1086-
// is `NULL` or `-1`, forcing `coalesce` to evaluate its second
1087-
// argument, the query to count entities. In all other cases,
1088-
// `coalesce` does not evaluate its second argument
1089-
format!(
1090-
"coalesce((nullif(entity_count, -1)) + ({count}),
1091-
({full_count_query}))",
1092-
full_count_query = full_count_query,
1093-
count = count
1094-
)
1068+
fn entity_count_sql(count: i32) -> String {
1069+
format!("entity_count + ({count})")
10951070
}
10961071

1097-
pub fn update_entity_count(
1098-
conn: &PgConnection,
1099-
site: &Site,
1100-
full_count_query: &str,
1101-
count: i32,
1102-
) -> Result<(), StoreError> {
1072+
pub fn update_entity_count(conn: &PgConnection, site: &Site, count: i32) -> Result<(), StoreError> {
11031073
use subgraph_deployment as d;
11041074

11051075
if count == 0 {
11061076
return Ok(());
11071077
}
11081078

1109-
let count_sql = entity_count_sql(full_count_query, count);
1079+
let count_sql = entity_count_sql(count);
11101080
update(d::table.filter(d::id.eq(site.id)))
11111081
.set(d::entity_count.eq(sql(&count_sql)))
11121082
.execute(conn)?;

store/postgres/src/deployment_store.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,14 +1164,7 @@ impl DeploymentStore {
11641164
)?;
11651165
}
11661166

1167-
deployment::transact_block(
1168-
&conn,
1169-
&site,
1170-
block_ptr_to,
1171-
firehose_cursor,
1172-
layout.count_query.as_str(),
1173-
count,
1174-
)?;
1167+
deployment::transact_block(&conn, &site, block_ptr_to, firehose_cursor, count)?;
11751168

11761169
Ok(event)
11771170
})
@@ -1227,12 +1220,7 @@ impl DeploymentStore {
12271220
// changes that might need to be reverted
12281221
Layout::revert_metadata(conn, &site, block)?;
12291222

1230-
deployment::update_entity_count(
1231-
conn,
1232-
site.as_ref(),
1233-
layout.count_query.as_str(),
1234-
count,
1235-
)?;
1223+
deployment::update_entity_count(conn, site.as_ref(), count)?;
12361224
Ok(event)
12371225
})
12381226
})?;

0 commit comments

Comments
 (0)