From 63a3ee90bd6d53e736406dce0d4f36e9a7dd042c Mon Sep 17 00:00:00 2001
From: Matthijs Brobbel <m1brobbel@gmail.com>
Date: Wed, 21 Sep 2022 16:15:46 +0200
Subject: [PATCH] fix: add note to summary of read relation about filter

---
 rs/src/parse/relations/read.rs                | 13 +++-
 .../relations/read/filter-project/both.yaml   | 68 ++++++++++++-------
 .../filter-project/projection-multiple.yaml   | 38 +++++------
 3 files changed, 76 insertions(+), 43 deletions(-)

diff --git a/rs/src/parse/relations/read.rs b/rs/src/parse/relations/read.rs
index 848c5a69..62a7aedd 100644
--- a/rs/src/parse/relations/read.rs
+++ b/rs/src/parse/relations/read.rs
@@ -334,7 +334,7 @@ pub fn parse_read_rel(x: &substrait::ReadRel, y: &mut context::Context) -> diagn
     y.set_schema(schema.clone());
 
     // Handle filter.
-    proto_boxed_field!(x, y, filter, expressions::parse_predicate);
+    let filter = proto_boxed_field!(x, y, filter, expressions::parse_predicate);
 
     // Handle projection.
     if x.projection.is_some() {
@@ -353,6 +353,17 @@ pub fn parse_read_rel(x: &substrait::ReadRel, y: &mut context::Context) -> diagn
         (true, true) => describe!(y, Relation, "Filtered partial read from {}", source.name),
     }
 
+    // Add filter summary.
+    if let (Some(filter_node), Some(filter_expr)) = filter {
+        let nullable = filter_node.data_type().nullable();
+        summary!(
+            y,
+            "This relation discards all rows for which the expression {} yields {}.",
+            filter_expr,
+            if nullable { "false or null" } else { "false" }
+        );
+    }
+
     // Handle the common field.
     handle_rel_common!(x, y);
 
diff --git a/tests/tests/relations/read/filter-project/both.yaml b/tests/tests/relations/read/filter-project/both.yaml
index ff56ad40..795e2692 100644
--- a/tests/tests/relations/read/filter-project/both.yaml
+++ b/tests/tests/relations/read/filter-project/both.yaml
@@ -1,26 +1,48 @@
 name: read-filter-project
 plan:
-  __test: [ level: i ]
+  __test: [level: i]
   relations:
-  - rel:
-      read:
-        baseSchema:
-          names: [a, b]
-          struct:
-            nullability: NULLABILITY_REQUIRED
-            types:
-            - string: { nullability: NULLABILITY_REQUIRED }
-            - bool: { nullability: NULLABILITY_REQUIRED }
-        namedTable:
-          names:
-          - test
-        filter:
-          selection:
-            rootReference: {}
-            directReference: { structField: { field: 1 } }
-        projection:
-          maintain_singular_struct: true
-          select:
-            structItems:
-            - field: 0
-        __test: [ type: "STRUCT<string>" ]
+    - rel:
+        read:
+          baseSchema:
+            names: [a, b]
+            struct:
+              nullability: NULLABILITY_REQUIRED
+              types:
+                - string: { nullability: NULLABILITY_REQUIRED }
+                - bool: { nullability: NULLABILITY_REQUIRED }
+          namedTable:
+            names:
+              - test
+          filter:
+            selection:
+              rootReference: {}
+              directReference: { structField: { field: 1 } }
+          projection:
+            maintain_singular_struct: true
+            select:
+              structItems:
+                - field: 0
+          __test: [comment: "*yields false.", type: "STRUCT<string>"]
+    - rel:
+        read:
+          baseSchema:
+            names: [a, b]
+            struct:
+              nullability: NULLABILITY_REQUIRED
+              types:
+                - string: { nullability: NULLABILITY_REQUIRED }
+                - bool: { nullability: NULLABILITY_NULLABLE }
+          namedTable:
+            names:
+              - test
+          filter:
+            selection:
+              rootReference: {}
+              directReference: { structField: { field: 1 } }
+          projection:
+            maintain_singular_struct: true
+            select:
+              structItems:
+                - field: 0
+          __test: [comment: "*yields false or null.", type: "STRUCT<string>"]
diff --git a/tests/tests/relations/read/filter-project/projection-multiple.yaml b/tests/tests/relations/read/filter-project/projection-multiple.yaml
index f4e26577..d69dee71 100644
--- a/tests/tests/relations/read/filter-project/projection-multiple.yaml
+++ b/tests/tests/relations/read/filter-project/projection-multiple.yaml
@@ -1,22 +1,22 @@
 name: read-projection-multiple
 plan:
-  __test: [ level: i ]
+  __test: [level: i]
   relations:
-  - rel:
-      read:
-        baseSchema:
-          names: [a, b]
-          struct:
-            nullability: NULLABILITY_REQUIRED
-            types:
-            - string: { nullability: NULLABILITY_REQUIRED }
-            - bool: { nullability: NULLABILITY_REQUIRED }
-        namedTable:
-          names:
-          - test
-        projection:
-          select:
-            structItems:
-            - field: 1
-            - field: 0
-        __test: [ type: "STRUCT<boolean, string>" ]
+    - rel:
+        read:
+          baseSchema:
+            names: [a, b]
+            struct:
+              nullability: NULLABILITY_REQUIRED
+              types:
+                - string: { nullability: NULLABILITY_REQUIRED }
+                - bool: { nullability: NULLABILITY_REQUIRED }
+          namedTable:
+            names:
+              - test
+          projection:
+            select:
+              structItems:
+                - field: 1
+                - field: 0
+          __test: [comment: "", type: "STRUCT<boolean, string>"]