Skip to content

Commit

Permalink
fix: make trial log timestamp filters backwards compatible (#1944)
Browse files Browse the repository at this point in the history
This change makes the query to gather trial logs with timestamp filters backwards compatible with logs created before 0.13.8.

(cherry picked from commit 3184552)
  • Loading branch information
stoksc authored and determined-dsw committed Feb 9, 2021
1 parent 461288d commit 0c229de
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 8 deletions.
22 changes: 16 additions & 6 deletions master/internal/db/postgres_filters.go
Expand Up @@ -19,22 +19,32 @@ var validField = regexp.MustCompile(`^[a-zA-Z0-9_]+$`)
//
// The user input to the filters should always be contained in api.Filter.Values and
// never the field. If the field is taken from user input, SQL injection is possible.
func filtersToSQL(fs []api.Filter, params []interface{}) (string, []interface{}) {
func filtersToSQL(
fs []api.Filter, params []interface{}, fieldMap map[string]string,
) (string, []interface{}) {
paramID := len(params) + 1
var fragments []string
for _, f := range fs {
if !validField.MatchString(f.Field) {
panic(fmt.Sprintf("field in filter %s contains possible SQL injection", f.Field))
}
filterParams := filterToParams(f)
fragments = append(fragments, filterToSQL(f, filterParams, paramID))
fragments = append(fragments, filterToSQL(f, filterParams, paramID, fieldMap))
params = append(params, filterParams...)
paramID += len(filterParams)
}
return strings.Join(fragments, "\n"), params
}

func filterToSQL(f api.Filter, values []interface{}, paramID int) string {
func filterToSQL(
f api.Filter, values []interface{}, paramID int, fieldMap map[string]string,
) string {
var field string
if fm, ok := fieldMap[f.Field]; ok {
field = fm
} else {
field = f.Field
}
switch f.Operation {
case api.FilterOperationIn:
var fragment strings.Builder
Expand All @@ -45,11 +55,11 @@ func filterToSQL(f api.Filter, values []interface{}, paramID int) string {
}
_, _ = fragment.WriteString(strings.Join(paramFragments, ","))
_, _ = fragment.WriteString(")")
return fmt.Sprintf(fragment.String(), f.Field)
return fmt.Sprintf(fragment.String(), field)
case api.FilterOperationGreaterThan:
return fmt.Sprintf("AND %s > $%d", f.Field, paramID)
return fmt.Sprintf("AND %s > $%d", field, paramID)
case api.FilterOperationLessThanEqual:
return fmt.Sprintf("AND %s <= $%d", f.Field, paramID)
return fmt.Sprintf("AND %s <= $%d", field, paramID)
default:
panic(fmt.Sprintf("cannot convert operation %d to SQL", f.Operation))
}
Expand Down
15 changes: 13 additions & 2 deletions master/internal/db/postgres_trial_logs.go
Expand Up @@ -13,12 +13,23 @@ import (
"github.com/determined-ai/determined/master/pkg/model"
)

var trialLogsFieldMap = map[string]string{
// Map timestamp to an expression that provides backwards compatibility when timestamp is missing.
"timestamp": `coalesce(timestamp,
to_timestamp(
substring(convert_from(message, 'UTF-8') from
'\[([0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z)\]'),
'YYYY-MM-DD hh24:mi:ss'
)
)`,
}

// TrialLogs takes a trial ID and log offset, limit and filters and returns matching trial logs.
func (db *PgDB) TrialLogs(
trialID, offset, limit int, fs []api.Filter, order apiv1.OrderBy, _ interface{},
) ([]*model.TrialLog, interface{}, error) {
params := []interface{}{trialID, offset, limit}
fragment, params := filtersToSQL(fs, params)
fragment, params := filtersToSQL(fs, params, trialLogsFieldMap)
query := fmt.Sprintf(`
SELECT
l.id,
Expand Down Expand Up @@ -98,7 +109,7 @@ INSERT INTO trial_logs
// TrialLogCount returns the number of logs in postgres for the given trial.
func (db *PgDB) TrialLogCount(trialID int, fs []api.Filter) (int, error) {
params := []interface{}{trialID}
fragment, params := filtersToSQL(fs, params)
fragment, params := filtersToSQL(fs, params, trialLogsFieldMap)
query := fmt.Sprintf(`
SELECT count(*)
FROM trial_logs
Expand Down

0 comments on commit 0c229de

Please sign in to comment.