Skip to content

Commit

Permalink
Update repeated value filters with ValueNotIn support (#5110)
Browse files Browse the repository at this point in the history
Signed-off-by: troychiu <[email protected]>
  • Loading branch information
troychiu authored Mar 26, 2024
1 parent 46d1bd8 commit b35246b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 25 deletions.
7 changes: 4 additions & 3 deletions docs/concepts/admin.rst
Original file line number Diff line number Diff line change
Expand Up @@ -260,14 +260,15 @@ The fully supported set of filter functions are

- contains
- gt (greater than)
- gte (greter than or equal to)
- gte (greater than or equal to)
- lt (less than)
- lte (less than or equal to)
- eq (equal)
- ne (not equal)
- value_in (for repeated sets of values)
- value_in (value in repeated sets of values)
- value_not_in (value not in repeated sets of values)

"value_in" is a special case where multiple values are passed to the filter expression. For example::
"value_in" and "value_not_in" are special cases where multiple values are passed to the filter expression. For example::

value_in(phase, RUNNING;SUCCEEDED;FAILED)

Expand Down
26 changes: 22 additions & 4 deletions flyteadmin/pkg/common/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const (
Equal
NotEqual
ValueIn
ValueNotIn
)

// String formats for various filter expression queries
Expand All @@ -43,6 +44,7 @@ const (
equalQuery = "%s = ?"
notEqualQuery = "%s <> ?"
valueInQuery = "%s in (?)"
valueNotInQuery = "%s not in (?)"
)

// Set of available filters which exclusively accept a single argument value.
Expand All @@ -58,7 +60,8 @@ var singleValueFilters = map[FilterExpression]bool{

// Set of available filters which exclusively accept repeated argument values.
var repeatedValueFilters = map[FilterExpression]bool{
ValueIn: true,
ValueIn: true,
ValueNotIn: true,
}

const EqualExpression = "eq"
Expand All @@ -72,6 +75,19 @@ var filterNameMappings = map[string]FilterExpression{
EqualExpression: Equal,
"ne": NotEqual,
"value_in": ValueIn,
"value_not_in": ValueNotIn,
}

var filterQueryMappings = map[FilterExpression]string{
Contains: containsQuery,
GreaterThan: greaterThanQuery,
GreaterThanOrEqual: greaterThanOrEqualQuery,
LessThan: lessThanQuery,
LessThanOrEqual: lessThanOrEqualQuery,
Equal: equalQuery,
NotEqual: notEqualQuery,
ValueIn: valueInQuery,
ValueNotIn: valueNotInQuery,
}

var executionIdentifierFields = map[string]bool{
Expand Down Expand Up @@ -108,6 +124,8 @@ func getFilterExpressionName(expression FilterExpression) string {
return "not equal"
case ValueIn:
return "value in"
case ValueNotIn:
return "value not in"
default:
return ""
}
Expand Down Expand Up @@ -166,9 +184,9 @@ func (f *inlineFilterImpl) GetField() string {

func (f *inlineFilterImpl) getGormQueryExpr(formattedField string) (GormQueryExpr, error) {

// ValueIn is special because it uses repeating values.
if f.function == ValueIn {
queryStr := fmt.Sprintf(valueInQuery, formattedField)
// Filters that use repeated values
if _, ok := repeatedValueFilters[f.function]; ok {
queryStr := fmt.Sprintf(filterQueryMappings[f.function], formattedField)
return GormQueryExpr{
Query: queryStr,
Args: f.repeatedValue,
Expand Down
39 changes: 21 additions & 18 deletions flyteadmin/pkg/common/filters_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package common

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -83,6 +84,14 @@ func TestNewRepeatedValueFilter(t *testing.T) {
assert.Equal(t, "projects.project in (?)", expression.Query)
assert.Equal(t, vals, expression.Args)

filter, err = NewRepeatedValueFilter(Workflow, ValueNotIn, "project", vals)
assert.NoError(t, err)

expression, err = filter.GetGormJoinTableQueryExpr("projects")
assert.NoError(t, err)
assert.Equal(t, "projects.project not in (?)", expression.Query)
assert.Equal(t, vals, expression.Args)

_, err = NewRepeatedValueFilter(Workflow, Equal, "domain", []string{"production", "qa"})
assert.EqualError(t, err, "invalid repeated value filter expression: equal")
}
Expand All @@ -96,16 +105,6 @@ func TestGetGormJoinTableQueryExpr(t *testing.T) {
assert.Equal(t, "workflows.domain = ?", gormQueryExpr.Query)
}

var expectedQueriesForFilters = map[FilterExpression]string{
Contains: "field LIKE ?",
GreaterThan: "field > ?",
GreaterThanOrEqual: "field >= ?",
LessThan: "field < ?",
LessThanOrEqual: "field <= ?",
Equal: "field = ?",
NotEqual: "field <> ?",
}

var expectedArgsForFilters = map[FilterExpression]string{
Contains: "%value%",
GreaterThan: "value",
Expand All @@ -117,27 +116,31 @@ var expectedArgsForFilters = map[FilterExpression]string{
}

func TestQueryExpressions(t *testing.T) {
for expression, expectedQuery := range expectedQueriesForFilters {
for expression := range singleValueFilters {
filter, err := NewSingleValueFilter(Workflow, expression, "field", "value")
assert.NoError(t, err)

gormQueryExpr, err := filter.GetGormQueryExpr()
assert.NoError(t, err)
expectedQuery := fmt.Sprintf(filterQueryMappings[expression], "field")
assert.Equal(t, expectedQuery, gormQueryExpr.Query)

expectedArg, ok := expectedArgsForFilters[expression]
assert.True(t, ok, "Missing expected argument for expression %s", expression)
assert.Equal(t, expectedArg, gormQueryExpr.Args)
}

// Also test the one repeated value filter
filter, err := NewRepeatedValueFilter(Workflow, ValueIn, "field", []string{"value"})
assert.NoError(t, err)
// Also test the repeated value filters
for expression := range repeatedValueFilters {
filter, err := NewRepeatedValueFilter(Workflow, expression, "field", []string{"value"})
assert.NoError(t, err)

gormQueryExpr, err := filter.GetGormQueryExpr()
assert.NoError(t, err)
assert.Equal(t, "field in (?)", gormQueryExpr.Query)
assert.EqualValues(t, []string{"value"}, gormQueryExpr.Args)
gormQueryExpr, err := filter.GetGormQueryExpr()
assert.NoError(t, err)
expectedQuery := fmt.Sprintf(filterQueryMappings[expression], "field")
assert.Equal(t, expectedQuery, gormQueryExpr.Query)
assert.EqualValues(t, []string{"value"}, gormQueryExpr.Args)
}
}

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

0 comments on commit b35246b

Please sign in to comment.