Skip to content

Commit

Permalink
Merge branch 'refs/heads/docstore-array-contains' into develop
Browse files Browse the repository at this point in the history
# Conflicts:
#	docstore/drivertest/drivertest.go
#	docstore/query.go
  • Loading branch information
ybourgery committed Jun 18, 2024
2 parents b29fec0 + d3d3c42 commit 433ffde
Show file tree
Hide file tree
Showing 25 changed files with 62 additions and 32 deletions.
2 changes: 1 addition & 1 deletion docstore/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ type Query struct {
// TODO(#1762): support comparison of other types.
type Filter struct {
FieldPath []string // the field path to filter
Op string // the operation, supports `=`, `>`, `>=`, `<`, `<=`, `in`, `not-in`
Op string // the operation, supports `=`, `>`, `>=`, `<`, `<=`, `in`, `not-in`, `array-contains`
Value interface{} // the value to compare using the operation
}

Expand Down
42 changes: 32 additions & 10 deletions docstore/drivertest/drivertest.go
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,7 @@ type HighScore struct {
Score int
Time time.Time
WithGlitch bool
GameCategories []string
DocstoreRevision interface{}
}

Expand Down Expand Up @@ -1316,15 +1317,21 @@ const (
game3 = "Days Gone"
)

var (
game1Category = []string{"Action", "Adventure"}
game2Category = []string{"Horror", "Comedy"}
game3Category = []string{"Action", "Adventure", "Horror"}
)

var highScores = []*HighScore{
{game1, "pat", 49, date(3, 13), false, nil},
{game1, "mel", 60, date(4, 10), false, nil},
{game1, "andy", 81, date(2, 1), false, nil},
{game1, "fran", 33, date(3, 19), false, nil},
{game2, "pat", 120, date(4, 1), true, nil},
{game2, "billie", 111, date(4, 10), false, nil},
{game2, "mel", 190, date(4, 18), true, nil},
{game2, "fran", 33, date(3, 20), false, nil},
{game1, "pat", 49, date(3, 13), game1Category, nil},
{game1, "mel", 60, date(4, 10), game1Category, nil},
{game1, "andy", 81, date(2, 1), game1Category, nil},
{game1, "fran", 33, date(3, 19), game1Category, nil},
{game2, "pat", 120, date(4, 1), game2Category, nil},
{game2, "billie", 111, date(4, 10), game2Category, nil},
{game2, "mel", 190, date(4, 18), game2Category, nil},
{game2, "fran", 33, date(3, 20), game2Category, nil},
}

func addHighScores(t *testing.T, coll *docstore.Collection) {
Expand Down Expand Up @@ -1491,6 +1498,11 @@ func testGetQuery(t *testing.T, _ Harness, coll *docstore.Collection) {
q: coll.Query().Where("WithGlitch", "=", true),
want: func(h *HighScore) bool { return h.WithGlitch },
},
{
name: "GameCategories",
q: coll.Query().Where("GameCategories", "array-contains", game1Category[0]),
want: func(h *HighScore) bool { return arrayContains(h.GameCategories, game1Category[0]) },
},
{
name: "AllByPlayerAsc",
q: coll.Query().OrderBy("Player", docstore.Ascending),
Expand Down Expand Up @@ -1529,13 +1541,14 @@ func testGetQuery(t *testing.T, _ Harness, coll *docstore.Collection) {
h.Score = 0
h.Time = time.Time{}
h.WithGlitch = false
h.GameCategories = nil
return true
},
},
{
name: "AllWithScore",
q: coll.Query(),
fields: []docstore.FieldPath{"Game", "Player", "Score", "WithGlitch", docstore.FieldPath(docstore.DefaultRevisionField)},
fields: []docstore.FieldPath{"Game", "Player", "Score", "WithGlitch", "GameCategories", docstore.FieldPath(docstore.DefaultRevisionField)},
want: func(h *HighScore) bool {
h.Time = time.Time{}
return true
Expand Down Expand Up @@ -2125,7 +2138,7 @@ func testAs(t *testing.T, coll *docstore.Collection, st AsTest) {
}

// ErrorCheck
doc := &HighScore{game3, "steph", 24, date(4, 25), false, nil}
doc := &HighScore{game3, "steph", 24, date(4, 25), game3Category, nil}
if err := coll.Create(ctx, doc); err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -2164,3 +2177,12 @@ func checkCode(t *testing.T, err error, code gcerrors.ErrorCode) {
t.Errorf("got %v, want %s", err, code)
}
}

func arrayContains[T comparable](a []T, x T) bool {
for _, e := range a {
if e == x {
return true
}
}
return false
}
7 changes: 3 additions & 4 deletions docstore/gcpfirestore/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func splitFilters(fs []driver.Filter) (sendToFirestore, evaluateLocally []driver
// Enforce that only one field can have an inequality.
var rangeFP []string
for _, f := range fs {
if f.Op == driver.EqualOp {
if f.Op == driver.EqualOp || f.Op == "array-contains" {
sendToFirestore = append(sendToFirestore, f)
} else {
if rangeFP == nil || driver.FieldPathsEqual(rangeFP, f.FieldPath) {
Expand Down Expand Up @@ -366,9 +366,8 @@ func newFieldFilter(fp []string, op string, val *pb.Value) (*pb.StructuredQuery_
fop = pb.StructuredQuery_FieldFilter_IN
case "not-in":
fop = pb.StructuredQuery_FieldFilter_NOT_IN
// TODO(jba): can we support array-contains portably?
// case "array-contains":
// fop = pb.StructuredQuery_FieldFilter_ARRAY_CONTAINS
case "array-contains":
fop = pb.StructuredQuery_FieldFilter_ARRAY_CONTAINS
default:
return nil, gcerr.Newf(gcerr.InvalidArgument, nil, "invalid operator: %q", op)
}
Expand Down
Binary file not shown.
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/As/verify_As.replay
Binary file not shown.
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/BeforeDo.replay
Binary file not shown.
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Create.replay
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Data.replay
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Delete.replay
Binary file not shown.
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Get.replay
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/GetQuery.replay
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Proto.replay
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Put.replay
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Replace.replay
Binary file not shown.
Binary file not shown.
Binary file modified docstore/gcpfirestore/testdata/TestConformance/Update.replay
Binary file not shown.
7 changes: 7 additions & 0 deletions docstore/memdocstore/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ func applyComparison(op string, c int) bool {
return c >= 0
case "<=":
return c <= 0
case "array-contains":
return c == 0
case "in":
return c == 0
case "not-in":
Expand All @@ -123,6 +125,11 @@ func applyComparison(op string, c int) bool {
func compare(x1, x2 interface{}) (int, bool) {
v1 := reflect.ValueOf(x1)
v2 := reflect.ValueOf(x2)
// this if for array-contains queries. We are just inverting the parameters x1 and x2 as it's the same logic
// as the in/not-in queries.
if v1.Kind() == reflect.Slice {
return compare(x2, x1)
}
// this is for in/not-in queries.
// return 0 if x1 is in slice x2, -1 if not.
if v2.Kind() == reflect.Slice {
Expand Down
15 changes: 8 additions & 7 deletions docstore/mongodocstore/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,14 @@ func (c *collection) RunGetQuery(ctx context.Context, q *driver.Query) (driver.D
}

var mongoQueryOps = map[string]string{
driver.EqualOp: "$eq",
">": "$gt",
">=": "$gte",
"<": "$lt",
"<=": "$lte",
"in": "$in",
"not-in": "$nin",
driver.EqualOp: "$eq",
">": "$gt",
">=": "$gte",
"<": "$lt",
"<=": "$lte",
"in": "$in",
"not-in": "$nin",
"array-contains": "$eq",
}

// filtersToBSON converts a []driver.Filter to the MongoDB equivalent, expressed
Expand Down
21 changes: 11 additions & 10 deletions docstore/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ func (c *Collection) Query() *Query {
}

// Where expresses a condition on the query.
// Valid ops are: "=", ">", "<", ">=", "<=, "in", "not-in".
// Valid values are strings, integers, floating-point numbers, time.Time and boolean (only for "=", "in" and "not-in") values.
// Valid ops are: "=", ">", "<", ">=", "<=, "in", "not-in", "array-contains".
// Valid values are strings, integers, floating-point numbers, and time.Time and boolean (only for "=", "in" and "not-in") values.
func (q *Query) Where(fp FieldPath, op string, value interface{}) *Query {
if q.err != nil {
return q
Expand All @@ -50,7 +50,7 @@ func (q *Query) Where(fp FieldPath, op string, value interface{}) *Query {
}
validator, ok := validOp[op]
if !ok {
return q.invalidf("invalid filter operator: %q. Use one of: =, >, <, >=, <=, in, not-in", op)
return q.invalidf("invalid filter operator: %q. Use one of: =, >, <, >=, <=, in, not-in, array-contains", op)
}
if !validator(value) {
return q.invalidf("invalid filter value: %v", value)
Expand All @@ -66,13 +66,14 @@ func (q *Query) Where(fp FieldPath, op string, value interface{}) *Query {
type valueValidator func(interface{}) bool

var validOp = map[string]valueValidator{
"=": validEqualValue,
">": validFilterValue,
"<": validFilterValue,
">=": validFilterValue,
"<=": validFilterValue,
"in": validFilterSlice,
"not-in": validFilterSlice,
"=": validEqualValue,
">": validFilterValue,
"<": validFilterValue,
">=": validFilterValue,
"<=": validFilterValue,
"array-contains": validFilterValue,
"in": validFilterSlice,
"not-in": validFilterSlice,
}

func validEqualValue(v interface{}) bool {
Expand Down

0 comments on commit 433ffde

Please sign in to comment.