Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(sort): Only filter out nodes with positive offsets. #8077

Merged
merged 3 commits into from
Oct 18, 2021
Merged

fix(sort): Only filter out nodes with positive offsets. #8077

merged 3 commits into from
Oct 18, 2021

Conversation

danielmai
Copy link
Contributor

@danielmai danielmai commented Oct 18, 2021 •
edited
Loading

Negative offsets (e.g., offset: -4) can cause panics when sorting. This can happen when the query has the following characteristics:

  1. The query is sorting on an indexed predicate
  2. The results include nodes that also don't have the sorted predicate
  3. A negative offset is used.
panic: runtime error: slice bounds out of range [-4:]

goroutine 1762633 [running]:
github.com/dgraph-io/dgraph/worker.sortWithIndex(0x1fb12e0, 0xc00906a880, 0xc009068660, 0x0)
        /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:330 +0x244d
github.com/dgraph-io/dgraph/worker.processSort.func2(0x1fb12e0, 0xc00906a880, 0xc009068660, 0xc0090686c0)
        /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:515 +0x3f
created by github.com/dgraph-io/dgraph/worker.processSort
        /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:514 +0x52a

This change is Reviewable

This was referenced Oct 18, 2021
Closed
Merged
Closed
Merged
Closed
Closed
@danielmai danielmai merged commit 74d833c into master Oct 18, 2021
@danielmai danielmai deleted the danielmai/fix-sort-offset branch October 18, 2021 21:05
danielmai added a commit that referenced this pull request Oct 18, 2021
@danielmai
fix(sort): Only filter out nodes with positive offsets. (#8079)
6e520b1 
Cherry-picked from #8077.

Negative offsets (e.g., offset: -4) can cause panics when sorting. This can happen when the query has the following characteristics:

1. The query is sorting on an indexed predicate
2. The results include nodes that also don't have the sorted predicate
3. A negative offset is used.

    panic: runtime error: slice bounds out of range [-4:]
    goroutine 1762633 [running]:
    github.com/dgraph-io/dgraph/worker.sortWithIndex(0x1fb12e0, 0xc00906a880, 0xc009068660, 0x0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:330 +0x244d
    github.com/dgraph-io/dgraph/worker.processSort.func2(0x1fb12e0, 0xc00906a880, 0xc009068660, 0xc0090686c0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:515 +0x3f
    created by github.com/dgraph-io/dgraph/worker.processSort
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:514 +0x52a
danielmai added a commit that referenced this pull request Oct 18, 2021
@danielmai
fix(sort): Only filter out nodes with positive offsets. (#8081)
f9add83 
Cherry-picked from #8077

Negative offsets (e.g., offset: -4) can cause panics when sorting. This can happen when the query has the following characteristics:

1. The query is sorting on an indexed predicate
2. The results include nodes that also don't have the sorted predicate
3. A negative offset is used.

    (panic trace is from v20.11.2-rc1-23-gaf5030a5)
    panic: runtime error: slice bounds out of range [-4:]
    goroutine 1762633 [running]:
    github.com/dgraph-io/dgraph/worker.sortWithIndex(0x1fb12e0, 0xc00906a880, 0xc009068660, 0x0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:330 +0x244d
    github.com/dgraph-io/dgraph/worker.processSort.func2(0x1fb12e0, 0xc00906a880, 0xc009068660, 0xc0090686c0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:515 +0x3f
    created by github.com/dgraph-io/dgraph/worker.processSort
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:514 +0x52a
joshua-goldstein pushed a commit that referenced this pull request Nov 19, 2022
@danielmai @joshua-goldstein
fix(sort): Only filter out nodes with positive offsets. (#8077)
59aea0d 
Negative offsets (e.g., offset: -4) can cause panics when sorting. This can happen when the query has the following characteristics:

1. The query is sorting on an indexed predicate
2. The results include nodes that also don't have the sorted predicate
3. A negative offset is used.

    (panic trace is from v20.11.2-rc1-23-gaf5030a5)
    panic: runtime error: slice bounds out of range [-4:]
    goroutine 1762633 [running]:
    github.com/dgraph-io/dgraph/worker.sortWithIndex(0x1fb12e0, 0xc00906a880, 0xc009068660, 0x0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:330 +0x244d
    github.com/dgraph-io/dgraph/worker.processSort.func2(0x1fb12e0, 0xc00906a880, 0xc009068660, 0xc0090686c0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:515 +0x3f
    created by github.com/dgraph-io/dgraph/worker.processSort
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:514 +0x52a

(cherry picked from commit 74d833c)
@joshua-goldstein joshua-goldstein mentioned this pull request Nov 19, 2022
Merged
joshua-goldstein pushed a commit that referenced this pull request Nov 30, 2022
@danielmai @joshua-goldstein
fix(sort): Only filter out nodes with positive offsets. (#8077)
f2a5602 
Negative offsets (e.g., offset: -4) can cause panics when sorting. This can happen when the query has the following characteristics:

1. The query is sorting on an indexed predicate
2. The results include nodes that also don't have the sorted predicate
3. A negative offset is used.

    (panic trace is from v20.11.2-rc1-23-gaf5030a5)
    panic: runtime error: slice bounds out of range [-4:]
    goroutine 1762633 [running]:
    github.com/dgraph-io/dgraph/worker.sortWithIndex(0x1fb12e0, 0xc00906a880, 0xc009068660, 0x0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:330 +0x244d
    github.com/dgraph-io/dgraph/worker.processSort.func2(0x1fb12e0, 0xc00906a880, 0xc009068660, 0xc0090686c0)
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:515 +0x3f
    created by github.com/dgraph-io/dgraph/worker.processSort
            /ext-go/1/src/github.com/dgraph-io/dgraph/worker/sort.go:514 +0x52a

(cherry picked from commit 74d833c)
all-seeing-code pushed a commit that referenced this pull request Dec 2, 2022
@joshua-goldstein @danielmai
fix(sort): Only filter out nodes with positive offsets. (#8077) (#8441)
19febad 
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manishrjain manishrjain Awaiting requested review from manishrjain

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@danielmai