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

Write data models are not used on read #1065

Closed
AndrewSisley opened this issue Jan 31, 2023 · 3 comments
Closed

Write data models are not used on read #1065

AndrewSisley opened this issue Jan 31, 2023 · 3 comments
Labels
area/db-system Related to the core system related components of the DB refactor This issue specific to or requires *notable* refactoring of existing codebases and components

Comments

@AndrewSisley
Copy link
Contributor

Stuff like LWWRegDelta is used on write, but not on read. We should have a shared set of models so we more can safely parse persisted stuff and see where it is being used. It should probably live in core or similar.

Most/all should have some separation between use and storage (i.e. don't move LWWRegDelta to core, define a new, shared struct there and map (like an entity)).

@AndrewSisley AndrewSisley added area/db-system Related to the core system related components of the DB refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels Jan 31, 2023
@jsimnz
Copy link
Member

jsimnz commented Jan 31, 2023

Remembered why we use the "raw" untyped marshaling in the planner code. Its primarily because we don't know what "type" of Delta object a given commit is (or at least the current code makes no effort to figure it out, possibly becasue that info isn't persisnted into the DAG, which is changing based on the new schema work). So we parse that common shared data that all deltas have like "priority" from a map[string]any object, using a generic approach.

In general, it would be nice to have stronger typings there.

I do think it's important to have a tight and consistent representation between the IPLD model, on-disk serialization, and programmatic type API, compared to separating the two like you would for an "application" model and "DB" model (from traditional API-like applications). But, there's a current exception (not suggesting it should stay or needs to be this way, just that it is) in the code here, where the CompositeDelta object doesn't match the on-disk serialization (see CompositeDelta.Marshal).

So there's definately some work important outlined/highlighted by this issue 👍

@AndrewSisley
Copy link
Contributor Author

Its primarily because we don't know what "type" of Delta object a given commit is

That's not a problem any shared strongly typed model can have all the props on (is a common issue in app dev too):

entity {
     field1FromComposite
     field1FromFieldCommit
     sharedField1
     ...
}

Can even wrap the type-specific stuff (e.g. field1FromComposite) with an Option. Or if the Go Json lib allows it, handle dynamic parsing to a range of types depending on the string contents (dotnet json allows for this).

@AndrewSisley
Copy link
Contributor Author

Issue is out of date, write code is nicer than it was, closing issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/db-system Related to the core system related components of the DB refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

No branches or pull requests

2 participants