Skip to content

Commit

Permalink
fix(sort): Only filter out nodes with positive offsets. (#8077) (#8441)
Browse files Browse the repository at this point in the history
Cherry pick from #8077.

Steps to reproduce:
```
make image-local
cd contrib/local-test && make up
make schema-dql
make load-data-dql-rdf
make query-dql
curl: (52) Empty reply from server
make: *** [query-dql] Error 52
<alpha will crash and restart>
```

with the following minimal schema / data / query (should be placed in
contrib/local-test):

schema.dql:
```
type Person {
    name
    age
    friend
}

name: string @Index(term)  .
age: int @Index(int) .
friend: [uid] @count .
```

dql-data.rdf
```
{
  set {
    _:michael <name> "Michael" .
    _:michael <dgraph.type> "Person" .
    _:michael <age> "314" .
    _:michael <friend> _:alice .
    _:michael <friend> _:bob .
    _:michael <friend> _:charlie .

    _:alice <name> "Alice" .
    _:alice <dgraph.type> "Person" .
    _:alice <age> "1" .

    _:bob <name> "Bob" .
    _:bob <dgraph.type> "Person" .
    _:bob <age> "2" .

    _:charlie <name> "Charlie" .
    _:charlie <dgraph.type> "Person" .
  }
}
```

query.dql
```
{
  michael_friends_first(func: allofterms(name, "Michael")) {
    name
    age
    friend (orderasc: age, offset: -1) {
      name
      age
    }
  }
}
```

### Remarks

- The cherry-pick does resolve the above crash
- This issue exists in main
- Why do we allow negative offsets? If they are not useful, we should
return an error when a user attempt to query using a negative offset.

Co-authored-by: Daniel Mai <[email protected]>
  • Loading branch information
joshua-goldstein and danielmai authored Dec 2, 2022
1 parent 5ac56a1 commit 19febad
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
13 changes: 13 additions & 0 deletions query/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ type Person {
alive
}
type Person2 {
name2
age2
}
type Animal {
name
}
Expand Down Expand Up @@ -332,6 +337,8 @@ tweet-a : string @index(trigram) .
tweet-b : string @index(term) .
tweet-c : string @index(fulltext) .
tweet-d : string @index(trigram) .
name2 : string @index(term) .
age2 : int @index(int) .
`

func populateCluster() {
Expand Down Expand Up @@ -629,6 +636,7 @@ func populateCluster() {
<5> <dgraph.type> "Pet" .
<6> <dgraph.type> "Animal" .
<6> <dgraph.type> "Pet" .
<23> <dgraph.type> "Person" .
<24> <dgraph.type> "Person" .
<25> <dgraph.type> "Person" .
Expand All @@ -638,6 +646,8 @@ func populateCluster() {
<34> <dgraph.type> "SchoolInfo" .
<35> <dgraph.type> "SchoolInfo" .
<36> <dgraph.type> "SchoolInfo" .
<40> <dgraph.type> "Person2" .
<41> <dgraph.type> "Person2" .
<11100> <dgraph.type> "Node" .
<2> <pet> <5> .
Expand Down Expand Up @@ -851,6 +861,9 @@ func populateCluster() {
<61> <tweet-d> "aaabxxx" .
<62> <tweet-d> "aaacdxx" .
<63> <tweet-d> "aaabcd" .
<40> <name2> "Alice" .
<41> <age2> "20" .
`)
if err != nil {
panic(fmt.Sprintf("Could not able add triple to the cluster. Got error %v", err.Error()))
Expand Down
14 changes: 14 additions & 0 deletions query/query0_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,20 @@ func TestCascadeWithSort(t *testing.T) {
require.JSONEq(t, `{"data":{"me":[{"name": "Daryl Dixon","alive": false},{"name": "Rick Grimes","alive": true}]}}`, js)
}

// Regression test for issue described in https://github.com/dgraph-io/dgraph/pull/8441
func TestNegativeOffset(t *testing.T) {
query := `
{
me(func: type(Person2), offset: -1, orderasc: age2) {
name2
age2
}
}
`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data":{"me":[{"age2":20},{"name2":"Alice"}]}}`, js)
}

func TestLevelBasedFacetVarAggSum(t *testing.T) {
query := `
{
Expand Down
4 changes: 3 additions & 1 deletion worker/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,9 @@ BUCKETS:

// Apply the offset on null nodes, if the nodes with value were not enough.
if out[i].offset < len(nullNodes) {
nullNodes = nullNodes[out[i].offset:]
if out[i].offset >= 0 {
nullNodes = nullNodes[out[i].offset:]
}
} else {
nullNodes = nullNodes[:0]
}
Expand Down

0 comments on commit 19febad

Please sign in to comment.