-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
fix(sort): Only filter out nodes with positive offsets. (#8077) #8441
Conversation
|
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)
59aea0d
to
f2a5602
Compare
query/query0_test.go
Outdated
@@ -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) | |||
} | |||
|
|||
// Resolved in PR #8441 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other tests in the past have given a full link to the issue, i.e.
// Regression test for https://github.com/dgraph-io/dgraph/issues/3657.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving this Joshua. Please go ahead and merge
Note this fixes errors reported as |
Cherry pick from #8077.
Steps to reproduce:
with the following minimal schema / data / query (should be placed in contrib/local-test):
schema.dql:
dql-data.rdf
query.dql
Remarks