[COR-56] Unify VertexRef #22165
[COR-56] Unify VertexRef #22165
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| // need a mechanism here as well to distinguish between (non)required | ||
| // parameters. | ||
| callback(Step{to, edge, previous, fetchedType, step.getDepth() + 1, | ||
| callback(Step{VertexRef{to}, VertexRef{edge}, previous, fetchedType, |
There was a problem hiding this comment.
Bug: Edge ID incorrectly wrapped in VertexRef type
The edge variable (an edge ID of type EdgeType/HashedStringRef) is incorrectly wrapped in VertexRef{edge}. The ClusterProviderStep constructor expects EdgeType for the second parameter, not VertexRef. While this currently works due to the implicit conversion operator in VertexRef, it semantically treats an edge ID as a vertex reference. Only to (the vertex ID) should be wrapped in VertexRef; edge should be passed directly.
jvolmer
left a comment
jvolmer
left a comment
There was a problem hiding this comment.
Two small include improvements and the cursor-comment above looks legitimate. Otherwise, LGTM
| using VertexSet = arangodb::containers::HashSet<VertexRef, std::hash<VertexRef>, | ||
| std::equal_to<VertexRef>>; | ||
|
|
There was a problem hiding this comment.
should probably be exchanged with #include "Graph/Types/VertexSet.h"
| using VertexSet = arangodb::containers::HashSet<VertexRef, std::hash<VertexRef>, | ||
| std::equal_to<VertexRef>>; | ||
|
|
markuspf
force-pushed
the
chore/unify-vertex-ref
branch
from
94c2ae2 to
4e40c07
Compare
Scope & Purpose
Use the same definition of
VertexRefthroughout;Before this patch, there were various places in the codebase which used
using VertexRef = HashedStringRef, as well asSteps in the traverser code wrapping thisHashedStringRefinto a helper class, which is identical for every Step type.This PR introduces a class
VertexRefwhich replaces all these uses consistently (and is currently only a wrapper aroundHashedStringRefRequires Enterprise PR: https://github.com/arangodb/enterprise/pull/1563
Note
Introduces a unified
VertexRefwrapper (overHashedStringRef) and replaces ad-hoc vertex ID types/usages across graph components, updating APIs, data structures, and tests.graph::VertexRef(Graph/Types/VertexRef.{h,cpp}) andVertexSet(Graph/Types/VertexSet.h); migrateForbiddenVertices.hto use them.HashedStringRefvertex IDs withVertexRefin public methods (e.g.,reset,startVertex,addVertexToBuilder) across executors (TraversalExecutor,ShortestPathExecutor,EnumeratePathsExecutor).VertexRef; switch internal maps/sets to key byVertexRef.SingleServerProvider,ClusterProvider,ProviderTracer, and step types to store/returnVertexRefand propagate through expansion/builders.PathResultandSingleProviderPathResultto store vertices asVertexRef; update memory accounting and VPack emission;PathValidator*now usesVertexReffor uniqueness/forbidden checks.VertexRef.Written by Cursor Bugbot for commit 2cdb2fe. This will update automatically on new commits. Configure here.