Skip to content

Commit

Permalink
Revert "feat(datastore): adds in, not-in, and != query operations (#5975
Browse files Browse the repository at this point in the history 
)" (#6016)

This reverts commit db9991f.
  • Loading branch information
@telpirion
telpirion committed May 9, 2022
1 parent 13ef5c0 commit cca86d6
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 147 deletions.
43 changes: 5 additions & 38 deletions datastore/query.go
View file
Expand Up @@ -38,9 +38,6 @@ const (
equal
greaterEq
greaterThan
in
notIn
notEqual

keyFieldName = "__key__"
)
Expand All @@ -51,9 +48,6 @@ var operatorToProto = map[operator]pb.PropertyFilter_Operator{
equal: pb.PropertyFilter_EQUAL,
greaterEq: pb.PropertyFilter_GREATER_THAN_OR_EQUAL,
greaterThan: pb.PropertyFilter_GREATER_THAN,
in: pb.PropertyFilter_IN,
notIn: pb.PropertyFilter_NOT_IN,
notEqual: pb.PropertyFilter_NOT_EQUAL,
}

// filter is a conditional filter on query results.
Expand Down Expand Up @@ -177,46 +171,25 @@ func (q *Query) Transaction(t *Transaction) *Query {
}

// Filter returns a derivative query with a field-based filter.
//
// Deprecated: Use the FilterField method instead, which supports the same
// set of operations (and more).
//
// The filterStr argument must be a field name followed by optional space,
// followed by an operator, one of ">", "<", ">=", "<=", "=", and "!=".
// followed by an operator, one of ">", "<", ">=", "<=", or "=".
// Fields are compared against the provided value using the operator.
// Multiple filters are AND'ed together.
// Field names which contain spaces, quote marks, or operator characters
// should be passed as quoted Go string literals as returned by strconv.Quote
// or the fmt package's %q verb.
func (q *Query) Filter(filterStr string, value interface{}) *Query {
// TODO( #5977 ): Add better string parsing (or something)
q = q.clone()
filterStr = strings.TrimSpace(filterStr)
if filterStr == "" {
q.err = fmt.Errorf("datastore: invalid filter %q", filterStr)
return q
}
f := strings.TrimRight(filterStr, " ><=!")
op := strings.TrimSpace(filterStr[len(f):])
return q.FilterField(f, op, value)
}

// FilterField returns a derivative query with a field-based filter.
// The operation parameter takes the following strings: ">", "<", ">=", "<=",
// "=", "!=", "in", and "not-in".
// Fields are compared against the provided value using the operator.
// Multiple filters are AND'ed together.
// Field names which contain spaces, quote marks, or operator characters
// should be passed as quoted Go string literals as returned by strconv.Quote
// or the fmt package's %q verb.
func (q *Query) FilterField(fieldName, operator string, value interface{}) *Query {
q = q.clone()

f := filter{
FieldName: fieldName,
FieldName: strings.TrimRight(filterStr, " ><=!"),
Value: value,
}

switch o := strings.TrimSpace(operator); o {
switch op := strings.TrimSpace(filterStr[len(f.FieldName):]); op {
case "<=":
f.Op = lessEq
case ">=":
Expand All @@ -227,14 +200,8 @@ func (q *Query) FilterField(fieldName, operator string, value interface{}) *Quer
f.Op = greaterThan
case "=":
f.Op = equal
case "in":
f.Op = in
case "not-in":
f.Op = notIn
case "!=":
f.Op = notEqual
default:
q.err = fmt.Errorf("datastore: invalid operator %q in filter", operator)
q.err = fmt.Errorf("datastore: invalid operator %q in filter %q", op, filterStr)
return q
}
var err error
Expand Down
158 changes: 49 additions & 109 deletions datastore/query_test.go
View file
Expand Up @@ -391,61 +391,57 @@ func TestQueriesAreImmutable(t *testing.T) {
}
}

type testFilterCase struct {
filterStr string
fieldName string
operator string
wantOp operator
wantFieldName string
}

var (
// Supported ops both filters.
filterTestCases = []testFilterCase{
{"x<", "x", "<", lessThan, "x"},
{"x <", "x", "<", lessThan, "x"},
{"x <", "x", "<", lessThan, "x"},
{" x < ", "x", "<", lessThan, "x"},
{"x <=", "x", "<=", lessEq, "x"},
{"x =", "x", "=", equal, "x"},
{"x >=", "x", ">=", greaterEq, "x"},
{"x >", "x", ">", greaterThan, "x"},
{"in >", "in", ">", greaterThan, "in"},
{"in>", "in", ">", greaterThan, "in"},
{"x!=", "x", "!=", notEqual, "x"},
{"x !=", "x", "!=", notEqual, "x"},
{" x != ", "x", "!=", notEqual, "x"},
}
// Supported in FilterField only.
filterFieldTestCases = []testFilterCase{
{"x in", "x", "in", in, "x"},
{"x not-in", "x", "not-in", notIn, "x"},
{"ins in", "ins", "in", in, "ins"},
{"in not-in", "in", "not-in", notIn, "in"},
}
// Operators not supported in either filter method
filterUnsupported = []testFilterCase{
{"x IN", "x", "IN", 0, ""},
{"x NOT-IN", "x", "NOT-IN", 0, ""},
{"x EQ", "x", "EQ", 0, ""},
{"x lt", "x", "lt", 0, ""},
{"x <>", "x", "<>", 0, ""},
{"x >>", "x", ">>", 0, ""},
{"x ==", "x", "==", 0, ""},
{"x =<", "x", "=<", 0, ""},
{"x =>", "x", "=>", 0, ""},
{"x !", "x", "!", 0, ""},
{"x ", "x", "", 0, ""},
{"x", "x", "", 0, ""},
}
)

func TestFilterParser(t *testing.T) {
// Success cases
for _, tc := range filterTestCases {
testCases := []struct {
filterStr string
wantOK bool
wantFieldName string
wantOp operator
}{
// Supported ops.
{"x<", true, "x", lessThan},
{"x <", true, "x", lessThan},
{"x <", true, "x", lessThan},
{" x < ", true, "x", lessThan},
{"x <=", true, "x", lessEq},
{"x =", true, "x", equal},
{"x >=", true, "x", greaterEq},
{"x >", true, "x", greaterThan},
{"in >", true, "in", greaterThan},
{"in>", true, "in", greaterThan},
// Valid but (currently) unsupported ops.
{"x!=", false, "", 0},
{"x !=", false, "", 0},
{" x != ", false, "", 0},
{"x IN", false, "", 0},
{"x in", false, "", 0},
// Invalid ops.
{"x EQ", false, "", 0},
{"x lt", false, "", 0},
{"x <>", false, "", 0},
{"x >>", false, "", 0},
{"x ==", false, "", 0},
{"x =<", false, "", 0},
{"x =>", false, "", 0},
{"x !", false, "", 0},
{"x ", false, "", 0},
{"x", false, "", 0},
// Quoted and interesting field names.
{"x > y =", true, "x > y", equal},
{"` x ` =", true, " x ", equal},
{`" x " =`, true, " x ", equal},
{`" \"x " =`, true, ` "x `, equal},
{`" x =`, false, "", 0},
{`" x ="`, false, "", 0},
{"` x \" =", false, "", 0},
}
for _, tc := range testCases {
q := NewQuery("foo").Filter(tc.filterStr, 42)
if q.err != nil {
t.Errorf("%q: error=%v", tc.filterStr, q.err)
if ok := q.err == nil; ok != tc.wantOK {
t.Errorf("%q: ok=%t, want %t", tc.filterStr, ok, tc.wantOK)
continue
}
if !tc.wantOK {
continue
}
if len(q.filter) != 1 {
Expand All @@ -458,62 +454,6 @@ func TestFilterParser(t *testing.T) {
continue
}
}
// Failure cases
failureTestCases := append(filterFieldTestCases, filterUnsupported...)
for _, tc := range failureTestCases {
q := NewQuery("foo").Filter(tc.filterStr, 42)
if q.err == nil {
t.Errorf("%q: should have thrown error", tc.filterStr)
}
}
}

func TestFilterField(t *testing.T) {
successTestCases := append(filterTestCases, filterFieldTestCases...)
for _, tc := range successTestCases {
q := NewQuery("foo").FilterField(tc.fieldName, tc.operator, 42)
if q.err != nil {
t.Errorf("%q %q: error: %v", tc.fieldName, tc.operator, q.err)
continue
}
if len(q.filter) != 1 {
t.Errorf("%q: len=%d, want %d", tc.fieldName, len(q.filter), 1)
continue
}
got, want := q.filter[0], filter{tc.fieldName, tc.wantOp, 42}
if got != want {
t.Errorf("%q %q: got %v, want %v", tc.fieldName, tc.operator, got, want)
continue
}
}
for _, tc := range filterUnsupported {
q := NewQuery("foo").Filter(tc.filterStr, 42)
if q.err == nil {
t.Errorf("%q: should have thrown error", tc.filterStr)
}
}
}

func TestUnquote(t *testing.T) {
testCases := []struct {
input string
want string
}{
{`" x "`, ` x `},
{`"\" \\\"x \""`, `" \"x "`},
}

for _, tc := range testCases {
got, err := unquote(tc.input)

if err != nil {
t.Errorf("error parsing field name: %v", err)
}

if got != tc.want {
t.Errorf("field name parsing error: \nwant %v,\ngot %v", tc.want, got)
}
}
}

func TestNamespaceQuery(t *testing.T) {
Expand Down

0 comments on commit cca86d6

Please sign in to comment.