fix: Handle multiple nil values on unique indexed fields (sourcenetwo…

…rk#2178) ## Relevant issue(s) Resolves sourcenetwork#2174 sourcenetwork#2175 ## Description This change fixes how nil values are handled on unique indexes
shahzadlone · Jan 20, 2024 · 5b4ea63 · 5b4ea63
1 parent 5fb9243
commit 5b4ea63
Show file tree

Hide file tree

Showing 5 changed files with 168 additions and 12 deletions.
diff --git a/db/collection_index.go b/db/collection_index.go
@@ -13,6 +13,7 @@ package db
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -236,36 +237,33 @@ func (c *collection) iterateAllDocs(
 	df := c.newFetcher()
 	err := df.Init(ctx, txn, c, fields, nil, nil, false, false)
 	if err != nil {
-		_ = df.Close()
-		return err
+		return errors.Join(err, df.Close())
 	}
 	start := base.MakeDataStoreKeyWithCollectionDescription(c.Description())
 	spans := core.NewSpans(core.NewSpan(start, start.PrefixEnd()))
 
 	err = df.Start(ctx, spans)
 	if err != nil {
-		_ = df.Close()
-		return err
+		return errors.Join(err, df.Close())
 	}
 
 	for {
 		encodedDoc, _, err := df.FetchNext(ctx)
 		if err != nil {
-			_ = df.Close()
-			return err
+			return errors.Join(err, df.Close())
 		}
 		if encodedDoc == nil {
 			break
 		}
 
 		doc, err := fetcher.Decode(encodedDoc, c.Schema())
 		if err != nil {
-			return err
+			return errors.Join(err, df.Close())
 		}
 
 		err = exec(doc)
 		if err != nil {
-			return err
+			return errors.Join(err, df.Close())
 		}
 	}
 

diff --git a/db/errors.go b/db/errors.go
@@ -86,7 +86,7 @@ const (
 	errExpectedJSONArray                  string = "expected JSON array"
 	errOneOneAlreadyLinked                string = "target document is already linked to another document"
 	errIndexDoesNotMatchName              string = "the index used does not match the given name"
-	errCanNotIndexNonUniqueField          string = "can not create doc that violates unique index"
+	errCanNotIndexNonUniqueField          string = "can not index a doc's field that violates unique index"
 	errInvalidViewQuery                   string = "the query provided is not valid as a View"
 )
 
@@ -579,7 +579,7 @@ func NewErrInvalidViewQueryCastFailed(query string) error {
 	return errors.New(
 		errInvalidViewQuery,
 		errors.NewKV("Query", query),
-		errors.NewKV("Reason", "Internal errror, cast failed"),
+		errors.NewKV("Reason", "Internal error, cast failed"),
 	)
 }
 

diff --git a/db/index.go b/db/index.go
@@ -290,10 +290,18 @@ func (i *collectionUniqueIndex) newUniqueIndexError(
 	doc *client.Document,
 ) error {
 	fieldVal, err := doc.GetValue(i.fieldDesc.Name)
+	var val any
 	if err != nil {
-		return err
+		// If the error is ErrFieldNotExist, we leave `val` as is (e.g. nil)
+		// otherwise we return the error
+		if !errors.Is(err, client.ErrFieldNotExist) {
+			return err
+		}
+	} else {
+		val = fieldVal.Value()
 	}
-	return NewErrCanNotIndexNonUniqueField(doc.ID().String(), i.fieldDesc.Name, fieldVal.Value())
+
+	return NewErrCanNotIndexNonUniqueField(doc.ID().String(), i.fieldDesc.Name, val)
 }
 
 func (i *collectionUniqueIndex) Update(

diff --git a/tests/integration/index/create_unique_test.go b/tests/integration/index/create_unique_test.go
@@ -186,3 +186,115 @@ func TestUniqueIndexCreate_IfFieldValuesAreUnique_Succeed(t *testing.T) {
 
 	testUtils.ExecuteTestCase(t, test)
 }
+
+func TestUniqueIndexCreate_IfNilFieldsArePresent_ReturnError(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "If filter does not match any document, return empty result",
+		Actions: []any{
+			testUtils.SchemaUpdate{
+				Schema: `
+					type User {
+						name: String 
+						age: Int
+					}
+				`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"John",
+						"age":	21
+					}`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"Andy"
+					}`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"Keenan"
+					}`,
+			},
+			testUtils.CreateIndex{
+				CollectionID:  0,
+				FieldName:     "age",
+				Unique:        true,
+				ExpectedError: db.NewErrCanNotIndexNonUniqueField("bae-caba9876-89aa-5bcf-bc1c-387a52499b27", "age", nil).Error(),
+			},
+		},
+	}
+
+	testUtils.ExecuteTestCase(t, test)
+}
+
+func TestUniqueIndexCreate_AddingDocWithNilValue_ShouldSucceed(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "Test adding a doc with nil value for indexed field should succeed",
+		Actions: []any{
+			testUtils.SchemaUpdate{
+				Schema: `
+					type User {
+						name: String 
+						age: Int @index(unique: true)
+					}
+				`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"John"
+					}`,
+			},
+		},
+	}
+
+	testUtils.ExecuteTestCase(t, test)
+}
+
+func TestUniqueIndexCreate_UponAddingDocWithExistingNilValue_ReturnError(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "If filter does not match any document, return empty result",
+		Actions: []any{
+			testUtils.SchemaUpdate{
+				Schema: `
+					type User {
+						name: String 
+						age: Int @index(unique: true)
+					}
+				`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"John",
+						"age":	21
+					}`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"Keenan"
+					}`,
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"Andy"
+					}`,
+				ExpectedError: db.NewErrCanNotIndexNonUniqueField("bae-2159860f-3cd1-59de-9440-71331e77cbb8", "age", nil).Error(),
+			},
+		},
+	}
+
+	testUtils.ExecuteTestCase(t, test)
+}
diff --git a/tests/integration/index/query_with_unique_index_only_filter_test.go b/tests/integration/index/query_with_unique_index_only_filter_test.go
@@ -494,3 +494,41 @@ func TestQueryWithUniqueIndex_IfNoMatch_ReturnEmptyResult(t *testing.T) {
 
 	testUtils.ExecuteTestCase(t, test)
 }
+
+func TestQueryWithUniqueIndex_WithEqualFilterOnNilValue_ShouldFetch(t *testing.T) {
+	test := testUtils.TestCase{
+		Description: "Test index filtering with _eq filter on nil value",
+		Actions: []any{
+			testUtils.SchemaUpdate{
+				Schema: `
+					type User {
+						name: String 
+						age: Int @index(unique: true)
+					}`,
+			},
+			testUtils.CreatePredefinedDocs{
+				Docs: getUserDocs(),
+			},
+			testUtils.CreateDoc{
+				CollectionID: 0,
+				Doc: `
+					{
+						"name":	"Alice"
+					}`,
+			},
+			testUtils.Request{
+				Request: `
+					query {
+						User(filter: {age: {_eq: null}}) {
+							name
+						}
+					}`,
+				Results: []map[string]any{
+					{"name": "Alice"},
+				},
+			},
+		},
+	}
+
+	testUtils.ExecuteTestCase(t, test)
+}