imbajin commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3367950349
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -657,6 +663,184 @@ private IdHolder doIndexQuery(IndexLabel indexLabel,
ConditionQuery query) {
}
}
+ private boolean needHstoreRangeIndexOrder(IndexLabel indexLabel) {
+ return this.store().provider().isHstore() &&
+ indexLabel.indexType().isRange();
+ }
+
+ private IdHolder doHstoreRangeIndexQuery(IndexLabel indexLabel,
+ ConditionQuery query) {
+ if (!query.paging()) {
+ if (query.noLimitAndOffset()) {
+ return this.doIndexQueryBatch(indexLabel, query);
+ }
+ Set<Id> ids = this.querySortedRangeIndexIds(indexLabel, query);
+ return this.newSortedRangeIndexBatchHolder(query, ids);
+ }
+ return new PagingIdHolder(query, q -> {
+ return this.querySortedRangeIndexPage(indexLabel, q);
+ });
+ }
+
+ private BatchIdHolder newSortedRangeIndexBatchHolder(ConditionQuery query,
+ Set<Id> ids) {
+ List<Id> idList = new ArrayList<>(ids);
+ return new BatchIdHolder(query, Collections.emptyIterator(), batch -> {
+ throw new IllegalStateException("Unexpected sorted index fetcher");
+ }) {
+ private int offset = 0;
+
+ @Override
+ public boolean hasNext() {
+ return this.offset < idList.size();
+ }
+
+ @Override
+ public IdHolder next() {
+ if (!this.hasNext()) {
+ throw new java.util.NoSuchElementException();
+ }
+ return this;
+ }
+
+ @Override
+ public PageIds fetchNext(String page, long batchSize) {
+ E.checkArgument(page == null,
+ "Not support page parameter by BatchIdHolder");
+ if (!this.hasNext()) {
+ return PageIds.EMPTY;
+ }
+
+ int end;
+ if (batchSize == Query.NO_LIMIT) {
+ end = idList.size();
+ } else {
+ end = (int) Math.min((long) idList.size(),
+ this.offset + batchSize);
+ }
+ Set<Id> batchIds = InsertionOrderUtil.newSet();
+ batchIds.addAll(idList.subList(this.offset, end));
+ this.offset = end;
+ return new PageIds(batchIds, PageState.EMPTY);
+ }
+
+ @Override
+ public Set<Id> all() {
+ Set<Id> allIds = InsertionOrderUtil.newSet();
+ allIds.addAll(idList);
+ return allIds;
+ }
+
+ @Override
+ public void close() {
+ this.offset = idList.size();
+ }
+ };
+ }
+
+ private Set<Id> querySortedRangeIndexIds(IndexLabel indexLabel,
+ ConditionQuery query) {
+ List<HugeIndex> indexes = this.querySortedRangeIndexes(indexLabel,
+ query);
+ Set<Id> ids = InsertionOrderUtil.newSet();
+ for (HugeIndex index : indexes) {
+ ids.addAll(index.elementIds());
+ Query.checkForceCapacity(ids.size());
+ }
+ return ids;
+ }
+
+ private PageIds querySortedRangeIndexPage(IndexLabel indexLabel,
+ ConditionQuery query) {
+ List<HugeIndex> indexes = this.querySortedRangeIndexes(indexLabel,
+ query);
+ Set<Id> allIds = InsertionOrderUtil.newSet();
+ for (HugeIndex index : indexes) {
+ allIds.addAll(index.elementIds());
+ Query.checkForceCapacity(allIds.size());
+ }
+ if (allIds.isEmpty()) {
+ return PageIds.EMPTY;
+ }
+
+ int start = 0;
+ if (!query.page().isEmpty()) {
+ start = PageState.fromString(query.page()).offset();
+ }
+ if (start >= allIds.size()) {
+ return PageIds.EMPTY;
+ }
+
+ long total = allIds.size();
+ long end = query.noLimit() ? total :
+ Math.min(total, (long) start + query.limit());
+ Set<Id> pageIds = CollectionUtil.subSet(allIds, start, (int) end);
+ if (pageIds.isEmpty()) {
+ return PageIds.EMPTY;
+ }
+
+ int next = (int) end;
+ PageState pageState;
+ if (next < total) {
+ pageState = new PageState(new byte[]{1}, next, pageIds.size());
+ } else {
+ pageState = new PageState(PageState.EMPTY_BYTES, 0,
+ pageIds.size());
+ }
+ return new PageIds(pageIds, pageState);
+ }
+
+ private List<HugeIndex> querySortedRangeIndexes(IndexLabel indexLabel,
+ ConditionQuery query) {
+ List<HugeIndex> indexes = new ArrayList<>();
+ Iterator<BackendEntry> entries = null;
+ String spaceGraph = this.params()
+ .graph().spaceGraphName();
+ LockUtil.Locks locks = new LockUtil.Locks(spaceGraph);
+ ConditionQuery scanQuery = query.copy();
+ scanQuery.page(null);
+ scanQuery.limit(Query.NO_LIMIT);
Review Comment:
‼️ **Reset offset before the full sorted scan**
Evidence: `querySortedRangeIndexes()` copies the original query, clears
`page`, and changes only `limit` before calling `super.query(scanQuery)`. For
`range()` / offset queries, `scanQuery` still carries the original offset, so
the backend iterator can skip those entries during the full scan. The returned
`BatchIdHolder` still keeps the original query, and
`QueryList.IndexQuery.each()` applies `bindQuery.skipOffsetIfNeeded(ids)` again
to the sorted ids.
Impact: HStore range-index queries with offset/range can drop too many
results after this fallback path is selected. Please reset the scan query
offset before reading all range-index entries, then let the sorted holder apply
the original offset once.
```suggestion
ConditionQuery scanQuery = query.copy();
scanQuery.page(null);
scanQuery.offset(0L);
scanQuery.limit(Query.NO_LIMIT);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
