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

Restructure client.Description objects to correctly split Schema and Collections #1911

Closed
2 of 5 tasks
Tracked by #1025
AndrewSisley opened this issue Sep 25, 2023 · 0 comments
Closed
2 of 5 tasks
Tracked by #1025
Assignees
Labels
area/collections Related to the collections system refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Milestone

Comments

@AndrewSisley
Copy link
Contributor

AndrewSisley commented Sep 25, 2023

Proposed structure:

type CollectionDescription struct {
	Name string
	ID uint32
        GlobalID string // * ID to uniquely identify the collection on a network
	Description string // *
	SchemaVersionID string // **
	Fields []CollectionFieldDescription
	Indexes []IndexDescription
}
type CollectionFieldDescription struct {
	Name string
	ID FieldID // note: If all nodes are on exactly the same Defra version, this would be the same across all nodes, but it does not have to be - make sure this is documented
	Description string // *
	CollectionID uint32 // * for relations
	RelationName string // for relations
	RelationType RelationType // for relations, this type will be modified from the existing definition, it now only needs to say whether it is a one-one or a one-many
}
type SchemaDescription struct {
	SchemaID string
	Name string // note: this is just a label, // thought (from Fred/Keenan): This should probably not actually be defined here, and instead should be a local only thing, otherwise we increase the risk of clashes 
	VersionID string // thought (from Fred/Keenan):  This should probably just be `ID`, we could then rename this struct to `SchemaVersionDescription`
	VersionLabel string // *  // thought (from Fred/Keenan): This should probably not actually be defined here, and instead should be a local only thing, otherwise we increase the risk of clashes 
	Fields []SchemaFieldDescription
}
type SchemaFieldDescription struct {
	Name string
	Kind FieldKind
	CRDT CRDT // **
	SchemaID string // for relations
}
// * == new property
// ** == renamed

Additionally, PatchSchema will remain conceptually as-is, updating the schema object only - PatchCollection will be introduced to do the same for collection stuff. SetDefaultSchemaVersion will be removed, as it's functionality will be covered by PatchCollection.

Worth noting perhaps is that a SchemaFieldDescription will only ever exist for the primary side of a relation. CollectionFieldDescription will exist for both sides.

The pointer from CollectionDescription to SchemaDescription has been removed. I think this makes the separation and [lack of] ownership clearer, as well as making stuff like PatchSchema/Collection more obvious to the user. It may also force us to allow multiple collections from a single schema for free, as the data structure now demands it.

Tasks

Preview Give feedback
  1. area/query area/schema feature
  2. area/query area/schema feature
  3. 5 of 5
    area/collections refactor
    AndrewSisley
  4. area/schema refactor
    AndrewSisley
@AndrewSisley AndrewSisley added area/collections Related to the collections system refactor This issue specific to or requires *notable* refactoring of existing codebases and components labels Sep 25, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.8 milestone Sep 25, 2023
@AndrewSisley AndrewSisley self-assigned this Sep 25, 2023
@AndrewSisley AndrewSisley removed their assignment Sep 27, 2023
@AndrewSisley AndrewSisley removed this from the DefraDB v0.8 milestone Oct 24, 2023
@AndrewSisley AndrewSisley added this to the DefraDB v0.11 milestone Apr 15, 2024
@AndrewSisley AndrewSisley self-assigned this Apr 15, 2024
AndrewSisley added a commit that referenced this issue Apr 19, 2024
## Relevant issue(s)

Resolves #2451 #1911 #2408

## Description

Moves relation field properties onto collection description from schema
description.

This allows for one-sided relations to be defined via `PatchSchema`.
There is no way atm to create them using an SDL, but we could add a
directive or something at somepoint if we want.

As the adding of fields via `PatchCollection` has not yet been enabled,
this does prevent users from adding secondary relation fields to an
existing schema. I'll create a ticket before merging this to allow this,
it is my strong preference to not do that in this already large and
fiddly PR though.

As the internal codebase relies on the setting of secondary fields via
`client.Document`, `client.Document` now requires a collection, not just
the schema, when being constructed. We will likely want to find a way to
avoid that in the future.

This PR also moves some validation from the graphql package into `db`,
not all of it has been moved, but that is a long term wish of mine. The
`db` package validation can/should be improved further making rule reuse
across `PatchCollection`, `PatchSchema` and `CreateCollection` (SDL),
however I would rather not spend too much effort on that in this PR.
This includes moving it out of the `collection.go` file.

It might resolve #2380
but I'd rather not bring that into scope. If I'm waiting around for
reviews I might verify that here though.

This should conclude the transfer of local properties off of the schema
object :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/collections Related to the collections system refactor This issue specific to or requires *notable* refactoring of existing codebases and components
Projects
None yet
Development

No branches or pull requests

1 participant