Skip to content

Commit

Permalink
JobSet Sorting (#3415)
Browse files Browse the repository at this point in the history
* Adding Job Set sorting (#72)

* Adding JobSet sorting even when aggregated

* Gitignore

* wip

* Working version

* Working version improved

* Rename

* Make more general

* Allow ordering by groupable columns

* Remove gitignore yarn.lock

* Linting

* Change the name of canOrderBy function

* Linting

* Linting

* Lint

---------

Co-authored-by: Mia Mijovic <Mia.Mijovic@gresearch.co.uk>
  • Loading branch information
mijovicmia and Mia Mijovic committed Feb 19, 2024
1 parent 6d43648 commit ade4347
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 6 deletions.
43 changes: 39 additions & 4 deletions internal/lookout/ui/src/hooks/useJobsTableData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,16 @@ const columnToGroupSortFieldMap = new Map<ColumnId, string>([
[StandardColumnId.TimeSubmittedAgo, "submitted"],
[StandardColumnId.LastTransitionTimeUtc, "lastTransitionTime"],
[StandardColumnId.TimeInState, "lastTransitionTime"],
[StandardColumnId.JobSet, "jobSet"],
])

// Return ordering to request to API based on column
function getOrder(lookoutOrder: LookoutColumnOrder, isJobFetch: boolean): JobOrder {
function getOrder(
lookoutOrder: LookoutColumnOrder,
lookoutFilters: JobFilter[],
groupedColumns: ColumnId[],
isJobFetch: boolean,
): JobOrder {
const defaultJobOrder: JobOrder = {
field: "jobId",
direction: "DESC",
Expand All @@ -101,9 +107,13 @@ function getOrder(lookoutOrder: LookoutColumnOrder, isJobFetch: boolean): JobOrd
}
field = columnToJobSortFieldMap.get(lookoutOrder.id as ColumnId) as string
} else {
if (!columnToGroupSortFieldMap.has(lookoutOrder.id as ColumnId)) {
if (
!canOrderByField(lookoutFilters, groupedColumns, lookoutOrder.id) ||
!columnToGroupSortFieldMap.has(lookoutOrder.id as ColumnId)
) {
return defaultGroupOrder
}

field = columnToGroupSortFieldMap.get(lookoutOrder.id as ColumnId) as string
}

Expand All @@ -113,6 +123,30 @@ function getOrder(lookoutOrder: LookoutColumnOrder, isJobFetch: boolean): JobOrd
}
}

function canOrderByField(lookoutFilters: JobFilter[], groupedColumns: string[], id: string): boolean {
// A function that determines whether we can see the field in the UI, therefore allowing us to sort by it
// Extract 'field' values from lookoutFilters, excluding 'jobSet'
const filterFields = lookoutFilters.map((filter) => filter.field).filter((field) => field !== id)

// Exclude 'jobSet' from groupedColumns for direct comparison
const filteredGroupedColumns = groupedColumns.filter((col) => col !== id)

// Check if the lengths are different
if (filterFields.length !== filteredGroupedColumns.length) {
return false
}

// Check each element for an exact match
for (let i = 0; i < filterFields.length; i++) {
if (filterFields[i] !== filteredGroupedColumns[i]) {
return false
}
}

// If all checks pass, return true
return true
}

export const useFetchJobsTableData = ({
groupedColumns,
visibleColumns,
Expand Down Expand Up @@ -149,9 +183,10 @@ export const useFetchJobsTableData = ({
const expandedLevel = parentRowInfo ? parentRowInfo.rowIdPathFromRoot.length : 0
const isJobFetch = expandedLevel === groupingLevel

const order = getOrder(lookoutOrder, isJobFetch)
const filterValues = getFiltersForRows(lookoutFilters, columnMatches, parentRowInfo?.rowIdPartsPath ?? [])
const order = getOrder(lookoutOrder, filterValues, groupedColumns, isJobFetch)
const rowRequest: FetchRowRequest = {
filters: getFiltersForRows(lookoutFilters, columnMatches, parentRowInfo?.rowIdPartsPath ?? []),
filters: filterValues,
activeJobSets: activeJobSets,
skip: nextRequest.skip ?? paginationState.pageIndex * paginationState.pageSize,
take: nextRequest.take ?? paginationState.pageSize,
Expand Down
34 changes: 32 additions & 2 deletions internal/lookoutv2/repository/querybuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ func (qb *QueryBuilder) GroupBy(
}
}

groupBySql := fmt.Sprintf("GROUP BY %s.%s", groupCol.abbrev, groupCol.name)
whereSql, err := qb.queryFiltersToSql(queryFilters, true)
if err != nil {
return nil, err
Expand All @@ -239,6 +238,10 @@ func (qb *QueryBuilder) GroupBy(
if err != nil {
return nil, err
}
groupBySql, err := qb.createGroupBySQL(order, groupCol, aggregates)
if err != nil {
return nil, err
}
template := fmt.Sprintf(`
SELECT %[1]s.%[2]s, %[3]s
%[4]s
Expand All @@ -254,6 +257,31 @@ func (qb *QueryBuilder) GroupBy(
}, nil
}

func (qb *QueryBuilder) createGroupBySQL(order *model.Order, groupCol *queryColumn, aggregates []string) (string, error) {
expr := fmt.Sprintf("GROUP BY %s.%s", groupCol.abbrev, groupCol.name)
isInAggregators := len(aggregates) > 0 && func(sl []string, t string) bool {
for _, s := range sl {
if s == t {
return true
}
}
return false
}(aggregates, order.Field)
if orderIsNull(order) || order.Field == countCol || isInAggregators {
return expr, nil
}
col, err := qb.lookoutTables.ColumnFromField(order.Field)
if err != nil {
return expr, err
}

if groupCol.name == col {
return expr, nil
}
// If order is not already grouped by or aggregated by, include it in GROUP BY statement
return expr + fmt.Sprintf(", %s.%s", groupCol.abbrev, col), nil
}

func (qb *QueryBuilder) fieldsToCols(fields []string) ([]string, error) {
var cols []string
for _, field := range fields {
Expand Down Expand Up @@ -940,8 +968,10 @@ func (qb *QueryBuilder) validateGroupOrder(order *model.Order) error {
if err != nil {
return errors.Errorf("unsupported field for order: %s", order.Field)
}

_, err = qb.lookoutTables.GroupAggregateForCol(col)
if err != nil {
// If it is not an aggregate and not groupable, it can't be ordered by
if err != nil && !qb.lookoutTables.IsGroupable(col) {
return errors.Errorf("unsupported field for order: %s, cannot sort by column %s", order.Field, col)
}
return nil
Expand Down

0 comments on commit ade4347

Please sign in to comment.