Private data sources table #3742
Conversation
|
Grafting from a V0 subgraph isn't working, I'll fix that. Edit: Fixed |
leoyvens
force-pushed
the
leo/impl-private-data-sources
branch
from
06b8f6d to
ed0cae4
Compare
lutter
left a comment
lutter
left a comment
There was a problem hiding this comment.
LGTM! I have a few detail questions, but assuming they aren't a real issue this is good to merge.
| pub(crate) fn revert(&self, conn: &PgConnection, block: BlockNumber) -> Result<(), StoreError> { | ||
| todo!() | ||
| // Use `@>` to leverage the gist index. | ||
| // This assumes all ranges are of the form [x, +inf). |
There was a problem hiding this comment.
If block ranges are always [x, +inf), it would be better to not use a range at all, and just have a block$ that stores x since that could be supported by a normal BTree index. But if we do need the flexibility to disable data sources afte ra certain block, this is ok.
There was a problem hiding this comment.
Yes this is trying to be future compatible with removing data sources by closing their range.
store/postgres/src/dynds/private.rs
Outdated
| let query = format!( | ||
| "\ | ||
| insert into {dst}(block_range, causality_region, manifest_idx, parent, id, param, context) | ||
| select int4range(lower(e.block_range), null), e.causality_region, e.manifest_idx, |
There was a problem hiding this comment.
Why not select just e.block_range instead of creating a new range?
There was a problem hiding this comment.
The desired behaviour for this to be future compatible with closed ranges would be to literally copy the range [x, y] if y <= target_block, but if y > target_block then we'd open the range with [x, null). Do you know what would be the right SQL to achieve this?
There was a problem hiding this comment.
You will have fun with case expressions, something like
case
when upper(e.block_range) <= $target_block then e.block_range
else int4range(lower(e.block_range), null)
endThere was a problem hiding this comment.
That seems to work, thanks! I've added a test to exercise this function, and put the debug_assert to use.
| debug_assert!( | ||
| self.load(conn, target_block).map_err(|e| e.to_string()) | ||
| == dst.load(conn, target_block).map_err(|e| e.to_string()) | ||
| ); |
leoyvens
force-pushed
the
leo/impl-private-data-sources
branch
4 times, most recently
from
fb2231d to
878f27d
Compare
leoyvens
force-pushed
the
leo/impl-private-data-sources
branch
2 times, most recently
from
2263b47 to
587b8e6
Compare
This required plumbing through a map between manifest index and template name for backwards compatibility with the shared data sources table.
leoyvens
force-pushed
the
leo/impl-private-data-sources
branch
from
587b8e6 to
2389f93
Compare
leoyvens
force-pushed
the
leo/impl-private-data-sources
branch
from
2389f93 to
be98a6d
Compare
This fills in the sql implementation in
dynds/private.rs, and sets V1 as the deployment schema version for new deployments.This maintains compatibility with the deployment schema V0, which uses the shared table. This does not implement migrations from V0 to V1, so copies are enabled for all versions.
The new stores template references by manifest index, but the shared table stores them by name and the runtime creates the by name, so a map between template name and index had to be plumbed into various places.
This was tested manually with uniswap-v3, and the
data_source_revertend-to-end test was modified to additionally test restarting a subgraph with dynamic data sources.Part of #3405, the only remaining work being migrations which we'll want to do eventually.