Skip to content

Commit

Permalink
Refactor sort logic to take array instead of pb.List. (#4289)
Browse files Browse the repository at this point in the history 
pb.List will change to have a compressed list of uids. This refactore
changes the sort logic in the types package to not depend on pb.List so
the transition is easier.
  • Loading branch information
@martinmr
martinmr authored Dec 18, 2019
1 parent b9627f1 commit e3844d1
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 20 deletions.
7 changes: 3 additions & 4 deletions query/query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2339,8 +2339,8 @@ func (sg *SubGraph) sortAndPaginateUsingFacet(ctx context.Context) error {
if len(values) == 0 {
continue
}
if err := types.SortWithFacet(values, &pb.List{Uids: uids},
facetList, []bool{sg.Params.FacetOrderDesc}, ""); err != nil {
if err := types.SortWithFacet(values, &uids, facetList,
[]bool{sg.Params.FacetOrderDesc}, ""); err != nil {
return err
}
sg.uidMatrix[i].Uids = uids
Expand Down Expand Up @@ -2387,8 +2387,7 @@ func (sg *SubGraph) sortAndPaginateUsingVar(ctx context.Context) error {
if len(values) == 0 {
continue
}
if err := types.Sort(values, &pb.List{Uids: uids},
[]bool{sg.Params.Order[0].Desc}, ""); err != nil {
if err := types.Sort(values, &uids, []bool{sg.Params.Order[0].Desc}, ""); err != nil {
return err
}
sg.uidMatrix[i].Uids = uids
Expand Down
12 changes: 7 additions & 5 deletions types/sort.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,20 @@ import (
type sortBase struct {
values [][]Val // Each uid could have multiple values which we need to sort it by.
desc []bool // Sort orders for different values.
ul *pb.List
ul *[]uint64
o []*pb.Facets
cl *collate.Collator // Compares Unicode strings according to the given collation order.
}

// Len returns size of vector.
// skipcq: CRT-P0003
func (s sortBase) Len() int { return len(s.values) }

// Swap swaps two elements.
// skipcq: CRT-P0003
func (s sortBase) Swap(i, j int) {
s.values[i], s.values[j] = s.values[j], s.values[i]
data := s.ul.Uids
data[i], data[j] = data[j], data[i]
(*s.ul)[i], (*s.ul)[j] = (*s.ul)[j], (*s.ul)[i]
if s.o != nil {
s.o[i], s.o[j] = s.o[j], s.o[i]
}
Expand All @@ -51,6 +52,7 @@ func (s sortBase) Swap(i, j int) {
type byValue struct{ sortBase }

// Less compares two elements
// skipcq: CRT-P0003
func (s byValue) Less(i, j int) bool {
first, second := s.values[i], s.values[j]
if len(first) == 0 || len(second) == 0 {
Expand Down Expand Up @@ -84,7 +86,7 @@ func (s byValue) Less(i, j int) bool {

// SortWithFacet sorts the given array in-place and considers the given facets to calculate
// the proper ordering.
func SortWithFacet(v [][]Val, ul *pb.List, l []*pb.Facets, desc []bool, lang string) error {
func SortWithFacet(v [][]Val, ul *[]uint64, l []*pb.Facets, desc []bool, lang string) error {
if len(v) == 0 || len(v[0]) == 0 {
return nil
}
Expand Down Expand Up @@ -113,7 +115,7 @@ func SortWithFacet(v [][]Val, ul *pb.List, l []*pb.Facets, desc []bool, lang str
}

// Sort sorts the given array in-place.
func Sort(v [][]Val, ul *pb.List, desc []bool, lang string) error {
func Sort(v [][]Val, ul *[]uint64, desc []bool, lang string) error {
return SortWithFacet(v, ul, nil, desc, lang)
}

Expand Down
18 changes: 9 additions & 9 deletions types/sort_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func getUIDList(n int) *pb.List {
func TestSortStrings(t *testing.T) {
list := getInput(t, StringID, []string{"bb", "aaa", "aa", "bab"})
ul := getUIDList(4)
require.NoError(t, Sort(list, ul, []bool{false}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, ""))
require.EqualValues(t, []uint64{300, 200, 400, 100}, ul.Uids)
require.EqualValues(t, []string{"aa", "aaa", "bab", "bb"},
toString(t, list, StringID))
Expand All @@ -66,15 +66,15 @@ func TestSortLanguage(t *testing.T) {
list := getInput(t, StringID, []string{"öffnen", "zumachen"})
ul := getUIDList(2)

require.NoError(t, Sort(list, ul, []bool{false}, "de"))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, "de"))
require.EqualValues(t, []uint64{100, 200}, ul.Uids)
require.EqualValues(t, []string{"öffnen", "zumachen"},
toString(t, list, StringID))

// Sorting strings of swedish language.
list = getInput(t, StringID, []string{"öppna", "zon"})

require.NoError(t, Sort(list, ul, []bool{false}, "sv"))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, "sv"))
require.EqualValues(t, []uint64{200, 100}, ul.Uids)
require.EqualValues(t, []string{"zon", "öppna"},
toString(t, list, StringID))
Expand All @@ -83,7 +83,7 @@ func TestSortLanguage(t *testing.T) {
func TestSortInts(t *testing.T) {
list := getInput(t, IntID, []string{"22", "111", "11", "212"})
ul := getUIDList(4)
require.NoError(t, Sort(list, ul, []bool{false}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, ""))
require.EqualValues(t, []uint64{300, 100, 200, 400}, ul.Uids)
require.EqualValues(t, []string{"11", "22", "111", "212"},
toString(t, list, IntID))
Expand All @@ -92,7 +92,7 @@ func TestSortInts(t *testing.T) {
func TestSortFloats(t *testing.T) {
list := getInput(t, FloatID, []string{"22.2", "11.2", "11.5", "2.12"})
ul := getUIDList(4)
require.NoError(t, Sort(list, ul, []bool{false}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, ""))
require.EqualValues(t, []uint64{400, 200, 300, 100}, ul.Uids)
require.EqualValues(t,
[]string{"2.12", "11.2", "11.5", "22.2"},
Expand All @@ -102,7 +102,7 @@ func TestSortFloats(t *testing.T) {
func TestSortFloatsDesc(t *testing.T) {
list := getInput(t, FloatID, []string{"22.2", "11.2", "11.5", "2.12"})
ul := getUIDList(4)
require.NoError(t, Sort(list, ul, []bool{true}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{true}, ""))
require.EqualValues(t, []uint64{100, 300, 200, 400}, ul.Uids)
require.EqualValues(t,
[]string{"22.2", "11.5", "11.2", "2.12"},
Expand All @@ -118,7 +118,7 @@ func TestSortDateTimes(t *testing.T) {
}
list := getInput(t, DateTimeID, in)
ul := getUIDList(4)
require.NoError(t, Sort(list, ul, []bool{false}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, ""))
require.EqualValues(t, []uint64{400, 200, 300, 100}, ul.Uids)
require.EqualValues(t,
[]string{"2006-01-02T15:04:01Z", "2006-01-02T15:04:05Z",
Expand All @@ -133,7 +133,7 @@ func TestSortIntAndFloat(t *testing.T) {
{{Tid: IntID, Value: int64(100)}},
}
ul := getUIDList(3)
require.NoError(t, Sort(list, ul, []bool{false}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, ""))
require.EqualValues(t, []uint64{200, 100, 300}, ul.Uids)
require.EqualValues(t,
[]string{"21.5", "55", "100"},
Expand Down Expand Up @@ -188,7 +188,7 @@ func TestSortMismatchedTypes(t *testing.T) {
{{Tid: FloatID, Value: 33.33}},
}
ul := getUIDList(7)
require.NoError(t, Sort(list, ul, []bool{false}, ""))
require.NoError(t, Sort(list, &ul.Uids, []bool{false}, ""))

// Don't care about relative ordering between types. However, like types
// should be sorted with each other.
Expand Down
4 changes: 2 additions & 2 deletions worker/sort.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func multiSort(ctx context.Context, r *sortresult, ts *pb.SortMessage) error {
x.AssertTrue(idx >= 0)
vals[j] = sortVals[idx]
}
if err := types.Sort(vals, ul, desc, ""); err != nil {
if err := types.Sort(vals, &ul.Uids, desc, ""); err != nil {
return err
}
// Paginate
Expand Down Expand Up @@ -711,7 +711,7 @@ func sortByValue(ctx context.Context, ts *pb.SortMessage, ul *pb.List,
values = append(values, []types.Val{val})
}
}
err := types.Sort(values, &pb.List{Uids: uids}, []bool{order.Desc}, lang)
err := types.Sort(values, &uids, []bool{order.Desc}, lang)
ul.Uids = uids
if len(ts.Order) > 1 {
for _, v := range values {
Expand Down

0 comments on commit e3844d1

Please sign in to comment.