[GIE Physical] Support LateProject Strategy when Fetching Properties. #2451

BingqingLyu · 2023-02-22T12:27:04Z

What do these changes do?

As titled.

Related issue number

longbinlai · 2023-02-24T11:53:09Z

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

+        // This would be refined as:
+        // In Logical Plan: `Source + EdgeExpand(ExpandE) + GetV`
+        // In Physical Plan:
+        //       1. on distributed graph store, `Source + EdgeExpand(ExpandV) + Shuffle + Auxilia`


Now we do not have Auxilia? Why still comment it as Auxilia

Updated as GetV

longbinlai · 2023-02-24T11:54:03Z

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

-                merged_params.columns.push(column.clone());
-            }
-        }
+fn process_tag_columns(builder: &mut JobBuilder, tag: Option<TagId>, columns_opt: ColumnsOpt) {


Is JobBuilder still used to build the Pegasus plan? or Physical plan?

Physical plan.

Then probably PhysicalBuilder is better than JobBuilder.

longbinlai · 2023-02-24T12:00:53Z

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

+fn process_tag_columns(builder: &mut JobBuilder, tag: Option<TagId>, columns_opt: ColumnsOpt) {
+    if columns_opt.len() > 0 {
+        let tag_pb = tag.map(|tag_id| (tag_id as i32).into());
+        builder.shuffle(tag_pb.clone());


Why have to shuffle here? We do not verify wether it is single thread?

This function would be called only when plan_meta.is_partition()

Then why not put plan_meta.is_partition() in this function?
Let the caller be free of the context.
Moreover, better comment this function to improve the understanding.

Done. Move this into post_process_vars() function.

longbinlai · 2023-02-24T12:03:45Z

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

+    let node_meta = plan_meta.get_curr_node_meta().unwrap();
+    let tag_columns = node_meta.get_tag_columns();
+    let len = tag_columns.len();
+    if len == 0 {


Better not using return in the middle. Can do this instead:

if len == 1 && !is_order_or_group { } else if len != 0 { } else { Ok(()) }

longbinlai · 2023-02-24T13:57:51Z

...compiler/src/main/java/com/alibaba/graphscope/gremlin/integration/graph/RemoteTestGraph.java

+@Graph.OptOut(
+        method = "g_V_order_byXnameX_name",
+        test = "org.apache.tinkerpop.gremlin.process.traversal.step.map.OrderTest",
+        reason = "unsupoorted")


the reason should not be unsupported (btw: unsupoorted misspelled)

Updated as "project (with shuffle) after order occurs bug"

"Projection may introduce additional shuffling that can break the order. "

longbinlai · 2023-02-24T14:03:05Z

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

+            sample_ratio: 1.0,
+            extra: Default::default(),
+        };
+        let auxilia = pb::GetV { tag: tag_pb.clone(), opt: 4, params: Some(params), alias: tag_pb.clone() };


Better comment what is opt: 4 .

longbinlai · 2023-02-24T14:20:47Z

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

-    merge_query_param_columns(merged_params, other_params.is_all_columns, &other_params.columns);
-    other_params.columns.clear();
-    other_params.is_all_columns = false;
+// Fetch properties before used in select, order, dedup, group, join, and apply.


Can you explain more about how to process late project. Use some examples?

More comment with examples added.

codecov-commenter · 2023-02-27T13:34:13Z

Codecov Report

Merging #2451 (32442a1) into main (e9850d3) will decrease coverage by 33.27%.
The diff coverage is n/a.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2451       +/-   ##
===========================================
- Coverage   73.22%   39.95%   -33.27%     
===========================================
  Files          88       88               
  Lines        9769     9769               
===========================================
- Hits         7153     3903     -3250     
- Misses       2616     5866     +3250

Impacted Files	Coverage Δ
python/graphscope/tests/unittest/test_java_app.py	`0.00% <0.00%> (-100.00%)`	⬇️
...ython/graphscope/tests/unittest/test_cython_ast.py	`0.00% <0.00%> (-100.00%)`	⬇️
...ython/graphscope/tests/unittest/test_serailaize.py	`0.00% <0.00%> (-100.00%)`	⬇️
python/graphscope/analytical/udf/patch.py	`3.47% <0.00%> (-96.53%)`	⬇️
python/graphscope/tests/unittest/test_app.py	`0.00% <0.00%> (-96.32%)`	⬇️
python/graphscope/tests/unittest/test_lazy.py	`0.00% <0.00%> (-96.22%)`	⬇️
...thon/graphscope/tests/unittest/test_scalability.py	`0.00% <0.00%> (-96.16%)`	⬇️
...hon/graphscope/tests/unittest/test_create_graph.py	`0.00% <0.00%> (-92.89%)`	⬇️
python/graphscope/tests/unittest/test_graph.py	`0.00% <0.00%> (-85.92%)`	⬇️
python/graphscope/tests/unittest/test_context.py	`0.00% <0.00%> (-81.36%)`	⬇️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5d1338...32442a1. Read the comment docs.

longbinlai · 2023-02-28T02:56:23Z

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

-    other_params.is_all_columns = false;
+// Fetch properties before used in Project, Select, Order, Dedup, Group, Join, and Apply.
+// e.g.,
+// Case 1: For singe property (except used in Order or GroupVal), e.g., g.V().out().as("a").out().out().out().select("a").by("name"),


single property. Can use chatgpt to help check typo fyi.

… Physical Plan.

longbinlai · 2023-03-01T10:46:13Z

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

@@ -28,7 +28,7 @@ use ir_physical_client::physical_builder::{JobBuilder, Plan};

 use crate::error::{IrError, IrResult};
 use crate::plan::logical::{LogicalPlan, NodeType};
-use crate::plan::meta::PlanMeta;
+use crate::plan::meta::{ColumnsOpt, PlanMeta, TagId};

 /// A trait for building physical plan (pegasus) from the logical plan
 pub trait AsPhysical {


This should not be pegasus's JobBuilder

BingqingLyu marked this pull request as draft February 22, 2023 12:27

BingqingLyu force-pushed the ir_physical_late_project branch from 4b12c1f to 9ac564d Compare February 22, 2023 12:30

BingqingLyu closed this Feb 22, 2023

BingqingLyu reopened this Feb 23, 2023

BingqingLyu force-pushed the ir_physical_late_project branch from 443bac0 to ede8b7e Compare February 24, 2023 02:40

BingqingLyu marked this pull request as ready for review February 24, 2023 02:45

BingqingLyu requested a review from longbinlai February 24, 2023 02:45

BingqingLyu force-pushed the ir_physical_late_project branch from ede8b7e to e48e607 Compare February 24, 2023 04:15

longbinlai reviewed Feb 24, 2023

View reviewed changes

longbinlai reviewed Feb 28, 2023

View reviewed changes

BingqingLyu force-pushed the ir_physical_late_project branch from c2dddd9 to b080b9e Compare March 1, 2023 03:52

BingqingLyu added 9 commits March 1, 2023 12:35

[IR Physical] Support late project

00fe5fc

[WIP] Refine Late Project

6706c46

[IR Physical] Refine Late Project

a1adbb2

[IR Physical] Translate As into Project rather than GetV

ce19ec5

[IR Physical] Make sure Auxilia won't change Head, and Simplify the…

f4a3a4e

… Physical Plan.

[IR Physical] Fix CI tests.

19662f0

[CI Test] Skip some order tests

60c3abc

[IR Physical] refine the codes.

182cc16

Minor fix.

32442a1

BingqingLyu force-pushed the ir_physical_late_project branch from b080b9e to 32442a1 Compare March 1, 2023 04:37

longbinlai approved these changes Mar 1, 2023

View reviewed changes

longbinlai merged commit 1209e3e into alibaba:main Mar 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GIE Physical] Support LateProject Strategy when Fetching Properties. #2451

[GIE Physical] Support LateProject Strategy when Fetching Properties. #2451

BingqingLyu commented Feb 22, 2023 •

edited

Loading

longbinlai Feb 24, 2023

BingqingLyu Feb 27, 2023

longbinlai Feb 24, 2023

BingqingLyu Feb 27, 2023

longbinlai Feb 28, 2023

longbinlai Feb 24, 2023

BingqingLyu Feb 27, 2023

longbinlai Feb 28, 2023

BingqingLyu Mar 1, 2023

longbinlai Feb 24, 2023

BingqingLyu Feb 27, 2023

longbinlai Feb 24, 2023 •

edited

Loading

BingqingLyu Feb 27, 2023

longbinlai Feb 28, 2023

longbinlai Feb 24, 2023

BingqingLyu Feb 27, 2023

longbinlai Feb 24, 2023

BingqingLyu Feb 27, 2023

codecov-commenter commented Feb 27, 2023 •

edited

Loading

longbinlai Feb 28, 2023

longbinlai Mar 1, 2023

[GIE Physical] Support LateProject Strategy when Fetching Properties. #2451

[GIE Physical] Support LateProject Strategy when Fetching Properties. #2451

Conversation

BingqingLyu commented Feb 22, 2023 • edited Loading

What do these changes do?

Related issue number

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

longbinlai Feb 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Feb 27, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BingqingLyu commented Feb 22, 2023 •

edited

Loading

longbinlai Feb 24, 2023 •

edited

Loading

codecov-commenter commented Feb 27, 2023 •

edited

Loading