From 3c953af882ba45faa8fd6ece30f17015c8060218 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Fri, 3 Mar 2023 15:20:50 +0000 Subject: [PATCH] Fix `$mul` operator handling dot notation (#2094) Closes #1744. --- integration/shareddata/composites.go | 5 +++++ integration/update_field_compat_test.go | 16 +++++++--------- internal/handlers/common/update.go | 16 +++++++++++++++- internal/types/path.go | 9 ++++----- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/integration/shareddata/composites.go b/integration/shareddata/composites.go index 367821a7f4fd..e9ce98f82d49 100644 --- a/integration/shareddata/composites.go +++ b/integration/shareddata/composites.go @@ -40,6 +40,11 @@ var Composites = &Values[string]{ {"42", "foo"}, {"foo", int32(42)}, }, + "document-composite-numerical-field-name": bson.D{ + {"foo", int32(42)}, + {"42", "foo"}, + {"array", bson.D{{"42", int32(42)}}}, + }, "document-null": bson.D{{"foo", nil}}, "document-empty": bson.D{}, diff --git a/integration/update_field_compat_test.go b/integration/update_field_compat_test.go index 2bd224136bad..5b8fab67ad79 100644 --- a/integration/update_field_compat_test.go +++ b/integration/update_field_compat_test.go @@ -1202,17 +1202,15 @@ func TestUpdateFieldCompatMul(t *testing.T) { "DotNotationMissingField": { update: bson.D{{"$mul", bson.D{{"v..", int32(45)}}}}, resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/1744", - }, - "DotNotationNegativeIndex": { - update: bson.D{{"$mul", bson.D{{"v.-1.bar", int32(45)}}}}, - resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/2050", }, "DotNotationIndexExceedsArrayLength": { - update: bson.D{{"$mul", bson.D{{"v.100.bar", int32(45)}}}}, - resultType: emptyResult, - skip: "https://github.com/FerretDB/FerretDB/issues/1744", + update: bson.D{{"$mul", bson.D{{"v.100.bar", int32(45)}}}}, + }, + "DotNotationFieldNumericName": { + update: bson.D{{"$mul", bson.D{{"v.array.42", int32(42)}}}}, + }, + "DotNotationNegativeIndex": { + update: bson.D{{"$mul", bson.D{{"v.array.-1", int32(42)}}}}, }, } diff --git a/internal/handlers/common/update.go b/internal/handlers/common/update.go index fd52a6cb4d56..54dbf0b84def 100644 --- a/internal/handlers/common/update.go +++ b/internal/handlers/common/update.go @@ -583,7 +583,21 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) { path, err = types.NewPathFromString(mulKey) if err != nil { - return false, lazyerrors.Error(err) + var pathErr *types.DocumentPathError + + if errors.As(err, &pathErr) { + switch pathErr.Code() { + case types.ErrDocumentPathEmptyKey: + return false, commonerrors.NewWriteErrorMsg( + commonerrors.ErrEmptyName, + fmt.Sprintf("Cannot apply $mul to a value of non-numeric type. "+ + "{_id: %s} has the field '%s' of non-numeric type object", + must.NotFail(doc.Get("_id")), + mulKey, + ), + ) + } + } } if !doc.HasByPath(path) { diff --git a/internal/types/path.go b/internal/types/path.go index 0c68359306e8..5baf7a15eebb 100644 --- a/internal/types/path.go +++ b/internal/types/path.go @@ -15,6 +15,7 @@ package types import ( + "errors" "fmt" "math" "strconv" @@ -23,7 +24,6 @@ import ( "golang.org/x/exp/slices" - "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" ) @@ -41,6 +41,8 @@ const ( ErrDocumentPathIndexOutOfBound // ErrDocumentPathCannotCreateField indicates that it's impossible to create a specific field. ErrDocumentPathCannotCreateField + // ErrDocumentPathEmptyKey indicates that provided path contains empty key. + ErrDocumentPathEmptyKey ) // DocumentPathError describes an error that could occur on document path related operations. @@ -84,13 +86,10 @@ func NewPathFromString(s string) (Path, error) { var res Path path := strings.Split(s, ".") - if len(path) == 0 { - return res, lazyerrors.New("empty path") - } for _, s := range path { if s == "" { - return res, lazyerrors.New("path element must not be empty") + return res, newDocumentPathError(ErrDocumentPathEmptyKey, errors.New("path element must not be empty")) } }