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.

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

cherry-pick v20.11: fix(query): Fix pagination with match functions (#7668) #7672

Merged
merged 1 commit into from
Mar 31, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix(query): Fix pagination with match functions (#7668)
Fix queries involving `regexp`, `allofterms`, `alloftext` and `match`
function with pagination. These functions relies on indexes and
needs to fetch postings/uids for each relevant index key to generate
the final result. Remove early pagination for these cases.

(cherry picked from commit e00ed67)
  • Loading branch information
ahsanbarkati committed Mar 31, 2021

Verified

This commit was signed with the committer’s verified signature.
nikukyugamer Osamu Takiya
commit b163c969ca9b82288196c440b823ff52c624cd9a
23 changes: 23 additions & 0 deletions query/common_test.go
Original file line number Diff line number Diff line change
@@ -328,6 +328,10 @@ gender : string .
indexpred : string @index(exact) .
pred : string .
pname : string .
tweet-a : string @index(trigram) .
tweet-b : string @index(term) .
tweet-c : string @index(fulltext) .
tweet-d : string @index(trigram) .
`

func populateCluster() {
@@ -828,6 +832,25 @@ func populateCluster() {
<67> <index-pred2> "I" .
<68> <index-pred2> "J" .
<69> <index-pred2> "K" .

<61> <tweet-a> "aaa" .
<62> <tweet-a> "aaaa" .
<63> <tweet-a> "aaaab" .
<64> <tweet-a> "aaaabb" .

<61> <tweet-b> "indiana" .
<62> <tweet-b> "indiana" .
<63> <tweet-b> "indiana jones" .
<64> <tweet-b> "indiana pop" .

<61> <tweet-c> "I am a citizen" .
<62> <tweet-c> "I am a citizen" .
<63> <tweet-c> "I am a citizen" .
<64> <tweet-c> "I am a citizen of Paradis Island" .

<61> <tweet-d> "aaabxxx" .
<62> <tweet-d> "aaacdxx" .
<63> <tweet-d> "aaabcd" .
`)
if err != nil {
panic(fmt.Sprintf("Could not able add triple to the cluster. Got error %v", err.Error()))
27 changes: 15 additions & 12 deletions query/query.go
Original file line number Diff line number Diff line change
@@ -943,19 +943,22 @@ func calculateFirstN(sg *SubGraph) int32 {
// name
// }
// }
// - should be has function (Right now, I'm doing it for has, later it can be extended)
// {
// q(func: has(name), first:1) {
// name
// }
// }
// isSupportedFunction := sg.SrcFunc != nil && sg.SrcFunc.Name == "has"
// - should not be one of those function which fetches some results and then do further
// processing to narrow down the result. For example: allofterm will fetch the index postings
// for each term and then do an intersection.
// TODO: Look into how we can optimize queries involving these functions.

shouldExclude := false
if sg.SrcFunc != nil {
switch sg.SrcFunc.Name {
case "regexp", "alloftext", "allofterms", "match":
shouldExclude = true
default:
shouldExclude = false
}
}

// Manish: Shouldn't all functions allow this? If we don't have a order and we don't have a
// filter, then we can respect the first N, offset Y arguments when retrieving data.
isSupportedFunction := true
if len(sg.Filters) == 0 && len(sg.Params.Order) == 0 &&
isSupportedFunction {
if len(sg.Filters) == 0 && len(sg.Params.Order) == 0 && !shouldExclude {
// Offset also added because, we need n results to trim the offset.
if sg.Params.Count != 0 {
count = sg.Params.Count + sg.Params.Offset
52 changes: 52 additions & 0 deletions query/query0_test.go
Original file line number Diff line number Diff line change
@@ -3386,6 +3386,58 @@ func TestEqFilterWithoutIndex(t *testing.T) {

}

func TestMatchingWithPagination(t *testing.T) {
tests := []struct {
name string
query string
expected string
}{
{
`Test regexp matching with pagination`,
`{
me(func: regexp(tweet-a, /aaa.b/), first:1){
tweet-a
}
}`,
`{"data":{"me":[{"tweet-a":"aaaab"}]}}`,
},
{
`Test term matching with pagination`,
`{
me(func: allofterms(tweet-b, "indiana jones"), first:1){
tweet-b
}
}`,
`{"data":{"me":[{"tweet-b":"indiana jones"}]}}`,
},
{
`Test full-text matching with pagination`,
`{
me(func: alloftext(tweet-c, "I am a citizen of Paradis Island"), first:1){
tweet-c
}
}`,
`{"data":{"me":[{"tweet-c":"I am a citizen of Paradis Island"}]}}`,
},
{
`Test match function with pagination`,
`{
me(func: match(tweet-d, "aaaaaa", 3), first:1) {
tweet-d
}
}`,
`{"data":{"me":[{"tweet-d":"aaabcd"}]}}`,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := processQueryNoErr(t, tc.query)
require.JSONEq(t, tc.expected, result)
})
}
}

var client *dgo.Dgraph

func TestMain(m *testing.M) {