Skip to content

Commit

Permalink
feat(backend): sort by run metrics - step 3. Part of #3591 (#4251)
Browse files Browse the repository at this point in the history
* enable pagination when expanding experiment in both the home page and the archive page

* Revert "enable pagination when expanding experiment in both the home page and the archive page"

This reverts commit 5b67273.

* sorting by run metrics is different from sorting by name, uuid, created at, etc. The lattre are direct field in listable object, the former is an element in an arrary-typed field in listable object. In other words, the latter are columns in table, the former is not.

* unit test: add sorting on metrics with both asc and desc order

* GetFieldValue in all models

* fix unit test

* whether to test in list_test. It's hacky when check mode == 'run'

* move model specific code to model; prevent model package depends on list package; let list package depends on modelpackage; marshal/unmarshal listable interface; include listable interface in token.

* some assumption on token's Model field

* fix the regular field checking logic

* add comment to help devs to use the new field

* add a validation check

* Listable object can be too large to be in token. So replace it with only
relevant fields taken out of it. In the future, if more fields in
Listable object become relevant, manually add it to token

* matches func update
  • Loading branch information
jingzhang36 authored Jul 31, 2020
1 parent 8a7ea0f commit 9c6738f
Show file tree
Hide file tree
Showing 14 changed files with 523 additions and 286 deletions.
108 changes: 34 additions & 74 deletions backend/src/apiserver/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,15 @@ type token struct {
SortByFieldName string
// SortByFieldValue is the value of the sorted field of the next row to be
// returned.
SortByFieldValue interface{}
// SortByFieldIsRunMetric indicates whether the SortByFieldName field is
// a run metric field or not.
SortByFieldIsRunMetric bool
SortByFieldValue interface{}
SortByFieldPrefix string

// KeyFieldName is the name of the primary key for the model being queried.
KeyFieldName string
// KeyFieldValue is the value of the sorted field of the next row to be
// returned.
KeyFieldValue interface{}
KeyFieldValue interface{}
KeyFieldPrefix string

// IsDesc is true if the sorting order should be descending.
IsDesc bool
Expand Down Expand Up @@ -101,7 +100,7 @@ type Options struct {
// Matches returns trues if the sorting and filtering criteria in o matches that
// of the one supplied in opts.
func (o *Options) Matches(opts *Options) bool {
return o.SortByFieldName == opts.SortByFieldName && o.SortByFieldIsRunMetric == opts.SortByFieldIsRunMetric &&
return o.SortByFieldName == opts.SortByFieldName && o.SortByFieldPrefix == opts.SortByFieldPrefix &&
o.IsDesc == opts.IsDesc &&
reflect.DeepEqual(o.Filter, opts.Filter)
}
Expand Down Expand Up @@ -147,24 +146,16 @@ func NewOptions(listable Listable, pageSize int, sortBy string, filterProto *api
}

token.SortByFieldName = listable.DefaultSortField()
token.SortByFieldIsRunMetric = false
if len(queryList) > 0 {
var err error
n, ok := listable.APIToModelFieldMap()[queryList[0]]
n, ok := listable.GetField(queryList[0])
if ok {
token.SortByFieldName = n
} else if strings.HasPrefix(queryList[0], "metric:") {
// Sorting on metrics is only available on certain runs.
model := reflect.ValueOf(listable).Elem().Type().Name()
if model != "Run" {
return nil, util.NewInvalidInputError("Invalid sorting field: %q on %q : %s", queryList[0], model, err)
}
token.SortByFieldName = queryList[0][7:]
token.SortByFieldIsRunMetric = true
} else {
return nil, util.NewInvalidInputError("Invalid sorting field: %q: %s", queryList[0], err)
return nil, util.NewInvalidInputError("Invalid sorting field: %q on listable type %s", queryList[0], reflect.ValueOf(listable).Elem().Type().Name())
}
}
token.SortByFieldPrefix = listable.GetSortByFieldPrefix(token.SortByFieldName)
token.KeyFieldPrefix = listable.GetKeyFieldPrefix()

if len(queryList) == 2 {
token.IsDesc = queryList[1] == "desc"
Expand Down Expand Up @@ -196,31 +187,18 @@ func (o *Options) AddPaginationToSelect(sqlBuilder sq.SelectBuilder) sq.SelectBu
// AddSortingToSelect adds Order By clause.
func (o *Options) AddSortingToSelect(sqlBuilder sq.SelectBuilder) sq.SelectBuilder {
// When sorting by a direct field in the listable model (i.e., name in Run or uuid in Pipeline), a sortByFieldPrefix can be specified; when sorting by a field in an array-typed dictionary (i.e., a run metric inside the metrics in Run), a sortByFieldPrefix is not needed.
var keyFieldPrefix string
var sortByFieldPrefix string
if len(o.ModelName) == 0 {
keyFieldPrefix = ""
sortByFieldPrefix = ""
} else if o.SortByFieldIsRunMetric {
keyFieldPrefix = o.ModelName + "."
sortByFieldPrefix = ""
} else {
keyFieldPrefix = o.ModelName + "."
sortByFieldPrefix = o.ModelName + "."
}

// If next row's value is specified, set those values in the clause.
if o.SortByFieldValue != nil && o.KeyFieldValue != nil {
if o.IsDesc {
sqlBuilder = sqlBuilder.
Where(sq.Or{sq.Lt{sortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.And{sq.Eq{sortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.LtOrEq{keyFieldPrefix + o.KeyFieldName: o.KeyFieldValue}}})
Where(sq.Or{sq.Lt{o.SortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.And{sq.Eq{o.SortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.LtOrEq{o.KeyFieldPrefix + o.KeyFieldName: o.KeyFieldValue}}})
} else {
sqlBuilder = sqlBuilder.
Where(sq.Or{sq.Gt{sortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.And{sq.Eq{sortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.GtOrEq{keyFieldPrefix + o.KeyFieldName: o.KeyFieldValue}}})
Where(sq.Or{sq.Gt{o.SortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.And{sq.Eq{o.SortByFieldPrefix + o.SortByFieldName: o.SortByFieldValue},
sq.GtOrEq{o.KeyFieldPrefix + o.KeyFieldName: o.KeyFieldValue}}})
}
}

Expand All @@ -229,25 +207,12 @@ func (o *Options) AddSortingToSelect(sqlBuilder sq.SelectBuilder) sq.SelectBuild
order = "DESC"
}
sqlBuilder = sqlBuilder.
OrderBy(fmt.Sprintf("%v %v", sortByFieldPrefix+o.SortByFieldName, order)).
OrderBy(fmt.Sprintf("%v %v", keyFieldPrefix+o.KeyFieldName, order))
OrderBy(fmt.Sprintf("%v %v", o.SortByFieldPrefix+o.SortByFieldName, order)).
OrderBy(fmt.Sprintf("%v %v", o.KeyFieldPrefix+o.KeyFieldName, order))

return sqlBuilder
}

// Add a metric as a new field to the select clause by join the passed-in SQL query with run_metrics table.
// With the metric as a field in the select clause enable sorting on this metric afterwards.
func (o *Options) AddSortByRunMetricToSelect(sqlBuilder sq.SelectBuilder) sq.SelectBuilder {
if !o.SortByFieldIsRunMetric {
return sqlBuilder
}
// TODO(jingzhang36): address the case where runs doesn't have the specified metric.
return sq.
Select("selected_runs.*, run_metrics.numbervalue as "+o.SortByFieldName).
FromSelect(sqlBuilder, "selected_runs").
LeftJoin("run_metrics ON selected_runs.uuid=run_metrics.runuuid AND run_metrics.name='" + o.SortByFieldName + "'")
}

// AddFilterToSelect adds WHERE clauses with the filtering criteria in the
// Options o to the supplied SelectBuilder, and returns the new SelectBuilder
// containing these.
Expand Down Expand Up @@ -345,6 +310,12 @@ type Listable interface {
APIToModelFieldMap() map[string]string
// GetModelName returns table name used as sort field prefix.
GetModelName() string
// Get the prefix of sorting field.
GetSortByFieldPrefix(string) string
// Get the prefix of key field.
GetKeyFieldPrefix() string
// Get a valid field for sorting/filtering in a listable model from the given string.
GetField(name string) (string, bool)
// Find the value of a given field in a listable object.
GetFieldValue(name string) interface{}
}
Expand All @@ -365,20 +336,8 @@ func (o *Options) nextPageToken(listable Listable) (*token, error) {
elemName := elem.Type().Name()

var sortByField interface{}
// TODO(jingzhang36): this if-else block can be simplified to one call to
// GetFieldValue after all the models (run, job, experiment, etc.) implement
// GetFieldValue method in listable interface.
if !o.SortByFieldIsRunMetric {
if value := elem.FieldByName(o.SortByFieldName); value.IsValid() {
sortByField = value.Interface()
} else {
return nil, util.NewInvalidInputError("cannot sort by field %q on type %q", o.SortByFieldName, elemName)
}
} else {
sortByField = listable.GetFieldValue(o.SortByFieldName)
if sortByField == nil {
return nil, util.NewInvalidInputError("Unable to find run metric %s", o.SortByFieldName)
}
if sortByField = listable.GetFieldValue(o.SortByFieldName); sortByField == nil {
return nil, util.NewInvalidInputError("cannot sort by field %q on type %q", o.SortByFieldName, elemName)
}

keyField := elem.FieldByName(listable.PrimaryKeyColumnName())
Expand All @@ -387,14 +346,15 @@ func (o *Options) nextPageToken(listable Listable) (*token, error) {
}

return &token{
SortByFieldName: o.SortByFieldName,
SortByFieldValue: sortByField,
SortByFieldIsRunMetric: o.SortByFieldIsRunMetric,
KeyFieldName: listable.PrimaryKeyColumnName(),
KeyFieldValue: keyField.Interface(),
IsDesc: o.IsDesc,
Filter: o.Filter,
ModelName: o.ModelName,
SortByFieldName: o.SortByFieldName,
SortByFieldValue: sortByField,
SortByFieldPrefix: listable.GetSortByFieldPrefix(o.SortByFieldName),
KeyFieldName: listable.PrimaryKeyColumnName(),
KeyFieldValue: keyField.Interface(),
KeyFieldPrefix: listable.GetKeyFieldPrefix(),
IsDesc: o.IsDesc,
Filter: o.Filter,
ModelName: o.ModelName,
}, nil
}

Expand Down
Loading

0 comments on commit 9c6738f

Please sign in to comment.