Skip to content

Commit

Permalink
fix(interactive): Fix bug in computing path length in GIE (#3172)
Browse files Browse the repository at this point in the history 
<!--
Thanks for your contribution! please review
https://github.com/alibaba/GraphScope/blob/main/CONTRIBUTING.md before
opening an issue.
-->

## What do these changes do?

<!-- Please give a short brief about these changes. -->

As titled.

## Related issue number

<!-- Are there any issues opened that will be resolved by merging this
change? -->

Fixes

Co-authored-by: Longbin Lai <longbin.lailb@alibaba-inc.com>
  • Loading branch information
@BingqingLyu @longbinlai
BingqingLyu and longbinlai committed Sep 2, 2023
1 parent 465877d commit ae00929
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 33 deletions.
62 changes: 45 additions & 17 deletions interactive_engine/executor/ir/graph_proxy/src/apis/graph/element/path.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,26 @@ impl VertexOrEdge {
}
}

pub fn is_vertex(&self) -> bool {
match self {
VertexOrEdge::V(_) => true,
_ => false,
}
}

pub fn as_edge(&self) -> Option<&Edge> {
match self {
VertexOrEdge::E(e) => Some(e),
_ => None,
}
}

pub fn is_edge(&self) -> bool {
match self {
VertexOrEdge::E(_) => true,
_ => false,
}
}
}

#[derive(Clone, Debug)]
Expand All @@ -73,7 +87,7 @@ pub enum GraphPath {
/// Arbitrary path with only end vertices preserved, which may contain both vertices and edges, or only vertices.
EndV((VertexOrEdge, usize)),
/// Simple path with only end vertex preserved, which may contains both vertices and edges, or only vertices.
SimpleEndV((VertexOrEdge, Vec<ID>)),
SimpleEndV((VertexOrEdge, Vec<ID>, usize)),
}

impl GraphPath {
Expand All @@ -86,7 +100,7 @@ impl GraphPath {
pb::path_expand::PathOpt::Simple => {
let entry = entry.into();
let id = entry.id();
GraphPath::SimpleEndV((entry, vec![id]))
GraphPath::SimpleEndV((entry, vec![id], 1))
}
},
pb::path_expand::ResultOpt::AllV | pb::path_expand::ResultOpt::AllVE => match path_opt {
Expand Down Expand Up @@ -114,16 +128,23 @@ impl GraphPath {
}
GraphPath::EndV((ref mut e, ref mut weight)) => {
*e = entry.into();
*weight += 1;
// we only increase the weight when the entry is a vertex.
if e.is_vertex() {
*weight += 1;
}
true
}
GraphPath::SimpleEndV((ref mut e, ref mut path)) => {
GraphPath::SimpleEndV((ref mut e, ref mut path, ref mut weight)) => {
let entry = entry.into();
if path.contains(&entry.id()) {
false
} else {
path.push(entry.id());
*e = entry.into();
*e = entry;
// we only increase the weight when the entry is a vertex.
if e.is_vertex() {
*weight += 1;
}
true
}
}
Expand All @@ -133,7 +154,7 @@ impl GraphPath {
pub fn get_path_end(&self) -> &VertexOrEdge {
match self {
GraphPath::AllPath(ref p) | GraphPath::SimpleAllPath(ref p) => p.last().unwrap(),
GraphPath::EndV((ref e, _)) | GraphPath::SimpleEndV((ref e, _)) => e,
GraphPath::EndV((ref e, _)) | GraphPath::SimpleEndV((ref e, _, _)) => e,
}
}

Expand Down Expand Up @@ -206,9 +227,14 @@ impl Element for GraphPath {
// the path len is the number of edges in the path;
fn len(&self) -> usize {
match self {
GraphPath::AllPath(p) | GraphPath::SimpleAllPath(p) => p.len() - 1,
GraphPath::AllPath(p) | GraphPath::SimpleAllPath(p) => {
p.iter()
.filter(|v_or_e| v_or_e.is_vertex())
.count()
- 1
}
GraphPath::EndV((_, weight)) => *weight - 1,
GraphPath::SimpleEndV((_, p)) => p.len() - 1,
GraphPath::SimpleEndV((_, _, weight)) => *weight - 1,
}
}

Expand Down Expand Up @@ -247,9 +273,9 @@ impl PartialEq for GraphPath {
| (GraphPath::SimpleAllPath(p1), GraphPath::AllPath(p2))
| (GraphPath::SimpleAllPath(p1), GraphPath::SimpleAllPath(p2)) => p1.eq(p2),
(GraphPath::EndV((p1, _)), GraphPath::EndV((p2, _)))
| (GraphPath::EndV((p1, _)), GraphPath::SimpleEndV((p2, _)))
| (GraphPath::SimpleEndV((p1, _)), GraphPath::EndV((p2, _)))
| (GraphPath::SimpleEndV((p1, _)), GraphPath::SimpleEndV((p2, _))) => p1.eq(p2),
| (GraphPath::EndV((p1, _)), GraphPath::SimpleEndV((p2, _, _)))
| (GraphPath::SimpleEndV((p1, _, _)), GraphPath::EndV((p2, _)))
| (GraphPath::SimpleEndV((p1, _, _)), GraphPath::SimpleEndV((p2, _, _))) => p1.eq(p2),
_ => false,
}
}
Expand All @@ -263,9 +289,9 @@ impl PartialOrd for GraphPath {
| (GraphPath::SimpleAllPath(p1), GraphPath::AllPath(p2))
| (GraphPath::SimpleAllPath(p1), GraphPath::SimpleAllPath(p2)) => p1.partial_cmp(p2),
(GraphPath::EndV((p1, _)), GraphPath::EndV((p2, _)))
| (GraphPath::EndV((p1, _)), GraphPath::SimpleEndV((p2, _)))
| (GraphPath::SimpleEndV((p1, _)), GraphPath::EndV((p2, _)))
| (GraphPath::SimpleEndV((p1, _)), GraphPath::SimpleEndV((p2, _))) => p1.partial_cmp(p2),
| (GraphPath::EndV((p1, _)), GraphPath::SimpleEndV((p2, _, _)))
| (GraphPath::SimpleEndV((p1, _, _)), GraphPath::EndV((p2, _)))
| (GraphPath::SimpleEndV((p1, _, _)), GraphPath::SimpleEndV((p2, _, _))) => p1.partial_cmp(p2),
_ => None,
}
}
Expand Down Expand Up @@ -320,10 +346,11 @@ impl Encode for GraphPath {
writer.write_u8(2)?;
path.write_to(writer)?;
}
GraphPath::SimpleEndV((path_end, path)) => {
GraphPath::SimpleEndV((path_end, path, weight)) => {
writer.write_u8(3)?;
path_end.write_to(writer)?;
path.write_to(writer)?;
writer.write_u64(*weight as u64)?;
}
}
Ok(())
Expand All @@ -350,7 +377,8 @@ impl Decode for GraphPath {
3 => {
let vertex_or_edge = <VertexOrEdge>::read_from(reader)?;
let path = <Vec<ID>>::read_from(reader)?;
Ok(GraphPath::SimpleEndV((vertex_or_edge, path)))
let weight = <u64>::read_from(reader)? as usize;
Ok(GraphPath::SimpleEndV((vertex_or_edge, path, weight)))
}
_ => Err(std::io::Error::new(std::io::ErrorKind::Other, "unreachable")),
}
Expand Down Expand Up @@ -392,7 +420,7 @@ impl Hash for GraphPath {
fn hash<H: Hasher>(&self, state: &mut H) {
match self {
GraphPath::AllPath(p) | GraphPath::SimpleAllPath(p) => p.hash(state),
GraphPath::EndV((e, _)) | GraphPath::SimpleEndV((e, _)) => e.hash(state),
GraphPath::EndV((e, _)) | GraphPath::SimpleEndV((e, _, _)) => e.hash(state),
}
}
}
Expand Down
Loading

0 comments on commit ae00929

Please sign in to comment.