Skip to content

Commit 047063c

Browse files
authored
fix(interactive): Fix a bug in insight runtime when eval empty string (#4582)
<!-- 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. --> As titled, this bug occurs because an Object with an empty string is incorrectly treated as NULL, leading `NULL==NULL` to evaluate as false. However, the comparison `""==""` should be true. This PR addresses and fixes the issue. ## Related issue number <!-- Are there any issues opened that will be resolved by merging this change? --> #4581
1 parent d39709a commit 047063c

File tree

3 files changed

+21
-5
lines changed

3 files changed

+21
-5
lines changed

interactive_engine/executor/common/dyn_type/tests/object.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
mod tests {
1818
use std::cmp::Ordering;
1919

20-
use dyn_type::{object, BorrowObject, Object, Primitives};
20+
use dyn_type::{object, BorrowObject, Object, OwnedOrRef, Primitives};
2121

2222
#[test]
2323
fn test_as_primitive() {
@@ -139,7 +139,7 @@ mod tests {
139139
fn test_owned_or_ref() {
140140
let a = object!(8_u128);
141141
let left = 0u128;
142-
let right = a.get().unwrap();
142+
let right: OwnedOrRef<'_, u128> = a.get().unwrap();
143143
assert_eq!(left.partial_cmp(&*right), Some(Ordering::Less));
144144
assert_eq!(right.partial_cmp(&left), Some(Ordering::Greater));
145145
assert_eq!(*&*right, 8_u128);

interactive_engine/executor/engine/pegasus/pegasus/tests/iteration_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ fn iterate_x_map_reduce_unfold_x_test() {
337337
.unwrap();
338338
println!("i2 = {}", i2);
339339
assert_eq!(count, 1000);
340-
assert_eq!(sum, (1..i2).rev().take(1000).sum());
340+
assert_eq!(sum, (1..i2).rev().take(1000).sum::<u32>());
341341
}
342342

343343
#[test]

interactive_engine/executor/ir/graph_proxy/src/utils/expr/eval.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use std::panic;
2121

2222
use dyn_type::arith::{BitOperand, Exp};
2323
use dyn_type::object;
24+
use dyn_type::object::RawType;
2425
use dyn_type::{BorrowObject, Object};
2526
use ir_common::error::{ParsePbError, ParsePbResult};
2627
use ir_common::expr_parse::to_suffix_expr;
@@ -330,7 +331,9 @@ pub(crate) fn apply_logical<'a>(
330331
if b_opt.is_some() {
331332
let b = b_opt.unwrap();
332333
// process null values
333-
if a.eq(&Object::None) || b.eq(&Object::None) {
334+
// "a.eq(&Object::None)" has a potential bug, that if a is empty string "", it will be treated as None.
335+
// Fix it by using raw_type() == RawType::None
336+
if a.raw_type() == RawType::None || b.raw_type() == RawType::None {
334337
match logical {
335338
And => {
336339
if (a != Object::None && !a.eval_bool::<(), NoneContext>(None)?)
@@ -713,7 +716,7 @@ impl TryFrom<common_pb::ExprOpr> for Operand {
713716
Ok(Self::VarMap(vec))
714717
}
715718
Map(key_vals) => Operand::try_from(key_vals),
716-
_ => Err(ParsePbError::ParseError("invalid operators for an Operand".to_string())),
719+
_ => Err(ParsePbError::ParseError(format!("invalid operators for an Operand {:?}", item))),
717720
}
718721
} else {
719722
Err(ParsePbError::from("empty value provided"))
@@ -1616,4 +1619,17 @@ mod tests {
16161619
assert_eq!(eval.eval::<_, Vertices>(Some(&ctxt)).unwrap(), expected);
16171620
}
16181621
}
1622+
1623+
#[test]
1624+
fn test_eval_empty_string() {
1625+
let map1: HashMap<NameOrId, Object> =
1626+
vec![(NameOrId::from("emptyProp".to_string()), Object::String("".to_string()))]
1627+
.into_iter()
1628+
.collect();
1629+
1630+
let ctxt = Vertices { vec: vec![Vertex::new(1, Some(9.into()), DynDetails::new(map1))] };
1631+
let case = "@0.emptyProp == \"\""; // true
1632+
let eval = Evaluator::try_from(str_to_expr_pb(case.to_string()).unwrap()).unwrap();
1633+
assert_eq!(eval.eval::<_, Vertices>(Some(&ctxt)).unwrap(), object!(true));
1634+
}
16191635
}

0 commit comments

Comments
 (0)