query: positive first pushdown is unsound for allof, lossy-index eq, and geo functions
Nobody has claimed this yet.
- 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
firstpushdown for lossyeq, 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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