VGalaxies commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3371394607
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -657,6 +663,185 @@ 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) {
Review Comment:
high
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:707`
- Preserve peeked ids in the custom range-index holder
Evidence: `doJointIndex()` calls `((BatchIdHolder)
holder).peekNext(this.indexIntersectThresh)` and may return that same `holder`
when filtering is enabled. The base `BatchIdHolder.peekNext()` caches the
peeked batch in `currentBatch`, but this override of `fetchNext()` advances its
own `offset` and never serves the cached batch.
Impact: For HStore joint-index queries involving a range index with at
least `indexIntersectThresh` matches, the first peeked batch can be skipped
when the returned holder is later consumed, causing missing results.
Requested fix: Use the normal `BatchIdHolder` fetcher path, or override
`peekNext()`/`fetchNext()` with a local pending batch so a peek does not
advance the final cursor without being consumed.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:
##########
@@ -657,6 +663,185 @@ 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();
Review Comment:
medium
`hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphIndexTransaction.java:745`
- Sorted HStore range ids are fetched back unordered
Evidence: The new path sorts range-index entries and inserts ids into an
insertion-ordered set, but `QueryList` later calls `queryByIndexIds(ids)` for
batch and page paths, which sets `mustSortByInput(false)`. For paging,
`QueryResults.keepInputOrderIfNeeded()` also skips input-order sorting.
Impact: HStore range queries may still return vertices/edges in backend
id order rather than range-index order, so limit/page results can be visibly
unordered even though this path sorted the index ids.
Requested fix: Preserve input order when consuming these sorted ids,
including the batch and paging paths, or return elements through a path that
explicitly keeps the sorted range-index order.
--
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]
