rdblue commented on code in PR #4815:
URL: https://github.com/apache/iceberg/pull/4815#discussion_r883871877
##########
python/tests/expressions/test_expressions_base.py:
##########
@@ -257,3 +317,111 @@ def test_bound_reference(table_schema_simple, foo_struct):
assert bound_ref1.eval(foo_struct) == "foovalue"
assert bound_ref2.eval(foo_struct) == 123
assert bound_ref3.eval(foo_struct) == True
+
+
+def test_boolean_expression_visitor():
+ """Test post-order traversal of boolean expression visit method"""
+ expr = base.And(
+ base.Or(base.Not(TestExpressionA()), base.Not(TestExpressionB()),
TestExpressionA(), TestExpressionB()),
+ base.Not(TestExpressionA()),
+ TestExpressionB(),
+ )
+ visitor = TestBooleanExpressionVisitor()
+ result = base.visit(expr, visitor=visitor)
+ assert result == [
+ "TestExpressionA",
+ "NOT",
+ "TestExpressionB",
+ "NOT",
+ "OR",
+ "TestExpressionA",
+ "OR",
+ "TestExpressionB",
+ "OR",
+ "TestExpressionA",
+ "NOT",
+ "AND",
+ "TestExpressionB",
+ "AND",
+ ]
+
+
+def test_or_with_always_true():
+ """Test visiting an Or expression with AlwaysTrue
+
+ Any Or expression with an AlwaysTrue child should be reduced to a single
AlwaysTrue expression and a visitor should only call the `visit_true` method.
+ """
+ expr = base.Or(
+ base.And(
+ base.Or(base.Not(TestExpressionA()), base.Not(TestExpressionB()),
TestExpressionA(), TestExpressionB()),
+ base.Not(TestExpressionA()),
+ TestExpressionB(),
+ ),
+ base.AlwaysTrue(),
+ )
+ visitor = TestBooleanExpressionVisitor()
+ result = base.visit(expr, visitor=visitor)
+ assert result == ["TRUE"]
+
+
+def test_or_with_always_false():
+ """Test visiting an Or expression with AlwaysFalse
+
+ An Or expression with an AlwaysFalse child should skip calling
`visit_false`, yet visit the rest of the child expressions.
+ """
+ expr = base.Or(
+ base.And(
+ base.Or(base.Not(TestExpressionA()), base.Not(TestExpressionB()),
TestExpressionA(), TestExpressionB()),
+ base.Not(TestExpressionA()),
+ TestExpressionB(),
+ ),
+ base.AlwaysFalse(),
+ )
+ visitor = TestBooleanExpressionVisitor()
+ result = base.visit(expr, visitor=visitor)
+ assert result == [
+ "TestExpressionA",
+ "NOT",
+ "TestExpressionB",
+ "NOT",
+ "OR",
+ "TestExpressionA",
+ "OR",
+ "TestExpressionB",
+ "OR",
+ "TestExpressionA",
+ "NOT",
+ "AND",
+ "TestExpressionB",
+ "AND",
+ ]
+
+
+def test_or_with_only_always_false():
+ """Test visiting an Or expression with only AlwaysFalse expressions
+
+ An Or expression with only AlwaysFalse expressions should be reduced to a
single AlwaysFalse expression
+ and only require a single call to the `visit_false` method.
+ """
+ expr = base.Or(
+ base.AlwaysFalse(),
+ base.AlwaysFalse(),
+ base.AlwaysFalse(),
+ base.AlwaysFalse(),
+ base.AlwaysFalse(),
+ base.AlwaysFalse(),
+ base.AlwaysFalse(),
+ )
+ visitor = TestBooleanExpressionVisitor()
+ result = base.visit(expr, visitor=visitor)
+ assert isinstance(expr, base.AlwaysFalse)
+ assert result == ["FALSE"]
+
+
+def test_boolean_expression_visit_raise_not_implemented_error():
Review Comment:
I'd keep this one, but you just need the one test above.
##########
python/tests/expressions/test_expressions_base.py:
##########
@@ -257,3 +317,111 @@ def test_bound_reference(table_schema_simple, foo_struct):
assert bound_ref1.eval(foo_struct) == "foovalue"
assert bound_ref2.eval(foo_struct) == 123
assert bound_ref3.eval(foo_struct) == True
+
+
+def test_boolean_expression_visitor():
Review Comment:
This test looks good to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
