From 3d9f9ea53e0b1762278c5496b185021b32241ea3 Mon Sep 17 00:00:00 2001 From: Edd Robinson Date: Thu, 21 Oct 2021 10:17:41 +0100 Subject: [PATCH 1/3] refactor: use Expr Display impl --- predicate/src/predicate.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/predicate/src/predicate.rs b/predicate/src/predicate.rs index d7f3b624b3..cc7c590c21 100644 --- a/predicate/src/predicate.rs +++ b/predicate/src/predicate.rs @@ -234,12 +234,15 @@ impl fmt::Display for Predicate { } if !self.exprs.is_empty() { - // Expr doesn't implement `Display` yet, so just the debug version - // See https://github.com/apache/arrow-datafusion/issues/347 - let display_exprs = self.exprs.iter().map(|e| format!("{:?}", e)); - write!(f, " exprs: [{}]", iter_to_str(display_exprs))?; + write!(f, " exprs: [")?; + for (i, expr) in self.exprs.iter().enumerate() { + write!(f, "{}", expr)?; + if i < self.exprs.len() - 1 { + write!(f, " ")?; + } + } + write!(f, "]")?; } - Ok(()) } } From a3e750c9fcca211cf528574fa39f75bab5889870 Mon Sep 17 00:00:00 2001 From: Edd Robinson Date: Fri, 22 Oct 2021 11:27:06 +0100 Subject: [PATCH 2/3] test: add null column to test --- read_buffer/src/chunk.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/read_buffer/src/chunk.rs b/read_buffer/src/chunk.rs index 94ac209ba4..f6e7df7cb1 100644 --- a/read_buffer/src/chunk.rs +++ b/read_buffer/src/chunk.rs @@ -974,6 +974,7 @@ mod test { .field("sketchy_sensor", Int64) .non_null_field("active", Boolean) .field("msg", Utf8) + .field("all_null", Utf8) .timestamp() .build() .unwrap(); @@ -997,6 +998,7 @@ mod test { Some("message b"), None, ])), + Arc::new(StringArray::from(vec![None, None, None])), Arc::new(TimestampNanosecondArray::from_vec( vec![i, 2 * i, 3 * i], None, @@ -1058,6 +1060,7 @@ mod test { ); assert_rb_column_equals(&first_row_group, "active", &exp_active_values); assert_rb_column_equals(&first_row_group, "msg", &exp_msg_values); + assert_rb_column_equals(&first_row_group, "all_null", &Values::String(vec![None])); assert_rb_column_equals(&first_row_group, "time", &Values::I64(vec![100])); // first row from first record batch let second_row_group = itr.next().unwrap(); @@ -1070,8 +1073,16 @@ mod test { &exp_sketchy_sensor_values, ); assert_rb_column_equals(&first_row_group, "active", &exp_active_values); + assert_rb_column_equals(&first_row_group, "all_null", &Values::String(vec![None])); assert_rb_column_equals(&second_row_group, "time", &Values::I64(vec![200])); // first row from second record batch + // No rows returned when filtering on all_null column + let predicate = Predicate::new(vec![BinaryExpr::from(("all_null", "!=", "a string"))]); + let mut itr = chunk + .read_filter(predicate, Selection::All, vec![]) + .unwrap(); + assert!(itr.next().is_none()); + // Error when predicate is invalid let predicate = Predicate::with_time_range(&[BinaryExpr::from(("env", "=", 22.3))], 100, 205); From 84a00cd5b69cbc31e22f63d87fac31684d2bad80 Mon Sep 17 00:00:00 2001 From: Edd Robinson Date: Fri, 22 Oct 2021 15:35:24 +0100 Subject: [PATCH 3/3] refactor: comma --- predicate/src/predicate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/predicate/src/predicate.rs b/predicate/src/predicate.rs index cc7c590c21..b0d6dcf910 100644 --- a/predicate/src/predicate.rs +++ b/predicate/src/predicate.rs @@ -238,7 +238,7 @@ impl fmt::Display for Predicate { for (i, expr) in self.exprs.iter().enumerate() { write!(f, "{}", expr)?; if i < self.exprs.len() - 1 { - write!(f, " ")?; + write!(f, ", ")?; } } write!(f, "]")?;