contrueCT commented on code in PR #2994:
URL: https://github.com/apache/hugegraph/pull/2994#discussion_r3371869110
##########
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:
Good catch, thanks. I replaced the ad hoc holder with a dedicated
SortedRangeBatchIdHolder that preserves the batch returned by peekNext() and
then serves the same batch from fetchNext(), so joint-index/filtering won't
skip the first prefetched ids. I also added GraphIndexTransactionTest coverage
for both the peek-then-fetch path and the zero-remaining batch case.
##########
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:
Thanks, fixed. The sorted range fallback now propagates keepOrder() through
QueryList batch/paging paths, and GraphTransaction will keep the input id order
whenever the generated IdQuery explicitly requires it, even on backends that
report supportsQuerySortByInputIds(). I also added QueryResultsTest coverage
and registered the new unit tests in UnitTestSuite so this path is exercised by
the unit-test profile.
--
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]
