fix(query): avoid eager UID materialization on posting reads

[gooohgb]

Description

Related @matthewmcneely : #9807

This PR avoids eagerly materializing the full UID slice for posting lists on read paths where that work is not useful or actively bypasses existing optimizations.

Changes:

• Gate calculateUids() behind the posting-list cache being enabled, so --cache percentage=0,... does not build and immediately discard a full []uint64.
• Avoid using calculatedUids for bounded Uids() reads with First or Intersect, preserving early-stop and compressed-intersection paths.
• Reduce Uids() allocation size for bounded and small-intersect reads.
• Avoid GetUids() in worker paths that do not consume a full UID list, including count, scalar comparison, has, uid_in, facets, pagination, and intersect paths.

Checklist

• The PR title follows the
  Conventional Commits syntax, leading
  with fix:, feat:, chore:, ci:, etc.
• Code compiles correctly and linting (via trunk) passes locally
• Tests added for new functionality, or regression tests for bug fixes added as applicable

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[gooohgb]

Hi, @matthewmcneely , could you please help review this PR? It’s affecting our production memory monitoring metrics and causing alerts. We’d really appreciate it if this could be reviewed and fixed as soon as possible. Thanks!

[matthewmcneely]

requestedFirst > 0 won't distinguish bounded from unbounded here. The worker's sentinel for "no limit" is math.MaxInt32, not 0:

• calculatePaginationParams in query/query.go starts with count := math.MaxInt32 and returns it whenever there's a filter, an order, an excluded func, or Params.Count == 0 (the common case).
• worker/task.go builds opts with First: int(q.First + q.Offset), so a plain read arrives here with opt.First == math.MaxInt32.

So this branch is skipped for nearly every query that reaches handleUidPostings, and the calculatedUids work from #9795/#9801 stops being used at all. Note the normalization six lines up (if opt.First == 0 { opt.First = math.MaxInt32 }) asserts exactly the equivalence this gate contradicts.

Suggest:

bounded := requestedFirst > 0 && requestedFirst < math.MaxInt32
if opt.Intersect == nil && !bounded && l.canUseCalculatedUids(opt.ReadTs) {

Separately: with opt.Intersect == nil now required to enter this branch, the return out, nil, opt.Intersect != nil at the bottom of it is always false. Worth returning false literally so the next reader doesn't have to prove the dead branch.

[matthewmcneely]

Same sentinel problem as the gate in Uids(): opts.First is int(q.First + q.Offset) and q.First defaults to math.MaxInt32 for an unbounded query, so opts.First > 0 is true almost always and this returns false for the common case.

Corroborating evidence that MaxInt32 really is the default rather than 0: handleHasFunction further down uses len(result.Uids) >= int(q.First) as its stop condition, which would fire immediately if q.First were ever 0 on an unbounded read.

if opts.Intersect != nil || (opts.First > 0 && opts.First < math.MaxInt32) {
	return false
}

[matthewmcneely]

The hasCache() gate itself is right. The interaction with the worker change is what concerns me: calculateUids() only ever runs on this disk path, never on the readFromCache hit path, and MutableLayer.clone() carries isUidsCalculated/calculatedUids through verbatim. So whichever query touches a key first decides whether the cached entry is warm:

1. q(func: eq(name,"x"), first: 10) now takes qs.cache.Get(key) → cache miss → readFromDisk(readUids=false) → saveInCache stores a list with isUidsCalculated == false.
2. Any later unbounded read of that same key → GetUids → cache hit → copy without calculated UIDs → full iterate().

The entry stays cold until maxTs < readTs forces a re-read from disk. Before this PR handleUidPostings always called GetUids, so it was always warm. Either calculate on the cache-hit path in ReadData when readUids is set, or accept it with a comment saying so, but it shouldn't be load-order-dependent by accident.

[matthewmcneely]

This cap works against the goal on two of the three paths below.

In the small-intersect branch, res accumulates every matching UID and the First truncation doesn't happen until the tail of Uids(), so capping at requestedFirst forces repeated regrowth whenever the intersection is larger than First.

In the iterate branch the stop check is len(res) > opt.First after the append, so res always reaches First+1 and reallocs exactly once. The new test encodes that: require.LessOrEqual(t, cap(first.Uids), 2) for First: 1 passes because the slice already doubled from 1 to 2.

Apply the cap only on the iterate path, and use requestedFirst + 1.

[matthewmcneely]

Pre-existing, but you're rewriting the line so it's cheap to close out: ApproxLen() takes l.RLock(), and we already hold RLock from the defer l.RUnlock() a few lines up. Recursive RLock on sync.RWMutex deadlocks if a writer arrives between the two acquisitions. The diff already collapses two calls into one; an unlocked approxLen() helper (asserting AssertRLock) would finish the job.

[matthewmcneely]

This is loop-invariant (q, srcFn, facetsTree, and opts are all fixed for the call), but it's evaluated once per UID inside the inner loop across numGo goroutines. Hoist it above calculate and capture the bool.

[matthewmcneely]

This assertion is passing because the slice regrew from cap 1 to cap 2, which is the allocation behavior flagged on the resCap line rather than the intended outcome.

More importantly, this test only covers the skip cases. The gap that lets the math.MaxInt32 bug through is the missing positive case: an unbounded read (First: 0, no Intersect) should still consume calculatedUids, and there should be a sibling case for First: math.MaxInt32 asserting the same thing, since that's what the worker actually sends. The existing sentinel trick (l.mutationMap.calculatedUids = []uint64{100, 101}) inverts nicely for both.