[GIE/IR] Introduce a new strategy for pattern matching #2159
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2159 +/- ##
===========================================
- Coverage 71.95% 40.19% -31.76%
===========================================
Files 89 87 -2
Lines 9884 9847 -37
===========================================
- Hits 7112 3958 -3154
- Misses 2772 5889 +3117
Continue to review full report at Codecov.
|
|
|
||
| /// Given a DefiniteExtendEdge, we can uniquely locate an edge with dir in the pattern | ||
| #[derive(Debug, Clone)] | ||
| pub struct DefiniteExtendEdge { |
There was a problem hiding this comment.
Not sure what DefiniteExtendEdge is? A comment may help. In addition, ExactExtendEdge could be a better name.
|
|
||
| /// Given a DefiniteExtendStep, we can uniquely find which part of the pattern to extend | ||
| #[derive(Debug, Clone)] | ||
| pub struct DefiniteExtendStep { |
There was a problem hiding this comment.
Similar to the comments of DefiniteExtendEdge
| pub type DynIter<'a, T> = Box<dyn Iterator<Item = T> + 'a>; | ||
|
|
||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] | ||
| pub enum PatternDirection { |
There was a problem hiding this comment.
Looks like PatternEdgeDirection. How about Both direction?
| } | ||
| } | ||
|
|
||
| pub mod canonical_label; |
There was a problem hiding this comment.
Move mod declaration to the top, after use.
| } | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub enum EdgeData { |
There was a problem hiding this comment.
This is not EdgeData. PbEdgeOrPath?
| match self.cmp_vertices(pattern, current_v_id, vertex_group[j])? { | ||
| Ordering::Greater => vertex_group_tmp_vec[i] += 1, | ||
| Ordering::Less => vertex_group_tmp_vec[j] += 1, | ||
| Ordering::Equal => (), |
There was a problem hiding this comment.
Ordering::Equal => /* add comment for why this needs no processing */
| updated_vertex_groups | ||
| .entry((v_label, v_group)) | ||
| .and_modify(|vertex_group| vertex_group.push(current_v_id)) | ||
| .or_insert(vec![current_v_id]); |
There was a problem hiding this comment.
or_insert_with(Vec::new)
.push(current_v_vid);
| .get(¤t_v_id) | ||
| .unwrap_or(&None) | ||
| .is_some(); | ||
| if !is_vertex_visited { |
There was a problem hiding this comment.
if veretx_rank_map.containsKey(¤t_v_id) { ... }
do not need is_vertex_visited above
| // Since vertices in the same pattern will never be given the same rank, two adjacencies cannot be equal. | ||
| let adj1_v_rank = self.get_vertex_rank(adj1_v_id); | ||
| let adj2_v_rank = self.get_vertex_rank(adj2_v_id); | ||
| if adj1_v_rank.is_none() && adj2_v_rank.is_none() { |
There was a problem hiding this comment.
if adj1_v_rank.is_none() && adj2_v_rank.is_none() {
Ordering::Equal
} else if adj1_v_rank.is_none() {
Ordering::Less
} else if adj2_v_rank.is_none() {
Ordering::Greater
} else {
adj1_v_rank.unwrap().cmp(&adj2_v_rank.unwrap())
}
| // Since vertices in the same pattern will never be given the same rank, two adjacencies cannot be equal. | ||
| let adj1_v_rank = vertex_rank_map.get(&adj1_v_id); | ||
| let adj2_v_rank = vertex_rank_map.get(&adj2_v_id); | ||
| if adj1_v_rank.is_none() && adj2_v_rank.is_none() { |
There was a problem hiding this comment.
Similar to the above codes
…table(label) value, and end with `Project` op with user-given alias
…stem-given alias exists.
… only when any system-given alias exists.
BingqingLyu
force-pushed
the
ir_catalog_naive_path_expand
branch
from
e193213
to
2bae396
Compare
…Data` for simplicity
BingqingLyu
force-pushed
the
ir_catalog_naive_path_expand
branch
from
f1fe383
to
49d8f65
Compare
… simplicity, instead, maintain max_tag_id
BingqingLyu
force-pushed
the
ir_catalog_naive_path_expand
branch
from
49d8f65
to
bdfcd93
Compare
BingqingLyu
force-pushed
the
ir_catalog_naive_path_expand
branch
from
99405ab
to
77a2262
Compare
BingqingLyu
force-pushed
the
ir_catalog_naive_path_expand
branch
2 times, most recently
from
f535fda
to
446bd46
Compare
BingqingLyu
force-pushed
the
ir_catalog_naive_path_expand
branch
from
446bd46
to
ff0a6aa
Compare
| @@ -75,7 +74,7 @@ pub struct LogicalPlan { | |||
| pub(crate) nodes: VecMap<NodeType>, | |||
| /// To record the nodes' maximum id in the logical plan. Note that the nodes | |||
| /// **include the removed ones** | |||
| pub(crate) max_node_id: NodeId, | |||
| pub max_node_id: NodeId, | |||
There was a problem hiding this comment.
如果想要在crate外面可见，那就用一个函数而非直接改这个可见范围
| if !source.is_queryable() { | ||
| if let Ok(store_meta) = STORE_META.read() { | ||
| if let Some(pattern_meta) = store_meta.pattern_meta.as_ref() { | ||
| if let Ok(extend_strategy) = |
There was a problem hiding this comment.
这里两种不同的strategy的应用应该更清晰一点，类似于
if 满足xx条件：
Apply ExtendStrategy
else:
Apply Naive Strategy
| @@ -37,6 +38,16 @@ lazy_static! { | |||
| pub static ref STORE_META: RwLock<StoreMeta> = RwLock::new(StoreMeta::default()); | |||
| } | |||
|
|
|||
| pub fn set_meta_from_json<R: io::Read>(read: R) { | |||
There was a problem hiding this comment.
这个用set schema的方式设置pattern meta，是不是可以仔细考虑一下。set schema一般只会设置一次，而set_pattern_meta，每个user frontend都可能会去设置。是不是现在的方式不太合理？
There was a problem hiding this comment.
因为目前schema中信息够了，暂时先把pattern_meta去掉了
| /// Key: vertex label name, Value: vertex labal id | ||
| /// | ||
| /// Usage: get a vertex label id from its label name | ||
| vertex_label_map: BiBTreeMap<String, PatternLabelId>, |
There was a problem hiding this comment.
这些在schema里面不是有吗？为啥需要在PatternMeta里存一遍？
| /// 3. vertex adjacent to more path_expand should be extended later; | ||
| /// 4. vertex with larger degree will be extended later | ||
| fn compare_vertices(v1_id: PatternId, v2_id: PatternId, trace_pattern: &Pattern) -> Ordering { | ||
| let v1_weight = estimate_vertex_weight(v1_id, trace_pattern); |
There was a problem hiding this comment.
这些estimate_xx_weight() 的函数可不可以给个抽象，然后现在这个estimation的方法只是一个naive实现？
…gy`; and Remove `PatternMeta`.
| @@ -42,3 +43,12 @@ pub(crate) fn query_params( | |||
| extra: HashMap::new(), | |||
| } | |||
| } | |||
|
|
|||
| pub trait PatternOrderTrait<D> { | |||
There was a problem hiding this comment.
Why do not just implement std::cmp::Ord?
There was a problem hiding this comment.
这个其实是给了一个sort_by()里的udf，因为可能是输入pattern_id来做order，而并不是直接compare pattern本身
| } | ||
|
|
||
| pub trait PatternWeightTrait<W: PartialOrd> { | ||
| fn estimate_vertex_weight(&self, vid: PatternId) -> W; |
There was a problem hiding this comment.
get_vertex_weight() and get_adjacencies_weight() are better names.
What do these changes do?
As titled.
Related issue number
Fixes #2033