Fix has pagination when predicate is queried with @lang#4331
Conversation
animesh2049
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @golangcibot, @mangalaman93, @manishrjain, and @pawanrawal)
pawanrawal
left a comment
There was a problem hiding this comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @ashish-goswami, @golangcibot, @mangalaman93, and @manishrjain)
query/query4_test.go, line 602 at r2 (raw file):
// func TestHasFirstLangPredicate(t *testing.T) { // query := `{ // q(func:has(name@lang), orderasc: name, first:5) {
This won't work but a test case which is just calling the has function on a predicate with a lang and first should work? Could we add that please?
worker/task.go, line 2096 at r2 (raw file):
lang := langForFunc(q.Langs) needFiltering := needsStringFiltering(srcFn, q.Langs, q.Attr) checkInclusion := func(uid uint64) error {
Add some comments that what we are fetching the value for the uid + predicate@lang here before deciding if we want to include the uid in the result.
worker/task.go, line 2101 at r2 (raw file):
} _, err := qs.getValsForUID(q.Attr, lang, uid, q.ReadTs)
Since this would only be called if there is a language, we don't need the extra logic in getValsForUID corresponding to lang == ""
ashish-goswami
left a comment
There was a problem hiding this comment.
Dismissed @golangcibot from a discussion.
Reviewable status: 1 of 2 files reviewed, 3 unresolved discussions (waiting on @animesh2049, @lang, @mangalaman93, @manishrjain, and @pawanrawal)
query/query4_test.go, line 602 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
This won't work but a test case which is just calling the
hasfunction on a predicate with a lang and first should work? Could we add that please?
First might return result in any order. Hence I won't be able to compare it. I have added test for count though.
worker/task.go, line 2096 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Add some comments that what we are fetching the value for the uid +
predicate@langhere before deciding if we want to include the uid in the result.
Done.
worker/task.go, line 2101 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Since this would only be called if there is a language, we don't need the extra logic in getValsForUID corresponding to
lang == ""
We need it in case predicate supports @lang. When lang is "", we need to get all uids which does not have @lang(there might be no posting for this). We predicate is list, this function won't be called from
handleHasFunction.
pawanrawal
left a comment
There was a problem hiding this comment.
Reviewed 1 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ashish-goswami, @mangalaman93, and @manishrjain)
query/query4_test.go, line 633 at r3 (raw file):
func TestHasCountPredicateWithLang(t *testing.T) { query := `{ q(func:has(name@en), first: 5) {
Could we use a higher number here please which is equal to number of nodes which have name@en?
ashish-goswami
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @animesh2049, @mangalaman93, @manishrjain, and @pawanrawal)
query/query4_test.go, line 633 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Could we use a higher number here please which is equal to number of nodes which have
name@en?
Done.
manishrjain
left a comment
There was a problem hiding this comment.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @animesh2049, @mangalaman93, @manishrjain, and @pawanrawal)
Fixes #4282
This change is