Skip to content

Commit 94eaf9c

Browse files
[Bug Fix] fix some bugs found in cypher e2e test (#2579)
<!-- 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. --> Several bugs fixed, including: * In schema 1. modern graph schema with a wrong label id; * In IrCore 1. in pattern matching, when generating source opr, assign the proper table info. 2. when processing PathExpand in pattern, throw the unsupported error for PathExpand ranging from 0, only when it is in the Intersection * In runtime 1. do not throw error when GetV Opt is not `EndV` for path anymore, as this can be either `StartV` or `EndV` for now, according to the direction of EdgeExpand in PathExpand's ExpandBase. ## Related issue number <!-- Are there any issues opened that will be resolved by merging this change? --> Fixes --------- Co-authored-by: MeloYang <[email protected]>
1 parent 51eeb99 commit 94eaf9c

File tree

5 files changed

+66
-32
lines changed

5 files changed

+66
-32
lines changed

interactive_engine/executor/ir/core/resource/modern_schema.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090
"name": "person"
9191
},
9292
"dst": {
93-
"id": 0,
93+
"id": 1,
9494
"name": "software"
9595
}
9696
}

interactive_engine/executor/ir/core/resource/modern_schema_pk.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090
"name": "person"
9191
},
9292
"dst": {
93-
"id": 0,
93+
"id": 1,
9494
"name": "software"
9595
}
9696
}

interactive_engine/executor/ir/core/src/glogue/pattern.rs

+17-15
Original file line numberDiff line numberDiff line change
@@ -647,18 +647,31 @@ fn generate_source_operator(
647647
pattern: &Pattern, source_extend: &ExactExtendStep,
648648
) -> IrPatternResult<pb::logical_plan::Operator> {
649649
let source_vertex_id = source_extend.get_target_vertex_id();
650-
let source_vertex_table = generate_vertex_table(pattern, source_vertex_id)?;
650+
let inferred_vertex_table = generate_vertex_table(pattern, source_vertex_id)?;
651651
// Fuse `source_vertex_label` into source, for efficiently scan
652652
let source_vertex_param = if let Some(mut vertex_param) = pattern
653653
.get_vertex_parameters(source_vertex_id)?
654654
.cloned()
655655
{
656-
if source_vertex_table.len() > 0 {
657-
vertex_param.tables = source_vertex_table;
656+
// if user given vertex table is empty, using the inferred vertex table
657+
// and vise versa
658+
if vertex_param.tables.len() == 0 {
659+
vertex_param.tables = inferred_vertex_table;
660+
}
661+
// if the two tables are both not empty
662+
// intersect user given labels and inferred labels
663+
else if inferred_vertex_table.len() > 0 {
664+
let user_given_vertex_table = vertex_param.tables;
665+
vertex_param.tables = vec![];
666+
for inferred_vertex_label in inferred_vertex_table {
667+
if user_given_vertex_table.contains(&inferred_vertex_label) {
668+
vertex_param.tables.push(inferred_vertex_label)
669+
}
670+
}
658671
}
659672
vertex_param
660673
} else {
661-
query_params(source_vertex_table, vec![], None)
674+
query_params(inferred_vertex_table, vec![], None)
662675
};
663676
let source_scan = pb::Scan {
664677
scan_opt: 0,
@@ -876,17 +889,6 @@ fn get_edge_expand_from_binder<'a, 'b>(
876889
) -> IrPatternResult<&'a pb::EdgeExpand> {
877890
use pb::pattern::binder::Item as BinderItem;
878891
if let Some(BinderItem::Path(path_expand)) = binder.item.as_ref() {
879-
let hop_range = path_expand
880-
.hop_range
881-
.as_ref()
882-
.ok_or(ParsePbError::EmptyFieldError("pb::PathExpand::hop_range".to_string()))?;
883-
if hop_range.lower < 1 {
884-
// The path with range from 0 cannot be translated to oprs that can be intersected.
885-
return Err(IrPatternError::Unsupported(format!(
886-
"PathExpand in Pattern with lower range of {:?}",
887-
hop_range.lower
888-
)))?;
889-
}
890892
let expand_base = path_expand
891893
.base
892894
.as_ref()

interactive_engine/executor/ir/core/src/plan/physical.rs

+32-6
Original file line numberDiff line numberDiff line change
@@ -805,9 +805,17 @@ fn add_intersect_job_builder(
805805
if get_v.alias.is_none() || !get_v.alias.as_ref().unwrap().eq(intersect_tag) {
806806
Err(IrError::InvalidPattern("Cannot intersect on different tags".to_string()))?
807807
}
808+
// If the first operator is PathExpand, pick its last expand out from the path
808809
let mut edge_expand = if let Some(Edge(edge_expand)) = first_opr.borrow().opr.opr.as_ref() {
809-
Ok(edge_expand.clone())
810+
edge_expand.clone()
810811
} else if let Some(Path(path_expand)) = first_opr.borrow().opr.opr.as_ref() {
812+
// Process path_expand as follows:
813+
// 1. If path_expand range from 0, it is unsupported;
814+
// 2. If it is path_expand(1,2), optimized as edge_expand;
815+
// 3. Otherwise, translate path_expand(l,h) to path_expand(l-1, h-1) + endV() + edge_expand,
816+
// and the last edge_expand is the one to intersect.
817+
// Notice that if we have predicates for vertices in path_expand, or for the last vertex of path_expand,
818+
// do the filtering after intersection.
811819
let mut path_expand = path_expand.clone();
812820
let path_expand_base = path_expand
813821
.base
@@ -828,37 +836,52 @@ fn add_intersect_job_builder(
828836
Err(IrError::Unsupported(
829837
"Edge Only PathExpand in Intersection's subplan has not been supported yet"
830838
.to_string(),
831-
))?;
839+
))?
832840
}
841+
// Combine the params for the last vertex in path.
842+
// That is, it should satisfy both params in `GetV` in PathExpand's ExpandBase,
843+
// and the params in `EndV` following PathExpand.
833844
if let Some(path_get_v) = path_get_v_opt {
834845
get_v = combine_get_v_by_query_params(get_v, path_get_v);
835846
}
847+
// pick the last edge expand out from the path expand
836848
let mut last_edge_expand = base_edge_expand.clone();
837849
last_edge_expand.v_tag = None;
838850
let hop_range = path_expand
839851
.hop_range
840852
.as_mut()
841853
.ok_or(ParsePbError::EmptyFieldError("pb::PathExpand::hop_range".to_string()))?;
854+
if hop_range.lower < 1 {
855+
Err(IrError::Unsupported(format!(
856+
"PathExpand in Intersection with lower range of {:?}",
857+
hop_range.lower
858+
)))?
859+
}
842860
if hop_range.lower == 1 && hop_range.upper == 2 {
861+
// optimized Path(1..2) to as EdgeExpand
843862
last_edge_expand.v_tag = path_expand.start_tag;
844863
} else {
864+
// translate path_expand(l,h) to path_expand(l-1, h-1) + endV() + edge_expand,
845865
hop_range.lower -= 1;
846866
hop_range.upper -= 1;
847867
let mut end_v = pb::GetV::default();
848868
end_v.opt = pb::get_v::VOpt::End as i32;
869+
// build the path expansion
849870
path_expand.add_job_builder(builder, plan_meta)?;
850871
end_v.add_job_builder(builder, plan_meta)?;
851872
}
852-
Ok(last_edge_expand)
873+
last_edge_expand
853874
} else {
854875
Err(IrError::InvalidPattern(
855876
"First node of Intersection's subplan is neither EdgeExpand or PathExpand".to_string(),
856-
))
857-
}?;
877+
))?
878+
};
879+
// build the edge expansion
880+
// the opt should be vertex because now only intersection on vertex is supported
858881
edge_expand.expand_opt = pb::edge_expand::ExpandOpt::Vertex as i32;
859882
edge_expand.alias = get_v.alias.clone();
860883
edge_expand.add_job_builder(&mut sub_bldr, plan_meta)?;
861-
884+
// vertex parameter after the intersection
862885
if let Some(params) = get_v.params.as_ref() {
863886
// the case that we need to further process getV's filter.
864887
if params.is_queryable() || !params.tables.is_empty() {
@@ -870,13 +893,16 @@ fn add_intersect_job_builder(
870893
let sub_plan = sub_bldr.take_plan();
871894
intersect_plans.push(sub_plan);
872895
}
896+
// intersect
873897
builder.intersect(intersect_plans, intersect_tag.clone());
898+
// unfold the intersection
874899
let unfold = pb::Unfold {
875900
tag: Some(intersect_tag.clone()),
876901
alias: Some(intersect_tag.clone()),
877902
meta_data: None,
878903
};
879904
unfold.add_job_builder(builder, plan_meta)?;
905+
// add vertex filters
880906
if let Some(mut auxilia) = auxilia {
881907
auxilia.tag = Some(intersect_tag.clone());
882908
builder.get_v(auxilia);

interactive_engine/executor/ir/runtime/src/process/operator/map/get_v.rs

+15-9
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,10 @@ impl FilterMapFunction<Record, Record> for GetVertexOperator {
4949
input.append(vertex, self.alias.clone());
5050
Ok(Some(input))
5151
} else if let Some(graph_path) = entry.as_graph_path() {
52-
if let VOpt::End = self.opt {
53-
let path_end = graph_path.get_path_end().clone();
54-
input.append(path_end, self.alias.clone());
55-
Ok(Some(input))
56-
} else {
57-
Err(FnExecError::unsupported_error(
58-
"Only support `GetV` with VOpt::End on a path entry",
59-
))?
60-
}
52+
// TODO: we do not check VOpt here, and we treat all cases as to get the end vertex of the path.
53+
let path_end = graph_path.get_path_end().clone();
54+
input.append(path_end, self.alias.clone());
55+
Ok(Some(input))
6156
} else {
6257
Err(FnExecError::unexpected_data_error(
6358
"Can only apply `GetV` (`Auxilia` instead) on an edge or path entry",
@@ -146,6 +141,17 @@ impl FilterMapFunction<Record, Record> for AuxiliaOperator {
146141
// e.g., for g.V().out().as("a").has("name", "marko"), we should compile as:
147142
// g.V().out().auxilia(as("a"))... where we give alias in auxilia,
148143
// then we set tag=None and alias="a" in auxilia
144+
// 1. filter by labels.
145+
if !self.query_params.labels.is_empty() && entry.label().is_some() {
146+
if !self
147+
.query_params
148+
.labels
149+
.contains(&entry.label().unwrap())
150+
{
151+
return Ok(None);
152+
}
153+
}
154+
// 2. further fetch properties, e.g., filter by columns.
149155
match entry.get_type() {
150156
EntryType::Vertex => {
151157
let graph = get_graph().ok_or(FnExecError::NullGraphError)?;

0 commit comments

Comments
 (0)