Skip to content

Commit

Permalink
fix: Make return type for FieldKind_INT an int64 (#1982)
Browse files Browse the repository at this point in the history
## Relevant issue(s)

Resolves #1681 
Resolves #1981 

## Description

The PR ensures the return type for `FieldKind_INT` is always of the same
go type `int64`.

Fixing this also surface a small bug in the indexes where a broken index
should have returned an error but was returning an error for a different
reason. That reason being that the expected return type was `int64` but
the actual returned type was `uint64`.
  • Loading branch information
fredcarle authored Oct 19, 2023
1 parent 2dbfbd6 commit 9e1ad62
Show file tree
Hide file tree
Showing 86 changed files with 458 additions and 461 deletions.
11 changes: 8 additions & 3 deletions core/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,14 @@ func DecodeFieldValue(fieldDesc client.FieldDescription, val any) (any, error) {
case client.FieldKind_INT:
switch v := val.(type) {
case float64:
if v >= 0 {
return uint64(v), nil
}
return int64(v), nil
case int64:
return int64(v), nil
case int:
return int64(v), nil
case uint64:
return int64(v), nil
case uint:
return int64(v), nil
}
}
Expand Down
7 changes: 3 additions & 4 deletions datastore/multi.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@ var _ MultiStore = (*multistore)(nil)
func MultiStoreFrom(rootstore ds.Datastore) MultiStore {
rootRW := AsDSReaderWriter(rootstore)
ms := &multistore{
root: rootRW,
data: prefix(rootRW, dataStoreKey),
head: prefix(rootRW, headStoreKey),
// the `peers` prefix is assigned by the libp2p peerstore
root: rootRW,
data: prefix(rootRW, dataStoreKey),
head: prefix(rootRW, headStoreKey),
peer: namespace.Wrap(rootstore, peerStoreKey),
system: prefix(rootRW, systemStoreKey),
dag: NewDAGStore(prefix(rootRW, blockStoreKey)),
Expand Down
22 changes: 0 additions & 22 deletions db/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,6 @@ func (db *db) basicExport(ctx context.Context, txn datastore.Txn, config *client
return err
}

// Temporary until https://github.com/sourcenetwork/defradb/issues/1681 is resolved.
ensureIntIsInt(foreignCol.Schema().Fields, oldForeignDoc)

delete(oldForeignDoc, "_key")
if foreignDoc.Key().String() == foreignDocKey.String() {
delete(oldForeignDoc, field.Name+request.RelatedObjectID)
Expand Down Expand Up @@ -289,9 +286,6 @@ func (db *db) basicExport(ctx context.Context, txn datastore.Txn, config *client
return err
}

// Temporary until https://github.com/sourcenetwork/defradb/issues/1681 is resolved.
ensureIntIsInt(col.Schema().Fields, docM)

delete(docM, "_key")
if isSelfReference {
delete(docM, refFieldName)
Expand Down Expand Up @@ -374,19 +368,3 @@ func writeString(f *os.File, normal, pretty string, isPretty bool) error {
}
return nil
}

// Temporary until https://github.com/sourcenetwork/defradb/issues/1681 is resolved.
func ensureIntIsInt(fields []client.FieldDescription, docMap map[string]any) {
for _, field := range fields {
if field.Kind == client.FieldKind_INT {
if val, ok := docMap[field.Name]; ok {
switch v := val.(type) {
case uint64:
docMap[field.Name] = int(v)
case int64:
docMap[field.Name] = int(v)
}
}
}
}
}
11 changes: 11 additions & 0 deletions db/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const (
errCanNotDropIndexWithPatch string = "dropping indexes via patch is not supported"
errCanNotChangeIndexWithPatch string = "changing indexes via patch is not supported"
errIndexWithNameDoesNotExists string = "index with name doesn't exists"
errCorruptedIndex string = "corrupted index. Please delete and recreate the index"
errInvalidFieldValue string = "invalid field value"
errUnsupportedIndexFieldType string = "unsupported index field type"
errIndexDescriptionHasNoFields string = "index description has no fields"
Expand Down Expand Up @@ -147,6 +148,7 @@ var (
ErrIndexFieldMissingName = errors.New(errIndexFieldMissingName)
ErrIndexFieldMissingDirection = errors.New(errIndexFieldMissingDirection)
ErrIndexSingleFieldWrongDirection = errors.New(errIndexSingleFieldWrongDirection)
ErrCorruptedIndex = errors.New(errCorruptedIndex)
ErrCanNotChangeIndexWithPatch = errors.New(errCanNotChangeIndexWithPatch)
ErrFieldOrAliasToFieldNotExist = errors.New(errFieldOrAliasToFieldNotExist)
ErrCreateFile = errors.New(errCreateFile)
Expand Down Expand Up @@ -478,6 +480,15 @@ func NewErrIndexWithNameDoesNotExists(indexName string) error {
)
}

// NewErrCorruptedIndex returns a new error indicating that an index with the
// given name has been corrupted.
func NewErrCorruptedIndex(indexName string) error {
return errors.New(
errCorruptedIndex,
errors.NewKV("Name", indexName),
)
}

// NewErrCannotAddIndexWithPatch returns a new error indicating that an index cannot be added
// with a patch.
func NewErrCannotAddIndexWithPatch(proposedName string) error {
Expand Down
7 changes: 7 additions & 0 deletions db/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,13 @@ func (i *collectionSimpleIndex) Update(
if err != nil {
return err
}
exists, err := txn.Datastore().Has(ctx, key.ToDS())
if err != nil {
return err
}
if !exists {
return NewErrCorruptedIndex(i.desc.Name)
}
err = txn.Datastore().Delete(ctx, key.ToDS())
if err != nil {
return err
Expand Down
9 changes: 3 additions & 6 deletions db/indexed_docs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -828,19 +828,14 @@ func TestNonUniqueUpdate_IfFailsToUpdateIndex_ReturnError(t *testing.T) {
f.commitTxn()

validKey := newIndexKeyBuilder(f).Col(usersColName).Field(usersAgeFieldName).Doc(doc).Build()
invalidKey := newIndexKeyBuilder(f).Col(usersColName).Field(usersAgeFieldName).Doc(doc).
Values([]byte("invalid")).Build()

err := f.txn.Datastore().Delete(f.ctx, validKey.ToDS())
require.NoError(f.t, err)
err = f.txn.Datastore().Put(f.ctx, invalidKey.ToDS(), []byte{})
require.NoError(f.t, err)
f.commitTxn()

err = doc.Set(usersAgeFieldName, 23)
require.NoError(t, err)
err = f.users.Update(f.ctx, doc)
require.Error(t, err)
require.ErrorIs(t, err, ErrCorruptedIndex)
}

func TestNonUniqueUpdate_ShouldPassToFetcherOnlyRelevantFields(t *testing.T) {
Expand Down Expand Up @@ -888,6 +883,7 @@ func TestNonUniqueUpdate_IfDatastoreFails_ReturnError(t *testing.T) {
Name: "Delete old value",
StubDataStore: func(ds *mocks.DSReaderWriter_Expecter) {
ds.Delete(mock.Anything, mock.Anything).Return(testErr)
ds.Has(mock.Anything, mock.Anything).Maybe().Return(true, nil)
ds.Get(mock.Anything, mock.Anything).Maybe().Return([]byte{}, nil)
},
},
Expand All @@ -896,6 +892,7 @@ func TestNonUniqueUpdate_IfDatastoreFails_ReturnError(t *testing.T) {
StubDataStore: func(ds *mocks.DSReaderWriter_Expecter) {
ds.Delete(mock.Anything, mock.Anything).Maybe().Return(nil)
ds.Get(mock.Anything, mock.Anything).Maybe().Return([]byte{}, nil)
ds.Has(mock.Anything, mock.Anything).Maybe().Return(true, nil)
ds.Put(mock.Anything, mock.Anything, mock.Anything).Maybe().Return(testErr)
},
},
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/backup/one_to_many/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollections_NoError(t *testing
Results: []map[string]any{
{
"name": "Smith",
"age": uint64(31),
"age": int64(31),
},
{
"name": "Bob",
"age": uint64(32),
"age": int64(32),
},
{
"name": "John",
"age": uint64(30),
"age": int64(30),
},
},
},
Expand Down Expand Up @@ -123,11 +123,11 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollectionsAndUpdatedDocs_NoEr
Results: []map[string]any{
{
"name": "Bob",
"age": uint64(31),
"age": int64(31),
},
{
"name": "John",
"age": uint64(31),
"age": int64(31),
},
},
},
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/backup/one_to_one/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollections_NoError(t *testing
Results: []map[string]any{
{
"name": "Smith",
"age": uint64(31),
"age": int64(31),
},
{
"name": "Bob",
"age": uint64(32),
"age": int64(32),
},
{
"name": "John",
"age": uint64(30),
"age": int64(30),
},
},
},
Expand Down Expand Up @@ -117,11 +117,11 @@ func TestBackupImport_WithMultipleNoKeyAndMultipleCollectionsAndUpdatedDocs_NoEr
Results: []map[string]any{
{
"name": "Bob",
"age": uint64(31),
"age": int64(31),
},
{
"name": "John",
"age": uint64(31),
"age": int64(31),
},
},
},
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/backup/simple/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestBackupImport_Simple_NoError(t *testing.T) {
Results: []map[string]any{
{
"name": "John",
"age": uint64(30),
"age": int64(30),
},
},
},
Expand Down Expand Up @@ -103,7 +103,7 @@ func TestBackupImport_WithNoKeys_NoError(t *testing.T) {
Results: []map[string]any{
{
"name": "John",
"age": uint64(30),
"age": int64(30),
},
},
},
Expand Down Expand Up @@ -134,15 +134,15 @@ func TestBackupImport_WithMultipleNoKeys_NoError(t *testing.T) {
Results: []map[string]any{
{
"name": "Smith",
"age": uint64(31),
"age": int64(31),
},
{
"name": "Bob",
"age": uint64(32),
"age": int64(32),
},
{
"name": "John",
"age": uint64(30),
"age": int64(30),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/collection/update/simple/with_keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func TestUpdateWithKeys(t *testing.T) {
return err
}

assert.Equal(t, uint64(40), name)
assert.Equal(t, int64(40), name)

d2, err := c.Get(ctx, doc2.Key(), false)
if err != nil {
Expand All @@ -172,7 +172,7 @@ func TestUpdateWithKeys(t *testing.T) {
return err
}

assert.Equal(t, uint64(40), name2)
assert.Equal(t, int64(40), name2)

return nil
},
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/index/create_drop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestIndexDrop_ShouldNotHinderQuerying(t *testing.T) {
Results: []map[string]any{
{
"Name": "John",
"Age": uint64(21),
"Age": int64(21),
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/index/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestIndexCreateWithCollection_ShouldNotHinderQuerying(t *testing.T) {
Results: []map[string]any{
{
"Name": "John",
"Age": uint64(21),
"Age": int64(21),
},
},
},
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestIndexCreate_ShouldNotHinderQuerying(t *testing.T) {
Results: []map[string]any{
{
"Name": "John",
"Age": uint64(21),
"Age": int64(21),
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/index/drop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestIndexDrop_IfIndexDoesNotExist_ReturnError(t *testing.T) {
Results: []map[string]any{
{
"Name": "John",
"Age": uint64(21),
"Age": int64(21),
},
},
},
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/index/query_with_index_only_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestQueryWithIndex_WithNonIndexedFields_ShouldFetchAllOfThem(t *testing.T)
Request: req,
Results: []map[string]any{{
"name": "Islam",
"age": uint64(32),
"age": int64(32),
}},
},
testUtils.Request{
Expand Down Expand Up @@ -104,8 +104,8 @@ func TestQueryWithIndex_IfSeveralDocsWithEqFilter_ShouldFetchAll(t *testing.T) {
testUtils.Request{
Request: req,
Results: []map[string]any{
{"age": uint64(32)},
{"age": uint64(18)},
{"age": int64(32)},
{"age": int64(18)},
},
},
testUtils.Request{
Expand Down Expand Up @@ -340,8 +340,8 @@ func TestQueryWithIndex_IfSeveralDocsWithInFilter_ShouldFetchAll(t *testing.T) {
testUtils.Request{
Request: req,
Results: []map[string]any{
{"age": uint64(32)},
{"age": uint64(18)},
{"age": int64(32)},
{"age": int64(18)},
},
},
testUtils.Request{
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/mutation/create/simple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestMutationCreate(t *testing.T) {
{
"_key": "bae-88b63198-7d38-5714-a9ff-21ba46374fd1",
"name": "John",
"age": uint64(27),
"age": int64(27),
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestDeletionOfADocumentUsingSingleKeyWithShowDeletedDocumentQuery(t *testin
{
"_deleted": false,
"name": "John",
"age": uint64(30),
"age": int64(30),
"published": []map[string]any{
{
"_deleted": true,
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/mutation/delete/with_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,10 @@ func TestMutationDeletion_WithFilterMatchingMultipleDocs(t *testing.T) {
}`,
Results: []map[string]any{
{
"age": uint64(2),
"age": int64(2),
},
{
"age": uint64(1),
"age": int64(1),
},
},
},
Expand Down
Loading

0 comments on commit 9e1ad62

Please sign in to comment.