Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework generation of the VID #5737

Open
wants to merge 27 commits into
base: zoran/rename-structs
Choose a base branch
from

Conversation

zorancv
Copy link
Contributor

@zorancv zorancv commented Dec 18, 2024

This PR changes the generation of the VID in such a way that entities have an order determined by the order of the blockchain transactions from which they originate, rather than to be ordered by the current implementation of the graph-node and the VID autogenerated by the database. This in turn will ensure that the dependant subgraphs would have a deterministic order of their input entities and it would be consistent across different graph-node deployments producing the same POI.

@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch from 1bbef27 to 5910d01 Compare December 18, 2024 15:47
@zorancv zorancv self-assigned this Dec 18, 2024
@zorancv zorancv added this to the Subgraph Composition milestone Dec 18, 2024
@zorancv zorancv force-pushed the zoran/rename-structs branch from 632ef9f to 9658d8c Compare December 18, 2024 18:40
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch from 9e6aa6d to 02ce854 Compare December 18, 2024 18:41
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch from 02ce854 to 62d1e2b Compare January 3, 2025 14:25
@zorancv zorancv marked this pull request as draft January 16, 2025 16:42
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch 2 times, most recently from 11c68a7 to 950a0fd Compare January 16, 2025 23:14
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch 7 times, most recently from 5151b66 to b6f7492 Compare January 19, 2025 17:31
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch from b6f7492 to 640e7d1 Compare January 19, 2025 17:34
@zorancv zorancv changed the base branch from zoran/rename-structs to master January 19, 2025 17:36
@zorancv zorancv changed the base branch from master to zoran/rename-structs January 19, 2025 17:37
@zorancv
Copy link
Contributor Author

zorancv commented Jan 20, 2025

A general question: we already require that every Entity has an id, if it doesn't, things will fail in funky ways. Wouldn't it be simpler to just treat the vid as another attribute of Entity, i.e., require that entity.get("vid") always returns something instead of keeping it separate from the entity?

@lutter I've decided to go that route. It simplifies the code a lot. Also I've added a check in the EntityCache set() that there was no VID previously set.

Finally when deserializing the entities from the database, I added deserialization of the the VID in addition to the list of fields from InputSchema, in order to keep the presence of the VID consistent in the Entity struct.

Those are two major changes since your last review.

@zorancv zorancv marked this pull request as ready for review January 20, 2025 16:20
@zorancv zorancv changed the base branch from zoran/rename-structs to zoran/subgraph-composition-rework-vid-wrap January 20, 2025 22:56
@zorancv zorancv changed the base branch from zoran/subgraph-composition-rework-vid-wrap to zoran/subgraph-composition-rework-vid-wrap5 January 20, 2025 22:56
@zorancv zorancv changed the base branch from zoran/subgraph-composition-rework-vid-wrap5 to zoran/rename-structs January 21, 2025 09:48
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch 3 times, most recently from 6feee8e to 14671db Compare January 21, 2025 14:34
@zorancv zorancv changed the base branch from zoran/rename-structs to zoran/subgraph-composition-rework-vid-wrap5 January 21, 2025 15:12
@zorancv zorancv closed this Jan 21, 2025
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch from 14671db to 640e7d1 Compare January 21, 2025 15:24
@zorancv zorancv reopened this Jan 21, 2025
@zorancv zorancv changed the base branch from zoran/subgraph-composition-rework-vid-wrap5 to zoran/rename-structs January 21, 2025 15:47
@zorancv zorancv force-pushed the zoran/subgraph-composition-rework-vid-wrap2 branch from 14671db to 640e7d1 Compare January 21, 2025 15:48
@zorancv zorancv requested a review from lutter January 21, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants