diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Query.java b/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Query.java index 5c96297a9..6ac50df7d 100644 --- a/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Query.java +++ b/hugegraph-core/src/main/java/org/apache/hugegraph/backend/query/Query.java @@ -37,6 +37,7 @@ import org.apache.hugegraph.util.CollectionUtil; import org.apache.hugegraph.util.E; import org.apache.hugegraph.util.InsertionOrderUtil; import org.apache.hugegraph.util.collection.IdSet; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -421,6 +422,10 @@ public class Query implements Cloneable { this.aggregate = new Aggregate(func, property); } + public void aggregate(Aggregate aggregate) { + this.aggregate = aggregate; + } + public boolean showHidden() { return this.showHidden; } diff --git a/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java index 66986407c..3300d3777 100644 --- a/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java @@ -32,19 +32,7 @@ import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; -import jakarta.ws.rs.ForbiddenException; import org.apache.commons.collections.CollectionUtils; -import org.apache.hugegraph.backend.store.BackendEntry; -import org.apache.hugegraph.backend.store.BackendMutation; -import org.apache.hugegraph.backend.store.BackendStore; -import org.apache.tinkerpop.gremlin.structure.Edge; -import org.apache.tinkerpop.gremlin.structure.Element; -import org.apache.tinkerpop.gremlin.structure.Graph; -import org.apache.tinkerpop.gremlin.structure.Vertex; -import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; -import org.apache.tinkerpop.gremlin.structure.util.ElementHelper; -import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; - import org.apache.hugegraph.HugeException; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.HugeGraphParams; @@ -64,6 +52,9 @@ import org.apache.hugegraph.backend.query.ConditionQueryFlatten; import org.apache.hugegraph.backend.query.IdQuery; import org.apache.hugegraph.backend.query.Query; import org.apache.hugegraph.backend.query.QueryResults; +import org.apache.hugegraph.backend.store.BackendEntry; +import org.apache.hugegraph.backend.store.BackendMutation; +import org.apache.hugegraph.backend.store.BackendStore; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.exception.LimitExceedException; @@ -98,8 +89,18 @@ import org.apache.hugegraph.type.define.IdStrategy; import org.apache.hugegraph.util.E; import org.apache.hugegraph.util.InsertionOrderUtil; import org.apache.hugegraph.util.LockUtil; +import org.apache.tinkerpop.gremlin.structure.Edge; +import org.apache.tinkerpop.gremlin.structure.Element; +import org.apache.tinkerpop.gremlin.structure.Graph; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; +import org.apache.tinkerpop.gremlin.structure.util.ElementHelper; +import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; + import com.google.common.collect.ImmutableList; +import jakarta.ws.rs.ForbiddenException; + public class GraphTransaction extends IndexableTransaction { public static final int COMMIT_BATCH = (int) Query.COMMIT_BATCH; @@ -528,12 +529,12 @@ public class GraphTransaction extends IndexableTransaction { @Override public QueryResults query(Query query) { if (!(query instanceof ConditionQuery)) { + // It's a sysprop-query, don't need to optimize LOG.debug("Query{final:{}}", query); return super.query(query); } - QueryList queries = this.optimizeQueries(query, - super::query); + QueryList queries = this.optimizeQueries(query, super::query); LOG.debug("{}", queries); return queries.empty() ? QueryResults.empty() : queries.fetch(this.pageSize); @@ -541,39 +542,51 @@ public class GraphTransaction extends IndexableTransaction { @Override public Number queryNumber(Query query) { - E.checkArgument(!this.hasUpdate(), - "It's not allowed to query number when " + - "there are uncommitted records."); - - if (!(query instanceof ConditionQuery)) { - return super.queryNumber(query); - } - + boolean hasUpdate = this.hasUpdate(); Aggregate aggregate = query.aggregateNotNull(); QueryList queries = this.optimizeQueries(query, q -> { - boolean indexQuery = q.getClass() == IdQuery.class; - OptimizedType optimized = ((ConditionQuery) query).optimized(); + boolean isIndexQuery = q instanceof IdQuery; + boolean isConditionQuery = query instanceof ConditionQuery; + assert isIndexQuery || isConditionQuery || q == query; + // Need to fallback if there are uncommitted records + boolean fallback = hasUpdate; Number result; - if (!indexQuery) { + + if (fallback) { + // Here just ignore it, and do fallback later + result = null; + } else if (!isIndexQuery || !isConditionQuery) { + // It's a sysprop-query, let parent tx do it + assert !fallback; result = super.queryNumber(q); } else { E.checkArgument(aggregate.func() == AggregateFunc.COUNT, "The %s operator on index is not supported now", aggregate.func().string()); - if (this.optimizeAggrByIndex && - optimized == OptimizedType.INDEX) { + assert query instanceof ConditionQuery; + OptimizedType optimized = ((ConditionQuery) query).optimized(); + if (this.optimizeAggrByIndex && optimized == OptimizedType.INDEX) { // The ids size means results count (assume no left index) result = q.idsSize(); } else { - assert optimized == OptimizedType.INDEX_FILTER || - optimized == OptimizedType.INDEX; - assert q.resultType().isVertex() || q.resultType().isEdge(); - result = IteratorUtils.count(q.resultType().isVertex() ? - this.queryVertices(q) : - this.queryEdges(q)); + assert !fallback; + fallback = true; + result = null; } } + + // Can't be optimized, then do fallback + if (fallback) { + assert result == null; + assert q.resultType().isVertex() || q.resultType().isEdge(); + // Reset aggregate to fallback and scan + q.aggregate(null); + result = IteratorUtils.count(q.resultType().isVertex() ? + this.queryVertices(q) : + this.queryEdges(q)); + } + return new QueryResults<>(IteratorUtils.of(result), q); }); @@ -1335,14 +1348,20 @@ public class GraphTransaction extends IndexableTransaction { private QueryList optimizeQueries(Query query, QueryResults.Fetcher fetcher) { - boolean supportIn = this.storeFeatures().supportsQueryWithInCondition(); QueryList queries = new QueryList<>(query, fetcher); + if (!(query instanceof ConditionQuery)) { + // It's a sysprop-query, add itself as subquery, don't need to flatten + queries.add(query); + return queries; + } + + boolean supportIn = this.storeFeatures().supportsQueryWithInCondition(); for (ConditionQuery cq: ConditionQueryFlatten.flatten( (ConditionQuery) query, supportIn)) { // Optimize by sysprop Query q = this.optimizeQuery(cq); /* - * NOTE: There are two possibilities for this query: + * NOTE: There are two possibilities for the returned q: * 1.sysprop-query, which would not be empty. * 2.index-query result(ids after optimization), which may be empty. */ diff --git a/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java b/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java index 023fbcba4..119b85af1 100644 --- a/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java +++ b/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java @@ -31,23 +31,6 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.function.Function; -import org.apache.tinkerpop.gremlin.process.traversal.Order; -import org.apache.tinkerpop.gremlin.process.traversal.P; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; -import org.apache.tinkerpop.gremlin.structure.Direction; -import org.apache.tinkerpop.gremlin.structure.Edge; -import org.apache.tinkerpop.gremlin.structure.T; -import org.apache.tinkerpop.gremlin.structure.Vertex; -import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; -import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; -import org.apache.hugegraph.testutil.FakeObjects; -import org.apache.hugegraph.testutil.Utils; -import org.junit.Assume; -import org.junit.Before; -import org.junit.Test; - import org.apache.hugegraph.HugeException; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.backend.BackendException; @@ -69,6 +52,8 @@ import org.apache.hugegraph.schema.Userdata; import org.apache.hugegraph.structure.HugeEdge; import org.apache.hugegraph.structure.HugeVertex; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.FakeObjects; +import org.apache.hugegraph.testutil.Utils; import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.traversal.optimize.ConditionP; import org.apache.hugegraph.traversal.optimize.Text; @@ -78,6 +63,21 @@ import org.apache.hugegraph.type.define.Directions; import org.apache.hugegraph.type.define.HugeKeys; import org.apache.hugegraph.util.Events; import org.apache.hugegraph.util.collection.CollectionFactory; +import org.apache.tinkerpop.gremlin.process.traversal.Order; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.structure.Direction; +import org.apache.tinkerpop.gremlin.structure.Edge; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; +import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -1835,19 +1835,20 @@ public class EdgeCoreTest extends BaseCoreTest { // Query all with limit after delete graph.traversal().E().limit(14).drop().iterate(); - Assert.assertThrows(IllegalArgumentException.class, () -> { - // Query count with uncommitted records - graph.traversal().E().count().next(); - }); + // Query count with uncommitted records + Assert.assertEquals(4L, graph.traversal().E().count().next()); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with limit graph.traversal().E().limit(3).toList(); }); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with offset graph.traversal().E().range(1, -1).toList(); }); + // Query all with limit after delete once graph.tx().commit(); Assert.assertEquals(4L, graph.traversal().E().count().next()); List edges = graph.traversal().E().limit(3).toList(); @@ -1877,21 +1878,30 @@ public class EdgeCoreTest extends BaseCoreTest { "comment", "good book!", "comment", "good book too!"); - Assert.assertThrows(IllegalArgumentException.class, () -> { - // Query count with uncommitted records - graph.traversal().E().count().next(); - }); + // Query count with uncommitted records + Assert.assertEquals(19L, graph.traversal().E().count().next()); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with limit graph.traversal().E().limit(3).toList(); }); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with offset graph.traversal().E().range(1, -1).toList(); }); + // Do commit graph.tx().commit(); + + // Query count Assert.assertEquals(19L, graph.traversal().E().count().next()); + + // Query with limit + Assert.assertEquals(3L, graph.traversal().E().limit(3).toList().size()); + + // Query with offset + Assert.assertEquals(18L, graph.traversal().E().range(1, -1).toList().size()); } @Test diff --git a/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java b/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java index 6cfce9a12..c2855bd9f 100644 --- a/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java +++ b/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java @@ -33,20 +33,6 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; -import org.apache.tinkerpop.gremlin.process.traversal.P; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; -import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; -import org.apache.tinkerpop.gremlin.structure.T; -import org.apache.tinkerpop.gremlin.structure.Vertex; -import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; -import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; -import org.apache.hugegraph.testutil.FakeObjects; -import org.apache.hugegraph.testutil.Utils; -import org.junit.Assume; -import org.junit.Before; -import org.junit.Test; - import org.apache.hugegraph.HugeException; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.backend.BackendException; @@ -71,6 +57,8 @@ import org.apache.hugegraph.schema.Userdata; import org.apache.hugegraph.schema.VertexLabel; import org.apache.hugegraph.structure.HugeElement; import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.testutil.FakeObjects; +import org.apache.hugegraph.testutil.Utils; import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.traversal.optimize.ConditionP; import org.apache.hugegraph.traversal.optimize.Text; @@ -82,6 +70,18 @@ import org.apache.hugegraph.type.define.WriteType; import org.apache.hugegraph.util.Blob; import org.apache.hugegraph.util.CollectionUtil; import org.apache.hugegraph.util.LongEncoding; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; +import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -2778,19 +2778,20 @@ public class VertexCoreTest extends BaseCoreTest { // Query all with limit after delete graph.traversal().V().limit(6).drop().iterate(); - Assert.assertThrows(IllegalArgumentException.class, () -> { - // Query count with uncommitted records - graph.traversal().V().count().next(); - }); + // Query count with uncommitted records + graph.traversal().V().count().next(); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with limit graph.traversal().V().limit(3).toList(); }); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with offset graph.traversal().V().range(1, -1).toList(); }); + // Query all with limit after delete once this.commitTx(); Assert.assertEquals(4L, graph.traversal().V().count().next()); List vertices = graph.traversal().V().limit(3).toList(); @@ -2813,21 +2814,30 @@ public class VertexCoreTest extends BaseCoreTest { "name", "test", "age", 88, "lived", "California"); - Assert.assertThrows(IllegalArgumentException.class, () -> { - // Query count with uncommitted records - graph.traversal().V().count().next(); - }); + // Query count with uncommitted records + Assert.assertEquals(11L, graph.traversal().V().count().next()); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with limit graph.traversal().V().limit(3).toList(); }); + Assert.assertThrows(IllegalArgumentException.class, () -> { // Query with offset graph.traversal().V().range(1, -1).toList(); }); + // Do commit this.commitTx(); + + // Query count Assert.assertEquals(11L, graph.traversal().V().count().next()); + + // Query with limit + Assert.assertEquals(3L, graph.traversal().V().limit(3).toList().size()); + + // Query with offset + Assert.assertEquals(10L, graph.traversal().V().range(1, -1).toList().size()); } @Test @@ -5968,7 +5978,7 @@ public class VertexCoreTest extends BaseCoreTest { @Test public void testQueryByUniqueIndex() { - HugeGraph graph = this.graph(); + HugeGraph graph = graph(); SchemaManager schema = graph.schema(); schema.vertexLabel("node") .properties("name")