sadpandajoe commented on code in PR #45771:
URL: https://github.com/apache/superset/pull/45771#discussion_r4393162151
##########
tests/unit_tests/sql/parse_tests.py:
##########
@@ -3477,6 +3487,33 @@ def
test_deduplicate_oracle_output_names_ignores_non_select() -> None:
assert values.sql() == "VALUES (1, 1)"
[email protected](
+ "condition, expected",
+ [
+ ("x = 1", ["x = 1"]),
+ ("x = 1 AND y = 2", ["x = 1", "y = 2"]),
+ ("x = 1 AND y = 2 AND z = 3", ["x = 1", "y = 2", "z = 3"]),
+ # A nested `AND` grouped in parentheses is still unwrapped, on
+ # either side of the chain.
+ ("x = 1 AND (y = 2 AND z = 3)", ["x = 1", "y = 2", "z = 3"]),
+ ("(x = 1 AND y = 2) AND z = 3", ["x = 1", "y = 2", "z = 3"]),
+ ("(x = 1 AND (y = 2 AND z = 3))", ["x = 1", "y = 2", "z = 3"]),
+ # An `OR` is a leaf, not a chain to look through.
+ ("x = 1 OR y = 2", ["x = 1 OR y = 2"]),
+ ("x = 1 AND (y = 2 OR z = 3)", ["x = 1", "y = 2 OR z = 3"]),
+ ],
+)
+def test_flatten_and(condition: str, expected: list[str]) -> None:
+ """
+ `_flatten_and` returns every `AND`-ed leaf, regardless of grouping.
+ """
+ from superset.sql.parse import _flatten_and
+
+ parsed = parse_one(f"SELECT * FROM t WHERE {condition}", dialect="oracle")
# noqa: S608
+ leaves = _flatten_and(parsed.args["where"].this)
+ assert [leaf.sql() for leaf in leaves] == expected
Review Comment:
The docstring on `_flatten_and` says the explicit stack exists so a long
chain of predicates cannot hit the recursion limit, but the cases here top out
at three leaves. A regression back to a recursive walk would pass every test in
this file. Would a case that builds an `exp.And` chain directly (parenthesized,
depth above `sys.getrecursionlimit()`) and asserts `_flatten_and` returns every
leaf in order without a `RecursionError` be worth adding?
--
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]
