From ba97b15b8554ca99296e1642ea914caa3a339f3c Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Fri, 7 Jul 2023 17:41:34 +0900
Subject: [PATCH 1/8] add test remove code

---
 integration/update_field_compat_test.go | 10 ++++++++++
 internal/handlers/common/update.go      | 26 -------------------------
 2 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
index 97100b7bdb27..9739e5d2a43e 100644
--- a/integration/update_field_compat_test.go
+++ b/integration/update_field_compat_test.go
@@ -791,6 +791,16 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			},
 			resultType: emptyResult,
 		},
+		"SameID": {
+			filter:     bson.D{{"_id", "int32"}},
+			update:     bson.D{{"$set", bson.D{{"_id", "int32"}}}},
+			resultType: emptyResult,
+		},
+		"DifferentID": {
+			filter:     bson.D{{"_id", "int32"}},
+			update:     bson.D{{"$set", bson.D{{"_id", "another-id"}}}},
+			resultType: emptyResult,
+		},
 	}
 
 	testUpdateCompat(t, testCases)
diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
index 566141273595..88a69cf4f1c0 100644
--- a/internal/handlers/common/update.go
+++ b/internal/handlers/common/update.go
@@ -848,10 +848,6 @@ func ValidateUpdateOperators(command string, update *types.Document) error {
 		return err
 	}
 
-	if err = validateSetExpression(command, update); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -994,28 +990,6 @@ func extractValueFromUpdateOperator(command, op string, update *types.Document)
 	return doc, nil
 }
 
-// validateSetExpression validates $set expression input.
-func validateSetExpression(command string, update *types.Document) error {
-	if !update.Has("$set") {
-		return nil
-	}
-
-	updateExpression := must.NotFail(update.Get("$set"))
-
-	// updateExpression is document, checked in ValidateUpdateOperators.
-	doc := updateExpression.(*types.Document)
-
-	if doc.Has("_id") {
-		return newUpdateError(
-			commonerrors.ErrImmutableField,
-			"Performing an update on the path '_id' would modify the immutable field '_id'",
-			command,
-		)
-	}
-
-	return nil
-}
-
 // validateRenameExpression validates $rename input on correctness.
 func validateRenameExpression(command string, update *types.Document) error {
 	if !update.Has("$rename") {

From d7e53c4337a4fc23b4b055ef489d3fc3ab1b8dc5 Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 15:34:41 +0900
Subject: [PATCH 2/8] fix immutable _id

---
 internal/handlers/common/findandmodify.go |  8 +++----
 internal/handlers/common/update.go        | 29 +++++++++++++++++++----
 internal/handlers/pg/msg_findandmodify.go |  4 +++-
 internal/handlers/pg/msg_update.go        |  7 ++++--
 internal/handlers/sqlite/msg_update.go    |  6 +++--
 5 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/internal/handlers/common/findandmodify.go b/internal/handlers/common/findandmodify.go
index 8ed88cbed6bd..e8c65ce37fbb 100644
--- a/internal/handlers/common/findandmodify.go
+++ b/internal/handlers/common/findandmodify.go
@@ -168,7 +168,7 @@ func PrepareDocumentForUpsert(docs []*types.Document, params *FindAndModifyParam
 
 	if len(docs) == 0 {
 		res.Operation = UpsertOperationInsert
-		res.Upsert, err = prepareDocumentForInsert(params)
+		res.Upsert, err = prepareDocumentForInsert(docs, params)
 
 		// insert operation returns null since no document existed before upsert.
 		res.ReturnValue = types.Null
@@ -196,11 +196,11 @@ func PrepareDocumentForUpsert(docs []*types.Document, params *FindAndModifyParam
 // prepareDocumentForInsert creates an insert document from the parameter.
 // When inserting new document we must check that `_id` is present, so we must extract `_id`
 // from query or generate a new one.
-func prepareDocumentForInsert(params *FindAndModifyParams) (*types.Document, error) {
+func prepareDocumentForInsert(docs []*types.Document, params *FindAndModifyParams) (*types.Document, error) {
 	insert := must.NotFail(types.NewDocument())
 
 	if params.HasUpdateOperators {
-		if _, err := UpdateDocument("findAndModify", insert, params.Update); err != nil {
+		if _, err := UpdateDocument("findAndModify", insert, params.Update, params.Query, GetIDs(docs)); err != nil {
 			return nil, err
 		}
 	} else {
@@ -224,7 +224,7 @@ func prepareDocumentForUpdate(docs []*types.Document, params *FindAndModifyParam
 	update := docs[0].DeepCopy()
 
 	if params.HasUpdateOperators {
-		if _, err := UpdateDocument("findAndModify", update, params.Update); err != nil {
+		if _, err := UpdateDocument("findAndModify", update, params.Update, params.Query, GetIDs(docs)); err != nil {
 			return nil, err
 		}
 
diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
index 88a69cf4f1c0..fd6432ab9784 100644
--- a/internal/handlers/common/update.go
+++ b/internal/handlers/common/update.go
@@ -37,7 +37,7 @@ import (
 // To validate update document, must call ValidateUpdateOperators before calling UpdateDocument.
 // UpdateDocument returns CommandError for findAndModify case-insensitive command name,
 // WriteError for other commands.
-func UpdateDocument(command string, doc, update *types.Document) (bool, error) {
+func UpdateDocument(command string, doc, update, filter *types.Document, ids []any) (bool, error) {
 	var changed bool
 	var err error
 
@@ -65,13 +65,13 @@ func UpdateDocument(command string, doc, update *types.Document) (bool, error) {
 			}
 
 		case "$set":
-			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), false)
+			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), filter, ids, false)
 			if err != nil {
 				return false, err
 			}
 
 		case "$setOnInsert":
-			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), true)
+			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), filter, ids, true)
 			if err != nil {
 				return false, err
 			}
@@ -187,17 +187,38 @@ func UpdateDocument(command string, doc, update *types.Document) (bool, error) {
 	return changed, nil
 }
 
+// GetIDs iterates each document and returns slices of IDs.
+func GetIDs(docs []*types.Document) []any {
+	ids := make([]any, len(docs))
+	for i := 0; i < len(docs); i++ {
+		ids[i] = must.NotFail(docs[i].Get("_id"))
+	}
+
+	return ids
+}
+
 // processSetFieldExpression changes document according to $set and $setOnInsert operators.
 // If the document was changed it returns true.
-func processSetFieldExpression(command string, doc, setDoc *types.Document, setOnInsert bool) (bool, error) {
+func processSetFieldExpression(command string, doc, setDoc, filter *types.Document, ids []any, setOnInsert bool) (bool, error) {
 	var changed bool
 
+	filterID, _ := filter.Get("_id")
+
 	setDocKeys := setDoc.Keys()
 	sort.Strings(setDocKeys)
 
 	for _, setKey := range setDocKeys {
 		setValue := must.NotFail(setDoc.Get(setKey))
 
+		collectionHasID := filter == nil && slices.Contains(ids, setValue)
+		if setKey == "_id" && setValue != filterID && !collectionHasID {
+			return false, newUpdateError(
+				commonerrors.ErrImmutableField,
+				"Performing an update on the path '_id' would modify the immutable field '_id'",
+				command,
+			)
+		}
+
 		if setOnInsert {
 			// $setOnInsert do not set null and empty array value.
 			if _, ok := setValue.(types.NullType); ok {
diff --git a/internal/handlers/pg/msg_findandmodify.go b/internal/handlers/pg/msg_findandmodify.go
index 372e89736d9d..7446f505e7e4 100644
--- a/internal/handlers/pg/msg_findandmodify.go
+++ b/internal/handlers/pg/msg_findandmodify.go
@@ -96,6 +96,8 @@ func (h *Handler) MsgFindAndModify(ctx context.Context, msg *wire.OpMsg) (*wire.
 			return err
 		}
 
+		ids := common.GetIDs(resDocs)
+
 		if params.Update != nil { // we have update part
 			var resValue any
 			var insertedID any
@@ -129,7 +131,7 @@ func (h *Handler) MsgFindAndModify(ctx context.Context, msg *wire.OpMsg) (*wire.
 
 				if params.HasUpdateOperators {
 					upsert = resDocs[0].DeepCopy()
-					_, err = common.UpdateDocument(document.Command(), upsert, params.Update)
+					_, err = common.UpdateDocument(document.Command(), upsert, params.Update, params.Query, ids)
 					if err != nil {
 						return err
 					}
diff --git a/internal/handlers/pg/msg_update.go b/internal/handlers/pg/msg_update.go
index 3fcc3485b81e..b0307dbbd5ed 100644
--- a/internal/handlers/pg/msg_update.go
+++ b/internal/handlers/pg/msg_update.go
@@ -79,6 +79,8 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg,
 				return err
 			}
 
+			ids := common.GetIDs(resDocs)
+
 			if len(resDocs) == 0 {
 				if !u.Upsert {
 					// nothing to do, continue to the next update operation
@@ -86,7 +88,8 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg,
 				}
 
 				doc := u.Filter.DeepCopy()
-				if _, err = common.UpdateDocument(document.Command(), doc, u.Update); err != nil {
+
+				if _, err = common.UpdateDocument(document.Command(), doc, u.Update, u.Filter, ids); err != nil {
 					return err
 				}
 				if !doc.Has("_id") {
@@ -114,7 +117,7 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg,
 			matched += int32(len(resDocs))
 
 			for _, doc := range resDocs {
-				changed, err := common.UpdateDocument(document.Command(), doc, u.Update)
+				changed, err := common.UpdateDocument(document.Command(), doc, u.Update, u.Filter, ids)
 				if err != nil {
 					return err
 				}
diff --git a/internal/handlers/sqlite/msg_update.go b/internal/handlers/sqlite/msg_update.go
index 8f0f479afc10..583219e6262e 100644
--- a/internal/handlers/sqlite/msg_update.go
+++ b/internal/handlers/sqlite/msg_update.go
@@ -117,6 +117,8 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara
 
 		res.Iter.Close()
 
+		ids := common.GetIDs(resDocs)
+
 		if len(resDocs) == 0 {
 			if !u.Upsert {
 				// nothing to do, continue to the next update operation
@@ -124,7 +126,7 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara
 			}
 
 			doc := u.Filter.DeepCopy()
-			if _, err = common.UpdateDocument("update", doc, u.Update); err != nil {
+			if _, err = common.UpdateDocument("update", doc, u.Update, u.Filter, ids); err != nil {
 				return 0, 0, nil, lazyerrors.Error(err)
 			}
 
@@ -161,7 +163,7 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara
 		matched += int32(len(resDocs))
 
 		for _, doc := range resDocs {
-			changed, err := common.UpdateDocument("update", doc, u.Update)
+			changed, err := common.UpdateDocument("update", doc, u.Update, u.Filter, ids)
 			if err != nil {
 				return 0, 0, nil, lazyerrors.Error(err)
 			}

From 5f65da5ea7568c0d4418c8e86beaf11edfe016f9 Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 15:45:08 +0900
Subject: [PATCH 3/8] additional tests

---
 integration/findandmodify_compat_test.go | 36 ++++++++++++++++++++++--
 integration/update_field_compat_test.go  | 15 ++++++++--
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/integration/findandmodify_compat_test.go b/integration/findandmodify_compat_test.go
index 2b9a87b7d585..13a990c5d191 100644
--- a/integration/findandmodify_compat_test.go
+++ b/integration/findandmodify_compat_test.go
@@ -333,9 +333,20 @@ func TestFindAndModifyCompatUpdateSet(t *testing.T) {
 		},
 		"UpdateID": {
 			command: bson.D{
-				{"update", bson.D{{"$set", bson.D{{"_id", "non-existent"}}}}},
+				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}}}}},
+			},
+		},
+		"UpdateExistingID": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int32"}}},
+				{"update", bson.D{{"$set", bson.D{{"_id", "int32-1"}}}}},
+			},
+		},
+		"UpdateSameID": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int32"}}},
+				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}}}}},
 			},
-			resultType: emptyResult, // _id must be an immutable field
 		},
 	}
 
@@ -644,6 +655,27 @@ func TestFindAndModifyCompatUpsertSet(t *testing.T) {
 			},
 			resultType: emptyResult, // _id must be an immutable field
 		},
+		"UpsertIDNoQuery": {
+			command: bson.D{
+				{"upsert", true},
+				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}}},
+			},
+		},
+		"UpsertExistingID": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int32"}}},
+				{"upsert", true},
+				{"update", bson.D{{"$set", bson.D{{"_id", "int32-1"}, {"v", int32(2)}}}}},
+			},
+			resultType: emptyResult, // _id must be an immutable field
+		},
+		"UpsertSameID": {
+			command: bson.D{
+				{"query", bson.D{{"_id", "int32"}}},
+				{"upsert", true},
+				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}}},
+			},
+		},
 	}
 
 	testFindAndModifyCompat(t, testCases)
diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
index 9739e5d2a43e..151ace37d70d 100644
--- a/integration/update_field_compat_test.go
+++ b/integration/update_field_compat_test.go
@@ -777,6 +777,11 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			update:     bson.D{{"$set", bson.D{{"_id", "non-existent"}}}},
 			resultType: emptyResult,
 		},
+		"SetID": {
+			update:      bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
+			unsetFilter: true,
+			resultType:  emptyResult, // _id must be an immutable field
+		},
 		"ConflictKey": {
 			update: bson.D{
 				{"$set", bson.D{{"v", "val"}}},
@@ -791,14 +796,18 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			},
 			resultType: emptyResult,
 		},
-		"SameID": {
+		"ExistingID": {
 			filter:     bson.D{{"_id", "int32"}},
-			update:     bson.D{{"$set", bson.D{{"_id", "int32"}}}},
+			update:     bson.D{{"$set", bson.D{{"_id", "int32-1"}, {"v", int32(2)}}}},
 			resultType: emptyResult,
 		},
+		"SameID": {
+			filter: bson.D{{"_id", "int32"}},
+			update: bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
+		},
 		"DifferentID": {
 			filter:     bson.D{{"_id", "int32"}},
-			update:     bson.D{{"$set", bson.D{{"_id", "another-id"}}}},
+			update:     bson.D{{"$set", bson.D{{"_id", "another-id"}, {"v", int32(2)}}}},
 			resultType: emptyResult,
 		},
 	}

From 2b7660b525951310645211003c778d8035fcc447 Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 15:54:48 +0900
Subject: [PATCH 4/8] update comments

---
 integration/findandmodify_compat_test.go | 2 +-
 integration/update_field_compat_test.go  | 5 ++---
 internal/handlers/common/update.go       | 2 ++
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/integration/findandmodify_compat_test.go b/integration/findandmodify_compat_test.go
index 13a990c5d191..bc6ca15127dd 100644
--- a/integration/findandmodify_compat_test.go
+++ b/integration/findandmodify_compat_test.go
@@ -667,7 +667,7 @@ func TestFindAndModifyCompatUpsertSet(t *testing.T) {
 				{"upsert", true},
 				{"update", bson.D{{"$set", bson.D{{"_id", "int32-1"}, {"v", int32(2)}}}}},
 			},
-			resultType: emptyResult, // _id must be an immutable field
+			resultType: emptyResult,
 		},
 		"UpsertSameID": {
 			command: bson.D{
diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
index 151ace37d70d..230e94b18e3d 100644
--- a/integration/update_field_compat_test.go
+++ b/integration/update_field_compat_test.go
@@ -778,9 +778,8 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			resultType: emptyResult,
 		},
 		"SetID": {
-			update:      bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
-			unsetFilter: true,
-			resultType:  emptyResult, // _id must be an immutable field
+			update:     bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
+			resultType: emptyResult,
 		},
 		"ConflictKey": {
 			update: bson.D{
diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
index fd6432ab9784..095d7c8618c7 100644
--- a/internal/handlers/common/update.go
+++ b/internal/handlers/common/update.go
@@ -198,6 +198,7 @@ func GetIDs(docs []*types.Document) []any {
 }
 
 // processSetFieldExpression changes document according to $set and $setOnInsert operators.
+// It uses filter and existing IDs in collection to check error on immutable _id field.
 // If the document was changed it returns true.
 func processSetFieldExpression(command string, doc, setDoc, filter *types.Document, ids []any, setOnInsert bool) (bool, error) {
 	var changed bool
@@ -210,6 +211,7 @@ func processSetFieldExpression(command string, doc, setDoc, filter *types.Docume
 	for _, setKey := range setDocKeys {
 		setValue := must.NotFail(setDoc.Get(setKey))
 
+		// when filter is not set and if collection contains ID to set, it does not error
 		collectionHasID := filter == nil && slices.Contains(ids, setValue)
 		if setKey == "_id" && setValue != filterID && !collectionHasID {
 			return false, newUpdateError(

From 85c5ab044a749918969a41ecbba8f9f989da587b Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 16:05:42 +0900
Subject: [PATCH 5/8] add error test

---
 integration/findandmodify_test.go | 12 ++++++++++++
 integration/update_field_test.go  |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/integration/findandmodify_test.go b/integration/findandmodify_test.go
index 59d333b241ff..8f98f5d61add 100644
--- a/integration/findandmodify_test.go
+++ b/integration/findandmodify_test.go
@@ -150,6 +150,18 @@ func TestFindAndModifyCommandErrors(t *testing.T) {
 			altMessage: "Cannot create field 'foo' in element " +
 				"{v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
 		},
+		"SetImmutableID": {
+			command: bson.D{
+				{"update", bson.D{{"$set", bson.D{{"_id", "non-existent"}}}}},
+			},
+			err: &mongo.CommandError{
+				Code: 66,
+				Name: "ImmutableField",
+				Message: "Plan executor error during findAndModify :: caused by :: " +
+					"Performing an update on the path '_id' would modify the immutable field '_id'",
+			},
+			altMessage: "Performing an update on the path '_id' would modify the immutable field '_id'",
+		},
 		"RenameEmptyFieldName": {
 			command: bson.D{
 				{"query", bson.D{{"_id", "array-documents-nested"}}},
diff --git a/integration/update_field_test.go b/integration/update_field_test.go
index 67c9a8f6e599..e1ab32c816ec 100644
--- a/integration/update_field_test.go
+++ b/integration/update_field_test.go
@@ -106,6 +106,14 @@ func TestUpdateFieldErrors(t *testing.T) {
 			},
 			altMessage: "cannot use path 'v.foo' to traverse the document",
 		},
+		"SetImmutableID": {
+			id:     "array-documents-nested",
+			update: bson.D{{"$set", bson.D{{"_id", "another-id"}}}},
+			err: &mongo.WriteError{
+				Code:    66,
+				Message: "Performing an update on the path '_id' would modify the immutable field '_id'",
+			},
+		},
 		"RenameEmptyFieldName": {
 			id:     "array-documents-nested",
 			update: bson.D{{"$rename", bson.D{{"", "v"}}}},

From b51b8245b395d2f8265c6bc83576ff7f2f3fb161 Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 16:27:32 +0900
Subject: [PATCH 6/8] add todo, fix failing test assertion

---
 integration/update_field_compat_test.go | 3 +--
 internal/handlers/common/update.go      | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
index 230e94b18e3d..3e1dbb67d4a6 100644
--- a/integration/update_field_compat_test.go
+++ b/integration/update_field_compat_test.go
@@ -778,8 +778,7 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			resultType: emptyResult,
 		},
 		"SetID": {
-			update:     bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
-			resultType: emptyResult,
+			update: bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
 		},
 		"ConflictKey": {
 			update: bson.D{
diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
index 095d7c8618c7..40b925ebf14c 100644
--- a/internal/handlers/common/update.go
+++ b/internal/handlers/common/update.go
@@ -37,6 +37,7 @@ import (
 // To validate update document, must call ValidateUpdateOperators before calling UpdateDocument.
 // UpdateDocument returns CommandError for findAndModify case-insensitive command name,
 // WriteError for other commands.
+// TODO https://github.com/FerretDB/FerretDB/issues/3013
 func UpdateDocument(command string, doc, update, filter *types.Document, ids []any) (bool, error) {
 	var changed bool
 	var err error

From 00597517af0d48c2bde41a4eead2b7653163dc79 Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 16:47:54 +0900
Subject: [PATCH 7/8] update test name

---
 integration/findandmodify_compat_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/integration/findandmodify_compat_test.go b/integration/findandmodify_compat_test.go
index bc6ca15127dd..a57415a9dbc2 100644
--- a/integration/findandmodify_compat_test.go
+++ b/integration/findandmodify_compat_test.go
@@ -331,7 +331,7 @@ func TestFindAndModifyCompatUpdateSet(t *testing.T) {
 				{"update", bson.D{{"$set", bson.D{{"v", "foo"}}}}},
 			},
 		},
-		"UpdateID": {
+		"UpdateIDNoQuery": {
 			command: bson.D{
 				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}}}}},
 			},

From 5a32fa4a61044bf448e8d8801d4943036dfb8512 Mon Sep 17 00:00:00 2001
From: Chi Fujii <chi.fujii@ferretdb.io>
Date: Mon, 10 Jul 2023 19:14:04 +0900
Subject: [PATCH 8/8] just remove validation and add todo

---
 integration/findandmodify_compat_test.go  |  5 ++++
 integration/findandmodify_test.go         |  1 +
 integration/update_field_compat_test.go   |  4 +++
 integration/update_field_test.go          |  1 +
 internal/handlers/common/findandmodify.go |  8 +++---
 internal/handlers/common/update.go        | 31 ++++-------------------
 internal/handlers/pg/msg_findandmodify.go |  4 +--
 internal/handlers/pg/msg_update.go        |  7 ++---
 internal/handlers/sqlite/msg_update.go    |  6 ++---
 9 files changed, 25 insertions(+), 42 deletions(-)

diff --git a/integration/findandmodify_compat_test.go b/integration/findandmodify_compat_test.go
index a57415a9dbc2..e0f58cb23f2c 100644
--- a/integration/findandmodify_compat_test.go
+++ b/integration/findandmodify_compat_test.go
@@ -335,12 +335,14 @@ func TestFindAndModifyCompatUpdateSet(t *testing.T) {
 			command: bson.D{
 				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}}}}},
 			},
+			skip: "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"UpdateExistingID": {
 			command: bson.D{
 				{"query", bson.D{{"_id", "int32"}}},
 				{"update", bson.D{{"$set", bson.D{{"_id", "int32-1"}}}}},
 			},
+			skip: "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"UpdateSameID": {
 			command: bson.D{
@@ -654,12 +656,14 @@ func TestFindAndModifyCompatUpsertSet(t *testing.T) {
 				{"update", bson.D{{"$set", bson.D{{"_id", "double"}}}}},
 			},
 			resultType: emptyResult, // _id must be an immutable field
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"UpsertIDNoQuery": {
 			command: bson.D{
 				{"upsert", true},
 				{"update", bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}}},
 			},
+			skip: "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"UpsertExistingID": {
 			command: bson.D{
@@ -668,6 +672,7 @@ func TestFindAndModifyCompatUpsertSet(t *testing.T) {
 				{"update", bson.D{{"$set", bson.D{{"_id", "int32-1"}, {"v", int32(2)}}}}},
 			},
 			resultType: emptyResult,
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"UpsertSameID": {
 			command: bson.D{
diff --git a/integration/findandmodify_test.go b/integration/findandmodify_test.go
index 8f98f5d61add..f8070f38022a 100644
--- a/integration/findandmodify_test.go
+++ b/integration/findandmodify_test.go
@@ -161,6 +161,7 @@ func TestFindAndModifyCommandErrors(t *testing.T) {
 					"Performing an update on the path '_id' would modify the immutable field '_id'",
 			},
 			altMessage: "Performing an update on the path '_id' would modify the immutable field '_id'",
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"RenameEmptyFieldName": {
 			command: bson.D{
diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go
index 3e1dbb67d4a6..d8c459470852 100644
--- a/integration/update_field_compat_test.go
+++ b/integration/update_field_compat_test.go
@@ -776,9 +776,11 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 		"ID": {
 			update:     bson.D{{"$set", bson.D{{"_id", "non-existent"}}}},
 			resultType: emptyResult,
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"SetID": {
 			update: bson.D{{"$set", bson.D{{"_id", "int32"}, {"v", int32(2)}}}},
+			skip:   "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"ConflictKey": {
 			update: bson.D{
@@ -798,6 +800,7 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			filter:     bson.D{{"_id", "int32"}},
 			update:     bson.D{{"$set", bson.D{{"_id", "int32-1"}, {"v", int32(2)}}}},
 			resultType: emptyResult,
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"SameID": {
 			filter: bson.D{{"_id", "int32"}},
@@ -807,6 +810,7 @@ func TestUpdateFieldCompatSet(t *testing.T) {
 			filter:     bson.D{{"_id", "int32"}},
 			update:     bson.D{{"$set", bson.D{{"_id", "another-id"}, {"v", int32(2)}}}},
 			resultType: emptyResult,
+			skip:       "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 	}
 
diff --git a/integration/update_field_test.go b/integration/update_field_test.go
index e1ab32c816ec..efc9beda9726 100644
--- a/integration/update_field_test.go
+++ b/integration/update_field_test.go
@@ -113,6 +113,7 @@ func TestUpdateFieldErrors(t *testing.T) {
 				Code:    66,
 				Message: "Performing an update on the path '_id' would modify the immutable field '_id'",
 			},
+			skip: "https://github.com/FerretDB/FerretDB/issues/3017",
 		},
 		"RenameEmptyFieldName": {
 			id:     "array-documents-nested",
diff --git a/internal/handlers/common/findandmodify.go b/internal/handlers/common/findandmodify.go
index e8c65ce37fbb..8ed88cbed6bd 100644
--- a/internal/handlers/common/findandmodify.go
+++ b/internal/handlers/common/findandmodify.go
@@ -168,7 +168,7 @@ func PrepareDocumentForUpsert(docs []*types.Document, params *FindAndModifyParam
 
 	if len(docs) == 0 {
 		res.Operation = UpsertOperationInsert
-		res.Upsert, err = prepareDocumentForInsert(docs, params)
+		res.Upsert, err = prepareDocumentForInsert(params)
 
 		// insert operation returns null since no document existed before upsert.
 		res.ReturnValue = types.Null
@@ -196,11 +196,11 @@ func PrepareDocumentForUpsert(docs []*types.Document, params *FindAndModifyParam
 // prepareDocumentForInsert creates an insert document from the parameter.
 // When inserting new document we must check that `_id` is present, so we must extract `_id`
 // from query or generate a new one.
-func prepareDocumentForInsert(docs []*types.Document, params *FindAndModifyParams) (*types.Document, error) {
+func prepareDocumentForInsert(params *FindAndModifyParams) (*types.Document, error) {
 	insert := must.NotFail(types.NewDocument())
 
 	if params.HasUpdateOperators {
-		if _, err := UpdateDocument("findAndModify", insert, params.Update, params.Query, GetIDs(docs)); err != nil {
+		if _, err := UpdateDocument("findAndModify", insert, params.Update); err != nil {
 			return nil, err
 		}
 	} else {
@@ -224,7 +224,7 @@ func prepareDocumentForUpdate(docs []*types.Document, params *FindAndModifyParam
 	update := docs[0].DeepCopy()
 
 	if params.HasUpdateOperators {
-		if _, err := UpdateDocument("findAndModify", update, params.Update, params.Query, GetIDs(docs)); err != nil {
+		if _, err := UpdateDocument("findAndModify", update, params.Update); err != nil {
 			return nil, err
 		}
 
diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go
index 40b925ebf14c..b10e87090bd6 100644
--- a/internal/handlers/common/update.go
+++ b/internal/handlers/common/update.go
@@ -38,7 +38,7 @@ import (
 // UpdateDocument returns CommandError for findAndModify case-insensitive command name,
 // WriteError for other commands.
 // TODO https://github.com/FerretDB/FerretDB/issues/3013
-func UpdateDocument(command string, doc, update, filter *types.Document, ids []any) (bool, error) {
+func UpdateDocument(command string, doc, update *types.Document) (bool, error) {
 	var changed bool
 	var err error
 
@@ -66,13 +66,13 @@ func UpdateDocument(command string, doc, update, filter *types.Document, ids []a
 			}
 
 		case "$set":
-			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), filter, ids, false)
+			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), false)
 			if err != nil {
 				return false, err
 			}
 
 		case "$setOnInsert":
-			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), filter, ids, true)
+			changed, err = processSetFieldExpression(command, doc, updateV.(*types.Document), true)
 			if err != nil {
 				return false, err
 			}
@@ -188,39 +188,18 @@ func UpdateDocument(command string, doc, update, filter *types.Document, ids []a
 	return changed, nil
 }
 
-// GetIDs iterates each document and returns slices of IDs.
-func GetIDs(docs []*types.Document) []any {
-	ids := make([]any, len(docs))
-	for i := 0; i < len(docs); i++ {
-		ids[i] = must.NotFail(docs[i].Get("_id"))
-	}
-
-	return ids
-}
-
 // processSetFieldExpression changes document according to $set and $setOnInsert operators.
-// It uses filter and existing IDs in collection to check error on immutable _id field.
 // If the document was changed it returns true.
-func processSetFieldExpression(command string, doc, setDoc, filter *types.Document, ids []any, setOnInsert bool) (bool, error) {
+func processSetFieldExpression(command string, doc, setDoc *types.Document, setOnInsert bool) (bool, error) {
 	var changed bool
 
-	filterID, _ := filter.Get("_id")
-
 	setDocKeys := setDoc.Keys()
 	sort.Strings(setDocKeys)
 
 	for _, setKey := range setDocKeys {
 		setValue := must.NotFail(setDoc.Get(setKey))
 
-		// when filter is not set and if collection contains ID to set, it does not error
-		collectionHasID := filter == nil && slices.Contains(ids, setValue)
-		if setKey == "_id" && setValue != filterID && !collectionHasID {
-			return false, newUpdateError(
-				commonerrors.ErrImmutableField,
-				"Performing an update on the path '_id' would modify the immutable field '_id'",
-				command,
-			)
-		}
+		// TODO: validate immutable _id https://github.com/FerretDB/FerretDB/issues/3017
 
 		if setOnInsert {
 			// $setOnInsert do not set null and empty array value.
diff --git a/internal/handlers/pg/msg_findandmodify.go b/internal/handlers/pg/msg_findandmodify.go
index 7446f505e7e4..372e89736d9d 100644
--- a/internal/handlers/pg/msg_findandmodify.go
+++ b/internal/handlers/pg/msg_findandmodify.go
@@ -96,8 +96,6 @@ func (h *Handler) MsgFindAndModify(ctx context.Context, msg *wire.OpMsg) (*wire.
 			return err
 		}
 
-		ids := common.GetIDs(resDocs)
-
 		if params.Update != nil { // we have update part
 			var resValue any
 			var insertedID any
@@ -131,7 +129,7 @@ func (h *Handler) MsgFindAndModify(ctx context.Context, msg *wire.OpMsg) (*wire.
 
 				if params.HasUpdateOperators {
 					upsert = resDocs[0].DeepCopy()
-					_, err = common.UpdateDocument(document.Command(), upsert, params.Update, params.Query, ids)
+					_, err = common.UpdateDocument(document.Command(), upsert, params.Update)
 					if err != nil {
 						return err
 					}
diff --git a/internal/handlers/pg/msg_update.go b/internal/handlers/pg/msg_update.go
index b0307dbbd5ed..3fcc3485b81e 100644
--- a/internal/handlers/pg/msg_update.go
+++ b/internal/handlers/pg/msg_update.go
@@ -79,8 +79,6 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg,
 				return err
 			}
 
-			ids := common.GetIDs(resDocs)
-
 			if len(resDocs) == 0 {
 				if !u.Upsert {
 					// nothing to do, continue to the next update operation
@@ -88,8 +86,7 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg,
 				}
 
 				doc := u.Filter.DeepCopy()
-
-				if _, err = common.UpdateDocument(document.Command(), doc, u.Update, u.Filter, ids); err != nil {
+				if _, err = common.UpdateDocument(document.Command(), doc, u.Update); err != nil {
 					return err
 				}
 				if !doc.Has("_id") {
@@ -117,7 +114,7 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg,
 			matched += int32(len(resDocs))
 
 			for _, doc := range resDocs {
-				changed, err := common.UpdateDocument(document.Command(), doc, u.Update, u.Filter, ids)
+				changed, err := common.UpdateDocument(document.Command(), doc, u.Update)
 				if err != nil {
 					return err
 				}
diff --git a/internal/handlers/sqlite/msg_update.go b/internal/handlers/sqlite/msg_update.go
index 583219e6262e..8f0f479afc10 100644
--- a/internal/handlers/sqlite/msg_update.go
+++ b/internal/handlers/sqlite/msg_update.go
@@ -117,8 +117,6 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara
 
 		res.Iter.Close()
 
-		ids := common.GetIDs(resDocs)
-
 		if len(resDocs) == 0 {
 			if !u.Upsert {
 				// nothing to do, continue to the next update operation
@@ -126,7 +124,7 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara
 			}
 
 			doc := u.Filter.DeepCopy()
-			if _, err = common.UpdateDocument("update", doc, u.Update, u.Filter, ids); err != nil {
+			if _, err = common.UpdateDocument("update", doc, u.Update); err != nil {
 				return 0, 0, nil, lazyerrors.Error(err)
 			}
 
@@ -163,7 +161,7 @@ func (h *Handler) updateDocument(ctx context.Context, params *common.UpdatesPara
 		matched += int32(len(resDocs))
 
 		for _, doc := range resDocs {
-			changed, err := common.UpdateDocument("update", doc, u.Update, u.Filter, ids)
+			changed, err := common.UpdateDocument("update", doc, u.Update)
 			if err != nil {
 				return 0, 0, nil, lazyerrors.Error(err)
 			}