diff --git a/integration/aggregate_documents_compat_test.go b/integration/aggregate_documents_compat_test.go index 291fef076275..41fd43e8f4ea 100644 --- a/integration/aggregate_documents_compat_test.go +++ b/integration/aggregate_documents_compat_test.go @@ -835,6 +835,13 @@ func TestAggregateCompatLimit(t *testing.T) { bson.D{{"$limit", math.MaxInt64}}, }, }, + "MinInt64": { + pipeline: bson.A{ + bson.D{{"$sort", bson.D{{"_id", -1}}}}, + bson.D{{"$limit", math.MinInt64}}, + }, + resultType: emptyResult, + }, "Negative": { pipeline: bson.A{ bson.D{{"$sort", bson.D{{"_id", -1}}}}, @@ -1367,6 +1374,13 @@ func TestAggregateCompatSkip(t *testing.T) { }, resultType: emptyResult, }, + "MinInt64": { + pipeline: bson.A{ + bson.D{{"$sort", bson.D{{"_id", -1}}}}, + bson.D{{"$skip", math.MinInt64}}, + }, + resultType: emptyResult, + }, "Int64Overflow": { pipeline: bson.A{ bson.D{{"$sort", bson.D{{"_id", -1}}}}, diff --git a/integration/aggregate_stats_compat_test.go b/integration/aggregate_stats_compat_test.go index 98000eb901fa..7cee60a72cab 100644 --- a/integration/aggregate_stats_compat_test.go +++ b/integration/aggregate_stats_compat_test.go @@ -49,6 +49,17 @@ func TestAggregateCompatCollStats(t *testing.T) { "StorageStatsWithScale": { collStats: bson.D{{"storageStats", bson.D{{"scale", 1000}}}}, }, + "StorageStatsNegativeScale": { + collStats: bson.D{{"storageStats", bson.D{{"scale", -1000}}}}, + resultType: emptyResult, + }, + "StorageStatsFloatScale": { + collStats: bson.D{{"storageStats", bson.D{{"scale", 42.42}}}}, + }, + "StorageStatsInvalidScale": { + collStats: bson.D{{"storageStats", bson.D{{"scale", "invalid"}}}}, + resultType: emptyResult, + }, "CountAndStorageStats": { collStats: bson.D{{"count", bson.D{}}, {"storageStats", bson.D{}}}, }, diff --git a/integration/commands_administration_compat_test.go b/integration/commands_administration_compat_test.go new file mode 100644 index 000000000000..276f9e783d63 --- /dev/null +++ b/integration/commands_administration_compat_test.go @@ -0,0 +1,178 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package integration + +import ( + "errors" + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/mongo" + + "github.com/FerretDB/FerretDB/integration/setup" + "github.com/FerretDB/FerretDB/integration/shareddata" + "github.com/FerretDB/FerretDB/internal/util/must" +) + +func TestCommandsAdministrationCompatCollStatsWithScale(t *testing.T) { + t.Parallel() + + s := setup.SetupCompatWithOpts(t, &setup.SetupCompatOpts{ + Providers: []shareddata.Provider{shareddata.DocumentsDocuments}, + AddNonExistentCollection: true, + }) + + ctx, targetCollection, compatCollection := s.Ctx, s.TargetCollections[0], s.CompatCollections[0] + + for name, tc := range map[string]struct { //nolint:vet // for readability + scale any + resultType compatTestCaseResultType + altMessage string + }{ + "scaleOne": {scale: int32(1)}, + "scaleBig": {scale: int64(1000)}, + "scaleMaxInt": {scale: math.MaxInt64}, + "scaleZero": {scale: int32(0), resultType: emptyResult}, + "scaleNegative": {scale: int32(-100), resultType: emptyResult}, + "scaleFloat": {scale: 2.8}, + "scaleFloatNegative": {scale: -2.8, resultType: emptyResult}, + "scaleMinFloat": {scale: -math.MaxFloat64, resultType: emptyResult}, + "scaleMaxFloat": {scale: math.MaxFloat64}, + "scaleString": { + scale: "1", + resultType: emptyResult, + altMessage: `BSON field 'collStats.scale' is the wrong type 'string', expected types '[long, int, decimal, double]'`, + }, + "scaleObject": { + scale: bson.D{{"a", 1}}, + resultType: emptyResult, + altMessage: `BSON field 'collStats.scale' is the wrong type 'object', expected types '[long, int, decimal, double]'`, + }, + "scaleNull": {scale: nil}, + } { + name, tc := name, tc + + t.Run(name, func(t *testing.T) { + t.Helper() + + t.Parallel() + + var targetRes bson.D + targetCommand := bson.D{{"collStats", targetCollection.Name()}, {"scale", tc.scale}} + targetErr := targetCollection.Database().RunCommand(ctx, targetCommand).Decode(&targetRes) + + var compatRes bson.D + compatCommand := bson.D{{"collStats", compatCollection.Name()}, {"scale", tc.scale}} + compatErr := compatCollection.Database().RunCommand(ctx, compatCommand).Decode(&compatRes) + + if tc.resultType == emptyResult { + require.Error(t, compatErr) + + targetErr = UnsetRaw(t, targetErr) + compatErr = UnsetRaw(t, compatErr) + + if tc.altMessage != "" { + var expectedErr mongo.CommandError + require.True(t, errors.As(compatErr, &expectedErr)) + AssertEqualAltError(t, expectedErr, tc.altMessage, targetErr) + } else { + assert.Equal(t, compatErr, targetErr) + } + + return + } + + require.NoError(t, compatErr) + require.NoError(t, targetErr) + + targetDoc := ConvertDocument(t, targetRes) + compatDoc := ConvertDocument(t, compatRes) + + targetFactor := must.NotFail(targetDoc.Get("scaleFactor")) + compatFactor := must.NotFail(compatDoc.Get("scaleFactor")) + + assert.Equal(t, compatFactor, targetFactor) + }) + } +} + +func TestCommandsAdministrationCompatDBStatsWithScale(t *testing.T) { + t.Parallel() + + s := setup.SetupCompatWithOpts(t, &setup.SetupCompatOpts{ + Providers: []shareddata.Provider{shareddata.DocumentsDocuments}, + AddNonExistentCollection: true, + }) + + ctx, targetCollection, compatCollection := s.Ctx, s.TargetCollections[0], s.CompatCollections[0] + + for name, tc := range map[string]struct { //nolint:vet // for readability + scale any + resultType compatTestCaseResultType + altMessage string + }{ + "scaleOne": {scale: int32(1)}, + "scaleBig": {scale: int64(1000)}, + "scaleFloat": {scale: 2.8}, + "scaleNull": {scale: nil}, + } { + name, tc := name, tc + + t.Run(name, func(t *testing.T) { + t.Helper() + + t.Parallel() + + var targetRes bson.D + targetCommand := bson.D{{"dbStats", int32(1)}, {"scale", tc.scale}} + targetErr := targetCollection.Database().RunCommand(ctx, targetCommand).Decode(&targetRes) + + var compatRes bson.D + compatCommand := bson.D{{"dbStats", int32(1)}, {"scale", tc.scale}} + compatErr := compatCollection.Database().RunCommand(ctx, compatCommand).Decode(&compatRes) + + if tc.resultType == emptyResult { + require.Error(t, compatErr) + + targetErr = UnsetRaw(t, targetErr) + compatErr = UnsetRaw(t, compatErr) + + if tc.altMessage != "" { + var expectedErr mongo.CommandError + require.True(t, errors.As(compatErr, &expectedErr)) + AssertEqualAltError(t, expectedErr, tc.altMessage, targetErr) + } else { + assert.Equal(t, compatErr, targetErr) + } + + return + } + + require.NoError(t, compatErr) + require.NoError(t, targetErr) + + targetDoc := ConvertDocument(t, targetRes) + compatDoc := ConvertDocument(t, compatRes) + + targetFactor := must.NotFail(targetDoc.Get("scaleFactor")) + compatFactor := must.NotFail(compatDoc.Get("scaleFactor")) + + assert.Equal(t, compatFactor, targetFactor) + }) + } +} diff --git a/integration/count_compat_test.go b/integration/count_compat_test.go index f9f7520382bf..eb3173869d56 100644 --- a/integration/count_compat_test.go +++ b/integration/count_compat_test.go @@ -16,6 +16,7 @@ package integration import ( "errors" + "math" "testing" "github.com/stretchr/testify/assert" @@ -210,6 +211,11 @@ func TestCountCompat(t *testing.T) { optSkip: -1.888, resultType: emptyResult, }, + "SkipMinFloat": { + filter: bson.D{}, + optSkip: -math.MaxFloat64, + resultType: emptyResult, + }, "SkipNull": { filter: bson.D{}, }, diff --git a/integration/query_compat_test.go b/integration/query_compat_test.go index 1136aab43191..24189fcecb8e 100644 --- a/integration/query_compat_test.go +++ b/integration/query_compat_test.go @@ -318,6 +318,11 @@ func TestQueryCompatSkip(t *testing.T) { optSkip: pointer.ToInt64(math.MaxInt64), resultType: emptyResult, }, + "MinInt64": { + filter: bson.D{}, + optSkip: pointer.ToInt64(math.MinInt64), + resultType: emptyResult, + }, } testQueryCompat(t, testCases) diff --git a/internal/handlers/common/aggregations/collstats.go b/internal/handlers/common/aggregations/collstats.go index ca4fb21371dc..8592eb6c04b7 100644 --- a/internal/handlers/common/aggregations/collstats.go +++ b/internal/handlers/common/aggregations/collstats.go @@ -62,13 +62,13 @@ func newCollStats(stage *types.Document) (Stage, error) { if fields.Has("storageStats") { cs.storageStats = new(storageStats) - // TODO Add proper support for scale: https://github.com/FerretDB/FerretDB/issues/1346 - cs.storageStats.scale, err = common.GetOptionalPositiveNumber( - must.NotFail(fields.Get("storageStats")).(*types.Document), - "scale", - ) - if err != nil || cs.storageStats.scale == 0 { - cs.storageStats.scale = 1 + storageStatsFields := must.NotFail(fields.Get("storageStats")).(*types.Document) + + var s any + if s, err = storageStatsFields.Get("scale"); err == nil { + if cs.storageStats.scale, err = common.GetScaleParam("$collStats.storageStats", s); err != nil { + return nil, err + } } } diff --git a/internal/handlers/common/params.go b/internal/handlers/common/params.go index 68103c11834d..53f7095c321b 100644 --- a/internal/handlers/common/params.go +++ b/internal/handlers/common/params.go @@ -121,7 +121,8 @@ var ( errNotBinaryMask = fmt.Errorf("not a binary mask") errUnexpectedLeftOpType = fmt.Errorf("unexpected left operand type") errUnexpectedRightOpType = fmt.Errorf("unexpected right operand type") - errLongExceeded = fmt.Errorf("long exceeded") + errLongExceededPositive = fmt.Errorf("long exceeded - positive value") + errLongExceededNegative = fmt.Errorf("long exceeded - negative value") errIntExceeded = fmt.Errorf("int exceeded") errInfinity = fmt.Errorf("infinity") ) @@ -132,16 +133,14 @@ func GetWholeNumberParam(value any) (int64, error) { switch value := value.(type) { // TODO: add string support https://github.com/FerretDB/FerretDB/issues/1089 case float64: - if math.IsInf(value, 1) { + switch { + case math.IsInf(value, 1): return 0, errInfinity - } - - if value > float64(math.MaxInt64) || - value < float64(math.MinInt64) { - return 0, errLongExceeded - } - - if value != math.Trunc(value) { + case value > float64(math.MaxInt64): + return 0, errLongExceededPositive + case value < float64(math.MinInt64): + return 0, errLongExceededNegative + case value != math.Trunc(value): return 0, errNotWholeNumber } @@ -174,7 +173,7 @@ func GetLimitStageParam(value any) (int64, error) { fmt.Sprintf("invalid argument to $limit stage: Expected an integer: $limit: %#v", value), "$limit (stage)", ) - case errors.Is(err, errLongExceeded): + case errors.Is(err, errLongExceededPositive), errors.Is(err, errLongExceededNegative): return 0, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrStageLimitInvalidArg, fmt.Sprintf("invalid argument to $limit stage: Cannot represent as a 64-bit integer: $limit: %#v", value), @@ -215,7 +214,7 @@ func GetSkipStageParam(value any) (int64, error) { fmt.Sprintf("invalid argument to $skip stage: Expected an integer: $skip: %#v", value), "$skip (stage)", ) - case errors.Is(err, errLongExceeded): + case errors.Is(err, errLongExceededPositive), errors.Is(err, errLongExceededNegative): return 0, commonerrors.NewCommandErrorMsgWithArgument( commonerrors.ErrStageSkipBadValue, fmt.Sprintf("invalid argument to $skip stage: Cannot represent as a 64-bit integer: $skip: %#v", value), @@ -363,11 +362,11 @@ func addNumbers(v1, v2 any) (any, error) { case int64: if v2 > 0 { if int64(v1) > math.MaxInt64-v2 { - return nil, errLongExceeded + return nil, errLongExceededPositive } } else { if int64(v1) < math.MinInt64-v2 { - return nil, errLongExceeded + return nil, errLongExceededNegative } } @@ -394,11 +393,11 @@ func addNumbers(v1, v2 any) (any, error) { case int64: if v2 > 0 { if v1 > math.MaxInt64-v2 { - return nil, errLongExceeded + return nil, errLongExceededPositive } } else { if v1 < math.MinInt64-v2 { - return nil, errLongExceeded + return nil, errLongExceededNegative } } @@ -483,12 +482,12 @@ func multiplyLongSafely(v1, v2 int64) (int64, error) { // This check is necessary only for MinInt64, as multiplying MinInt64 by -1 // results in overflow with the MinInt64 as result. case v1 == math.MinInt64 || v2 == math.MinInt64: - return 0, errLongExceeded + return 0, errLongExceededNegative } res := v1 * v2 if res/v2 != v1 { - return 0, errLongExceeded + return 0, errLongExceededPositive } return res, nil diff --git a/internal/handlers/common/params_test.go b/internal/handlers/common/params_test.go index 1f1eee4e5e79..464784dc1606 100644 --- a/internal/handlers/common/params_test.go +++ b/internal/handlers/common/params_test.go @@ -51,12 +51,12 @@ func TestMultiplyLongSafely(t *testing.T) { "OverflowLarge": { v1: 1 << 60, v2: 42, - err: errLongExceeded, + err: errLongExceededPositive, }, "OverflowMax": { v1: math.MaxInt64, v2: 2, - err: errLongExceeded, + err: errLongExceededPositive, }, "MaxMinusOne": { v1: math.MaxInt64, @@ -66,17 +66,17 @@ func TestMultiplyLongSafely(t *testing.T) { "OverflowMaxMinusTwo": { v1: math.MaxInt64, v2: -2, - err: errLongExceeded, + err: errLongExceededPositive, }, "OverflowMin": { v1: math.MinInt64, v2: 2, - err: errLongExceeded, + err: errLongExceededNegative, }, "OverflowMinMinusOne": { v1: math.MinInt64, v2: -1, - err: errLongExceeded, + err: errLongExceededNegative, }, } { name, tc := name, tc diff --git a/internal/handlers/common/scale.go b/internal/handlers/common/scale.go new file mode 100644 index 000000000000..94de0ac25ee9 --- /dev/null +++ b/internal/handlers/common/scale.go @@ -0,0 +1,88 @@ +// Copyright 2021 FerretDB Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package common + +import ( + "errors" + "fmt" + "math" + + "github.com/FerretDB/FerretDB/internal/handlers/commonerrors" + "github.com/FerretDB/FerretDB/internal/types" + "github.com/FerretDB/FerretDB/internal/util/lazyerrors" +) + +// GetScaleParam validates the given scale value for collStats and dbStats command. +// +// If the value is valid, it returns its int32 representation, +// otherwise it returns a command error with the given command being mentioned. +func GetScaleParam(command string, value any) (int32, error) { + scaleValue, err := GetWholeNumberParam(value) + + if err == nil { + if scaleValue <= 0 { + return 0, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrValueNegative, + fmt.Sprintf("BSON field 'scale' value must be >= 1, actual value '%d'", scaleValue), + "scale", + ) + } + + if scaleValue > math.MaxInt32 { + return math.MaxInt32, nil + } + + return int32(scaleValue), nil + } + + switch { + case errors.Is(err, errUnexpectedType): + if _, ok := value.(types.NullType); ok { + return 1, nil + } + + return 0, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrTypeMismatch, + fmt.Sprintf( + `BSON field '%s.scale' is the wrong type '%s', expected types '[long, int, decimal, double]'`, + command, AliasFromType(value), + ), + "scale", + ) + case errors.Is(err, errNotWholeNumber): + if math.Signbit(value.(float64)) { + return 0, commonerrors.NewCommandError( + commonerrors.ErrValueNegative, + fmt.Errorf("BSON field 'scale' value must be >= 1, actual value '%d'", int(math.Ceil(value.(float64)))), + ) + } + + // for non-integer numbers, scale value is rounded to the greatest integer value less than the given value. + return int32(math.Floor(value.(float64))), nil + + case errors.Is(err, errLongExceededPositive): + return math.MaxInt32, nil + + case errors.Is(err, errLongExceededNegative): + return 0, commonerrors.NewCommandErrorMsgWithArgument( + commonerrors.ErrValueNegative, + fmt.Sprintf("BSON field 'scale' value must be >= 1, actual value '%d'", math.MinInt32), + "scale", + ) + + default: + return 0, lazyerrors.Error(err) + } +} diff --git a/internal/handlers/common/skip.go b/internal/handlers/common/skip.go index 25d7e7301053..006aa4b6c3ad 100644 --- a/internal/handlers/common/skip.go +++ b/internal/handlers/common/skip.go @@ -67,9 +67,15 @@ func GetSkipParam(command string, value any) (int64, error) { // for non-integer numbers, skip value is rounded to the greatest integer value less than the given value. return int64(math.Floor(value.(float64))), nil - case errors.Is(err, errLongExceeded): + case errors.Is(err, errLongExceededPositive): return math.MaxInt64, nil + case errors.Is(err, errLongExceededNegative): + return 0, commonerrors.NewCommandError( + commonerrors.ErrValueNegative, + fmt.Errorf("BSON field 'skip' value must be >= 0, actual value '%d'", int(math.Ceil(value.(float64)))), + ) + default: return 0, lazyerrors.Error(err) } diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go index 580a5c584221..0bb2f5e6de3e 100644 --- a/internal/handlers/common/update.go +++ b/internal/handlers/common/update.go @@ -407,8 +407,8 @@ func processIncFieldExpression(doc *types.Document, updateV any) (bool, error) { continue } - switch err { - case errUnexpectedLeftOpType: + switch { + case errors.Is(err, errUnexpectedLeftOpType): return false, commonerrors.NewWriteErrorMsg( commonerrors.ErrTypeMismatch, fmt.Sprintf( @@ -417,7 +417,7 @@ func processIncFieldExpression(doc *types.Document, updateV any) (bool, error) { incValue, ), ) - case errUnexpectedRightOpType: + case errors.Is(err, errUnexpectedRightOpType): k := incKey if path.Len() > 1 { k = path.Suffix() @@ -433,7 +433,7 @@ func processIncFieldExpression(doc *types.Document, updateV any) (bool, error) { AliasFromType(docValue), ), ) - case errLongExceeded: + case errors.Is(err, errLongExceededPositive), errors.Is(err, errLongExceededNegative): return false, commonerrors.NewWriteErrorMsg( commonerrors.ErrBadValue, fmt.Sprintf( @@ -442,7 +442,7 @@ func processIncFieldExpression(doc *types.Document, updateV any) (bool, error) { must.NotFail(doc.Get("_id")), ), ) - case errIntExceeded: + case errors.Is(err, errIntExceeded): return false, commonerrors.NewWriteErrorMsg( commonerrors.ErrBadValue, fmt.Sprintf( @@ -452,7 +452,7 @@ func processIncFieldExpression(doc *types.Document, updateV any) (bool, error) { ), ) default: - return false, err + return false, lazyerrors.Error(err) } } @@ -734,7 +734,7 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) { AliasFromType(docValue), ), ) - case errors.Is(err, errLongExceeded): + case errors.Is(err, errLongExceededPositive), errors.Is(err, errLongExceededNegative): return false, commonerrors.NewWriteErrorMsg( commonerrors.ErrBadValue, fmt.Sprintf( diff --git a/internal/handlers/pg/msg_collstats.go b/internal/handlers/pg/msg_collstats.go index 6d2d462e802c..a415c2b0c37f 100644 --- a/internal/handlers/pg/msg_collstats.go +++ b/internal/handlers/pg/msg_collstats.go @@ -50,12 +50,13 @@ func (h *Handler) MsgCollStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs return nil, err } - // TODO Add proper support for scale: https://github.com/FerretDB/FerretDB/issues/1346 - var scale int32 + scale := int32(1) - scale, err = common.GetOptionalPositiveNumber(document, "scale") - if err != nil || scale == 0 { - scale = 1 + var s any + if s, err = document.Get("scale"); err == nil { + if scale, err = common.GetScaleParam(command, s); err != nil { + return nil, err + } } stats, err := dbPool.Stats(ctx, db, collection) diff --git a/internal/handlers/pg/msg_dbstats.go b/internal/handlers/pg/msg_dbstats.go index 9289f7a97d0c..9122008b3207 100644 --- a/internal/handlers/pg/msg_dbstats.go +++ b/internal/handlers/pg/msg_dbstats.go @@ -36,17 +36,20 @@ func (h *Handler) MsgDBStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return nil, lazyerrors.Error(err) } + command := document.Command() + db, err := common.GetRequiredParam[string](document, "$db") if err != nil { return nil, err } - // TODO Add proper support for scale: https://github.com/FerretDB/FerretDB/issues/1346 - var scale int32 + scale := int32(1) - scale, err = common.GetOptionalPositiveNumber(document, "scale") - if err != nil || scale == 0 { - scale = 1 + var s any + if s, err = document.Get("scale"); err == nil { + if scale, err = common.GetScaleParam(command, s); err != nil { + return nil, err + } } stats, err := dbPool.Stats(ctx, db, "") diff --git a/internal/handlers/pg/msg_update.go b/internal/handlers/pg/msg_update.go index bf6fea7a092b..80d49eb86f7f 100644 --- a/internal/handlers/pg/msg_update.go +++ b/internal/handlers/pg/msg_update.go @@ -76,13 +76,15 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error { return pgdb.CreateCollectionIfNotExists(ctx, tx, qp.DB, qp.Collection) }) - if err != nil { - if errors.Is(err, pgdb.ErrInvalidCollectionName) || - errors.Is(err, pgdb.ErrInvalidDatabaseName) { - msg := fmt.Sprintf("Invalid namespace: %s.%s", qp.DB, qp.Collection) - return nil, commonerrors.NewCommandErrorMsg(commonerrors.ErrInvalidNamespace, msg) - } - return nil, err + + switch { + case err == nil: + // do nothing + case errors.Is(err, pgdb.ErrInvalidCollectionName), errors.Is(err, pgdb.ErrInvalidDatabaseName): + msg := fmt.Sprintf("Invalid namespace: %s.%s", qp.DB, qp.Collection) + return nil, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrInvalidNamespace, msg, document.Command()) + default: + return nil, lazyerrors.Error(err) } var matched, modified int32 @@ -203,7 +205,7 @@ func (h *Handler) MsgUpdate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, }) if err != nil { - return nil, err + return nil, lazyerrors.Error(err) } res := must.NotFail(types.NewDocument( diff --git a/internal/handlers/tigris/msg_collstats.go b/internal/handlers/tigris/msg_collstats.go index 1b2cd441bed1..c810cd844918 100644 --- a/internal/handlers/tigris/msg_collstats.go +++ b/internal/handlers/tigris/msg_collstats.go @@ -49,12 +49,13 @@ func (h *Handler) MsgCollStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs return nil, err } - // TODO Add proper support for scale: https://github.com/FerretDB/FerretDB/issues/1346 - var scale int32 + scale := int32(1) - scale, err = common.GetOptionalPositiveNumber(document, "scale") - if err != nil || scale == 0 { - scale = 1 + var s any + if s, err = document.Get("scale"); err == nil { + if scale, err = common.GetScaleParam(command, s); err != nil { + return nil, err + } } querier := dbPool.Driver.UseDatabase(db) diff --git a/internal/handlers/tigris/msg_dbstats.go b/internal/handlers/tigris/msg_dbstats.go index f30ed60eab8e..feee7cf3834c 100644 --- a/internal/handlers/tigris/msg_dbstats.go +++ b/internal/handlers/tigris/msg_dbstats.go @@ -39,15 +39,20 @@ func (h *Handler) MsgDBStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return nil, lazyerrors.Error(err) } + command := document.Command() + db, err := common.GetRequiredParam[string](document, "$db") if err != nil { return nil, err } - m := document.Map() - scale, ok := m["scale"].(float64) - if !ok { - scale = 1 + scale := int32(1) + + var s any + if s, err = document.Get("scale"); err == nil { + if scale, err = common.GetScaleParam(command, s); err != nil { + return nil, err + } } stats, err := dbPool.Driver.DescribeDatabase(ctx, db) @@ -98,7 +103,7 @@ func (h *Handler) MsgDBStats(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, "indexes", int32(0), "indexSize", int32(0), "totalSize", int32(0), - "scaleFactor", scale, + "scaleFactor", float64(scale), "ok", float64(1), ))}, })) diff --git a/website/docs/diff.md b/website/docs/diff.md index 8c50259fef62..e07770891f49 100644 --- a/website/docs/diff.md +++ b/website/docs/diff.md @@ -29,6 +29,8 @@ slug: /diff/ # referenced in README.md and beacon * database name cannot contain capital letters; 9. For Tigris, FerretDB requires Tigris schema validation for `create` command: validator must be set as `$tigrisSchemaString`. The value must be a JSON string representing JSON schema in [Tigris format](https://docs.tigrisdata.com/overview/schema). +10. FerretDB offers the same validation rules for the `scale` parameter in both the `collStats` and `dbStats` commands. + If an invalid `scale` value is provided in the `dbStats` command, the same error codes will be triggered as with the `collStats` command. If you encounter some other difference in behavior, please [join our community](/#community) to report a problem.