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.

Sign up for GitHub

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

feat: Allow new fields to be added locally to schema #1139

Merged
merged 12 commits into from
Mar 6, 2023
Merged

feat: Allow new fields to be added locally to schema #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
eb7c28b
Correct P2P error message
AndrewSisley Feb 24, 2023
e71a52d
Remove field kind decimal
AndrewSisley Feb 20, 2023
5eb5495
Remove field kind bytes
AndrewSisley Feb 20, 2023
4377581
Remove field embedded object kinds
AndrewSisley Feb 20, 2023
05d5078
Add documentation to FieldDesc.Kind
AndrewSisley Feb 24, 2023
362d204
Assert for collection name uniquenss within SDL
AndrewSisley Feb 16, 2023
7f6f92d
Correctly query all existing collections
AndrewSisley Feb 17, 2023
4b5d326
Make collection persistance transactional
AndrewSisley Feb 16, 2023
e8978ce
Make SetSchema (GQL) transactional
AndrewSisley Feb 16, 2023
12fb03e
Add support for adding fields to schema
AndrewSisley Feb 7, 2023
adf435b
PR FIXUP - Reword ValidateUpdateCollectionTxn docs
AndrewSisley Mar 6, 2023
27093dd
PR FIXUP - errors.Is and ds.ErrNotFound for err-if
AndrewSisley Mar 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion client/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,44 @@ import (
type DB interface {
AddSchema(context.Context, string) error

// PatchSchema takes the given JSON patch string and applies it to the set of CollectionDescriptions
// present in the database.
//
// It will also update the GQL types used by the query system. It will error and not apply any of the
// requested, valid updates should the net result of the patch result in an invalid state. The
// individual operations defined in the patch do not need to result in a valid state, only the net result
// of the full patch.
//
// The collections (including the schema version ID) will only be updated if any changes have actually
// been made, if the net result of the patch matches the current persisted description then no changes
// will be applied.
PatchSchema(context.Context, string) error

CreateCollection(context.Context, CollectionDescription) (Collection, error)
CreateCollectionTxn(context.Context, datastore.Txn, CollectionDescription) (Collection, error)

// UpdateCollectionTxn updates the persisted collection description matching the name of the given
// description, to the values in the given description.
//
// It will validate the given description using [ValidateUpdateCollectionTxn] before updating it.
//
// The collection (including the schema version ID) will only be updated if any changes have actually
// been made, if the given description matches the current persisted description then no changes will be
// applied.
UpdateCollectionTxn(context.Context, datastore.Txn, CollectionDescription) (Collection, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: UpdateCollection should take a name (string) as a parameter to make it clear what is the intended "collection" youre trying to update.

Obviously the description itself has the name too, but it should be more explicit from an API perspective.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I disagree with this quite strongly and it creates additional points of failure and confusion where the name parameter does not pair up with the description. Such a param can also be added later if we feel the need.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really convinced by that argument, but looking at the current system, CreateCollection also doesn't take a named value, so its good for now 👍.


// ValidateUpdateCollectionTxn validates that the given collection description is a valid update.
//
// Will return true if the given desctiption differs from the current persisted state of the
// collection. Will return an error if it fails validation.
ValidateUpdateCollectionTxn(context.Context, datastore.Txn, CollectionDescription) (bool, error)

GetCollectionByName(context.Context, string) (Collection, error)
GetCollectionByNameTxn(context.Context, datastore.Txn, string) (Collection, error)
GetCollectionBySchemaID(context.Context, string) (Collection, error)
GetAllCollections(ctx context.Context) ([]Collection, error)
GetCollectionBySchemaIDTxn(context.Context, datastore.Txn, string) (Collection, error)
GetAllCollections(context.Context) ([]Collection, error)
GetAllCollectionsTxn(context.Context, datastore.Txn) ([]Collection, error)

Root() datastore.RootStore
Blockstore() blockstore.Blockstore
Expand Down
22 changes: 11 additions & 11 deletions client/descriptions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,14 @@ const (
FieldKind_INT_ARRAY FieldKind = 5
FieldKind_FLOAT FieldKind = 6
FieldKind_FLOAT_ARRAY FieldKind = 7
FieldKind_DECIMAL FieldKind = 8
_ FieldKind = 8 // safe to repurpose (was never used)
_ FieldKind = 9 // safe to repurpose (previoulsy old field)
FieldKind_DATETIME FieldKind = 10
FieldKind_STRING FieldKind = 11
FieldKind_STRING_ARRAY FieldKind = 12
FieldKind_BYTES FieldKind = 13

// Embedded object within the type
FieldKind_OBJECT FieldKind = 14

// Array of embedded objects
FieldKind_OBJECT_ARRAY FieldKind = 15
_ FieldKind = 13 // safe to repurpose (was never used)
_ FieldKind = 14 // safe to repurpose (was never used)
_ FieldKind = 15 // safe to repurpose (was never used)

// Embedded object, but accessed via foreign keys
FieldKind_FOREIGN_OBJECT FieldKind = 16
Expand Down Expand Up @@ -185,8 +181,12 @@ func (f FieldID) String() string {

// FieldDescription describes a field on a Schema and its associated metadata.
type FieldDescription struct {
Name string
ID FieldID
Name string
ID FieldID

// The data type that this field holds.
//
// Must contain a valid value.
Kind FieldKind
Schema string // If the field is an OBJECT type, then it has a target schema
RelationName string // The name of the relation index if the field is of type FOREIGN_OBJECT
Expand All @@ -201,7 +201,7 @@ type FieldDescription struct {

// IsObject returns true if this field is an object type.
func (f FieldDescription) IsObject() bool {
return (f.Kind == FieldKind_OBJECT) || (f.Kind == FieldKind_FOREIGN_OBJECT) ||
return (f.Kind == FieldKind_FOREIGN_OBJECT) ||
(f.Kind == FieldKind_FOREIGN_OBJECT_ARRAY)
}

Expand Down
16 changes: 15 additions & 1 deletion client/p2p.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

package client

import "context"
import (
"context"

"github.com/sourcenetwork/defradb/datastore"
)

type P2P interface {
// SetReplicator adds a replicator to the persisted list or adds
Expand All @@ -27,10 +31,20 @@ type P2P interface {
// subscribes to to the the persisted list. It will error if the provided
// collection ID is invalid.
AddP2PCollection(ctx context.Context, collectionID string) error
// AddP2PCollectionTxn adds the given collection ID that the P2P system
// subscribes to to the the persisted list. It will error if the provided
// collection ID is invalid.
AddP2PCollectionTxn(ctx context.Context, txn datastore.Txn, collectionID string) error

// RemoveP2PCollection removes the given collection ID that the P2P system
// subscribes to from the the persisted list. It will error if the provided
// collection ID is invalid.
RemoveP2PCollection(ctx context.Context, collectionID string) error
// RemoveP2PCollectionTxn removes the given collection ID that the P2P system
// subscribes to from the the persisted list. It will error if the provided
// collection ID is invalid.
RemoveP2PCollectionTxn(ctx context.Context, txn datastore.Txn, collectionID string) error

// GetAllP2PCollections returns the list of persisted collection IDs that
// the P2P system subscribes to.
GetAllP2PCollections(ctx context.Context) ([]string, error)
Expand Down
3 changes: 2 additions & 1 deletion core/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/sourcenetwork/defradb/client"
"github.com/sourcenetwork/defradb/client/request"
"github.com/sourcenetwork/defradb/datastore"
)

// SchemaDefinition represents a schema definition.
Expand Down Expand Up @@ -49,5 +50,5 @@ type Parser interface {
ParseSDL(ctx context.Context, schemaString string) ([]client.CollectionDescription, error)

// Adds the given schema to this parser's model.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Is the documentation still valid for SetSchema?

Copy link
Contributor Author

@AndrewSisley AndrewSisley Mar 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, what it does conceptually has not changed. The only difference is that changes will only be applied if/until the txn is successfully committed.

The doc could probably be expanded to mention this (although it is somewhat a universal implicit-given for anything transaction-based), but it is internal, not public, and if left up to me laziness would probably win out. Do you want it expanded?

AddSchema(ctx context.Context, collections []client.CollectionDescription) error
SetSchema(ctx context.Context, txn datastore.Txn, collections []client.CollectionDescription) error
}
Loading