merge

FerretDB · AlekSi · Jun 5, 2023 · May 30, 2023 · May 31, 2023 · May 31, 2023
commit a6401b26568e284518dea9ae88c8beffcb06f711
diff --git a/.golangci-new.yml b/.golangci-new.yml
@@ -6,6 +6,11 @@ run:
   timeout: 3m
 
 linters-settings:
+  asasalint:
+    exclude:
+      - ^lazyerrors\.Errorf$
+    use-builtin-exclusions: true
+    ignore-test: false
   errorlint:
     # see caveats at https://github.com/polyfloyd/go-errorlint#fmterrorf-wrapping-verb
     errorf: false

diff --git a/integration/.golangci-new.yml b/integration/.golangci-new.yml
@@ -6,6 +6,11 @@ run:
   timeout: 3m
 
 linters-settings:
+  asasalint:
+    exclude:
+      - ^lazyerrors\.Errorf$
+    use-builtin-exclusions: true
+    ignore-test: false
   errorlint:
     # see caveats at https://github.com/polyfloyd/go-errorlint#fmterrorf-wrapping-verb
     errorf: false
@@ -17,7 +22,7 @@ linters-settings:
   revive:
     ignore-generated-header: true
     severity: warning
-    # TODO enable-all-rules: true
+    # TODO enable-all-rules: true https://github.com/FerretDB/FerretDB/issues/2748
     rules:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md
       - name: exported
@@ -62,8 +67,9 @@ linters:
     - unused
     - whitespace
 
-    # TODO configure and enable one by one
+    # TODO https://github.com/FerretDB/FerretDB/issues/2748
     - bidichk
+    - bodyclose
     - containedctx
     - contextcheck
     - cyclop
@@ -110,6 +116,8 @@ linters:
     - prealloc
     - predeclared
     - promlinter
+    - rowserrcheck
+    - sqlclosecheck
     - stylecheck
     - tagliatelle
     - tenv
@@ -120,13 +128,8 @@ linters:
     - unparam
     - varcheck
     - varnamelen
-    - wrapcheck
-
-    # TODO https://github.com/golangci/golangci-lint/issues/2649
-    - bodyclose
-    - rowserrcheck
-    - sqlclosecheck
     - wastedassign
+    - wrapcheck
 
     # deprecated
     - golint

diff --git a/integration/findandmodify_compat_test.go b/integration/findandmodify_compat_test.go
@@ -197,7 +197,7 @@ func TestFindAndModifyCompatUpdate(t *testing.T) {
 			},
 			skipForTigris: "schema validation would fail",
 		},
-		"InvalidOperator": {
+		"Conflict": {
 			command: bson.D{
 				{"query", bson.D{{"_id", bson.D{{"$exists", false}}}}},
 				{"update", bson.D{{"$invalid", "non-existent-field"}}},
@@ -214,6 +214,88 @@ func TestFindAndModifyCompatUpdate(t *testing.T) {
 			},
 			resultType: emptyResult,
 		},
+		"NoConflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int64"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v", 4}}},
+					{"$inc", bson.D{{"foo", 4}}},
+				}},
+			},
+		},
+		"EmptyKey": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int64"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"", 4}}},
+					{"$inc", bson.D{{"", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+		"EmptyKeyAndKey": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int64"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"", 4}}},
+					{"$inc", bson.D{{"v", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+		"InvalidOperator": {
+			command: bson.D{
+				{"query", bson.D{{"_id", bson.D{{"$exists", false}}}}},
+				{"update", bson.D{{"$invalid", "non-existent-field"}}},
+			},
+			resultType: emptyResult,
+		},
+	}
+
+	testFindAndModifyCompat(t, testCases)
+}
+
+func TestFindAndModifyCompatUpdateDotNotation(t *testing.T) {
+	testCases := map[string]findAndModifyCompatTestCase{
+		"Conflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v.0.field", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+		"NoConflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v.0.foo", 4}}},
+				}},
+			},
+		},
+		"NoIndex": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v.field", 4}}},
+				}},
+			},
+		},
+		"ParentConflict": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "array-documents-two-fields"}}},
+				{"update", bson.D{
+					{"$set", bson.D{{"v.0.field", 4}}},
+					{"$inc", bson.D{{"v", 4}}},
+				}},
+			},
+			resultType: emptyResult,
+		},
+
 		"ConflictKey": {
 			command: bson.D{
 				{"query", bson.D{{"_id", bson.D{{"$exists", false}}}}},

diff --git a/integration/shareddata/composites.go b/integration/shareddata/composites.go
@@ -98,6 +98,16 @@ var ArrayAndDocuments = &Values[string]{
 			bson.D{{"field", int32(44)}},
 			bson.D{{"foo", int32(42)}},
 		},
+		"array-documents-two-fields": bson.A{
+			bson.D{
+				{"field", int32(42)},
+				{"foo", int32(44)},
+			},
+			bson.D{
+				{"field", int32(44)},
+				{"foo", int32(42)},
+			},
+		},
 	},
 }
 

diff --git a/internal/handlers/common/distinct.go b/internal/handlers/common/distinct.go
@@ -15,6 +15,7 @@
 package common
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 
@@ -23,6 +24,7 @@ import (
 	"github.com/FerretDB/FerretDB/internal/handlers/commonerrors"
 	"github.com/FerretDB/FerretDB/internal/handlers/commonparams"
 	"github.com/FerretDB/FerretDB/internal/types"
+	"github.com/FerretDB/FerretDB/internal/util/iterator"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
 )
 
@@ -84,10 +86,21 @@ func GetDistinctParams(document *types.Document, l *zap.Logger) (*DistinctParams
 //
 // If the key is found in the document, and the value is an array, each element of the array is added to the result.
 // Otherwise, the value itself is added to the result.
-func FilterDistinctValues(docs []*types.Document, key string) (*types.Array, error) {
-	distinct := types.MakeArray(len(docs))
+func FilterDistinctValues(iter types.DocumentsIterator, key string) (*types.Array, error) {
+	distinct := types.MakeArray(0)
+
+	defer iter.Close()
+
+	for {
+		_, doc, err := iter.Next()
+		if errors.Is(err, iterator.ErrIteratorDone) {
+			break
+		}
+
+		if err != nil {
+			return nil, lazyerrors.Error(err)
+		}
 
-	for _, doc := range docs {
 		// docsAtSuffix contains all documents exist at the suffix key.
 		docsAtSuffix := []*types.Document{doc}
 		suffix := key

diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
@@ -902,11 +902,11 @@ func newUpdateError(code commonerrors.ErrorCode, msg, command string) error {
 // validateOperatorKeys returns error if any key contains empty path or
 // the same path prefix exists in other key or other document.
 func validateOperatorKeys(command string, docs ...*types.Document) error {
-	seen := map[string]struct{}{}
+	visitedPaths := []*types.Path{}
 
 	for _, doc := range docs {
 		for _, key := range doc.Keys() {
-			path, err := types.NewPathFromString(key)
+			nextPath, err := types.NewPathFromString(key)
 			if err != nil {
 				return newUpdateError(
 					commonerrors.ErrEmptyName,
@@ -918,23 +918,47 @@ func validateOperatorKeys(command string, docs ...*types.Document) error {
 				)
 			}
 
-			if _, ok := seen[path.Prefix()]; ok {
-				return newUpdateError(
-					commonerrors.ErrConflictingUpdateOperators,
-					fmt.Sprintf(
-						"Updating the path '%[1]s' would create a conflict at '%[1]s'", key,
-					),
-					command,
-				)
+			for _, oldPath := range visitedPaths {
+				if checkSlicePrefix(oldPath.Slice(), nextPath.Slice()) {
+					return newUpdateError(
+						commonerrors.ErrConflictingUpdateOperators,
+						fmt.Sprintf(
+							"Updating the path '%[1]s' would create a conflict at '%[1]s'", key,
+						),
+						command,
+					)
+				}
 			}
-
-			seen[path.Prefix()] = struct{}{}
+			visitedPaths = append(visitedPaths, &nextPath)
 		}
 	}
 
 	return nil
 }
 
+// checkSlicePrefix returns true if one slice is the beginning of another slice.
+// The example of slice prefix: arr1 = ["a","b","c"] arr2 = ["a","b"];
+// If both slices are empty, it returns true. If one slice is empty and another slice is not, it returns false.
+func checkSlicePrefix(arr1, arr2 []string) bool {
+	target, prefix := arr1, arr2
+
+	if len(target) < len(prefix) {
+		target, prefix = prefix, target
+	}
+
+	if len(prefix) == 0 {
+		return len(target) == 0
+	}
+
+	for i := range prefix {
+		if prefix[i] != target[i] {
+			return false
+		}
+	}
+
+	return true
+}
+
 // extractValueFromUpdateOperator gets operator "op" value and returns CommandError for `findAndModify`
 // WriteError error other commands if it is not a document.
 // For example, for update document

diff --git a/internal/handlers/pg/msg_count.go b/internal/handlers/pg/msg_count.go
@@ -22,6 +22,7 @@ import (
 	"github.com/FerretDB/FerretDB/internal/handlers/common"
 	"github.com/FerretDB/FerretDB/internal/handlers/pg/pgdb"
 	"github.com/FerretDB/FerretDB/internal/types"
+	"github.com/FerretDB/FerretDB/internal/util/iterator"
 	"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
 	"github.com/FerretDB/FerretDB/internal/util/must"
 	"github.com/FerretDB/FerretDB/internal/wire"
@@ -50,26 +51,40 @@ func (h *Handler) MsgCount(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, e
 		Collection: params.Collection,
 	}
 
-	qp.Filter = params.Filter
+	if !h.DisableFilterPushdown {
+		qp.Filter = params.Filter
+	}
 
 	var resDocs []*types.Document
 	err = dbPool.InTransaction(ctx, func(tx pgx.Tx) error {
-		resDocs, err = fetchAndFilterDocs(ctx, &fetchParams{tx, &qp, h.DisableFilterPushdown})
-		return err
+		var iter types.DocumentsIterator
+
+		iter, _, err = pgdb.QueryDocuments(ctx, tx, &qp)
+		if err != nil {
+			return lazyerrors.Error(err)
+		}
+
+		closer := iterator.NewMultiCloser(iter)
+		defer closer.Close()
+
+		iter = common.FilterIterator(iter, closer, qp.Filter)
+
+		iter = common.SkipIterator(iter, closer, params.Skip)
+
+		iter = common.LimitIterator(iter, closer, params.Limit)
+
+		resDocs, err = iterator.ConsumeValues(iterator.Interface[struct{}, *types.Document](iter))
+		if err != nil {
+			return lazyerrors.Error(err)
+		}
+
+		return nil
 	})
 
 	if err != nil {
 		return nil, err
 	}
 
-	if resDocs, err = common.SkipDocuments(resDocs, params.Skip); err != nil {
-		return nil, lazyerrors.Error(err)
-	}
-
-	if resDocs, err = common.LimitDocuments(resDocs, params.Limit); err != nil {
-		return nil, lazyerrors.Error(err)
-	}
-
 	var reply wire.OpMsg
 	must.NoError(reply.SetSections(wire.OpMsgSection{
 		Documents: []*types.Document{must.NotFail(types.NewDocument(