keksmd commented on code in PR #968: URL: https://github.com/apache/incubator-graphar/pull/968#discussion_r3966509368
########## maven-projects/core/src/main/java/org/apache/graphar/core/EdgeRange.java: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.graphar.core; + +/** A validated half-open range of edge rows within one GraphAr vertex partition. */ +public final class EdgeRange { + private final long begin; + private final long end; + + private EdgeRange(long begin, long end) { + this.begin = begin; + this.end = end; + } + + /** + * Creates an edge range from the two adjacent values of an ordered offset table. + * + * @param begin the first included edge row + * @param end the first excluded edge row + * @return the validated half-open edge range + */ + public static EdgeRange fromOffsets(long begin, long end) { + if (begin < 0) { + throw new IllegalArgumentException("Edge range begin must be non-negative: " + begin); + } + if (end < begin) { + throw new IllegalArgumentException( + "Offset values must be monotonic: begin=" + begin + ", end=" + end); + } + return new EdgeRange(begin, end); + } + + /** + * Returns the first included edge row. + * + * @return the first included edge row + */ + public long begin() { + return begin; + } + + /** + * Returns the first excluded edge row. + * + * @return the first excluded edge row + */ + public long end() { + return end; + } + + /** + * Returns the number of selected edge rows. + * + * @return the number of selected edge rows + */ + public long length() { + return end - begin; + } + + /** + * Returns whether this range selects no edge rows. + * + * @return whether the range is empty + */ + public boolean isEmpty() { + return begin == end; + } + + /** + * Returns the half-open range of edge chunks intersecting this edge range. + * + * @param edgeChunkSize a positive number of edge rows per chunk + * @return the chunk range intersecting this edge range + */ + public ChunkRange edgeChunks(long edgeChunkSize) { + ChunkMath.validateChunkSize(edgeChunkSize); Review Comment: Done in 2412d03 — `EdgeRange.edgeChunks` now calls `ChunkMath.chunkIndex(begin, size)` and `ChunkMath.chunkCount(end, size)` instead of repeating the division, so the arithmetic (and its validation) lives in one place. ########## maven-projects/core/src/main/java/org/apache/graphar/core/ChunkRange.java: ########## Review Comment: Added in 2412d03: `equals`/`hashCode`/`toString` on `ChunkRange`, `EdgeRange` and `OffsetLocation`. The fixture test now asserts whole values (`assertEquals(new ChunkRange(0, 2), resolved.edgeChunks())`) instead of begin/end field by field. `ResolvedAdjacency` got `toString` only — it holds an `EdgeInfo`, which has no value equality, so equals there would not be well defined. ########## maven-projects/core/src/main/java/org/apache/graphar/core/OrderedAdjacencyResolver.java: ########## @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.graphar.core; + +import java.net.URI; +import java.util.Objects; +import org.apache.graphar.info.EdgeInfo; +import org.apache.graphar.info.type.AdjListType; + +/** Resolves GraphAr ordered adjacency metadata into offset and edge-chunk locations. */ +public final class OrderedAdjacencyResolver { + private final EdgeInfo edgeInfo; + private final AdjListType adjListType; + private final long vertexChunkSize; + + public OrderedAdjacencyResolver(EdgeInfo edgeInfo, AdjListType adjListType) { + this.edgeInfo = Objects.requireNonNull(edgeInfo, "Edge info cannot be null."); + this.adjListType = + Objects.requireNonNull(adjListType, "Adjacency list type cannot be null."); + if (!adjListType.isOrdered()) { + throw new IllegalArgumentException( + "An ordered adjacency resolver requires an ordered layout: " + adjListType); + } + if (!edgeInfo.hasAdjListType(adjListType)) { + throw new IllegalArgumentException( + "Edge info does not declare adjacency layout: " + adjListType); + } + this.vertexChunkSize = + adjListType == AdjListType.ordered_by_source + ? edgeInfo.getSrcChunkSize() + : edgeInfo.getDstChunkSize(); + ChunkMath.validateChunkSize(vertexChunkSize); + ChunkMath.validateChunkSize(edgeInfo.getChunkSize()); + } + + /** Locates the offset pair that the physical reader must fetch for {@code vertexId}. */ + public OffsetLocation locate(long vertexId) { + long vertexChunkIndex = ChunkMath.chunkIndex(vertexId, vertexChunkSize); + long offsetIndex = ChunkMath.offsetInChunk(vertexId, vertexChunkSize); + URI offsetChunkUri = edgeInfo.getOffsetChunkUri(adjListType, vertexChunkIndex); + return new OffsetLocation(vertexId, vertexChunkIndex, offsetIndex, offsetChunkUri); + } + + /** Combines a vertex location and its two ordered-offset values into exact edge chunks. */ + public ResolvedAdjacency resolve(long vertexId, long offsetBegin, long offsetEnd) { + OffsetLocation offsetLocation = locate(vertexId); + EdgeRange edgeRange = EdgeRange.fromOffsets(offsetBegin, offsetEnd); + return resolved(offsetLocation, edgeRange); + } + + /** Resolves a vertex using a complete, validated offset chunk read by a physical backend. */ + public ResolvedAdjacency resolve(long vertexId, OffsetChunk offsetChunk) { + OffsetLocation offsetLocation = locate(vertexId); + Objects.requireNonNull(offsetChunk, "Offset chunk cannot be null."); Review Comment: Fixed in 2412d03, and the null check was only half the problem. `OffsetChunk.of` now takes the vertex chunk index it was read from and exposes `vertexChunkIndex()`; `resolve(vertexId, offsetChunk)` does `requireNonNull` first, then rejects a chunk whose index does not match the located vertex chunk. Before this, passing an offset chunk from another vertex chunk returned a plausible but wrong edge range instead of failing. New test `rejectsAnOffsetChunkReadFromAnotherVertexChunk` pins it. The API is unreleased, so I changed the factory rather than adding an overload. -- 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]
