diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/query/Query.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/query/Query.java index b278709dfe..b798da3003 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/query/Query.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/query/Query.java @@ -212,6 +212,11 @@ private long goStoreOffsetBySubQuery(long offset) { } public Set skipOffsetIfNeeded(Set elems) { + /* + * Skip index(index query with offset) for performance optimization. + * We assume one result is returned by each index, but if there are + * overridden index it will cause confusing offset and results. + */ long fromIndex = this.offset() - this.actualOffset(); if (fromIndex < 0L) { // Skipping offset is overhead, no need to skip diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphIndexTransaction.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphIndexTransaction.java index 0bb19acb4c..9a460bec2a 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphIndexTransaction.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphIndexTransaction.java @@ -397,10 +397,22 @@ private IdHolderList queryByLabel(ConditionQuery query) { indexQuery.eq(HugeKeys.INDEX_LABEL_ID, il.id()); indexQuery.eq(HugeKeys.FIELD_VALUES, label); /* - * Set offset and limit to avoid redundant element ids - * NOTE: the backend itself will skip the offset + * We can avoid redundant element ids if set limit, but if there are + * label index overridden by other vertices with different label, + * query with limit like g.V().hasLabel('xx').limit(10) may lose some + * results, so can't set limit here. But in this case, the following + * query results may be still different: + * g.V().hasLabel('xx').count() // label index count + * g.V().hasLabel('xx').limit(-1).count() // actual vertices count + * It’s a similar situation for the offset, like: + * g.V().hasLabel('xx').range(26, 27) + * g.V().hasLabel('xx').range(27, 28) + * we just reset limit here, but don't reset offset due to performance + * optimization with index+offset query, see Query.skipOffsetIfNeeded(). + * NOTE: if set offset the backend itself will skip the offset */ indexQuery.copyBasic(query); + indexQuery.limit(Query.NO_LIMIT); IdHolder idHolder = this.doIndexQuery(il, indexQuery); diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphTransaction.java index c594f40acf..1acc771cb7 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphTransaction.java @@ -72,6 +72,7 @@ import com.baidu.hugegraph.iterator.FlatMapperIterator; import com.baidu.hugegraph.iterator.ListIterator; import com.baidu.hugegraph.iterator.MapperIterator; +import com.baidu.hugegraph.iterator.WrappedIterator; import com.baidu.hugegraph.job.system.DeleteExpiredJob; import com.baidu.hugegraph.perf.PerfUtil.Watched; import com.baidu.hugegraph.schema.EdgeLabel; @@ -747,7 +748,7 @@ public Iterator queryVertices(Query query) { @SuppressWarnings("unchecked") Iterator r = (Iterator) joinTxVertices(query, results); - return this.filterOffsetLimitRecords(r, query); + return this.skipOffsetOrStopLimit(r, query); } protected Iterator queryVerticesFromBackend(Query query) { @@ -930,7 +931,7 @@ public Iterator queryEdges(Query query) { @SuppressWarnings("unchecked") Iterator r = (Iterator) joinTxEdges(query, results, this.removedVertices); - return this.filterOffsetLimitRecords(r, query); + return this.skipOffsetOrStopLimit(r, query); } protected Iterator queryEdgesFromBackend(Query query) { @@ -1599,7 +1600,7 @@ private Iterator filterUnmatchedRecords( } // Process results that query from left index or primary-key if (query.resultType().isVertex() == elem.type().isVertex() && - !filterResultFromIndexQuery(query, elem)) { + !rightResultFromIndexQuery(query, elem)) { // Only index query will come here return false; } @@ -1607,7 +1608,7 @@ private Iterator filterUnmatchedRecords( }); } - private boolean filterResultFromIndexQuery(Query query, HugeElement elem) { + private boolean rightResultFromIndexQuery(Query query, HugeElement elem) { /* * If query is ConditionQuery or query.originQuery() is ConditionQuery * means it's index query @@ -1637,8 +1638,24 @@ private boolean filterResultFromIndexQuery(Query query, HugeElement elem) { return false; } - private Iterator filterOffsetLimitRecords(Iterator results, - Query query) { + private Iterator + filterExpiredResultFromFromBackend( + Query query, Iterator results) { + if (this.store().features().supportsTtl() || query.showExpired()) { + return results; + } + // Filter expired vertices/edges with TTL + return new FilterIterator<>(results, elem -> { + if (elem.expired()) { + DeleteExpiredJob.asyncDeleteExpiredObject(this.params(), elem); + return false; + } + return true; + }); + } + + private Iterator skipOffsetOrStopLimit(Iterator results, + Query query) { if (query.noLimitAndOffset()) { return results; } @@ -1655,26 +1672,10 @@ private Iterator filterOffsetLimitRecords(Iterator results, query.goOffset(1L); } } - // Filter limit - return new FilterIterator<>(results, elem -> { + // Stop if reach limit + return new LimitIterator<>(results, elem -> { long count = query.goOffset(1L); - return !query.reachLimit(count - 1L); - }); - } - - private Iterator - filterExpiredResultFromFromBackend( - Query query, Iterator results) { - if (this.store().features().supportsTtl() || query.showExpired()) { - return results; - } - // Filter expired vertices/edges with TTL - return new FilterIterator<>(results, elem -> { - if (elem.expired()) { - DeleteExpiredJob.asyncDeleteExpiredObject(this.params(), elem); - return false; - } - return true; + return query.reachLimit(count - 1L); }); } @@ -1986,4 +1987,47 @@ private void traverseByLabel(SchemaLabel label, } while (page != null); } } + + // TODO: move to common module + public static class LimitIterator extends WrappedIterator { + + private final Iterator originIterator; + private final Function filterCallback; + + public LimitIterator(Iterator origin, Function filter) { + this.originIterator = origin; + this.filterCallback = filter; + } + + @Override + protected Iterator originIterator() { + return this.originIterator; + } + + @Override + protected final boolean fetch() { + while (this.originIterator.hasNext()) { + T next = this.originIterator.next(); + // Do filter + boolean reachLimit = this.filterCallback.apply(next); + if (reachLimit) { + this.closeOriginIterator(); + return false; + } + if (next != null) { + assert this.current == none(); + this.current = next; + return true; + } + } + return false; + } + + protected final void closeOriginIterator() { + if (this.originIterator == null) { + return; + } + close(this.originIterator); + } + } }