diff --git a/integration/indexes_compat_test.go b/integration/indexes_compat_test.go index 03cd3a2951b9..f6b9a10daa04 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{ @@ -173,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{ @@ -336,21 +346,18 @@ 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", key: bson.D{{"v", -1}}, indexName: "", resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "NonStringIndexName": { collectionName: "test", @@ -362,7 +369,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", @@ -377,14 +383,12 @@ func TestCreateIndexesCommandCompat(t *testing.T) { "KeyNotSet": { collectionName: "test", resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, "UniqueFalse": { collectionName: "unique_false", key: bson.D{{"v", 1}}, indexName: "unique_false", unique: false, - skip: "https://github.com/FerretDB/FerretDB/issues/2845", }, "UniqueTypeDocument": { collectionName: "test", @@ -451,27 +455,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) }) } } @@ -806,7 +814,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..a4b2e242a05c 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() @@ -81,7 +81,11 @@ func TestIndexesDropCommandErrors(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\" }", + }, }, "NonExistentKey": { toDrop: bson.D{{"non-existent", 1}}, @@ -173,16 +177,19 @@ 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() 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 + 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 + skip string // optional, skip test with a specified reason }{ "EmptyIndexes": { indexes: bson.A{}, @@ -192,6 +199,14 @@ func TestCreateIndexesInvalidSpec(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,16 +214,30 @@ func TestCreateIndexesInvalidSpec(t *testing.T) { Name: "Location10065", Message: "invalid parameter: expected an object (indexes)", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", }, - "InvalidType": { + "InvalidTypeObject": { + indexes: bson.D{}, + err: &mongo.CommandError{ + Code: 14, + Name: "TypeMismatch", + Message: "BSON field 'createIndexes.indexes' is the wrong type 'object', expected type 'array'", + }, + }, + "InvalidTypeInt": { indexes: 42, err: &mongo.CommandError{ Code: 14, Name: "TypeMismatch", Message: "BSON field 'createIndexes.indexes' is the wrong type 'int', expected type 'array'", }, - skip: "https://github.com/FerretDB/FerretDB/issues/2311", + }, + "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'", + }, }, "IDIndex": { indexes: bson.A{ @@ -225,6 +254,106 @@ func TestCreateIndexesInvalidSpec(t *testing.T) { ` Specification: { key: { _id: 1 }, name: "_id_", unique: true, v: 2 }`, }, }, + "MissingName": { + 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`, + }, + }, + "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`, + }, + }, + "IdenticalIndex": { + 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`, + }, + }, + "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{ bson.D{ @@ -251,11 +380,109 @@ func TestCreateIndexesInvalidSpec(t *testing.T) { t.Parallel() + if tc.missingIndexes { + require.Nil(t, tc.indexes, "indexes must be nil if missingIndexes is true") + } + + 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 + + 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) + + require.Nil(t, res) + AssertEqualAltCommandError(t, *tc.err, tc.altMessage, err) + }) + } +} + +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", collection.Name()}, + {"createIndexes", tc.collectionName}, {"indexes", tc.indexes}, } @@ -268,7 +495,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 +513,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": { @@ -297,7 +524,7 @@ func TestDropIndexesInvalidCollection(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, @@ -307,7 +534,7 @@ func TestDropIndexesInvalidCollection(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: "", @@ -315,9 +542,8 @@ func TestDropIndexesInvalidCollection(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/commonerrors/error.go b/internal/handlers/commonerrors/error.go index fb9bc3534b46..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 @@ -116,6 +119,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 7522b5da5b16..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] @@ -37,6 +38,7 @@ func _() { _ = x[ErrInvalidIndexSpecificationOption-197] _ = x[ErrInvalidPipelineOperator-168] _ = x[ErrNotImplemented-238] + _ = x[ErrIndexesWrongType-10065] _ = x[ErrDuplicateKeyInsert-11000] _ = x[ErrSetBadExpression-40272] _ = x[ErrStageGroupInvalidFields-15947] @@ -99,7 +101,7 @@ func _() { _ = x[ErrStageCollStatsInvalidArg-5447000] } -const _ErrorCode_name = "UnsetInternalErrorBadValueFailedToParseUnauthorizedTypeMismatchAuthenticationFailedIllegalOperationNamespaceNotFoundIndexNotFoundPathNotViableConflictingUpdateOperatorsCursorNotFoundNamespaceExistsDollarPrefixedFieldNameInvalidIDEmptyFieldNameCommandNotFoundImmutableFieldCannotCreateIndexInvalidOptionsInvalidNamespaceIndexOptionsConflictIndexKeySpecsConflictOperationFailedDocumentValidationFailureInvalidPipelineOperatorInvalidIndexSpecificationOptionNotImplementedLocation11000Location15947Location15948Location15955Location15958Location15959Location15969Location15973Location15974Location15975Location15976Location15981Location15998Location16020Location16410Location16872Location17276Location28667Location28724Location28812Location28818Location31002Location31119Location31120Location31249Location31250Location31253Location31254Location31324Location31325Location31394Location31395Location40156Location40157Location40158Location40160Location40181Location40234Location40237Location40238Location40272Location40323Location40352Location40353Location40414Location40415Location50840Location51024Location51075Location51091Location51108Location51246Location51247Location51270Location51272Location4822819Location5107200Location5107201Location5447000" +const _ErrorCode_name = "UnsetInternalErrorBadValueFailedToParseUnauthorizedTypeMismatchAuthenticationFailedIllegalOperationNamespaceNotFoundIndexNotFoundPathNotViableConflictingUpdateOperatorsCursorNotFoundNamespaceExistsDollarPrefixedFieldNameInvalidIDEmptyFieldNameCommandNotFoundImmutableFieldCannotCreateIndexIndexAlreadyExistsInvalidOptionsInvalidNamespaceIndexOptionsConflictIndexKeySpecsConflictOperationFailedDocumentValidationFailureInvalidPipelineOperatorInvalidIndexSpecificationOptionNotImplementedLocation10065Location11000Location15947Location15948Location15955Location15958Location15959Location15969Location15973Location15974Location15975Location15976Location15981Location15998Location16020Location16410Location16872Location17276Location28667Location28724Location28812Location28818Location31002Location31119Location31120Location31249Location31250Location31253Location31254Location31324Location31325Location31394Location31395Location40156Location40157Location40158Location40160Location40181Location40234Location40237Location40238Location40272Location40323Location40352Location40353Location40414Location40415Location50840Location51024Location51075Location51091Location51108Location51246Location51247Location51270Location51272Location4822819Location5107200Location5107201Location5447000" var _ErrorCode_map = map[ErrorCode]string{ 0: _ErrorCode_name[0:5], @@ -122,74 +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], - 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], - 40181: _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], + 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 d29e34484fe2..c45dc311a2a8 100644 --- a/internal/handlers/pg/msg_createindexes.go +++ b/internal/handlers/pg/msg_createindexes.go @@ -59,9 +59,41 @@ 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 + 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( + commonerrors.ErrMissingField, + "BSON field 'createIndexes.indexes' is missing but a required field", + document.Command(), + ) + } + + 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, + fmt.Sprintf( + "BSON field 'createIndexes.indexes' is the wrong type '%s', expected type 'array'", + commonparams.AliasFromType(v), + ), + document.Command(), + ) } if idxArr.Len() == 0 { @@ -75,16 +107,41 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. iter := idxArr.Iterator() defer iter.Close() + indexes := map[*types.Document]*pgdb.Index{} + + var isUniqueSpecified bool + var numIndexesBefore, numIndexesAfter int32 err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error { + var indexesBefore []pgdb.Index + 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 val any - _, val, err = iter.Next() + var key, val any + key, val, err = iter.Next() switch { case err == nil: // do nothing case errors.Is(err, iterator.ErrIteratorDone): - // iterator is done, no more indexes to create + var indexesAfter []pgdb.Index + 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) @@ -92,8 +149,15 @@ 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 '%s', expected type 'object'", + key, + commonparams.AliasFromType(val), + ), + document.Command(), + ) } var index *pgdb.Index @@ -102,9 +166,63 @@ func (h *Handler) MsgCreateIndexes(ctx context.Context, msg *wire.OpMsg) (*wire. return err } - if err = pgdb.CreateIndexIfNotExists(ctx, tx, db, collection, index); err != nil { + 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(), + ) + } + + 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(), + ) + } + } + + indexes[indexDoc] = index + + 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 } + + isUniqueSpecified = index.Unique != nil } }) @@ -119,7 +237,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(), ) @@ -134,11 +252,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 @@ -151,6 +277,7 @@ func processIndexOptions(indexDoc *types.Document) (*pgdb.Index, error) { iter := indexDoc.Iterator() defer iter.Close() + var hasValue bool for { opt, _, err := iter.Next() @@ -158,11 +285,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 @@ -204,9 +344,23 @@ 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 { key: %s } :: caused by :: "+ + "The 'name' field is a required property of an index specification", + types.FormatAnyValue(keyDoc), + ), + "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", @@ -219,9 +373,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) + _, ok := v.(bool) if !ok { return nil, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrTypeMismatch, @@ -230,7 +384,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", ) @@ -309,8 +463,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", ) } 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