diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java index 0c962b11a2..ff11685636 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java @@ -775,6 +775,12 @@ protected Iterator queryVerticesByIds(Object[] vertexIds, boolean adjace boolean checkMustExist, HugeType type) { Query.checkForceCapacity(vertexIds.length); + if (vertexIds.length == 1) { + // Fast path: skip the id list, map and mapper iterator for one id + return this.queryVertexById(vertexIds[0], adjacentVertex, + checkMustExist, type); + } + // NOTE: allowed duplicated vertices if query by duplicated ids List ids = InsertionOrderUtil.newList(); Map vertices = new HashMap<>(vertexIds.length); @@ -808,22 +814,67 @@ protected Iterator queryVerticesByIds(Object[] vertexIds, boolean adjace } return new MapperIterator<>(ids.iterator(), id -> { - HugeVertex vertex = vertices.get(id); + return this.resolveVertex(vertices.get(id), id, + adjacentVertex, checkMustExist); + }); + } + + /** + * Query a single vertex by id, with the same semantics as the multi-id + * path of {@link #queryVerticesByIds(Object[], boolean, boolean, HugeType)} + * but without allocating the id list, the result map and the mapper + * iterator; only an {@link IdQuery.OneIdQuery} is created on a miss. + */ + private Iterator queryVertexById(Object vertexId, boolean adjacentVertex, + boolean checkMustExist, HugeType type) { + Id id = HugeVertex.getIdValue(vertexId); + if (id == null) { + return QueryResults.emptyIterator(); + } + + HugeVertex vertex = null; + if (this.verticesInTxSize() > 0) { + if (this.removedVertices.containsKey(id)) { + // The record has been deleted + return QueryResults.emptyIterator(); + } + vertex = this.addedVertices.get(id); if (vertex == null) { - if (checkMustExist) { - throw new NotFoundException( - "Vertex '%s' does not exist", id); - } else if (adjacentVertex) { - assert !checkMustExist; - // Return undefined if adjacentVertex but !checkMustExist - vertex = HugeVertex.undefined(this.graph(), id); - } else { - // Return null - assert vertex == null; - } + vertex = this.updatedVertices.get(id); } + if (vertex != null && vertex.expired()) { + // Found from local tx but expired + return QueryResults.emptyIterator(); + } + } + + if (vertex == null) { + // Query from backend store + IdQuery query = new IdQuery.OneIdQuery(type, id); + vertex = QueryResults.one(this.queryVerticesFromBackend(query)); + } + + vertex = this.resolveVertex(vertex, id, adjacentVertex, checkMustExist); + if (vertex == null) { + return QueryResults.emptyIterator(); + } + return QueryResults.iterator(vertex); + } + + private HugeVertex resolveVertex(HugeVertex vertex, Id id, + boolean adjacentVertex, boolean checkMustExist) { + if (vertex != null) { return vertex; - }); + } + if (checkMustExist) { + throw new NotFoundException("Vertex '%s' does not exist", id); + } + if (adjacentVertex) { + // Return undefined if adjacentVertex but !checkMustExist + return HugeVertex.undefined(this.graph(), id); + } + // Return null to skip the vertex + return null; } public Iterator queryVertices() { @@ -934,6 +985,11 @@ protected Iterator queryEdgesByIds(Object[] edgeIds, boolean verifyId) { Query.checkForceCapacity(edgeIds.length); + if (edgeIds.length == 1) { + // Fast path: skip the id list, map and mapper iterator for one id + return this.queryEdgeById(edgeIds[0], verifyId); + } + // NOTE: allowed duplicated edges if query by duplicated ids List ids = InsertionOrderUtil.newList(); Map edges = new HashMap<>(edgeIds.length); @@ -991,6 +1047,50 @@ protected Iterator queryEdgesByIds(Object[] edgeIds, }); } + /** + * Query a single edge by id, with the same semantics as the multi-id + * path of {@link #queryEdgesByIds(Object[], boolean)} but without + * allocating the id list, the result map and the mapper iterator; only + * an {@link IdQuery.OneIdQuery} is created on a miss. + */ + private Iterator queryEdgeById(Object edgeId, boolean verifyId) { + EdgeId id = HugeEdge.getIdValue(edgeId, !verifyId); + if (id == null) { + return QueryResults.emptyIterator(); + } + if (id.direction() == Directions.IN) { + id = id.switchDirection(); + } + + if (this.edgesInTxSize() > 0) { + if (this.removedEdges.containsKey(id)) { + // The record has been deleted + return QueryResults.emptyIterator(); + } + HugeEdge edge = this.addedEdges.get(id); + if (edge == null) { + edge = this.updatedEdges.get(id); + } + if (edge != null) { + // Found from local tx + if (edge.expired()) { + return QueryResults.emptyIterator(); + } + return QueryResults.iterator(edge); + } + } + + /* + * Query from backend store and return the results directly, just + * like the multi-id path does when there is no local edge. + */ + IdQuery query = new IdQuery.OneIdQuery(HugeType.EDGE, id); + Iterator it = this.queryEdgesFromBackend(query); + @SuppressWarnings({"unchecked", "rawtypes"}) + Iterator r = (Iterator) it; + return r; + } + public Iterator queryEdges() { Query q = new Query(HugeType.EDGE); return this.queryEdges(q); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java index cbb2b7d043..238e5a6eb0 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/EdgeCoreTest.java @@ -23,6 +23,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; @@ -33,6 +34,7 @@ import org.apache.hugegraph.HugeException; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.backend.BackendException; +import org.apache.hugegraph.backend.id.EdgeId; import org.apache.hugegraph.backend.id.Id; import org.apache.hugegraph.backend.id.IdGenerator; import org.apache.hugegraph.backend.page.PageInfo; @@ -46,6 +48,7 @@ import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.exception.LimitExceedException; import org.apache.hugegraph.exception.NoIndexException; +import org.apache.hugegraph.exception.NotFoundException; import org.apache.hugegraph.schema.SchemaManager; import org.apache.hugegraph.schema.Userdata; import org.apache.hugegraph.structure.HugeEdge; @@ -2727,6 +2730,176 @@ public void testQueryEdgesByIdWithGraphAPIAndNotCommittedRemoved() { Assert.assertTrue(graph.edges(edge1.id(), edge2.id()).hasNext()); } + @Test + public void testQueryEdgesByIdsWithLocalAndDuplicateIds() { + HugeGraph graph = graph(); + init18Edges(); + + Object id1 = graph.traversal().E().toList().get(0).id(); + // Added but not committed + Vertex james = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Vertex book = graph.addVertex(T.label, "book", "name", "c-book"); + Edge local = james.addEdge("authored", book, "score", 5); + Object id2 = local.id(); + + List edges = ImmutableList.copyOf(graph.edges(id2, id1, id2)); + Assert.assertEquals(3, edges.size()); + Assert.assertSame(local, edges.get(0)); + Assert.assertEquals(id1, edges.get(1).id()); + Assert.assertSame(local, edges.get(2)); + + graph.tx().rollback(); + } + + @Test + public void testQuerySingleEdgeByIdInLocalTx() { + HugeGraph graph = graph(); + init18Edges(); + + // Added but not committed + Vertex james = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Vertex book = graph.addVertex(T.label, "book", "name", "c-book"); + Edge edge = james.addEdge("authored", book, "score", 5); + Object id = edge.id(); + + List edges = ImmutableList.copyOf(graph.edges(id)); + Assert.assertEquals(1, edges.size()); + Assert.assertSame(edge, edges.get(0)); + Assert.assertSame(edge, graph.edge(id)); + graph.tx().commit(); + + // Updated but not committed + edge = graph.edge(id); + edge.property("score", 6); + + edges = ImmutableList.copyOf(graph.edges(id)); + Assert.assertEquals(1, edges.size()); + Assert.assertSame(edge, edges.get(0)); + Assert.assertEquals(6, (int) graph.edge(id).value("score")); + graph.tx().rollback(); + + Assert.assertEquals(5, (int) graph.edge(id).value("score")); + } + + @Test + public void testQuerySingleEdgeByIdRemovedInLocalTx() { + HugeGraph graph = graph(); + init18Edges(); + + Edge edge = graph.traversal().E().toList().get(0); + Object id = edge.id(); + Assert.assertTrue(graph.edges(id).hasNext()); + + edge.remove(); + Assert.assertFalse(graph.edges(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + graph.tx().rollback(); + Assert.assertTrue(graph.edges(id).hasNext()); + Assert.assertEquals(id, graph.edge(id).id()); + } + + @Test + public void testQuerySingleEdgeByIdNotFound() { + HugeGraph graph = graph(); + init18Edges(); + + String id = graph.traversal().E().toList().get(0).id() + "-not-exist"; + Assert.assertFalse(graph.edges(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + } + + @Test + public void testQuerySingleEdgeByInvalidId() { + HugeGraph graph = graph(); + init18Edges(); + + // Invalid id is skipped by edges() and rejected by edge() + Assert.assertFalse(graph.edges("invalid-edge-id").hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge("invalid-edge-id"); + }, e -> { + Assert.assertContains("Edge id must be formatted", e.getMessage()); + }); + + Assert.assertFalse(graph.edges((Object) null).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(null); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + } + + @Test + public void testQuerySingleEdgeByIdWithInDirection() { + HugeGraph graph = graph(); + init18Edges(); + + HugeEdge edge = (HugeEdge) graph.traversal().E().toList().get(0); + EdgeId inId = edge.idWithDirection().switchDirection(); + Assert.assertEquals(Directions.IN, inId.direction()); + + List edges = ImmutableList.copyOf(graph.edges(inId)); + Assert.assertEquals(1, edges.size()); + Assert.assertEquals(edge.id(), edges.get(0).id()); + Assert.assertEquals(edge.id(), graph.edge(inId).id()); + + edges = ImmutableList.copyOf(graph.edges(inId.asString())); + Assert.assertEquals(1, edges.size()); + Assert.assertEquals(edge.id(), edges.get(0).id()); + } + + @Test + public void testQuerySingleEdgeByIdExpiredInLocalTx() { + HugeGraph graph = graph(); + + Vertex baby = graph.addVertex(T.label, "person", "name", "Baby", + "age", 3, "city", "Beijing"); + Vertex java = graph.addVertex(T.label, "book", + "name", "Java in action"); + Edge edge = baby.addEdge("read", java, "place", "library of school", + "date", "2019-12-23 12:00:00"); + Object id = edge.id(); + graph.tx().commit(); + + edge = graph.edge(id); + Assert.assertTrue(graph.edges(id).hasNext()); + graph.tx().rollback(); + + try { + Thread.sleep(3100L); + } catch (InterruptedException e) { + // Ignore + } + + // Update the expired edge in a new tx (not committed) + edge.property("place", "home"); + Map updated = Whitebox.getInternalState( + params().graphTransaction(), "updatedEdges"); + Assert.assertTrue(updated.containsKey(edge.id())); + Assert.assertTrue(((HugeEdge) edge).expired()); + + Assert.assertFalse(graph.edges(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.edge(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + graph.tx().rollback(); + } + @Test public void testQueryEdgesByIdNotFound() { HugeGraph graph = graph(); @@ -3322,6 +3495,49 @@ public void testQueryAdjacentVerticesOfEdgesWithoutVertex() } } + @Test + public void testQueryAdjacentVertexRemovedInLocalTx() + throws InterruptedException, ExecutionException { + HugeGraph graph = graph(); + + Vertex james = graph.addVertex(T.label, "author", "id", 1, + "name", "James Gosling", "age", 62, + "lived", "Canadian"); + Vertex java = graph.addVertex(T.label, "book", "name", "java"); + james.addEdge("authored", java, "score", 3); + graph.tx().commit(); + params().graphEventHub().notify(Events.CACHE, "clear", null).get(); + + Edge edge = graph.traversal().V(james.id()).outE().next(); + HugeVertex adjacent = (HugeVertex) edge.inVertex(); + Assert.assertFalse(adjacent.isPropLoaded()); + Assert.assertEquals("book", adjacent.label()); + + // Remove the adjacent vertex but don't commit + graph.vertex(java.id()).remove(); + Assert.assertFalse(graph.vertices(java.id()).hasNext()); + Assert.assertFalse(graph.adjacentVertex(java.id()).hasNext()); + // Querying by one id must agree with querying by multiple ids + List vertices = ImmutableList.copyOf( + params().graphTransaction() + .queryAdjacentVertices(java.id(), james.id())); + Assert.assertEquals(1, vertices.size()); + Assert.assertEquals(james.id(), vertices.get(0).id()); + + /* + * Loading the adjacent vertex of an edge held before the removal + * must not turn it into an undefined vertex + */ + adjacent.forceLoad(); + Assert.assertFalse(adjacent.schemaLabel().undefined()); + Assert.assertEquals("book", adjacent.label()); + + graph.tx().rollback(); + Assert.assertTrue(graph.vertices(java.id()).hasNext()); + Assert.assertEquals("book", graph.adjacentVertex(java.id()) + .next().label()); + } + @Test public void testQueryAdjacentVerticesOfEdgesWithInvalidVertexLabel() throws InterruptedException, ExecutionException { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java index 9aa144542e..6feceee51b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/VertexCoreTest.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; import java.util.Random; @@ -52,11 +53,13 @@ import org.apache.hugegraph.exception.LimitExceedException; import org.apache.hugegraph.exception.NoIndexException; import org.apache.hugegraph.exception.NotAllowException; +import org.apache.hugegraph.exception.NotFoundException; import org.apache.hugegraph.schema.PropertyKey; import org.apache.hugegraph.schema.SchemaManager; import org.apache.hugegraph.schema.Userdata; import org.apache.hugegraph.schema.VertexLabel; import org.apache.hugegraph.structure.HugeElement; +import org.apache.hugegraph.structure.HugeVertex; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.FakeObjects; import org.apache.hugegraph.testutil.Utils; @@ -3188,6 +3191,174 @@ public void testQueryByIdWithGraphAPIAndNotCommittedRemoved() { Assert.assertTrue(graph.vertices(vertex1.id(), vertex2.id()).hasNext()); } + @Test + public void testQueryVerticesByNonConsecutiveDuplicateIds() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + List all = graph.traversal().V().toList(); + Object id1 = all.get(0).id(); + Object id2 = all.get(1).id(); + + List vertices = ImmutableList.copyOf(graph.vertices(id1, id2, id1)); + Assert.assertEquals(3, vertices.size()); + Assert.assertEquals(id1, vertices.get(0).id()); + Assert.assertEquals(id2, vertices.get(1).id()); + Assert.assertEquals(id1, vertices.get(2).id()); + } + + @Test + public void testQueryVerticesByIdsWithLocalAndDuplicateIds() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Object id1 = graph.traversal().V().toList().get(0).id(); + // Added but not committed + Vertex local = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Object id2 = local.id(); + + List vertices = ImmutableList.copyOf(graph.vertices(id2, id1, id2)); + Assert.assertEquals(3, vertices.size()); + Assert.assertSame(local, vertices.get(0)); + Assert.assertEquals(id1, vertices.get(1).id()); + Assert.assertSame(local, vertices.get(2)); + + graph.tx().rollback(); + } + + @Test + public void testQuerySingleVertexByIdInLocalTx() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + // Added but not committed + Vertex vertex = graph.addVertex(T.label, "author", "id", 3, + "name", "Dennis Ritchie", "age", 70, + "lived", "New York"); + Object id = vertex.id(); + + List vertices = ImmutableList.copyOf(graph.vertices(id)); + Assert.assertEquals(1, vertices.size()); + Assert.assertSame(vertex, vertices.get(0)); + Assert.assertSame(vertex, graph.vertex(id)); + Assert.assertSame(vertex, graph.adjacentVertex(id).next()); + this.commitTx(); + + // Updated but not committed + vertex = graph.vertex(id); + vertex.property("age", 71); + + vertices = ImmutableList.copyOf(graph.vertices(id)); + Assert.assertEquals(1, vertices.size()); + Assert.assertSame(vertex, vertices.get(0)); + Assert.assertEquals(71, (int) graph.vertex(id).value("age")); + graph.tx().rollback(); + + Assert.assertEquals(70, (int) graph.vertex(id).value("age")); + } + + @Test + public void testQuerySingleVertexByIdRemovedInLocalTx() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Vertex vertex = graph.traversal().V().hasLabel("author") + .has("id", 1).next(); + Object id = vertex.id(); + Assert.assertTrue(graph.vertices(id).hasNext()); + Assert.assertTrue(graph.adjacentVertex(id).hasNext()); + + vertex.remove(); + Assert.assertFalse(graph.vertices(id).hasNext()); + // Removed vertex must not be reported as an undefined adjacent vertex + Assert.assertFalse(graph.adjacentVertex(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + graph.tx().rollback(); + Assert.assertTrue(graph.vertices(id).hasNext()); + Assert.assertTrue(graph.adjacentVertex(id).hasNext()); + } + + @Test + public void testQuerySingleVertexByIdNotFound() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Id id = SplicingIdGenerator.splicing("author", "not-exists-id"); + Assert.assertFalse(graph.vertices(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + // Adjacent vertex not found is returned as an undefined vertex + // (vertex.check_adjacent_vertex_exist=false, the default) + List vertices = ImmutableList.copyOf(graph.adjacentVertex(id)); + Assert.assertEquals(1, vertices.size()); + Assert.assertEquals(id, vertices.get(0).id()); + Assert.assertEquals("~undefined", vertices.get(0).label()); + } + + @Test + public void testQuerySingleVertexByNullId() { + HugeGraph graph = graph(); + this.init10VerticesAndCommit(); + + Assert.assertFalse(graph.vertices((Object) null).hasNext()); + Assert.assertFalse(graph.adjacentVertex(null).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(null); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + + // Same as querying by multiple ids + Assert.assertFalse(graph.vertices(null, null).hasNext()); + } + + @Test + public void testQuerySingleVertexByIdExpiredInLocalTx() { + HugeGraph graph = graph(); + + Vertex vertex = graph.addVertex(T.label, "fan", "name", "Baby", + "age", 3, "city", "Beijing"); + Object id = vertex.id(); + this.commitTx(); + + vertex = graph.vertex(id); + Assert.assertTrue(graph.vertices(id).hasNext()); + graph.tx().rollback(); + + try { + Thread.sleep(3100L); + } catch (InterruptedException e) { + // Ignore + } + + // Update the expired vertex in a new tx (not committed) + vertex.property("age", 4); + Map updated = Whitebox.getInternalState( + params().graphTransaction(), "updatedVertices"); + Assert.assertTrue(updated.containsKey(vertex.id())); + Assert.assertTrue(((HugeVertex) vertex).expired()); + + Assert.assertFalse(graph.vertices(id).hasNext()); + Assert.assertFalse(graph.adjacentVertex(id).hasNext()); + Assert.assertThrows(NotFoundException.class, () -> { + graph.vertex(id); + }, e -> { + Assert.assertContains("does not exist", e.getMessage()); + }); + graph.tx().rollback(); + } + @Test public void testQueryByInvalidSysprop() { HugeGraph graph = graph();