From 4d57f0aa7787fd449d448550c95c0f548efd117a Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 23 Jun 2023 14:57:58 +0900 Subject: [PATCH 01/17] rename test names consistent --- integration/indexes_compat_test.go | 2 +- integration/indexes_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index 03cd3a2951b9..2dcc00af1322 100644 --- a/integration/indexes_compat_test.go +++ b/integration/indexes_compat_test.go @@ -806,7 +806,7 @@ func TestDropIndexesCommandCompat(t *testing.T) { } } -func TestCreateIndexesUniqueCompat(t *testing.T) { +func TestCreateIndexesCompatUnique(t *testing.T) { setup.SkipForTigrisWithReason(t, "Indexes creation is not supported for Tigris") t.Parallel() diff --git a/integration/indexes_test.go b/integration/indexes_test.go index d674f29e9836..258bbeb08ad0 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -26,7 +26,7 @@ import ( "github.com/FerretDB/FerretDB/integration/shareddata" ) -func TestIndexesDropCommandErrors(t *testing.T) { +func TestDropIndexesCommandErrors(t *testing.T) { setup.SkipForTigrisWithReason(t, "Indexes are not supported for Tigris") t.Parallel() @@ -173,7 +173,7 @@ func TestIndexesDropCommandErrors(t *testing.T) { } } -func TestCreateIndexesInvalidSpec(t *testing.T) { +func TestCreateIndexesCommandInvalidSpec(t *testing.T) { setup.SkipForTigrisWithReason(t, "Indexes are not supported for Tigris") t.Parallel() @@ -268,7 +268,7 @@ func TestCreateIndexesInvalidSpec(t *testing.T) { } } -func TestDropIndexesInvalidCollection(t *testing.T) { +func TestDropIndexesCommandInvalidCollection(t *testing.T) { setup.SkipForTigrisWithReason(t, "Indexes are not supported for Tigris") t.Parallel() @@ -286,7 +286,7 @@ func TestDropIndexesInvalidCollection(t *testing.T) { err: &mongo.CommandError{ Code: 26, Name: "NamespaceNotFound", - Message: "ns not found TestDropIndexesInvalidCollection-NonExistentCollection.non-existent", + Message: "ns not found TestDropIndexesCommandInvalidCollection-NonExistentCollection.non-existent", }, }, "InvalidTypeCollection": { From 90980a471e8778c8fdda0b6aa90686f739d9abea Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 23 Jun 2023 15:32:06 +0900 Subject: [PATCH 02/17] fix createIndexes error --- integration/indexes_test.go | 37 ++++-- internal/handlers/commonerrors/error.go | 3 + .../handlers/commonerrors/errorcode_string.go | 120 +++++++++--------- internal/handlers/pg/msg_createindexes.go | 19 ++- 4 files changed, 108 insertions(+), 71 deletions(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 258bbeb08ad0..a056db529fe1 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -179,10 +179,12 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { t.Parallel() for name, tc := range map[string]struct { - indexes any - err *mongo.CommandError - altMessage string - skip string + indexes any // optional + missingIndexes bool // optional, if set indexes must be nil + + err *mongo.CommandError // required, expected error from MongoDB + altMessage string // optional, alternative error message for FerretDB, ignored if empty + skip string // optional, skip test with a specified reason }{ "EmptyIndexes": { indexes: bson.A{}, @@ -192,6 +194,14 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Message: "Must specify at least one index to create", }, }, + "MissingIndexes": { + missingIndexes: true, + err: &mongo.CommandError{ + Code: 40414, + Name: "Location40414", + Message: "BSON field 'createIndexes.indexes' is missing but a required field", + }, + }, "NilIndexes": { indexes: nil, err: &mongo.CommandError{ @@ -199,7 +209,6 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "Location10065", Message: "invalid parameter: expected an object (indexes)", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "InvalidType": { indexes: 42, @@ -208,7 +217,6 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "TypeMismatch", Message: "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "IDIndex": { indexes: bson.A{ @@ -251,14 +259,25 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { t.Parallel() + if tc.missingIndexes { + require.Nil(t, tc.indexes, "indexes must be nil if missingIndexes is true") + } + provider := shareddata.ArrayDocuments // one provider is enough to check for errors ctx, collection := setup.Setup(t, provider) - command := bson.D{ - {"createIndexes", collection.Name()}, - {"indexes", tc.indexes}, + var rest bson.D + + if !tc.missingIndexes { + rest = append(rest, bson.E{Key: "indexes", Value: tc.indexes}) } + command := append(bson.D{ + {"createIndexes", collection.Name()}, + }, + rest..., + ) + var res bson.D err := collection.Database().RunCommand(ctx, command).Decode(&res) diff --git a/internal/handlers/commonerrors/error.go b/internal/handlers/commonerrors/error.go index d619aff1fa50..225872a5b2bb 100644 --- a/internal/handlers/commonerrors/error.go +++ b/internal/handlers/commonerrors/error.go @@ -116,6 +116,9 @@ const ( // ErrNotImplemented indicates that a flag or command is not implemented. ErrNotImplemented = ErrorCode(238) // NotImplemented + // ErrIndexesWrongType indicates that indexes parameter has wrong type. + ErrIndexesWrongType = ErrorCode(10065) // Location10065 + // ErrDuplicateKeyInsert indicates duplicate key violation on inserting document. ErrDuplicateKeyInsert = ErrorCode(11000) // Location11000 diff --git a/internal/handlers/commonerrors/errorcode_string.go b/internal/handlers/commonerrors/errorcode_string.go index a54647fcf5d7..ec027da4c037 100644 --- a/internal/handlers/commonerrors/errorcode_string.go +++ b/internal/handlers/commonerrors/errorcode_string.go @@ -37,6 +37,7 @@ func _() { _ = x[ErrInvalidIndexSpecificationOption-197] _ = x[ErrInvalidPipelineOperator-168] _ = x[ErrNotImplemented-238] + _ = x[ErrIndexesWrongType-10065] _ = x[ErrDuplicateKeyInsert-11000] _ = x[ErrSetBadExpression-40272] _ = x[ErrStageGroupInvalidFields-15947] @@ -98,7 +99,7 @@ func _() { _ = x[ErrStageCollStatsInvalidArg-5447000] } -const _ErrorCode_name = "UnsetInternalErrorBadValueFailedToParseUnauthorizedTypeMismatchAuthenticationFailedIllegalOperationNamespaceNotFoundIndexNotFoundPathNotViableConflictingUpdateOperatorsCursorNotFoundNamespaceExistsDollarPrefixedFieldNameInvalidIDEmptyFieldNameCommandNotFoundImmutableFieldCannotCreateIndexInvalidOptionsInvalidNamespaceIndexOptionsConflictIndexKeySpecsConflictOperationFailedDocumentValidationFailureInvalidPipelineOperatorInvalidIndexSpecificationOptionNotImplementedLocation11000Location15947Location15948Location15955Location15958Location15959Location15969Location15973Location15974Location15975Location15976Location15981Location15998Location16020Location16410Location16872Location17276Location28667Location28724Location28812Location28818Location31002Location31119Location31120Location31249Location31250Location31253Location31254Location31324Location31325Location31394Location31395Location40156Location40157Location40158Location40160Location40234Location40237Location40238Location40272Location40323Location40352Location40353Location40414Location40415Location50840Location51024Location51075Location51091Location51108Location51246Location51247Location51270Location51272Location4822819Location5107200Location5107201Location5447000" +const _ErrorCode_name = "UnsetInternalErrorBadValueFailedToParseUnauthorizedTypeMismatchAuthenticationFailedIllegalOperationNamespaceNotFoundIndexNotFoundPathNotViableConflictingUpdateOperatorsCursorNotFoundNamespaceExistsDollarPrefixedFieldNameInvalidIDEmptyFieldNameCommandNotFoundImmutableFieldCannotCreateIndexInvalidOptionsInvalidNamespaceIndexOptionsConflictIndexKeySpecsConflictOperationFailedDocumentValidationFailureInvalidPipelineOperatorInvalidIndexSpecificationOptionNotImplementedLocation10065Location11000Location15947Location15948Location15955Location15958Location15959Location15969Location15973Location15974Location15975Location15976Location15981Location15998Location16020Location16410Location16872Location17276Location28667Location28724Location28812Location28818Location31002Location31119Location31120Location31249Location31250Location31253Location31254Location31324Location31325Location31394Location31395Location40156Location40157Location40158Location40160Location40234Location40237Location40238Location40272Location40323Location40352Location40353Location40414Location40415Location50840Location51024Location51075Location51091Location51108Location51246Location51247Location51270Location51272Location4822819Location5107200Location5107201Location5447000" var _ErrorCode_map = map[ErrorCode]string{ 0: _ErrorCode_name[0:5], @@ -130,64 +131,65 @@ var _ErrorCode_map = map[ErrorCode]string{ 168: _ErrorCode_name[400:423], 197: _ErrorCode_name[423:454], 238: _ErrorCode_name[454:468], - 11000: _ErrorCode_name[468:481], - 15947: _ErrorCode_name[481:494], - 15948: _ErrorCode_name[494:507], - 15955: _ErrorCode_name[507:520], - 15958: _ErrorCode_name[520:533], - 15959: _ErrorCode_name[533:546], - 15969: _ErrorCode_name[546:559], - 15973: _ErrorCode_name[559:572], - 15974: _ErrorCode_name[572:585], - 15975: _ErrorCode_name[585:598], - 15976: _ErrorCode_name[598:611], - 15981: _ErrorCode_name[611:624], - 15998: _ErrorCode_name[624:637], - 16020: _ErrorCode_name[637:650], - 16410: _ErrorCode_name[650:663], - 16872: _ErrorCode_name[663:676], - 17276: _ErrorCode_name[676:689], - 28667: _ErrorCode_name[689:702], - 28724: _ErrorCode_name[702:715], - 28812: _ErrorCode_name[715:728], - 28818: _ErrorCode_name[728:741], - 31002: _ErrorCode_name[741:754], - 31119: _ErrorCode_name[754:767], - 31120: _ErrorCode_name[767:780], - 31249: _ErrorCode_name[780:793], - 31250: _ErrorCode_name[793:806], - 31253: _ErrorCode_name[806:819], - 31254: _ErrorCode_name[819:832], - 31324: _ErrorCode_name[832:845], - 31325: _ErrorCode_name[845:858], - 31394: _ErrorCode_name[858:871], - 31395: _ErrorCode_name[871:884], - 40156: _ErrorCode_name[884:897], - 40157: _ErrorCode_name[897:910], - 40158: _ErrorCode_name[910:923], - 40160: _ErrorCode_name[923:936], - 40234: _ErrorCode_name[936:949], - 40237: _ErrorCode_name[949:962], - 40238: _ErrorCode_name[962:975], - 40272: _ErrorCode_name[975:988], - 40323: _ErrorCode_name[988:1001], - 40352: _ErrorCode_name[1001:1014], - 40353: _ErrorCode_name[1014:1027], - 40414: _ErrorCode_name[1027:1040], - 40415: _ErrorCode_name[1040:1053], - 50840: _ErrorCode_name[1053:1066], - 51024: _ErrorCode_name[1066:1079], - 51075: _ErrorCode_name[1079:1092], - 51091: _ErrorCode_name[1092:1105], - 51108: _ErrorCode_name[1105:1118], - 51246: _ErrorCode_name[1118:1131], - 51247: _ErrorCode_name[1131:1144], - 51270: _ErrorCode_name[1144:1157], - 51272: _ErrorCode_name[1157:1170], - 4822819: _ErrorCode_name[1170:1185], - 5107200: _ErrorCode_name[1185:1200], - 5107201: _ErrorCode_name[1200:1215], - 5447000: _ErrorCode_name[1215:1230], + 10065: _ErrorCode_name[468:481], + 11000: _ErrorCode_name[481:494], + 15947: _ErrorCode_name[494:507], + 15948: _ErrorCode_name[507:520], + 15955: _ErrorCode_name[520:533], + 15958: _ErrorCode_name[533:546], + 15959: _ErrorCode_name[546:559], + 15969: _ErrorCode_name[559:572], + 15973: _ErrorCode_name[572:585], + 15974: _ErrorCode_name[585:598], + 15975: _ErrorCode_name[598:611], + 15976: _ErrorCode_name[611:624], + 15981: _ErrorCode_name[624:637], + 15998: _ErrorCode_name[637:650], + 16020: _ErrorCode_name[650:663], + 16410: _ErrorCode_name[663:676], + 16872: _ErrorCode_name[676:689], + 17276: _ErrorCode_name[689:702], + 28667: _ErrorCode_name[702:715], + 28724: _ErrorCode_name[715:728], + 28812: _ErrorCode_name[728:741], + 28818: _ErrorCode_name[741:754], + 31002: _ErrorCode_name[754:767], + 31119: _ErrorCode_name[767:780], + 31120: _ErrorCode_name[780:793], + 31249: _ErrorCode_name[793:806], + 31250: _ErrorCode_name[806:819], + 31253: _ErrorCode_name[819:832], + 31254: _ErrorCode_name[832:845], + 31324: _ErrorCode_name[845:858], + 31325: _ErrorCode_name[858:871], + 31394: _ErrorCode_name[871:884], + 31395: _ErrorCode_name[884:897], + 40156: _ErrorCode_name[897:910], + 40157: _ErrorCode_name[910:923], + 40158: _ErrorCode_name[923:936], + 40160: _ErrorCode_name[936:949], + 40234: _ErrorCode_name[949:962], + 40237: _ErrorCode_name[962:975], + 40238: _ErrorCode_name[975:988], + 40272: _ErrorCode_name[988:1001], + 40323: _ErrorCode_name[1001:1014], + 40352: _ErrorCode_name[1014:1027], + 40353: _ErrorCode_name[1027:1040], + 40414: _ErrorCode_name[1040:1053], + 40415: _ErrorCode_name[1053:1066], + 50840: _ErrorCode_name[1066:1079], + 51024: _ErrorCode_name[1079:1092], + 51075: _ErrorCode_name[1092:1105], + 51091: _ErrorCode_name[1105:1118], + 51108: _ErrorCode_name[1118:1131], + 51246: _ErrorCode_name[1131:1144], + 51247: _ErrorCode_name[1144:1157], + 51270: _ErrorCode_name[1157:1170], + 51272: _ErrorCode_name[1170:1183], + 4822819: _ErrorCode_name[1183:1198], + 5107200: _ErrorCode_name[1198:1213], + 5107201: _ErrorCode_name[1213:1228], + 5447000: _ErrorCode_name[1228:1243], } func (i ErrorCode) String() string { diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index d29e34484fe2..d1231f07e1b6 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -59,9 +59,22 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return nil, err } - idxArr, err := common.GetRequiredParam[*types.Array](document, "indexes") - if err != nil { - return nil, err + v, _ := document.Get("indexes") + if v == nil { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrMissingField, + "BSON field 'createIndexes.indexes' is missing but a required field", + document.Command(), + ) + } + + idxArr, ok := v.(*types.Array) + if !ok { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrTypeMismatch, + "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", + document.Command(), + ) } if idxArr.Len() == 0 { From d390ae9807568905fa0b893adaaa29f2323776ee Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 23 Jun 2023 15:52:27 +0900 Subject: [PATCH 03/17] createIndexes null returns different error --- integration/indexes_test.go | 10 +++++++++- internal/handlers/pg/msg_createindexes.go | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index a056db529fe1..736a8ea76ff8 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -210,7 +210,15 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Message: "invalid parameter: expected an object (indexes)", }, }, - "InvalidType": { + "InvalidTypeObject": { + indexes: bson.D{}, + err: &mongo.CommandError{ + Code: 14, + Name: "TypeMismatch", + Message: "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", + }, + }, + "InvalidTypeInt": { indexes: 42, err: &mongo.CommandError{ Code: 14, diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index d1231f07e1b6..e565902aa87c 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -70,6 +70,14 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. idxArr, ok := v.(*types.Array) if !ok { + if _, ok = v.(types.NullType); ok { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrIndexesWrongType, + "invalid parameter: expected an object (indexes)", + document.Command(), + ) + } + return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", From 2459f98170ec64fc20e4d7d47ebb56b11193c548 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 23 Jun 2023 16:12:26 +0900 Subject: [PATCH 04/17] fix empty collection error --- integration/indexes_test.go | 11 ++++++----- internal/handlers/pg/msg_createindexes.go | 2 +- internal/handlers/pg/msg_dropindexes.go | 8 ++++++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 736a8ea76ff8..5a6a4f2a1e44 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -215,8 +215,9 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { err: &mongo.CommandError{ Code: 14, Name: "TypeMismatch", - Message: "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", + Message: "BSON field 'createIndexes.indexes' is the wrong type 'object', expected type 'array'", }, + altMessage: "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", }, "InvalidTypeInt": { indexes: 42, @@ -225,6 +226,7 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "TypeMismatch", Message: "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", }, + altMessage: "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", }, "IDIndex": { indexes: bson.A{ @@ -324,7 +326,7 @@ func TestDropIndexesCommandInvalidCollection(t *testing.T) { Name: "BadValue", Message: "collection name has invalid type int", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", + altMessage: "required parameter \"dropIndexes\" has type int32 (expected string)", }, "NilCollection": { collectionName: nil, @@ -334,7 +336,7 @@ func TestDropIndexesCommandInvalidCollection(t *testing.T) { Name: "BadValue", Message: "collection name has invalid type null", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", + altMessage: "required parameter \"dropIndexes\" has type types.NullType (expected string)", }, "EmptyCollection": { collectionName: "", @@ -342,9 +344,8 @@ func TestDropIndexesCommandInvalidCollection(t *testing.T) { err: &mongo.CommandError{ Code: 73, Name: "InvalidNamespace", - Message: "Invalid namespace specified 'TestIndexesDropInvalidCollection-EmptyCollection.'", + Message: "Invalid namespace specified 'TestDropIndexesCommandInvalidCollection-EmptyCollection.'", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, } { name, tc := name, tc diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index e565902aa87c..963cdbb3e1b6 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -80,7 +80,7 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, - "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", + "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", document.Command(), ) } diff --git a/internal/handlers/pg/msg_dropindexes.go b/internal/handlers/pg/msg_dropindexes.go index b5b50c6827cc..bd344ddfd31b 100644 --- a/internal/handlers/pg/msg_dropindexes.go +++ b/internal/handlers/pg/msg_dropindexes.go @@ -58,6 +58,14 @@ func (h *Handler) MsgDropIndexes(ctx context.Context, msg *wire.OpMsg) (*wire.Op return nil, err } + if collection == "" { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrInvalidNamespace, + fmt.Sprintf("Invalid namespace specified '%s.'", db), + command, + ) + } + var nIndexesWas int32 var responseMsg string From 0a5dbf3a885e3bc19760a648d4c3813c484c73cd Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 23 Jun 2023 16:28:09 +0900 Subject: [PATCH 05/17] update error message --- integration/indexes_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 5a6a4f2a1e44..83f71234c1ea 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -81,7 +81,12 @@ func TestDropIndexesCommandErrors(t *testing.T) { }, "InvalidDocumentIndex": { toDrop: bson.D{{"invalid", "invalid"}}, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", + err: &mongo.CommandError{ + Code: 27, + Name: "IndexNotFound", + Message: "can't find index with key: { invalid: \"invalid\" }", + }, + skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "NonExistentKey": { toDrop: bson.D{{"non-existent", 1}}, From 6eeaa27fc1df4ff09bb14265de1c8119cb34f52c Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 15:28:10 +0900 Subject: [PATCH 06/17] duplicate ascending ID index does not error --- integration/indexes_compat_test.go | 1 - internal/handlers/pg/msg_createindexes.go | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index 2dcc00af1322..ab4f5c5f48ef 100644 --- a/integration/indexes_compat_test.go +++ b/integration/indexes_compat_test.go @@ -104,7 +104,6 @@ func TestCreateIndexesCompat(t *testing.T) { Keys: bson.D{{"_id", 1}}, // this index is already created by default }, }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "DescendingID": { models: []mongo.IndexModel{ diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index 963cdbb3e1b6..d821df17663f 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -123,7 +123,13 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return err } - if err = pgdb.CreateIndexIfNotExists(ctx, tx, db, collection, index); err != nil { + err = pgdb.CreateIndexIfNotExists(ctx, tx, db, collection, index) + if errors.Is(err, pgdb.ErrIndexKeyAlreadyExist) && index.Name == "_id_1" { + // ascending _id index is created by default + return nil + } + + if err != nil { return err } } From 9e92996c17996424bfca6d35cf5435aa98052d4c Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 16:19:45 +0900 Subject: [PATCH 07/17] check index creation duplicate, type errors --- integration/indexes_test.go | 9 ++ internal/handlers/commonerrors/error.go | 3 + .../handlers/commonerrors/errorcode_string.go | 142 +++++++++--------- internal/handlers/pg/msg_createindexes.go | 21 ++- 4 files changed, 102 insertions(+), 73 deletions(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 83f71234c1ea..3a11b90efee5 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -233,6 +233,15 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { }, altMessage: "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", }, + "InvalidTypeArrayString": { + indexes: bson.A{"invalid"}, + err: &mongo.CommandError{ + Code: 14, + Name: "TypeMismatch", + Message: "BSON field 'createIndexes.indexes.0' is the wrong type 'string', expected type 'object'", + }, + altMessage: "BSON field 'createIndexes.indexes.0' is the wrong type, expected type 'object'", + }, "IDIndex": { indexes: bson.A{ bson.D{ diff --git a/internal/handlers/commonerrors/error.go b/internal/handlers/commonerrors/error.go index 08667be85e7e..91297f05824b 100644 --- a/internal/handlers/commonerrors/error.go +++ b/internal/handlers/commonerrors/error.go @@ -89,6 +89,9 @@ const ( // ErrCannotCreateIndex indicates that index creation process failed because some data are not valid. ErrCannotCreateIndex = ErrorCode(67) // CannotCreateIndex + // ErrIndexAlreadyExists indicates that identical index already exists. + ErrIndexAlreadyExists = ErrorCode(68) // IndexAlreadyExists + // ErrInvalidOptions indicates that _id index cannot be deleted. ErrInvalidOptions = ErrorCode(72) // InvalidOptions diff --git a/internal/handlers/commonerrors/errorcode_string.go b/internal/handlers/commonerrors/errorcode_string.go index ad2f47f633d1..ea547a4c7dee 100644 --- a/internal/handlers/commonerrors/errorcode_string.go +++ b/internal/handlers/commonerrors/errorcode_string.go @@ -28,6 +28,7 @@ func _() { _ = x[ErrCommandNotFound-59] _ = x[ErrImmutableField-66] _ = x[ErrCannotCreateIndex-67] + _ = x[ErrIndexAlreadyExists-68] _ = x[ErrInvalidOptions-72] _ = x[ErrInvalidNamespace-73] _ = x[ErrIndexOptionsConflict-85] @@ -100,7 +101,7 @@ func _() { _ = x[ErrStageCollStatsInvalidArg-5447000] } -const _ErrorCode_name = "UnsetInternalErrorBadValueFailedToParseUnauthorizedTypeMismatchAuthenticationFailedIllegalOperationNamespaceNotFoundIndexNotFoundPathNotViableConflictingUpdateOperatorsCursorNotFoundNamespaceExistsDollarPrefixedFieldNameInvalidIDEmptyFieldNameCommandNotFoundImmutableFieldCannotCreateIndexInvalidOptionsInvalidNamespaceIndexOptionsConflictIndexKeySpecsConflictOperationFailedDocumentValidationFailureInvalidPipelineOperatorInvalidIndexSpecificationOptionNotImplementedLocation10065Location11000Location15947Location15948Location15955Location15958Location15959Location15969Location15973Location15974Location15975Location15976Location15981Location15998Location16020Location16410Location16872Location17276Location28667Location28724Location28812Location28818Location31002Location31119Location31120Location31249Location31250Location31253Location31254Location31324Location31325Location31394Location31395Location40156Location40157Location40158Location40160Location40181Location40234Location40237Location40238Location40272Location40323Location40352Location40353Location40414Location40415Location50840Location51024Location51075Location51091Location51108Location51246Location51247Location51270Location51272Location4822819Location5107200Location5107201Location5447000" +const _ErrorCode_name = "UnsetInternalErrorBadValueFailedToParseUnauthorizedTypeMismatchAuthenticationFailedIllegalOperationNamespaceNotFoundIndexNotFoundPathNotViableConflictingUpdateOperatorsCursorNotFoundNamespaceExistsDollarPrefixedFieldNameInvalidIDEmptyFieldNameCommandNotFoundImmutableFieldCannotCreateIndexIndexAlreadyExistsInvalidOptionsInvalidNamespaceIndexOptionsConflictIndexKeySpecsConflictOperationFailedDocumentValidationFailureInvalidPipelineOperatorInvalidIndexSpecificationOptionNotImplementedLocation10065Location11000Location15947Location15948Location15955Location15958Location15959Location15969Location15973Location15974Location15975Location15976Location15981Location15998Location16020Location16410Location16872Location17276Location28667Location28724Location28812Location28818Location31002Location31119Location31120Location31249Location31250Location31253Location31254Location31324Location31325Location31394Location31395Location40156Location40157Location40158Location40160Location40181Location40234Location40237Location40238Location40272Location40323Location40352Location40353Location40414Location40415Location50840Location51024Location51075Location51091Location51108Location51246Location51247Location51270Location51272Location4822819Location5107200Location5107201Location5447000" var _ErrorCode_map = map[ErrorCode]string{ 0: _ErrorCode_name[0:5], @@ -123,75 +124,76 @@ var _ErrorCode_map = map[ErrorCode]string{ 59: _ErrorCode_name[243:258], 66: _ErrorCode_name[258:272], 67: _ErrorCode_name[272:289], - 72: _ErrorCode_name[289:303], - 73: _ErrorCode_name[303:319], - 85: _ErrorCode_name[319:339], - 86: _ErrorCode_name[339:360], - 96: _ErrorCode_name[360:375], - 121: _ErrorCode_name[375:400], - 168: _ErrorCode_name[400:423], - 197: _ErrorCode_name[423:454], - 238: _ErrorCode_name[454:468], - 10065: _ErrorCode_name[468:481], - 11000: _ErrorCode_name[481:494], - 15947: _ErrorCode_name[494:507], - 15948: _ErrorCode_name[507:520], - 15955: _ErrorCode_name[520:533], - 15958: _ErrorCode_name[533:546], - 15959: _ErrorCode_name[546:559], - 15969: _ErrorCode_name[559:572], - 15973: _ErrorCode_name[572:585], - 15974: _ErrorCode_name[585:598], - 15975: _ErrorCode_name[598:611], - 15976: _ErrorCode_name[611:624], - 15981: _ErrorCode_name[624:637], - 15998: _ErrorCode_name[637:650], - 16020: _ErrorCode_name[650:663], - 16410: _ErrorCode_name[663:676], - 16872: _ErrorCode_name[676:689], - 17276: _ErrorCode_name[689:702], - 28667: _ErrorCode_name[702:715], - 28724: _ErrorCode_name[715:728], - 28812: _ErrorCode_name[728:741], - 28818: _ErrorCode_name[741:754], - 31002: _ErrorCode_name[754:767], - 31119: _ErrorCode_name[767:780], - 31120: _ErrorCode_name[780:793], - 31249: _ErrorCode_name[793:806], - 31250: _ErrorCode_name[806:819], - 31253: _ErrorCode_name[819:832], - 31254: _ErrorCode_name[832:845], - 31324: _ErrorCode_name[845:858], - 31325: _ErrorCode_name[858:871], - 31394: _ErrorCode_name[871:884], - 31395: _ErrorCode_name[884:897], - 40156: _ErrorCode_name[897:910], - 40157: _ErrorCode_name[910:923], - 40158: _ErrorCode_name[923:936], - 40160: _ErrorCode_name[936:949], - 40181: _ErrorCode_name[949:962], - 40234: _ErrorCode_name[962:975], - 40237: _ErrorCode_name[975:988], - 40238: _ErrorCode_name[988:1001], - 40272: _ErrorCode_name[1001:1014], - 40323: _ErrorCode_name[1014:1027], - 40352: _ErrorCode_name[1027:1040], - 40353: _ErrorCode_name[1040:1053], - 40414: _ErrorCode_name[1053:1066], - 40415: _ErrorCode_name[1066:1079], - 50840: _ErrorCode_name[1079:1092], - 51024: _ErrorCode_name[1092:1105], - 51075: _ErrorCode_name[1105:1118], - 51091: _ErrorCode_name[1118:1131], - 51108: _ErrorCode_name[1131:1144], - 51246: _ErrorCode_name[1144:1157], - 51247: _ErrorCode_name[1157:1170], - 51270: _ErrorCode_name[1170:1183], - 51272: _ErrorCode_name[1183:1196], - 4822819: _ErrorCode_name[1196:1211], - 5107200: _ErrorCode_name[1211:1226], - 5107201: _ErrorCode_name[1226:1241], - 5447000: _ErrorCode_name[1241:1256], + 68: _ErrorCode_name[289:307], + 72: _ErrorCode_name[307:321], + 73: _ErrorCode_name[321:337], + 85: _ErrorCode_name[337:357], + 86: _ErrorCode_name[357:378], + 96: _ErrorCode_name[378:393], + 121: _ErrorCode_name[393:418], + 168: _ErrorCode_name[418:441], + 197: _ErrorCode_name[441:472], + 238: _ErrorCode_name[472:486], + 10065: _ErrorCode_name[486:499], + 11000: _ErrorCode_name[499:512], + 15947: _ErrorCode_name[512:525], + 15948: _ErrorCode_name[525:538], + 15955: _ErrorCode_name[538:551], + 15958: _ErrorCode_name[551:564], + 15959: _ErrorCode_name[564:577], + 15969: _ErrorCode_name[577:590], + 15973: _ErrorCode_name[590:603], + 15974: _ErrorCode_name[603:616], + 15975: _ErrorCode_name[616:629], + 15976: _ErrorCode_name[629:642], + 15981: _ErrorCode_name[642:655], + 15998: _ErrorCode_name[655:668], + 16020: _ErrorCode_name[668:681], + 16410: _ErrorCode_name[681:694], + 16872: _ErrorCode_name[694:707], + 17276: _ErrorCode_name[707:720], + 28667: _ErrorCode_name[720:733], + 28724: _ErrorCode_name[733:746], + 28812: _ErrorCode_name[746:759], + 28818: _ErrorCode_name[759:772], + 31002: _ErrorCode_name[772:785], + 31119: _ErrorCode_name[785:798], + 31120: _ErrorCode_name[798:811], + 31249: _ErrorCode_name[811:824], + 31250: _ErrorCode_name[824:837], + 31253: _ErrorCode_name[837:850], + 31254: _ErrorCode_name[850:863], + 31324: _ErrorCode_name[863:876], + 31325: _ErrorCode_name[876:889], + 31394: _ErrorCode_name[889:902], + 31395: _ErrorCode_name[902:915], + 40156: _ErrorCode_name[915:928], + 40157: _ErrorCode_name[928:941], + 40158: _ErrorCode_name[941:954], + 40160: _ErrorCode_name[954:967], + 40181: _ErrorCode_name[967:980], + 40234: _ErrorCode_name[980:993], + 40237: _ErrorCode_name[993:1006], + 40238: _ErrorCode_name[1006:1019], + 40272: _ErrorCode_name[1019:1032], + 40323: _ErrorCode_name[1032:1045], + 40352: _ErrorCode_name[1045:1058], + 40353: _ErrorCode_name[1058:1071], + 40414: _ErrorCode_name[1071:1084], + 40415: _ErrorCode_name[1084:1097], + 50840: _ErrorCode_name[1097:1110], + 51024: _ErrorCode_name[1110:1123], + 51075: _ErrorCode_name[1123:1136], + 51091: _ErrorCode_name[1136:1149], + 51108: _ErrorCode_name[1149:1162], + 51246: _ErrorCode_name[1162:1175], + 51247: _ErrorCode_name[1175:1188], + 51270: _ErrorCode_name[1188:1201], + 51272: _ErrorCode_name[1201:1214], + 4822819: _ErrorCode_name[1214:1229], + 5107200: _ErrorCode_name[1229:1244], + 5107201: _ErrorCode_name[1244:1259], + 5447000: _ErrorCode_name[1259:1274], } func (i ErrorCode) String() string { diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index d821df17663f..97221cb463b5 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -96,10 +96,12 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. iter := idxArr.Iterator() defer iter.Close() + duplicateChecker := make(map[string]struct{}) + err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error { for { var val any - _, val, err = iter.Next() + i, val, err := iter.Next() switch { case err == nil: @@ -113,8 +115,11 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. indexDoc, ok := val.(*types.Document) if !ok { - // TODO Add better validation and return proper error: https://github.com/FerretDB/FerretDB/issues/2311 - return lazyerrors.Errorf("expected index document, got %T", val) + return commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrTypeMismatch, + fmt.Sprintf("BSON field 'createIndexes.indexes.%d' is the wrong type, expected type 'object'", i), + document.Command(), + ) } var index *pgdb.Index @@ -123,6 +128,16 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return err } + if _, ok := duplicateChecker[index.Name]; ok { + return commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrIndexAlreadyExists, + fmt.Sprintf("Identical index already exists: %s", index.Name), + document.Command(), + ) + } + + duplicateChecker[index.Name] = struct{}{} + err = pgdb.CreateIndexIfNotExists(ctx, tx, db, collection, index) if errors.Is(err, pgdb.ErrIndexKeyAlreadyExist) && index.Name == "_id_1" { // ascending _id index is created by default From efd664cc31cdf47eab99125a4352a5ef63168b2f Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 17:09:14 +0900 Subject: [PATCH 08/17] cover parameter tests for create --- integration/indexes_compat_test.go | 2 - integration/indexes_test.go | 95 +++++++++++++++++++++++ internal/handlers/pg/msg_createindexes.go | 24 +++++- 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index ab4f5c5f48ef..f4203d3183fb 100644 --- a/integration/indexes_compat_test.go +++ b/integration/indexes_compat_test.go @@ -335,14 +335,12 @@ func TestCreateIndexesCommandCompat(t *testing.T) { key: bson.D{{"v", -1}}, indexName: "custom-name", resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "IndexNameNotSet": { collectionName: "test", key: bson.D{{"v", -1}}, indexName: nil, resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "EmptyIndexName": { collectionName: "test", diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 3a11b90efee5..f70a73bfa6d9 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -257,6 +257,19 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { ` Specification: { key: { _id: 1 }, name: "_id_", unique: true, v: 2 }`, }, }, + "NameNotSet": { + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", 1}}}, + }, + }, + err: &mongo.CommandError{ + Code: 9, + Name: "FailedToParse", + Message: `Error in specification { key: { v: 1 } } :: caused by :: ` + + `The 'name' field is a required property of an index specification`, + }, + }, "UniqueTypeDocument": { indexes: bson.A{ bson.D{ @@ -311,6 +324,88 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { } } +func TestCreateIndexesCommandInvalidCollection(t *testing.T) { + setup.SkipForTigrisWithReason(t, "Indexes are not supported for Tigris") + + t.Parallel() + + for name, tc := range map[string]struct { + collectionName any + indexes any + err *mongo.CommandError + altMessage string + skip string + }{ + "InvalidTypeCollection": { + collectionName: 42, + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", 1}}}, + {"name", "v_1"}, + }, + }, + err: &mongo.CommandError{ + Code: 2, + Name: "BadValue", + Message: "collection name has invalid type int", + }, + altMessage: "required parameter \"createIndexes\" has type int32 (expected string)", + }, + "NilCollection": { + collectionName: nil, + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", 1}}}, + {"name", "v_1"}, + }, + }, + err: &mongo.CommandError{ + Code: 2, + Name: "BadValue", + Message: "collection name has invalid type null", + }, + altMessage: "required parameter \"createIndexes\" has type types.NullType (expected string)", + }, + "EmptyCollection": { + collectionName: "", + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", 1}}}, + {"name", "v_1"}, + }, + }, + err: &mongo.CommandError{ + Code: 73, + Name: "InvalidNamespace", + Message: "Invalid namespace specified 'TestCreateIndexesCommandInvalidCollection-EmptyCollection.'", + }, + }, + } { + name, tc := name, tc + t.Run(name, func(t *testing.T) { + if tc.skip != "" { + t.Skip(tc.skip) + } + + t.Parallel() + + provider := shareddata.ArrayDocuments // one provider is enough to check for errors + ctx, collection := setup.Setup(t, provider) + + command := bson.D{ + {"createIndexes", tc.collectionName}, + {"indexes", tc.indexes}, + } + + var res bson.D + err := collection.Database().RunCommand(ctx, command).Decode(&res) + + require.Nil(t, res) + AssertEqualAltCommandError(t, *tc.err, tc.altMessage, err) + }) + } +} + func TestDropIndexesCommandInvalidCollection(t *testing.T) { setup.SkipForTigrisWithReason(t, "Indexes are not supported for Tigris") diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index 97221cb463b5..d42ee1aaa218 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -59,6 +59,14 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return nil, err } + if collection == "" { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrInvalidNamespace, + fmt.Sprintf("Invalid namespace specified '%s.'", db), + command, + ) + } + v, _ := document.Get("indexes") if v == nil { return nil, commonerrors.NewCommandErrorMsgWithArgument( @@ -246,9 +254,19 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { return nil, err } - // Process required param "name" - index.Name, err = common.GetRequiredParam[string](indexDoc, "name") - if err != nil { + v, _ := indexDoc.Get("name") + if v == nil { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrFailedToParse, + fmt.Sprintf("Error in specification %s :: caused by :: The 'name' field is a required property of an index specification", types.FormatAnyValue(index.Key)), + "createIndexes", + ) + } + + var ok bool + index.Name, ok = v.(string) + + if !ok { return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, "'name' option must be specified as a string", From d88d84a32c3d4a50a028d16c97605052b4f7c9ab Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 17:18:14 +0900 Subject: [PATCH 09/17] lint --- internal/handlers/pg/msg_createindexes.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index d42ee1aaa218..90de41534701 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -104,12 +104,12 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. iter := idxArr.Iterator() defer iter.Close() - duplicateChecker := make(map[string]struct{}) + duplicateChecker := map[string]struct{}{} err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error { for { - var val any - i, val, err := iter.Next() + var key, val any + key, val, err = iter.Next() switch { case err == nil: @@ -125,7 +125,7 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. if !ok { return commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, - fmt.Sprintf("BSON field 'createIndexes.indexes.%d' is the wrong type, expected type 'object'", i), + fmt.Sprintf("BSON field 'createIndexes.indexes.%d' is the wrong type, expected type 'object'", key), document.Command(), ) } @@ -258,7 +258,11 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { if v == nil { return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrFailedToParse, - fmt.Sprintf("Error in specification %s :: caused by :: The 'name' field is a required property of an index specification", types.FormatAnyValue(index.Key)), + fmt.Sprintf( + "Error in specification { key: %s } :: caused by :: "+ + "The 'name' field is a required property of an index specification", + types.FormatAnyValue(keyDoc), + ), "createIndexes", ) } From 51c6da4f49fed7718e2f5c6e15214d346958d833 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 18:19:40 +0900 Subject: [PATCH 10/17] hanlde missing parameters --- integration/indexes_compat_test.go | 3 --- integration/indexes_test.go | 26 +++++++++++++++++++++- internal/handlers/pg/msg_createindexes.go | 27 ++++++++++++++++++++++- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index f4203d3183fb..3de19946637d 100644 --- a/integration/indexes_compat_test.go +++ b/integration/indexes_compat_test.go @@ -347,7 +347,6 @@ func TestCreateIndexesCommandCompat(t *testing.T) { key: bson.D{{"v", -1}}, indexName: "", resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "NonStringIndexName": { collectionName: "test", @@ -359,7 +358,6 @@ func TestCreateIndexesCommandCompat(t *testing.T) { collectionName: "test", key: bson.D{{"_id", 1}, {"v", 1}}, indexName: "_id_", // the same name as the default index - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "InvalidKey": { collectionName: "test", @@ -374,7 +372,6 @@ func TestCreateIndexesCommandCompat(t *testing.T) { "KeyNotSet": { collectionName: "test", resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "UniqueFalse": { collectionName: "unique_false", diff --git a/integration/indexes_test.go b/integration/indexes_test.go index f70a73bfa6d9..c1f60af6eed3 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -257,7 +257,7 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { ` Specification: { key: { _id: 1 }, name: "_id_", unique: true, v: 2 }`, }, }, - "NameNotSet": { + "MissingName": { indexes: bson.A{ bson.D{ {"key", bson.D{{"v", 1}}}, @@ -270,6 +270,30 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { `The 'name' field is a required property of an index specification`, }, }, + "EmptyName": { + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", -1}}}, + {"name", ""}, + }, + }, + err: &mongo.CommandError{ + Code: 67, + Name: "CannotCreateIndex", + Message: `Error in specification { key: { v: -1 }, name: "", v: 2 } :: caused by :: index name cannot be empty`, + }, + altMessage: `Error in specification { key: { v: -1 }, name: "" } :: caused by :: index name cannot be empty`, + }, + "MissingKey": { + indexes: bson.A{ + bson.D{}, + }, + err: &mongo.CommandError{ + Code: 9, + Name: "FailedToParse", + Message: `Error in specification {} :: caused by :: The 'key' field is a required property of an index specification`, + }, + }, "UniqueTypeDocument": { indexes: bson.A{ bson.D{ diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index 90de41534701..c08f6cf8afa0 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -136,6 +136,17 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return err } + if index.Name == "" { + return commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrCannotCreateIndex, + fmt.Sprintf( + "Error in specification %s :: caused by :: index name cannot be empty", + types.FormatAnyValue(indexDoc), + ), + document.Command(), + ) + } + if _, ok := duplicateChecker[index.Name]; ok { return commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrIndexAlreadyExists, @@ -169,7 +180,7 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. ) case errors.Is(err, pgdb.ErrIndexNameAlreadyExist): return nil, commonerrors.NewCommandErrorMsgWithArgument( - commonerrors.ErrIndexKeySpecsConflict, + commonerrors.ErrBadValue, "One of the specified indexes already exists with a different key", document.Command(), ) @@ -201,6 +212,7 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { iter := indexDoc.Iterator() defer iter.Close() + var hasValue bool for { opt, _, err := iter.Next() @@ -208,11 +220,24 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { case err == nil: // do nothing case errors.Is(err, iterator.ErrIteratorDone): + if !hasValue { + return nil, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrFailedToParse, + fmt.Sprintf( + "Error in specification {} :: caused by :: "+ + "The 'key' field is a required property of an index specification", + ), + "createIndexes", + ) + } + return &index, nil default: return nil, lazyerrors.Error(err) } + hasValue = true + // Process required param "key" var keyDoc *types.Document From 3a81b5238c0c38d9a4109b71a048b7ff04f41aa8 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 19:23:34 +0900 Subject: [PATCH 11/17] unique index return additional resp --- integration/indexes_compat_test.go | 37 ++++++++++--------- internal/handlers/pg/msg_createindexes.go | 44 +++++++++++++++++++---- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index 3de19946637d..d68b9949f526 100644 --- a/integration/indexes_compat_test.go +++ b/integration/indexes_compat_test.go @@ -378,7 +378,6 @@ func TestCreateIndexesCommandCompat(t *testing.T) { key: bson.D{{"v", 1}}, indexName: "unique_false", unique: false, - skip: "https://github.com/FerretDB/FerretDB/issues/2845", }, "UniqueTypeDocument": { collectionName: "test", @@ -445,27 +444,31 @@ func TestCreateIndexesCommandCompat(t *testing.T) { assert.Equal(t, compatRes, targetRes) - targetErr = targetCollection.Database().RunCommand( - ctx, bson.D{{"listIndexes", tc.collectionName}}, - ).Decode(&targetRes) + targetCursor, targetErr := targetCollection.Indexes().List(ctx) + compatCursor, compatErr := compatCollection.Indexes().List(ctx) - compatErr = compatCollection.Database().RunCommand( - ctx, bson.D{{"listIndexes", tc.collectionName}}, - ).Decode(&targetRes) + if targetCursor != nil { + defer targetCursor.Close(ctx) + } + if compatCursor != nil { + defer compatCursor.Close(ctx) + } - require.Nil(t, targetRes) - require.Nil(t, compatRes) + require.NoError(t, targetErr) + require.NoError(t, compatErr) - if targetErr != nil { - t.Logf("Target error: %v", targetErr) - t.Logf("Compat error: %v", compatErr) + targetListRes := FetchAll(t, ctx, targetCursor) + compatListRes := FetchAll(t, ctx, compatCursor) - // error messages are intentionally not compared - AssertMatchesCommandError(t, compatErr, targetErr) + assert.Equal(t, compatListRes, targetListRes) - return - } - require.NoError(t, compatErr, "compat error; target returned no error") + targetSpec, targetErr := targetCollection.Indexes().ListSpecifications(ctx) + compatSpec, compatErr := compatCollection.Indexes().ListSpecifications(ctx) + + require.NoError(t, compatErr) + require.NoError(t, targetErr) + + assert.Equal(t, compatSpec, targetSpec) }) } } diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index c08f6cf8afa0..afbfc681bc0c 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -106,7 +106,22 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. duplicateChecker := map[string]struct{}{} + var isUniqueSpecified bool + var numIndexesBefore, numIndexesAfter int32 err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error { + indexesBefore, err := pgdb.Indexes(ctx, tx, db, collection) + if err == nil { + numIndexesBefore = int32(len(indexesBefore)) + } + if errors.Is(err, pgdb.ErrTableNotExist) { + numIndexesBefore = 1 + err = nil + } + + if err != nil { + return lazyerrors.Error(err) + } + for { var key, val any key, val, err = iter.Next() @@ -116,6 +131,13 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. // do nothing case errors.Is(err, iterator.ErrIteratorDone): // iterator is done, no more indexes to create + indexesAfter, err := pgdb.Indexes(ctx, tx, db, collection) + if err != nil { + return lazyerrors.Error(err) + } + + numIndexesAfter = int32(len(indexesAfter)) + return nil default: return lazyerrors.Error(err) @@ -166,6 +188,8 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. if err != nil { return err } + + isUniqueSpecified = index.Unique != nil } }) @@ -195,11 +219,19 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return nil, lazyerrors.Error(err) } + res := new(types.Document) + + if isUniqueSpecified { + res.Set("numIndexesBefore", numIndexesBefore) + res.Set("numIndexesAfter", numIndexesAfter) + res.Set("createdCollectionAutomatically", true) + } + + res.Set("ok", float64(1)) + var reply wire.OpMsg must.NoError(reply.SetSections(wire.OpMsgSection{ - Documents: []*types.Document{must.NotFail(types.NewDocument( - "ok", float64(1), - ))}, + Documents: []*types.Document{res}, })) return &reply, nil @@ -308,9 +340,9 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { // already processed, do nothing case "unique": - uniqueVal := must.NotFail(indexDoc.Get("unique")) + v := must.NotFail(indexDoc.Get("unique")) - _, ok := uniqueVal.(bool) + unique, ok := v.(bool) if !ok { return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, @@ -319,7 +351,7 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { ":: caused by :: "+ "The field 'unique' has value unique: %[3]s, which is not convertible to bool", types.FormatAnyValue(must.NotFail(indexDoc.Get("key"))), - index.Name, types.FormatAnyValue(uniqueVal), + index.Name, types.FormatAnyValue(v), ), "createIndexes", ) From 34764d82f7121c4b48bb4673f9bac30cdfac22c0 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 19:32:06 +0900 Subject: [PATCH 12/17] fix build --- integration/indexes_test.go | 1 - internal/handlers/pg/msg_createindexes.go | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index c1f60af6eed3..951f57924549 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -86,7 +86,6 @@ func TestDropIndexesCommandErrors(t *testing.T) { Name: "IndexNotFound", Message: "can't find index with key: { invalid: \"invalid\" }", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "NonExistentKey": { toDrop: bson.D{{"non-existent", 1}}, diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index afbfc681bc0c..9ccf3659f1be 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -342,7 +342,7 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { case "unique": v := must.NotFail(indexDoc.Get("unique")) - unique, ok := v.(bool) + _, ok := v.(bool) if !ok { return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, @@ -430,8 +430,8 @@ func processIndexKey(keyDoc *types.Document) (pgdb.IndexKey, error) { if orderParam, err = commonparams.GetWholeNumberParam(order); err != nil { return nil, commonerrors.NewCommandErrorMsgWithArgument( - commonerrors.ErrNotImplemented, - fmt.Sprintf("Index key value %q is not implemented yet", order), + commonerrors.ErrIndexNotFound, + fmt.Sprintf("can't find index with key: { %s: \"%s\" }", field, order), "createIndexes", ) } From dfa6f9de5351b9cb03288d4211e1150308558391 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 19:34:16 +0900 Subject: [PATCH 13/17] linter --- internal/handlers/pg/msg_createindexes.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index 9ccf3659f1be..be345a8f4bad 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -109,7 +109,8 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. var isUniqueSpecified bool var numIndexesBefore, numIndexesAfter int32 err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error { - indexesBefore, err := pgdb.Indexes(ctx, tx, db, collection) + var indexesBefore []pgdb.Index + indexesBefore, err = pgdb.Indexes(ctx, tx, db, collection) if err == nil { numIndexesBefore = int32(len(indexesBefore)) } @@ -130,8 +131,8 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. case err == nil: // do nothing case errors.Is(err, iterator.ErrIteratorDone): - // iterator is done, no more indexes to create - indexesAfter, err := pgdb.Indexes(ctx, tx, db, collection) + var indexesAfter []pgdb.Index + indexesAfter, err = pgdb.Indexes(ctx, tx, db, collection) if err != nil { return lazyerrors.Error(err) } From e4ce5bfe859cd52b3d99a7a8540705d1fd448d0b Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 19:58:58 +0900 Subject: [PATCH 14/17] more test --- integration/indexes_compat_test.go | 13 ++++++++++++- integration/indexes_test.go | 29 +++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index d68b9949f526..f6b9a10daa04 100644 --- a/integration/indexes_compat_test.go +++ b/integration/indexes_compat_test.go @@ -172,7 +172,18 @@ func TestCreateIndexesCompat(t *testing.T) { {Keys: bson.D{{"v", 1}}}, }, resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", + skip: "https://github.com/FerretDB/FerretDB/issues/2910", + // the error for existing and non-existing collection are different, + // below is the error for existing collection. + // + // &mongo.CommandError{ + // Code: 96, + // Name: "OperationFailed", + // Message: `Index build failed: 7a1c4cc3-8ac6-44d3-92e0-57853e6bc837: Collection ` + + // `TestCreateIndexesCommandInvalidSpec-SameIndex.TestCreateIndexesCommandInvalidSpec-SameIndex ` + + // `( 020f17e0-7847-45f2-8397-c631c5e9bdaf ) :: caused by :: Cannot build two identical indexes. ` + + // `Try again without duplicate indexes.`, + // }, }, "MultiWithInvalid": { models: []mongo.IndexModel{ diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 951f57924549..410adf241118 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -185,6 +185,7 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { for name, tc := range map[string]struct { indexes any // optional missingIndexes bool // optional, if set indexes must be nil + noProvider bool // if set, no provider is added. err *mongo.CommandError // required, expected error from MongoDB altMessage string // optional, alternative error message for FerretDB, ignored if empty @@ -293,6 +294,25 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Message: `Error in specification {} :: caused by :: The 'key' field is a required property of an index specification`, }, }, + "SameIndex": { + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", 1}}}, + {"name", "v_1"}, + }, + bson.D{ + {"key", bson.D{{"v", 1}}}, + {"name", "v_1"}, + }, + }, + noProvider: true, + err: &mongo.CommandError{ + Code: 68, + Name: "IndexAlreadyExists", + Message: `Identical index already exists: v_1`, + }, + skip: "", + }, "UniqueTypeDocument": { indexes: bson.A{ bson.D{ @@ -323,8 +343,13 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { require.Nil(t, tc.indexes, "indexes must be nil if missingIndexes is true") } - provider := shareddata.ArrayDocuments // one provider is enough to check for errors - ctx, collection := setup.Setup(t, provider) + var providers []shareddata.Provider + if !tc.noProvider { + // one provider is enough to check for errors + providers = append(providers, shareddata.ArrayDocuments) + } + + ctx, collection := setup.Setup(t, providers...) var rest bson.D From efd4f8d878b6dda4a8434a00fbf41d50374a4461 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 26 Jun 2023 20:18:40 +0900 Subject: [PATCH 15/17] fix error --- internal/handlers/pg/msg_createindexes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index be345a8f4bad..895bacaa6eb3 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -172,7 +172,7 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. if _, ok := duplicateChecker[index.Name]; ok { return commonerrors.NewCommandErrorMsgWithArgument( - commonerrors.ErrIndexAlreadyExists, + commonerrors.ErrIndexKeySpecsConflict, fmt.Sprintf("Identical index already exists: %s", index.Name), document.Command(), ) From b2a2bd4ff69da08f4fedd6fa9b10719a8e806997 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 27 Jun 2023 12:17:51 +0900 Subject: [PATCH 16/17] make error message the same --- integration/indexes_test.go | 3 --- internal/handlers/pg/msg_createindexes.go | 11 +++++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index 410adf241118..ced28669104f 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -222,7 +222,6 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "TypeMismatch", Message: "BSON field 'createIndexes.indexes' is the wrong type 'object', expected type 'array'", }, - altMessage: "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", }, "InvalidTypeInt": { indexes: 42, @@ -231,7 +230,6 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "TypeMismatch", Message: "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", }, - altMessage: "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", }, "InvalidTypeArrayString": { indexes: bson.A{"invalid"}, @@ -240,7 +238,6 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "TypeMismatch", Message: "BSON field 'createIndexes.indexes.0' is the wrong type 'string', expected type 'object'", }, - altMessage: "BSON field 'createIndexes.indexes.0' is the wrong type, expected type 'object'", }, "IDIndex": { indexes: bson.A{ diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index 895bacaa6eb3..c34b7400c5fa 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -88,7 +88,10 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, - "BSON field 'createIndexes.indexes' is the wrong type, expected type 'array'", + fmt.Sprintf( + "BSON field 'createIndexes.indexes' is the wrong type '%s', expected type 'array'", + commonparams.AliasFromType(v), + ), document.Command(), ) } @@ -148,7 +151,11 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. if !ok { return commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, - fmt.Sprintf("BSON field 'createIndexes.indexes.%d' is the wrong type, expected type 'object'", key), + fmt.Sprintf( + "BSON field 'createIndexes.indexes.%d' is the wrong type '%s', expected type 'object'", + key, + commonparams.AliasFromType(val), + ), document.Command(), ) } From 381c58f7b86fb1b208e881cc0a3beea754ec1aad Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 27 Jun 2023 13:22:33 +0900 Subject: [PATCH 17/17] handle identical indexes, same index keys and same index names duplicates in index creation --- integration/indexes_test.go | 48 ++++++++++++++++++++++- internal/handlers/pg/msg_createindexes.go | 41 +++++++++++++++---- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/integration/indexes_test.go b/integration/indexes_test.go index ced28669104f..a4b2e242a05c 100644 --- a/integration/indexes_test.go +++ b/integration/indexes_test.go @@ -291,7 +291,7 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Message: `Error in specification {} :: caused by :: The 'key' field is a required property of an index specification`, }, }, - "SameIndex": { + "IdenticalIndex": { indexes: bson.A{ bson.D{ {"key", bson.D{{"v", 1}}}, @@ -308,7 +308,51 @@ func TestCreateIndexesCommandInvalidSpec(t *testing.T) { Name: "IndexAlreadyExists", Message: `Identical index already exists: v_1`, }, - skip: "", + }, + "SameName": { + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"foo", -1}}}, + {"name", "index-name"}, + }, + bson.D{ + {"key", bson.D{{"bar", -1}}}, + {"name", "index-name"}, + }, + }, + noProvider: true, + err: &mongo.CommandError{ + Code: 86, + Name: "IndexKeySpecsConflict", + Message: "An existing index has the same name as the requested index. " + + "When index names are not specified, they are auto generated and can " + + "cause conflicts. Please refer to our documentation. " + + "Requested index: { v: 2, key: { bar: -1 }, name: \"index-name\" }, " + + "existing index: { v: 2, key: { foo: -1 }, name: \"index-name\" }", + }, + altMessage: "An existing index has the same name as the requested index. " + + "When index names are not specified, they are auto generated and can " + + "cause conflicts. Please refer to our documentation. " + + "Requested index: { key: { bar: -1 }, name: \"index-name\" }, " + + "existing index: { key: { foo: -1 }, name: \"index-name\" }", + }, + "SameIndex": { + indexes: bson.A{ + bson.D{ + {"key", bson.D{{"v", -1}}}, + {"name", "foo"}, + }, + bson.D{ + {"key", bson.D{{"v", -1}}}, + {"name", "bar"}, + }, + }, + noProvider: true, + err: &mongo.CommandError{ + Code: 85, + Name: "IndexOptionsConflict", + Message: "Index already exists with a different name: foo", + }, }, "UniqueTypeDocument": { indexes: bson.A{ diff --git a/internal/handlers/pg/msg_createindexes.go b/internal/handlers/pg/msg_createindexes.go index c34b7400c5fa..c45dc311a2a8 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -107,7 +107,7 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. iter := idxArr.Iterator() defer iter.Close() - duplicateChecker := map[string]struct{}{} + indexes := map[*types.Document]*pgdb.Index{} var isUniqueSpecified bool var numIndexesBefore, numIndexesAfter int32 @@ -177,15 +177,40 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. ) } - if _, ok := duplicateChecker[index.Name]; ok { - return commonerrors.NewCommandErrorMsgWithArgument( - commonerrors.ErrIndexKeySpecsConflict, - fmt.Sprintf("Identical index already exists: %s", index.Name), - document.Command(), - ) + for doc, existing := range indexes { + if existing.Key.Equal(index.Key) && existing.Name == index.Name { + return commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrIndexAlreadyExists, + fmt.Sprintf("Identical index already exists: %s", existing.Name), + document.Command(), + ) + } + + if existing.Key.Equal(index.Key) { + return commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrIndexOptionsConflict, + fmt.Sprintf("Index already exists with a different name: %s", existing.Name), + document.Command(), + ) + } + + if existing.Name == index.Name { + return commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrIndexKeySpecsConflict, + fmt.Sprintf("An existing index has the same name as the requested index. "+ + "When index names are not specified, they are auto generated and can "+ + "cause conflicts. Please refer to our documentation. "+ + "Requested index: %s, "+ + "existing index: %s", + types.FormatAnyValue(indexDoc), + types.FormatAnyValue(doc), + ), + document.Command(), + ) + } } - duplicateChecker[index.Name] = struct{}{} + indexes[indexDoc] = index err = pgdb.CreateIndexIfNotExists(ctx, tx, db, collection, index) if errors.Is(err, pgdb.ErrIndexKeyAlreadyExist) && index.Name == "_id_1" {