Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Fix] Fix the alias of GetV of ExpandBase in PathExpand, an… #2763

Merged
merged 2 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public Builder getV(GetVConfig config) {
config.getOpt(),
innerBuilder.getTableConfig(
config.getLabels(), GraphOpt.Source.VERTEX),
config.getAlias()); // hack ways: to be consistent with runtime
AliasInference.DEFAULT_NAME);
// (the alias of endV is given in the getV
// base)
innerBuilder.push(this.getV);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ public void push_filter_2_test() {
.filter(
builder.call(
GraphStdOperatorTable.EQUALS,
pxdBuilder.variable("z", "age"),
pxdBuilder.literal(10)))
builder.variable("z", "age"),
builder.literal(10)))
.build();
RelOptPlanner planner = Utils.mockPlanner(FilterMatchRule.Config.DEFAULT.toRule());
planner.setRoot(before);
Expand All @@ -95,7 +95,7 @@ public void push_filter_2_test() {
+ " GraphLogicalPathExpand(expand=[GraphLogicalExpand(tableConfig=[{isAll=false,"
+ " tables=[knows]}], alias=[DEFAULT], opt=[OUT])\n"
+ "], getV=[GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[z], opt=[END])\n"
+ " alias=[DEFAULT], opt=[END])\n"
+ "], offset=[1], fetch=[3], path_opt=[SIMPLE], result_opt=[AllV],"
+ " alias=[DEFAULT])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void match_4_test() {
+ " tables=[knows]}], alias=[DEFAULT], fusedFilter=[[=(DEFAULT.weight,"
+ " 1.0E0)]], opt=[OUT])\n"
+ "], getV=[GraphLogicalGetV(tableConfig=[{isAll=false, tables=[person]}],"
+ " alias=[c], opt=[END])\n"
+ " alias=[DEFAULT], opt=[END])\n"
+ "], offset=[1], fetch=[2], path_opt=[ARBITRARY], result_opt=[EndV],"
+ " alias=[b])\n"
+ " GraphLogicalSource(tableConfig=[{isAll=false, tables=[person]}],"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ impl ExactExtendEdge {
&self, mut path_opr: pb::PathExpand,
) -> IrPatternResult<pb::logical_plan::Operator> {
path_opr.start_tag = Some((self.src_vertex_id as KeyId).into());
path_opr.alias = None;
path_opr
.base
.as_mut()
Expand Down
11 changes: 6 additions & 5 deletions interactive_engine/executor/ir/core/src/plan/physical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,15 +772,14 @@ impl AsPhysical for LogicalPlan {
// 3. To intersect a->c, b->c with key=c, with filters
// we support expanding vertices with filters on edges (i.e., filters on a->c, b->c), e.g., Intersect{{a-filter->c, b-filter->c}, key=c};
// if expanding vertices with filters on vertices (i.e., filters on c), translate into Intersect{{a->c, b->c}, key=c} + Select {filter on c}
// For now, this logic is in the translation rule in Pattern in logical plan
// TODO: move this logic into physical layer, as we may able to directly filter vertices during intersection if we have the global view of storage.
// 4. To intersect a->...->d->c and b->c with key=c, where a->..->d->c is a path from a to c,
// if so, translate into PathExpand{a->d} + Intersect{d->c, b->c, key=c}.

// Thus, after build intersect, the physical plan looks like:
// 1. the last ops in intersect's sub_plans are the ones to intersect;
// 2. the intersect op can be:
// 1) EdgeExpand with Opt = ExpandE followed by GetV with Opt = End, which is to expand and intersect on id-only vertices; (supported currently)
// 2. the intersect op includes:
// 1) EdgeExpand with Opt = ExpandV, which is to expand and intersect on id-only vertices; (supported currently)
// 2) EdgeExpand with Opt = ExpandE, which is to expand and intersect on edges (although, not considered in Pattern yet);
// and 3) GetV with Opt = Self, which is to expand and intersect on vertices, while there may be some filters on the intersected vertices. (TODO e2e)

fn add_intersect_job_builder(
builder: &mut JobBuilder, plan_meta: &mut PlanMeta, intersect_opr: &pb::Intersect,
Expand Down Expand Up @@ -827,6 +826,8 @@ fn add_intersect_job_builder(
// and the last edge_expand is the one to intersect.
// Notice that if we have predicates for vertices in path_expand, or for the last vertex of path_expand,
// do the filtering after intersection.
// TODO: there might be a bug here:
// if path_expand has an alias which indicates that the path would be referred later, it may not as expected.
let mut path_expand = path_expand.clone();
let path_expand_base = path_expand
.base
Expand Down