diff --git a/db/base/compare.go b/db/base/compare.go index 081cc250cd..c5636f9e15 100644 --- a/db/base/compare.go +++ b/db/base/compare.go @@ -26,6 +26,10 @@ import ( // and they are always the same type as each other. // @todo: Handle list/slice/array fields func Compare(a, b any) int { + if a == nil || b == nil { + return compareNil(a, b) + } + switch v := a.(type) { case bool: return compareBool(v, b.(bool)) @@ -48,6 +52,15 @@ func Compare(a, b any) int { } } +func compareNil(a, b any) int { + if a == nil && b == nil { + return 0 + } else if b == nil && a != nil { // a > b (1 > nil) + return 1 + } + return -1 +} + func compareBool(a, b bool) int { if a == b { return 0 diff --git a/planner/values.go b/planner/values.go index c7cf836a61..289b120a58 100644 --- a/planner/values.go +++ b/planner/values.go @@ -77,39 +77,43 @@ func (n *valuesNode) Source() planNode { return nil } // SortAll actually sorts all the data within the docContainer object func (n *valuesNode) SortAll() { - sort.Sort(n) + sort.Stable(n) } -// Less implements the golang sort.Sort interface. -// It compares the values the ith and jth index -// within the docContainer. -// returns true if i < j. -// returns false if i > j. +// Less implements the golang sort.Interface. +// Less reports whether the elements within the docContainer at index i must sort before the element with index j. +// Returns true if docs[i] < docs[j]. +// Returns false if docs[i] >= docs[j]. +// If both Less(i, j) and Less(j, i) are false, then the elements at index i and j are considered equal. func (n *valuesNode) Less(i, j int) bool { da, db := n.docs.At(i), n.docs.At(j) return n.docValueLess(da, db) } -// docValueLess extracts and compare field values of a document -func (n *valuesNode) docValueLess(da, db core.Doc) bool { - var ra, rb any +// docValueLess extracts and compare field values of a document, returns true only if strictly less when ASC, +// and true if greater than or equal when DESC, otherwise returns false. +func (n *valuesNode) docValueLess(docA, docB core.Doc) bool { for _, order := range n.ordering { - if order.Direction == mapper.ASC { - ra = getDocProp(da, order.FieldIndexes) - rb = getDocProp(db, order.FieldIndexes) - } else if order.Direction == mapper.DESC { // redundant, just else - ra = getDocProp(db, order.FieldIndexes) - rb = getDocProp(da, order.FieldIndexes) - } - - if c := base.Compare(ra, rb); c < 0 { - return true - } else if c > 0 { - return false + compare := base.Compare( + getDocProp(docA, order.FieldIndexes), + getDocProp(docB, order.FieldIndexes), + ) + + if order.Direction == mapper.DESC { + if compare > 0 { + return true + } else { + return false + } + } else { // Otherwise assume order.Direction == mapper.ASC + if compare < 0 { + return true + } else { + return false + } } } - - return true + return false } // Swap implements the golang sort.Sort interface. diff --git a/tests/integration/query/commits/with_dockey_order_test.go b/tests/integration/query/commits/with_dockey_order_test.go index 71b14f0eb0..57057d001e 100644 --- a/tests/integration/query/commits/with_dockey_order_test.go +++ b/tests/integration/query/commits/with_dockey_order_test.go @@ -44,15 +44,15 @@ func TestQueryCommitsWithDockeyAndOrderHeightDesc(t *testing.T) { }, Results: []map[string]any{ { - "cid": "bafybeibz3vbkt75siz3zogke6tlzvpcxttpiy4xivjvgyeaorjz6wsbguq", + "cid": "bafybeiacqac6scm7pmtlvqptvtljmoroevnoedku42qi5bmfdpaelcu5fm", "height": int64(2), }, { - "cid": "bafybeiacqac6scm7pmtlvqptvtljmoroevnoedku42qi5bmfdpaelcu5fm", + "cid": "bafybeibz3vbkt75siz3zogke6tlzvpcxttpiy4xivjvgyeaorjz6wsbguq", "height": int64(2), }, { - "cid": "bafybeid5l577igkgcn6wjqjeqxlta4dcc3a3iykwkborf4fklaenjuctoq", + "cid": "bafybeigju7dgicfq3fxvtlxtjao7won4xc7kusykkvumngjfx5i2c7ibny", "height": int64(1), }, { @@ -60,7 +60,7 @@ func TestQueryCommitsWithDockeyAndOrderHeightDesc(t *testing.T) { "height": int64(1), }, { - "cid": "bafybeigju7dgicfq3fxvtlxtjao7won4xc7kusykkvumngjfx5i2c7ibny", + "cid": "bafybeid5l577igkgcn6wjqjeqxlta4dcc3a3iykwkborf4fklaenjuctoq", "height": int64(1), }, }, @@ -97,7 +97,7 @@ func TestQueryCommitsWithDockeyAndOrderHeightAsc(t *testing.T) { }, Results: []map[string]any{ { - "cid": "bafybeid5l577igkgcn6wjqjeqxlta4dcc3a3iykwkborf4fklaenjuctoq", + "cid": "bafybeigju7dgicfq3fxvtlxtjao7won4xc7kusykkvumngjfx5i2c7ibny", "height": int64(1), }, { @@ -105,15 +105,15 @@ func TestQueryCommitsWithDockeyAndOrderHeightAsc(t *testing.T) { "height": int64(1), }, { - "cid": "bafybeigju7dgicfq3fxvtlxtjao7won4xc7kusykkvumngjfx5i2c7ibny", + "cid": "bafybeid5l577igkgcn6wjqjeqxlta4dcc3a3iykwkborf4fklaenjuctoq", "height": int64(1), }, { - "cid": "bafybeibz3vbkt75siz3zogke6tlzvpcxttpiy4xivjvgyeaorjz6wsbguq", + "cid": "bafybeiacqac6scm7pmtlvqptvtljmoroevnoedku42qi5bmfdpaelcu5fm", "height": int64(2), }, { - "cid": "bafybeiacqac6scm7pmtlvqptvtljmoroevnoedku42qi5bmfdpaelcu5fm", + "cid": "bafybeibz3vbkt75siz3zogke6tlzvpcxttpiy4xivjvgyeaorjz6wsbguq", "height": int64(2), }, }, @@ -227,3 +227,78 @@ func TestQueryCommitsWithDockeyAndOrderCidAsc(t *testing.T) { executeTestCase(t, test) } + +func TestQueryCommitsWithDockeyAndOrderAndMultiUpdatesCidAsc(t *testing.T) { + test := testUtils.RequestTestCase{ + Description: "Simple all commits query with dockey, multiple updates with order cid asc", + Request: `query { + commits(dockey: "bae-52b9170d-b77a-5887-b877-cbdbb99b009f", order: {height: ASC}) { + cid + height + } + }`, + Docs: map[int][]string{ + 0: { + `{ + "Name": "John", + "Age": 21 + }`, + }, + }, + Updates: map[int]map[int][]string{ + 0: { + 0: { + `{ + "Age": 22 + }`, + `{ + "Age": 23 + }`, + `{ + "Age": 24 + }`, + }, + }, + }, + Results: []map[string]any{ + { + "cid": "bafybeigju7dgicfq3fxvtlxtjao7won4xc7kusykkvumngjfx5i2c7ibny", + "height": int64(1), + }, + { + "cid": "bafybeiaqarrcayyoly2gdiam6mhh72ls4azwa7brozxxc3q2srnggkkqkq", + "height": int64(1), + }, + { + "cid": "bafybeid5l577igkgcn6wjqjeqxlta4dcc3a3iykwkborf4fklaenjuctoq", + "height": int64(1), + }, + { + "cid": "bafybeiacqac6scm7pmtlvqptvtljmoroevnoedku42qi5bmfdpaelcu5fm", + "height": int64(2), + }, + { + "cid": "bafybeibz3vbkt75siz3zogke6tlzvpcxttpiy4xivjvgyeaorjz6wsbguq", + "height": int64(2), + }, + { + "cid": "bafybeiho5z6seahwxgbdyobylzyarrdschgzmood7rkdtp4qpd2uxebaxy", + "height": int64(3), + }, + { + "cid": "bafybeib6yxcmbg2gz5ss6d67u5mu6wcatfjtdp2rv44difyznp3rqlyu4m", + "height": int64(3), + }, + { + "cid": "bafybeihhlr6teynoy534b32kihulsmtdnnzp5woebvhubvofoxqx4g4lha", + "height": int64(4), + }, + { + "cid": "bafybeicgc6k47su6263dm3mv5j5gkv3bbt43nb66x6wjashjxpfm2ds5nu", + "height": int64(4), + }, + }, + } + + executeTestCase(t, test) +} diff --git a/tests/integration/query/one_to_many_to_one/with_order_limit_test.go b/tests/integration/query/one_to_many_to_one/with_order_limit_test.go index 056c83348f..5f56941a3b 100644 --- a/tests/integration/query/one_to_many_to_one/with_order_limit_test.go +++ b/tests/integration/query/one_to_many_to_one/with_order_limit_test.go @@ -16,10 +16,10 @@ import ( testUtils "github.com/sourcenetwork/defradb/tests/integration" ) -// TODO: Fix this panic in #833. func TestOneToManyToOneDeepOrderBySubTypeOfBothDescAndAsc(t *testing.T) { test := testUtils.RequestTestCase{ Description: "1-N-1 deep orderby subtypes of both descending and ascending.", + Request: `query { Author { name @@ -129,8 +129,41 @@ func TestOneToManyToOneDeepOrderBySubTypeOfBothDescAndAsc(t *testing.T) { }`, }, }, - Results: []map[string]any{}, + + Results: []map[string]any{ + { + "name": "John Grisham", + "NewestPublishersBook": []map[string]any{ + { + "name": "Theif Lord", + }, + }, + "OldestPublishersBook": []map[string]any{ + { + "name": "The Associate", // oldest because has no publisher. + }, + }, + }, + { + "name": "Not a Writer", + "NewestPublishersBook": []map[string]any{}, + "OldestPublishersBook": []map[string]any{}, + }, + { + "name": "Cornelia Funke", + "NewestPublishersBook": []map[string]any{ + { + "name": "The Rooster Bar", + }, + }, + "OldestPublishersBook": []map[string]any{ + { + "name": "The Rooster Bar", + }, + }, + }, + }, } - testUtils.AssertPanicAndSkipChangeDetection(t, func() { executeTestCase(t, test) }) + executeTestCase(t, test) } diff --git a/tests/integration/query/one_to_many_to_one/with_order_test.go b/tests/integration/query/one_to_many_to_one/with_order_test.go index 741efe1686..31cd3c9da9 100644 --- a/tests/integration/query/one_to_many_to_one/with_order_test.go +++ b/tests/integration/query/one_to_many_to_one/with_order_test.go @@ -299,19 +299,6 @@ func TestMultipleOrderByWithDepthGreaterThanOneOrderSwitched(t *testing.T) { "yearOpened": uint64(2022), }, }, - { - "name": "Sooley", - "rating": 3.2, - "publisher": map[string]any{ - "name": "Only Publisher of Sooley", - "yearOpened": uint64(1999), - }, - }, - { - "name": "The Associate", - "rating": 4.2, - "publisher": nil, - }, { "name": "Theif Lord", "rating": 4.8, @@ -328,6 +315,14 @@ func TestMultipleOrderByWithDepthGreaterThanOneOrderSwitched(t *testing.T) { "yearOpened": uint64(2013), }, }, + { + "name": "Sooley", + "rating": 3.2, + "publisher": map[string]any{ + "name": "Only Publisher of Sooley", + "yearOpened": uint64(1999), + }, + }, { "name": "Painted House", "rating": 4.9, @@ -336,6 +331,11 @@ func TestMultipleOrderByWithDepthGreaterThanOneOrderSwitched(t *testing.T) { "yearOpened": uint64(1995), }, }, + { + "name": "The Associate", + "rating": 4.2, + "publisher": nil, + }, }, } diff --git a/tests/integration/query/one_to_many_to_one/with_sum_order_limit_test.go b/tests/integration/query/one_to_many_to_one/with_sum_order_limit_test.go index f07f3d7eac..288d0335a8 100644 --- a/tests/integration/query/one_to_many_to_one/with_sum_order_limit_test.go +++ b/tests/integration/query/one_to_many_to_one/with_sum_order_limit_test.go @@ -159,10 +159,10 @@ func TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeDescDirec executeTestCase(t, test) } -// TODO: Fix this panic in #833. func TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirections(t *testing.T) { test := testUtils.RequestTestCase{ Description: "1-N-1 sum of deep orderby subtypes and non-sum deep orderby, asc. directions.", + Request: `query { Author { name @@ -270,16 +270,21 @@ func TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirect }`, }, }, + Results: []map[string]any{ { "name": "John Grisham", - "s1": 4.9 + 3.2, // Because in ascending order years for John are [1995, 1999]. + + // Because in ascending order years for John are: + // 'The Associate' as it has no publisher (4.2 rating), then 'Painted House' 1995 (4.9 rating). + "s1": float64(4.2) + float64(4.9), + "NewestPublishersBook": []map[string]any{ { - "name": "Painted House", + "name": "The Associate", }, { - "name": "Sooley", + "name": "Painted House", }, }, }, @@ -300,13 +305,14 @@ func TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirect }, } - testUtils.AssertPanicAndSkipChangeDetection(t, func() { executeTestCase(t, test) }) + executeTestCase(t, test) } -// TODO: Fix this panic in #833. +// TODO: Fix this panic in #833 and #920. func TestOneToManyToOneWithSumOfDeepOrderBySubTypeOfBothDescAndAsc(t *testing.T) { test := testUtils.RequestTestCase{ Description: "1-N-1 sums of deep orderby subtypes of both descending and ascending.", + Request: `query { Author { name @@ -418,7 +424,7 @@ func TestOneToManyToOneWithSumOfDeepOrderBySubTypeOfBothDescAndAsc(t *testing.T) testUtils.AssertPanicAndSkipChangeDetection(t, func() { executeTestCase(t, test) }) } -// TODO: Fix this panic in #833. +// TODO: Fix this panic in #833 and #920. func TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeOppositeDirections(t *testing.T) { test := testUtils.RequestTestCase{ Description: "1-N-1 sum of deep orderby subtypes and non-sum deep orderby, opposite directions.",