dgraph-io / dgraph-io/dgraph

query: positive first pushdown is unsound for allof, lossy-index eq, and geo functions

Open
#9,830 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area/querylang kind/bug
Dominant language
Go
Stars
21.8k
Forks
1.6k
Avg merge
2d 5h
Merged PRs (30d)
9

Description

What

calculatePaginationParams pushes a positive first down to the worker for functions whose results are post-filtered or intersected after posting.(*List).Uids has already truncated the posting list. The truncation drops uids the later pass never sees, so the query can return fewer rows than it should, or none.

The exclusion list is incomplete

query/query.go, calculatePaginationParams:

switch sg.SrcFunc.Name {
case "regexp", "alloftext", "allofterms", "match", "ngram":
    shouldExclude = true

Those five are correctly kept off the pushdown. Three families are not:

Function Why the pushdown is unsound
allof (custom index) needsIntersect matches on the allof prefix/suffix, so the query layer runs algo.IntersectSorted over the per-token lists. head(A) ∩ head(B) ≠ head(A ∩ B).
eq / inequalities over a lossy-only index handleCompareFunction re-fetches the real values and drops non-matches when tokenizer.IsLossy(). A term index with no exact alongside it takes this path.
geo (near, within, contains, intersects) filterGeoFunction re-checks the real geometry over MergeSorted(arg.out.UidMatrix).

The ordering is what makes it wrong. In helpProcessTask, handleUidPostings (which calls pl.Uids(opts)) runs first; handleCompareFunction and filterGeoFunction run afterwards. x.PageRange at the query layer cannot restore a uid the worker already discarded.

Union functions are fine: anyofterms merges rather than intersects, and head_N(A) ∪ head_N(B) contains head_N(A ∪ B), so taking the first N of the merged result is still correct.

Reproduction shape
name: string @index(term) .
0x1,0x2 name "great wall"    0x3,0x4 name "great"

q(func: eq(name, "great"), first: 2)

The bucket for token "great" is {1,2,3,4}. Pushed down, Uids returns {1,2}, handleCompareFunction drops both, and the query answers nothing instead of 0x3, 0x4.

Confirmed reachable, not hypothetical: TestPaginationPushdownExcludesIntersectingFunctions (added in #9809) asserts the five excluded names return math.MaxInt32, and the same harness shows allof, anyofterms, eq and has all receive a pushed-down count.

Why the negative case is already fixed and this one is not

#9809 removed the negative-first trim from Uids entirely, which fixes this for first: -N across all three families, because a negative first never had a paired early stop and so the trim bought nothing.

The positive trim cannot be removed the same way: it is paired with the early stop in the iterate loop, so it stops the read rather than just truncating the result, and that saving is the point. The fix belongs at the planner — adding these functions to the exclusion list, which gives up the pushdown for them — not in Uids.

Notes

Pre-existing, and independent of #9809; that PR only made the negative half correct. Discussed with the PR author, who confirmed the positive half touches enough query-planning and post-filtering paths to want its own change:

I took a look at fixing positive first pushdown for lossy eq, but it touches a broader set of query-planning and post-filtering paths than expected, so I agree that it should be handled separately.

There is no test anywhere for x.PageRange, and no test for allof or geo with a first argument, so nothing currently guards this.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in query/query.go at calculatePaginationParams, then trace helpProcessTask through handleUidPostings, handleCompareFunction, and filterGeoFunction. Run TestPaginationPushdownExcludesIntersectingFunctions and inspect the existing pagination harness. Done means positive first pushdown is excluded for allof, lossy-index comparisons, and geo functions, with regression coverage for the affected cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.