Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "feat(datastore): adds in, not-in, and != query operations" #6016

Merged
merged 1 commit into from May 9, 2022
Merged

Revert "feat(datastore): adds in, not-in, and != query operations" #6016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 5 additions & 38 deletions datastore/query.go
View file
Open in desktop
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
Open in desktop
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