fix(interactive): fix the bug in filtering after intersection in GIE Runtime in distributed computation #3359
Conversation
Committed-by: bingqing.lbq from Dev container
Committed-by: bingqing.lbq from Dev container
BingqingLyu
changed the title
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3359 +/- ##
=======================================
Coverage 41.04% 41.04%
=======================================
Files 109 109
Lines 11298 11298
=======================================
Hits 4637 4637
Misses 6661 6661 Continue to review full report in Codecov by Sentry.
|
Committed-by: bingqing.lbq from Dev container
| // is_queryable doesn't consider tables as we assume that the table info can be inferred directly from current data. | ||
| pub fn is_queryable(&self) -> bool { | ||
| !(self.filter.is_none() | ||
| && self.limit.is_none() |
There was a problem hiding this comment.
Why we need to consider limit and sample_ratio then. And if this is the case, directly using ! ( self.filter.is_none() && self.columns.is_none()) without introducing this arbitrary function is a better idea.
Committed-by: bingqing.lbq from Dev container
Committed-by: bingqing.lbq from Dev container
| @@ -159,17 +159,22 @@ impl FilterMapFunction<Record, Record> for AuxiliaOperator { | |||
| // e.g., for g.V().out().as("a").has("name", "marko"), we should compile as: | |||
| // g.V().out().auxilia(as("a"))... where we give alias in auxilia, | |||
| // then we set tag=None and alias="a" in auxilia | |||
| // 1. filter by labels. | |||
|
|
|||
| // 1. If to filter by labels, and the entry itself carries label information already, directly eval it without query the store | |||
There was a problem hiding this comment.
Can the following code be replaced with params.ha_labels()?
| return Ok(None); | ||
| } else if self.query_params.filter.is_none() && self.query_params.columns.is_none() { |
There was a problem hiding this comment.
Use has_predicates & has_columns?
| || params.has_predicates() | ||
| || params.has_sample() | ||
| || params.has_limit()) | ||
| && is_params_all_labels(params) |
There was a problem hiding this comment.
Can is_params_all_labels be replaced with params.has_labels()?
There was a problem hiding this comment.
Maybe not. Here we have to confirm it is the "whole graph", that it either specifies all labels, or no labels (also indicates that all labels is allowed).
What do these changes do?
This pr:
Related issue number
Fixes #3360