Skip to content

Commit

Permalink
Fix embedded array query bug (#736)
Browse files Browse the repository at this point in the history 
Closes #709.
Closes #732.
  • Loading branch information
@ribaraka
ribaraka authored Jun 21, 2022
1 parent b7cc81b commit cf873e8
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 79 deletions.
79 changes: 76 additions & 3 deletions integration/query_array_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,19 @@ func TestQueryArrayDotNotation(t *testing.T) {

"PositionTypeNull": {
filter: bson.D{{"value.0", bson.D{{"$type", "null"}}}},
expectedIDs: []any{"array-null", "array-three-reverse"},
expectedIDs: []any{"array-last-embedded", "array-middle-embedded", "array-null", "array-three-reverse"},
},
"PositionRegex": {
filter: bson.D{{"value.1", primitive.Regex{Pattern: "foo"}}},
expectedIDs: []any{"array-three", "array-three-reverse"},
},
"PositionArray": {
filter: bson.D{{"value.0", primitive.A{}}},
expectedIDs: []any{},
filter: bson.D{{"value.0", bson.A{"42", "foo"}}},
expectedIDs: []any{"array-embedded"},
},
"PositionArrayEmpty": {
filter: bson.D{{"value.0", bson.A{}}},
expectedIDs: []any{"array-empty-nested"},
},

"NoSuchFieldPosition": {
Expand Down Expand Up @@ -340,3 +344,72 @@ func TestQueryElemMatchOperator(t *testing.T) {
})
}
}

func TestArrayEquality(t *testing.T) {
t.Parallel()
ctx, collection := setup(t, shareddata.Composites)

for name, tc := range map[string]struct {
array bson.A
expectedIDs []any
}{
"One": {
array: bson.A{int32(42)},
expectedIDs: []any{"array"},
},
"Two": {
array: bson.A{42, "foo"},
expectedIDs: []any{"array-first-embedded", "array-last-embedded", "array-middle-embedded"},
},
"Three": {
array: bson.A{int32(42), "foo", nil},
expectedIDs: []any{"array-three"},
},
"Three-reverse": {
array: bson.A{nil, "foo", int32(42)},
expectedIDs: []any{"array-three-reverse"},
},
"Empty": {
array: bson.A{},
expectedIDs: []any{"array-empty", "array-empty-nested"},
},
"Null": {
array: bson.A{nil},
expectedIDs: []any{"array-null"},
},
"EmptyNested": {
array: bson.A{bson.A{}},
expectedIDs: []any{"array-empty-nested"},
},
"OneEmbedded": {
array: bson.A{bson.A{"42", "foo"}},
expectedIDs: []any{"array-embedded"},
},
"FirstEmbedded": {
array: bson.A{bson.A{int32(42), "foo"}, nil},
expectedIDs: []any{"array-first-embedded"},
},
"MiddleEmbedded": {
array: bson.A{nil, bson.A{int32(42), "foo"}, nil},
expectedIDs: []any{"array-middle-embedded"},
},
"LastEmbedded": {
array: bson.A{nil, bson.A{int32(42), "foo"}},
expectedIDs: []any{"array-last-embedded"},
},
} {
name, tc := name, tc
t.Run(name, func(t *testing.T) {
t.Parallel()

filter := bson.D{{"value", tc.array}}
cursor, err := collection.Find(ctx, filter, options.Find().SetSort(bson.D{{"_id", 1}}))
require.NoError(t, err)

var actual []bson.D
err = cursor.All(ctx, &actual)
require.NoError(t, err)
assert.Equal(t, tc.expectedIDs, CollectIDs(t, actual))
})
}
}
64 changes: 42 additions & 22 deletions integration/query_comparison_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +82,23 @@ func TestQueryComparisonImplicit(t *testing.T) {
},
"ArrayEmpty": {
filter: bson.D{{"value", bson.A{}}},
expectedIDs: []any{"array-empty"},
expectedIDs: []any{"array-empty", "array-empty-nested"},
},
"ArrayNoSuchField": {
filter: bson.D{{"no-such-field", bson.A{42}}},
expectedIDs: []any{},
},
"ArrayEmbedded": {
filter: bson.D{{"value", bson.A{bson.A{int32(42), "foo"}, nil}}},
expectedIDs: []any{"array-embedded"},
expectedIDs: []any{"array-first-embedded"},
},
"LongArrayEmbedded": {
filter: bson.D{{"value", bson.A{bson.A{int32(42), "foo"}, nil, "foo"}}},
expectedIDs: []any{},
},
"ArraySlice": {
filter: bson.D{{"value", bson.A{int32(42), "foo"}}},
expectedIDs: []any{"array-embedded"},
expectedIDs: []any{"array-first-embedded", "array-last-embedded", "array-middle-embedded"},
},
"ArrayShuffledValues": {
filter: bson.D{{"value", bson.A{"foo", nil, int32(42)}}},
Expand Down Expand Up @@ -170,13 +170,17 @@ func TestQueryComparisonImplicit(t *testing.T) {
expectedIDs: []any{},
},
"ValueNull": {
filter: bson.D{{"value", nil}},
expectedIDs: []any{"array-embedded", "array-null", "array-three", "array-three-reverse", "null"},
filter: bson.D{{"value", nil}},
expectedIDs: []any{
"array-first-embedded", "array-last-embedded", "array-middle-embedded", "array-null",
"array-three", "array-three-reverse", "null",
},
},
"NoSuchFieldNull": {
filter: bson.D{{"no-such-field", nil}},
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-null", "array-three", "array-three-reverse", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-three", "array-three-reverse", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand Down Expand Up @@ -258,15 +262,15 @@ func TestQueryComparisonEq(t *testing.T) {
},
"ArrayEmbedded": {
filter: bson.D{{"value", bson.D{{"$eq", bson.A{bson.A{int32(42), "foo"}, nil}}}}},
expectedIDs: []any{"array-embedded"},
expectedIDs: []any{"array-first-embedded"},
},
"LongArrayEmbedded": {
filter: bson.D{{"value", bson.D{{"$eq", bson.A{bson.A{int32(42), "foo"}, nil, "foo"}}}}},
expectedIDs: []any{},
},
"ArraySlice": {
filter: bson.D{{"value", bson.D{{"$eq", bson.A{int32(42), "foo"}}}}},
expectedIDs: []any{"array-embedded"},
expectedIDs: []any{"array-first-embedded", "array-last-embedded", "array-middle-embedded"},
},
"ArrayShuffledValues": {
filter: bson.D{{"value", bson.D{{"$eq", bson.A{"foo", nil, int32(42)}}}}},
Expand All @@ -282,7 +286,7 @@ func TestQueryComparisonEq(t *testing.T) {
},
"ArrayEmpty": {
filter: bson.D{{"value", bson.D{{"$eq", bson.A{}}}}},
expectedIDs: []any{"array-empty"},
expectedIDs: []any{"array-empty", "array-empty-nested"},
},

"Double": {
Expand Down Expand Up @@ -392,8 +396,11 @@ func TestQueryComparisonEq(t *testing.T) {
},

"Null": {
filter: bson.D{{"value", bson.D{{"$eq", nil}}}},
expectedIDs: []any{"array-embedded", "array-null", "array-three", "array-three-reverse", "null"},
filter: bson.D{{"value", bson.D{{"$eq", nil}}}},
expectedIDs: []any{
"array-first-embedded", "array-last-embedded", "array-middle-embedded", "array-null", "array-three",
"array-three-reverse", "null",
},
},

"RegexWithoutOption": {
Expand Down Expand Up @@ -467,7 +474,8 @@ func TestQueryComparisonEq(t *testing.T) {
"NoSuchFieldNull": {
filter: bson.D{{"no-such-field", bson.D{{"$eq", nil}}}},
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-null", "array-three", "array-three-reverse", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-three", "array-three-reverse", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand Down Expand Up @@ -759,8 +767,11 @@ func TestQueryComparisonGte(t *testing.T) {
},

"Null": {
value: nil,
expectedIDs: []any{"array-embedded", "array-null", "array-three", "array-three-reverse", "null"},
value: nil,
expectedIDs: []any{
"array-first-embedded", "array-last-embedded", "array-middle-embedded", "array-null", "array-three",
"array-three-reverse", "null",
},
},

"Regex": {
Expand Down Expand Up @@ -1096,8 +1107,11 @@ func TestQueryComparisonLte(t *testing.T) {
},

"Null": {
value: nil,
expectedIDs: []any{"array-embedded", "array-null", "array-three", "array-three-reverse", "null"},
value: nil,
expectedIDs: []any{
"array-first-embedded", "array-last-embedded", "array-middle-embedded", "array-null",
"array-three", "array-three-reverse", "null",
},
},

"Regex": {
Expand Down Expand Up @@ -1192,8 +1206,11 @@ func TestQueryComparisonNin(t *testing.T) {
err *mongo.CommandError
}{
"ForScalarDataTypes": {
value: scalarDataTypesFilter,
expectedIDs: []any{"array-empty", "document", "document-composite", "document-composite-reverse", "document-empty", "document-null"},
value: scalarDataTypesFilter,
expectedIDs: []any{
"array-embedded", "array-empty", "array-empty-nested", "document", "document-composite",
"document-composite-reverse", "document-empty", "document-null",
},
},
"ForCompositeDataTypes": {
value: compositeDataTypesFilter,
Expand Down Expand Up @@ -1224,7 +1241,8 @@ func TestQueryComparisonNin(t *testing.T) {
"Regex": {
value: bson.A{primitive.Regex{Pattern: "foo", Options: "i"}},
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-null", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand Down Expand Up @@ -1302,7 +1320,8 @@ func TestQueryComparisonIn(t *testing.T) {
"ForScalarDataTypes": {
value: scalarDataTypesFilter,
expectedIDs: []any{
"array", "array-embedded", "array-null", "array-three", "array-three-reverse", "array-two",
"array", "array-first-embedded", "array-last-embedded", "array-middle-embedded", "array-null",
"array-three", "array-three-reverse", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand All @@ -1320,7 +1339,8 @@ func TestQueryComparisonIn(t *testing.T) {
"ForCompositeDataTypes": {
value: compositeDataTypesFilter,
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-null", "array-three", "array-three-reverse", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-three", "array-three-reverse", "array-two",
"document", "document-composite", "document-composite-reverse", "document-empty", "document-null",
},
},
Expand Down Expand Up @@ -1401,7 +1421,7 @@ func TestQueryComparisonNe(t *testing.T) {
},
"ArrayEmbedded": {
value: bson.A{bson.A{int32(42), "foo"}, nil},
unexpectedID: "array-embedded",
unexpectedID: "array-first-embedded",
},
"LongArrayEmbedded": {
value: bson.A{bson.A{int32(42), "foo"}, nil, "foo"},
Expand Down
14 changes: 10 additions & 4 deletions integration/query_element_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,11 @@ func TestQueryElementType(t *testing.T) {
expectedIDs: []any{"document", "document-composite", "document-composite-reverse", "document-empty", "document-null"},
},
"Array": {
v: "array",
expectedIDs: []any{"array", "array-embedded", "array-empty", "array-null", "array-three", "array-three-reverse", "array-two"},
v: "array",
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-three", "array-three-reverse", "array-two",
},
},
"Double": {
v: "double",
Expand Down Expand Up @@ -143,8 +146,11 @@ func TestQueryElementType(t *testing.T) {
expectedIDs: []any{"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min"},
},
"Null": {
v: "null",
expectedIDs: []any{"array-embedded", "array-null", "array-three", "array-three-reverse", "null"},
v: "null",
expectedIDs: []any{
"array-first-embedded", "array-last-embedded", "array-middle-embedded", "array-null", "array-three",
"array-three-reverse", "null",
},
},
"Regex": {
v: "regex",
Expand Down
11 changes: 7 additions & 4 deletions integration/query_logical_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,8 @@ func TestQueryLogicalNot(t *testing.T) {
"Not": {
filter: bson.D{{"value", bson.D{{"$not", bson.D{{"$eq", 42}}}}}},
expectedIDs: []any{
"array-embedded", "array-empty", "array-null", "array-two",
"array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand Down Expand Up @@ -307,7 +308,7 @@ func TestQueryLogicalNot(t *testing.T) {
"NotEqNull": {
filter: bson.D{{"value", bson.D{{"$not", bson.D{{"$eq", nil}}}}}},
expectedIDs: []any{
"array", "array-empty", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand All @@ -325,7 +326,8 @@ func TestQueryLogicalNot(t *testing.T) {
"ValueRegex": {
filter: bson.D{{"value", bson.D{{"$not", primitive.Regex{Pattern: "^fo"}}}}},
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-null", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded",
"array-last-embedded", "array-middle-embedded", "array-null", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand All @@ -344,7 +346,8 @@ func TestQueryLogicalNot(t *testing.T) {
"NoSuchFieldRegex": {
filter: bson.D{{"no-such-field", bson.D{{"$not", primitive.Regex{Pattern: "/someregex/"}}}}},
expectedIDs: []any{
"array", "array-embedded", "array-empty", "array-null", "array-three", "array-three-reverse", "array-two",
"array", "array-embedded", "array-empty", "array-empty-nested", "array-first-embedded", "array-last-embedded",
"array-middle-embedded", "array-null", "array-three", "array-three-reverse", "array-two",
"binary", "binary-empty",
"bool-false", "bool-true",
"datetime", "datetime-epoch", "datetime-year-max", "datetime-year-min",
Expand Down
4 changes: 2 additions & 2 deletions integration/query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func TestQueryCount(t *testing.T) {
}{
"CountAllDocuments": {
command: bson.D{{"count", collection.Name()}},
response: 50,
response: 54,
},
"CountExactlyOneDocument": {
command: bson.D{
Expand All @@ -332,7 +332,7 @@ func TestQueryCount(t *testing.T) {
{"count", collection.Name()},
{"query", bson.D{{"value", bson.D{{"$type", "array"}}}}},
},
response: 7,
response: 11,
},
"CountNonExistingCollection": {
command: bson.D{
Expand Down
21 changes: 11 additions & 10 deletions integration/shareddata/composites.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ var Composites = &Values[string]{
"document-null": bson.D{{"foo", nil}},
"document-empty": bson.D{},

"array": bson.A{int32(42)},
"array-two": bson.A{42.13, math.NaN()},
"array-three": bson.A{int32(42), "foo", nil},
"array-three-reverse": bson.A{nil, "foo", int32(42)},
"array-embedded": bson.A{bson.A{int32(42), "foo"}, nil},
"array-empty": bson.A{},
"array-null": bson.A{nil},

// TODO: This case demonstrates a bug, see https://github.com/FerretDB/FerretDB/issues/732
// "array-empty-nested": bson.A{bson.A{}},
"array": bson.A{int32(42)},
"array-two": bson.A{42.13, math.NaN()},
"array-three": bson.A{int32(42), "foo", nil},
"array-three-reverse": bson.A{nil, "foo", int32(42)},
"array-embedded": bson.A{bson.A{"42", "foo"}},
"array-first-embedded": bson.A{bson.A{int32(42), "foo"}, nil},
"array-middle-embedded": bson.A{nil, bson.A{int32(42), "foo"}, nil},
"array-last-embedded": bson.A{nil, bson.A{int32(42), "foo"}},
"array-empty": bson.A{},
"array-empty-nested": bson.A{bson.A{}},
"array-null": bson.A{nil},
},
}
2 changes: 1 addition & 1 deletion internal/handlers/common/filter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func filterDocumentPair(doc *types.Document, filterKey string, filterValue any)
}

if _, ok := value.(*types.Array); ok {
return false, nil
return types.Compare(value, filterValue) == types.Equal, nil
}

doc = must.NotFail(types.NewDocument(filterKey, value))
Expand Down
Loading

0 comments on commit cf873e8

Please sign in to comment.