fix(server): make query result batch boundaries explicit

[contrueCT]

Purpose of the PR

Closes #3190.

QueryResults currently detects a query boundary by probing hasNext() and then checking whether the active query changed. That probe can activate the next index query or backend page before the current batch finishes processing. Ordering is also selected from the first active segment, so an unsorted batch [1] followed by a batch with input IDs [3, 2] can incorrectly produce [1, 2, 3].

This change gives each batch its own results and captured processing context. Parsing, TTL checks, residual filtering and input-order restoration finish within that batch before the caller consumes a flattened stream.

Main Changes

• Introduce QueryBatch and QueryResultContext to carry the input IDs, ordering decision, visibility flags and effective results filter with the results that produced them.
• Make QueryResults, QueryList and PageEntryIterator compose explicit batches. Remove queryVersion/currentQueries boundary inference and shared results-filter propagation.
• Apply vertex and edge processing within each batch, preserving public/internal visibility, diagnostics and index-cleanup ownership. Keep native scans on their original single-batch path so backend page tokens and capacity checks remain intact.
• Cache candidates before residual matching and reapply the current batch context on hits. Removing the old post-filter cache exclusion intentionally makes more query shapes cacheable, including indexed edge lookups. Cache capacity limits remain unchanged, but workloads with many distinct predicate values may increase cache-key churn; no cache-performance improvement is claimed. Preserve mixed hit/miss input ordering, the original edge-cache request key, off-heap serialization, TASK/SERVER query types and the RamTable fast path.
• Share the remaining batch cursor between element iteration, mapping and materialization, preserving prefetched results and page metadata. Keep queries() diagnostics bounded to the current batch and clear them on close.
• Preserve the existing holder boundary when a backend page has no raw records. Keep following the cursor when an existing batch is emptied by parsing or TTL filtering, and retain negative edge-cache entries. This avoids scanning stale label-index pages during HStore schema cleanup.
• Bypass the vertex cache for paged backend queries so primary-key optimization retains PageState. Give each empty page an independent batch cursor, preventing one consumer from exhausting another consumer's cursor.
• Synchronize reflection filtering with the renamed transaction helpers and the cache overrides. Add lifecycle, edge-cache boundary, and index-cleanup side-effect regressions through the actual query-processing stages.
• Map and filter elements without creating a singleton collection/iterator per element. Skip the TTL wrapper for TTL-capable stores, copy edge-cache group indexes only on writes, and use an empty batch label when the batch key equals the outer cache key. Distinct batch keys remain explicit and retain the existing candidate/batch limits.
• Make batch wrappers close idempotently and preserve the primary exception when cleanup also fails. Native iterators that close themselves still rely on their existing backend close contract.

The flattened consumer can advance after A is exhausted; an iterator operating inside A never probes B to discover A's boundary. HStore partition merging, ORDER_BY_KEY and physical-key cursors are outside this change.

Multi-branch edge queries activate branches lazily. Errors raised while preparing a branch (including query optimization and index preparation) or starting its backend read can therefore surface from hasNext()/next() rather than the initial query call; earlier branches may already have yielded results. Validation reached while flattening the request still runs immediately, and the single-branch path retains immediate branch activation. This avoids preparing sibling branches before the current batch has finished.

Verifying these changes

• Trivial rework / code cleanup without any test coverage. (No Need)
• Already covered by existing tests, including VertexCoreTest, EdgeCoreTest and cache regressions.
• Need tests and can be verified as follows:

Three focused tests first failed on the old implementation: mixed ordering returned [1, 2, 3] instead of [1, 3, 2], and the next-query and next-page fetch counters reached 2 when only 1 batch should have been activated. All now pass. Additional regressions fail on the previous PR head for materialization/mapping after hasNext() and duplicate/accumulating query diagnostics; they pass with the shared cursor. The paging regression traverses 10,000 real PageEntryIterator pages and checks that only the current query is retained.

Verification on JDK 17 and Maven 3.9.16:

Scope	Snapshot	Selected	Skipped	Failures / errors
Allocation/cache follow-up: seven focused unit-test classes	9ad4a27e	80	0	0 / 0
Review cleanup: QueryResultsTest, QueryListTest, GraphTransactionTest	cf403002	37	0	0 / 0
Seven focused unit-test classes	05186a0e	78	0	0 / 0
RocksDB: primary-key/paging cases and QueryListTest	17788a4a	23	0	0 / 0
HStore: primary-key paging with cold/warm cache, filtered-empty and missing results	17788a4a	1	0	0 / 0
Memory: VertexCoreTest, EdgeCoreTest	c694bb0a	436	69	0 / 0
RocksDB: VertexCoreTest, EdgeCoreTest	c694bb0a	436	29	0 / 0
HStore: search/joint-index, input-order and paging regressions	c694bb0a	26	0	0 / 0

Both new review regressions failed on c694bb0a: primary-key paging raised Invalid PageState 'null', and interleaved empty-page consumption exposed the shared cursor. They pass with this follow-up. The primary-key test covers cold and warm caches, residual filtering to an empty result, a missing primary key, and terminal page metadata.

The latest supplement adds 10 tests. Seven isolated fault-injection variants are rejected by the new tests: omitted holder/source closing, a changed cache capacity boundary, a discarded oversized-batch tail, and incorrect cleanup decisions for invisible, mismatching, or matching-with-stale-index records. The unchanged implementation passes all selected diagnostic tests. Reflection-filter entries were checked against 15 actual declared methods across the transaction and cache classes.

Coverage includes raw-empty backend pages versus filtered-empty batches, missing and expired vertices through cache materialization, empty batches/holders, null and expanded mapper results, mixed ordering in both directions, real page-fetch counters and cursor metadata, limits, cross-batch materialization capacity, materialization after partial consumption, page metadata after closing the source, early close and suppressed exceptions, residual filtering on warm vertex caches, and an off-heap edge-cache hit with zero backend reads. The HStore selection also passed on the pre-change baseline using the same isolated PD/Store setup.

At c694bb0a, a separate four-method HStore reproduction exercises schema cleanup after creating 10,000 edges per label. Before the empty-page fix, three subsequent tests timed out in setup. With the fix, cleanup completes and the sequence matches baseline 36811483: three tests pass, while testQueryOutEdgesOfVertexBySortkeyWithMoreFieldsInPage fails in its body with Cardinality from code 0. This existing HStore serialization error remains unresolved; the 26-test selection above is not a full HStore-suite pass.

Reproduction commands

mvn test -pl hugegraph-server/hugegraph-test -am -P unit-test \
  -Dtest=QueryResultsTest,QueryListTest,GraphTransactionTest,CachedGraphTransactionTest,IdHolderTest,QueryTest,PageStateTest \
  -DfailIfNoTests=false -Drat.skip=true

# Run once with memory, then with rocksdb
mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,memory \
  -Dtest=VertexCoreTest,EdgeCoreTest -DfailIfNoTests=false -Drat.skip=true

# Requires an isolated, initialized PD/Store and an HStore properties file
mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,hstore \
  -Dbackend=hstore -Dconfig_path=/path/to/hstore-test.properties \
  '-Dtest=VertexCoreTest#testQueryByJointIndexesWithSearch*+testQueryByTextContainsPropertyOrderByMatchedCount*+testQueryByRangeIndexKeeps*+testQueryByPage*+testQueryByMultiLabelInPage,EdgeCoreTest#testQueryEdgeByPage*+testQuery*EdgesOfVertexInPaging' \
  -DfailIfNoTests=false -Drat.skip=true

# Four-method HStore cleanup reproduction (baseline has one serialization error)
mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,hstore \
  -Dbackend=hstore -Dconfig_path=/path/to/hstore-test.properties \
  '-Dtest=EdgeCoreTest#testQueryEdgesWithLimitOnSuperVertexAndFilterProp+testQueryByUnionHasDate+testQueryOutEdgesOfVertexBySortkeyWithMoreFieldsInPage+testUpdateEdgeProperty' \
  -DfailIfNoTests=false -Drat.skip=true

# Latest review follow-up on RocksDB
mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,rocksdb \
  '-Dtest=QueryListTest,VertexCoreTest#testQueryByPrimaryValues*+testQueryByPage*+testQueryByMultiLabelInPage' \
  -DfailIfNoTests=false -Drat.skip=true

# Repeat the new primary-key case with the HStore configuration above:
# -Dtest=VertexCoreTest#testQueryByPrimaryValuesInPageWithVertexCache

mvn editorconfig:format
mvn clean compile -Dmaven.javadoc.skip=true -Drat.skip=true
git diff --check

Formatting and full reactor compilation also passed at 9ad4a27e. The two additional tests cover deferred branch failure/cleanup in the shared flat-map iterator and compact root cache labels coexisting with distinct batches without mutating previously published values. Existing off-heap, lifecycle and cache-limit regressions pass in the 80-test selection. No throughput benchmark or backend-suite rerun was performed for this allocation/cache follow-up. Verification used -Drat.skip=true; the complete UnitTestSuite, API and TinkerPop suites were not run. The 78-test unit verification used Java files matching 05186a0e byte-for-byte. The accessor/comment cleanup at cf403002 passed formatting, full reactor compilation and the 37 targeted tests listed above. The backend suites remain labeled with the snapshots actually tested and were not rerun for this test/auth-registration supplement.

Does this PR potentially affect the following parts?

• Dependencies
• Modify configurations
• The public API (Gremlin/REST interfaces)
• Other affects (internal query iteration, paging and cache lifecycle)
• Nope

Documentation Status

• Doc - TODO
• Doc - Done
• Doc - No Need

[codecov]

Codecov Report

❌ Patch coverage is 70.91633% with 146 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.88%. Comparing base (3681148) to head (9ad4a27).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...org/apache/hugegraph/backend/query/QueryBatch.java	58.97%	37 Missing and 11 partials ⚠️
...g/apache/hugegraph/backend/query/QueryResults.java	68.49%	44 Missing and 2 partials ⚠️
.../apache/hugegraph/backend/tx/GraphTransaction.java	64.38%	15 Missing and 11 partials ⚠️
...ugegraph/backend/cache/CachedGraphTransaction.java	77.02%	5 Missing and 12 partials ⚠️
...ache/hugegraph/backend/page/PageEntryIterator.java	79.16%	1 Missing and 4 partials ⚠️
...apache/hugegraph/backend/query/ConditionQuery.java	50.00%	1 Missing and 1 partial ⚠️
...a/org/apache/hugegraph/backend/page/QueryList.java	95.65%	1 Missing ⚠️
...he/hugegraph/backend/query/QueryResultContext.java	97.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3193      +/-   ##
============================================
+ Coverage     37.77%   37.88%   +0.10%     
- Complexity     6560     6594      +34     
============================================
  Files           800      802       +2     
  Lines         68960    69100     +140     
  Branches       9166     9188      +22     
============================================
+ Hits          26052    26179     +127     
- Misses        39841    39861      +20     
+ Partials       3067     3060       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bitflicker64]

Blocking: yes. Summary: The batching design holds up. BatchIterator close and exception semantics, the shared cursor across mapBatches/flatMap, the raw-empty-page versus filtered-empty-batch distinction, and rebuilding the batch context on an edge-cache hit all check out. One blocker: moving the QueryList fetcher up from super::query to fetchVertexBatch puts the vertex cache inside the paging machinery, and the joined cache-plus-backend result it returns carries no page metadata, so a paged primary-key vertex query dies in PageInfo.pageState. Please add the paging() bypass plus a VertexCoreTest case for that shape; the other four comments are optional.

Evidence: the chain is traced through unmodified code and spelled out inline on CachedGraphTransaction.java:322, including why the existing green paging tests do not reach it. Static analysis only, not executed against a page-capable backend. CI at c694bb0 is green on all 24 checks.

[bitflicker64]

Blocking: no. Summary: The batch rewrite holds up at this head. BatchIterator close and suppression semantics, prefetch preservation across backendBatches into mapBatches, null mapper returns, filter ordering, root-of-chain visibility flags and paging metadata propagation all check out, and the previous review's Invalid PageState 'null' blocker is fixed by the query.paging() bypass. Two minor points below: the vertex and edge caches quietly lost their queryNeedsPostFilter exclusion, and PageResults.query()/get() are now dead code. Neither blocks.

Evidence: static reading of the exact-head diff plus the surrounding unchanged code, and a local build of 05186a0 (hugegraph-struct plus hugegraph-server/{hugegraph-core,hugegraph-api,hugegraph-test}) where the seven selected unit classes give Tests run: 78, Failures: 0, Errors: 0, matching the PR body. Backend suites were not re-run here; InMemoryDBStore.supportsQueryByPage() is false, so a local unit run cannot reach the paging and cache paths this PR reworks.

[bitflicker64]

Blocking: no. Summary: The batch rework holds up at this head, and four of my earlier points are addressed: the vertex cache now has the paging() bypass (CachedGraphTransaction.java:322), PageResults.emptyIterator() returns a fresh instance (QueryList.java:362-365), registerPrivateActions() matches the renamed members again, and the dead PageResults.query()/get() accessors are gone. My threads on the shared edgesCache monitor (:412) and the dropped queryNeedsPostFilter exclusion (:372) are still open. New here: the iterator plumbing allocates per surviving element at every stage, the TTL stage is now unconditional, multi-branch edge queries lost their eager validation, and the edge cache copies its group index on reads. Evidence: exact-head diff for cf40300 against origin/master, plus Query.toString():610, CoreOptions.QUERY_BATCH_SIZE default 1000, style/checkstyle.xml:23,59. All 24 check runs on cf40300 are green.

[bitflicker64]

⚠️ Each map or filter stage allocates a set and an iterator per surviving element.

map routes through flatMap and returns Collections.singleton(mapped).iterator() for every value that passes, so each surviving element costs one SingletonSet plus one iterator, per stage. filter at line 117 is built on map, so it pays the same. Rejected elements are free, since they get the shared Collections.emptyIterator().

A plain vertex read stacks four such stages: .map(this::parseEntry) and filterExpiredBatches in GraphTransaction.fetchVertexBatch:883, filterUnmatchedRecord in processBatches:892, and filterInvalidRecord in queryValidVerticesFromBackend:888. Edges stack three on top of the one real expansion at fetchEdgeBatch:1133. MapperIterator.fetch(), which this replaces, loops on a null result in place and allocates nothing.

Requested change: give map its own BatchIterator that applies the function and loops while the result is null; filter can keep delegating to it.

[bitflicker64]

🧹 The TTL stage is now unconditional.

Base filterExpiredResultFromBackend returned the source iterator untouched when store().features().supportsTtl() or query.showExpired() held, so a TTL-capable store carried no wrapper at all. This version always adds a filter stage, which is one of the four counted in the QueryBatch comment, and moves the store-feature test into the per-element predicate.

Requested change: read storeFeatures().supportsTtl() once and return batches unchanged when it is true, leaving context.showExpired() and elem.expired() in the predicate since those do vary.

[bitflicker64]

🧹 Multi-branch edge queries lost their eager validation, and only the single-query case says so.

Base built a Stream<Iterator<HugeEdge>> and ended with reduce(ExtendableIterator::concat). That is a terminal operation, so every queryEdgesFromBackendInternal(cq) ran at call time and any validation failure surfaced from the call itself. QueryResults.flatMap here runs each branch only when iteration reaches it, so a failure in the second or later flattened query now surfaces mid-iteration. Line 1109 documents the choice for flattened.size() == 1 and nothing covers the multi-branch case.

Requested change: say in the PR body which failures are expected to move from call time to iteration time for multi-branch edge queries, or apply the same eager first step there.

[bitflicker64]

🧹 The read path shallow-copies the group's index list for nothing.

Line 377 builds a CachedEdgeQuery on every edge fetch that reaches the cache, and the constructor at 427-430 does new ArrayList<>((List<Object>) cached). Nothing on the read path mutates it: get() only scans. cacheEdgeBatch:413 already makes its own copy inside synchronized (this.edgesCache), which is where the copy belongs.

Requested change: scan the cached list directly for the lookup and keep the copy in cacheEdgeBatch.

[bitflicker64]

🧹 The batch key stored in the value can be larger than the value it labels.

QueryId.asString() is Query.toString(), and Query.toString():610 appends id in <ids>. put stores that string next to the candidates, so an index-derived leaf writes up to query.batch_size ids of rendered text (CoreOptions.java:466-472, default 1000) beside a candidate list capped at MAX_CACHE_EDGES_PER_QUERY, which is 100 (line 63).

In the common adjacency shape the leaf is also the chain root, so batchKey and cacheKey render the same string and the value holds a verbatim copy of its own cache key.

Requested change: skip storing the label when batchKey.equals(cacheKey) and treat a group of one unlabelled entry as that case, which keeps the stored value a nested list and leaves off-heap serialization unchanged.

[bitflicker64]

🧹 Four imports are left unused, and two were reordered for no reason. Anchored here because lines 23, 68, 70 and 105 fall outside every hunk in this file.

java.util.Collections (23), org.apache.hugegraph.iterator.FlatMapperIterator (68), org.apache.hugegraph.iterator.ListIterator (70) and com.google.common.collect.Iterators (105) each appear exactly once in the file at this head, on their own import line, and all four are used at base. style/checkstyle.xml:59 enables UnusedImports, inheriting the info severity set at line 23, so the validate-bound execution logs them without failing the build.

Lines 47-48 and 50-51 also moved Aggregate.AggregateFunc above Aggregate and ConditionQuery.OptimizedType above ConditionQuery, which is neither alphabetical nor related to the change. PageEntryIterator.java:23-24 has the same swap.

Requested change: delete the four import lines and restore the original order of the two pairs.

[bitflicker64]

🧹 The offset check dropped the page token from its message.

Base parsePageInfo reported "Invalid page '%s' with an offset '%s' exceeds the size of IdHolderList" with both the raw page string and the offset. This check keeps only the offset, and it fires exactly when a caller supplies a stale or hand-written ~page value, which is the case where the token itself is the thing worth seeing in the log.

Requested change: put queries.parent().pageWithoutCheck() back in the message alongside the offset.

[bitflicker64]

Blocking: no. Summary: The batch rework holds at this head, and 9ad4a27e addresses all seven points from the cf40300 round: map allocation, the TTL short-circuit, the branch-validation note plus testLaterBranchFailureClosesEarlierBranch, the read-path copy, the batch-key size, the unused imports and reorders in GraphTransaction.java, and the offset message. Two caveats. First, only half of the base TTL short-circuit came back: base filterExpiredResultFromBackend returned early on supportsTtl() || query.showExpired(), while GraphTransaction.java:1982-1992 returns early on supportsTtl() alone and still builds a filter stage to evaluate context.showExpired() per element. That is fine given the per-batch context, but the wrapper is not gone for a showExpired query on a non-TTL store. Second, the backend suites were not re-run here. Three minor points below; none blocks. My earlier threads on the shared edgesCache monitor (:414) and the dropped queryNeedsPostFilter exclusion (:372) are unchanged at this head.

What I checked and found sound: the compact "" batch label cannot collide, because Query.toString():588 always starts with `Query and IdPrefixQuery/IdRangeQuery build on super.toString(), so no QueryId.asString() is empty. Off-heap storage of the nested List<Object> round-trips: OffheapCache.ValueType.valueOf maps String to STRING(DataType.TEXT) and serializeList/deserializeList recurse (OffheapCache.java:281-297). The ExtendableIterator(candidates, source) wrapper at CachedGraphTransaction.java:409-410 does not strand the backend cursor: hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/iterator/ExtendableIterator.java:69-90 closes every iterator still in itors, with suppression.

Evidence: static reading of the exact-head diff against 36811483a plus the surrounding unchanged code. Executed: a local JDK 11 build of hugegraph-struct and hugegraph-server/{hugegraph-core,hugegraph-api,hugegraph-test} at 9ad4a27e running the seven selected unit classes gives Tests run: 80, Failures: 0, Errors: 0, matching the PR body. InMemoryDBStore.supportsQueryByPage() is false (InMemoryDBStore.java:442-444), so a local unit run cannot reach the paging paths this PR reworks. All 24 check runs on 9ad4a27e are green.

[bitflicker64]

🧹 queries() has no main-code caller left after this change.

At base it had exactly one: QueryList.PageResults.query() (base QueryList.java:347), which cf403002 deleted. Ordering now comes from QueryResultContext, and base PageEntryIterator.java:79 (queryResults.setQuery(pageResults.query())) is gone. Over this head, git grep '\.queries()' finds only context.queries() calls plus assertions in QueryResultsTest.java:161,164,166 and QueryListTest.java:129,131,135.

The bookkeeping still runs on the hot path: an ArrayList per QueryResults (line 51), queries.clear() plus queries.add(...) on every batch activation (lines 91-92), a clear on close (line 100), and a re-seed in toList() (line 250). Same shape as the dead PageResults.query()/get() pair you removed last round.

Requested change: drop the queries field and this accessor, and rework the assertions onto results.batches() where the test structure allows. Note that java.util.Collections (line 21) is then unused, since lines 51 and 226 are its only uses. If it should stay, name its reader in the Javadoc at lines 221-224, because no main code reads it today.

[bitflicker64]

🧹 This paging() test is on the leaf, while fetchEdgeBatch tests the chain root.

Separate from my earlier thread that asked for this guard: now that it exists, the two overrides disagree about what "paged" means. Line 322 tests query.paging() on the QueryList fetcher's leaf; fetchEdgeBatch builds the same context and tests request.paging() on chain.get(chain.size() - 1), the chain root (lines 369-372).

The difference is observable. QueryList.IndexQuery.iterator(int, String, long) builds its leaf through indexIdQuery (QueryList.java:326-332) as new IdQuery(parent().resultType(), bindQuery), and Query(HugeType, Query) sets this.page = null (Query.java:95), so that leaf reaches line 322 with paging() == false, idsSize() > 0 and conditionsSize() == 0. The vertex cache therefore still runs inside a paged index query, and is correct only for the reason I traced on the earlier thread: that path takes its cursor from pageIds.pageState(), not from result metadata.

Requested change: state in a comment that the leaf test is deliberate and that the index-paging leaf is safe because its cursor comes from the IdHolder. Matching fetchEdgeBatch and testing the root instead would also work, but it bypasses the vertex cache for every paged index query, so it is the more expensive option.

[bitflicker64]

🧹 These two import pairs kept the ordering that 9ad4a27e reverted in the other two files.

This PR moved ConditionQuery below ConditionQuery.OptimizedType (lines 28-29) and added QueryBatch.BatchIterator above QueryBatch (lines 32-33). 9ad4a27e restored outer-class-first for exactly these two patterns elsewhere: Aggregate/Aggregate.AggregateFunc and ConditionQuery/ConditionQuery.OptimizedType in GraphTransaction.java, and QueryBatch/QueryBatch.BatchIterator in PageEntryIterator.java. This file was missed, and nothing flags it because style/checkstyle.xml:43 has ImportOrder commented out.

Requested change: swap both pairs so this file matches the ordering the same commit restored in the other two.