bharadwaj-pendyala opened a new pull request, #24763:
URL: https://github.com/apache/datafusion/pull/24763
## Which issue does this PR close?
No separate issue. I found this while reading `simplify_predicates`.
## Rationale for this change
`WHERE s = 'a' AND 'a' = s` returns no rows, where one row is expected:
```sql
> CREATE TABLE t(s VARCHAR) AS VALUES ('a'), ('b');
> SELECT * FROM t WHERE s = 'a' AND 'a' = s;
0 row(s) fetched.
```
Either half on its own returns `a`, and the same query against an INT column
returns the row.
On `main` (4d3e79e), `EXPLAIN VERBOSE` shows the filter turning into a
constant between two rules:
```
logical_plan after simplify_expressions Filter: t.s = Utf8View("a") AND
Utf8View("a") = t.s
logical_plan after push_down_filter Filter: Boolean(false)
logical_plan after eliminate_filter EmptyRelation: rows=0
```
`PushDownFilter` splits the conjuncts and calls `simplify_predicates`. It
accepts both `<col> <op> <literal>` and `<literal> <op> <col>`, but
`simplify_column_predicates` compares whole `Expr`s. `t.s = Utf8View("a")` and
`Utf8View("a") = t.s` aren't structurally equal, so the two equalities read as
a contradiction and the conjunction becomes `false`.
The INT version survives because the `Canonicalizer` reorders it first. It
can't do that here: it runs once at `expr_simplifier.rs:203`, ahead of the
const-evaluation loop, so it sees `CAST(Utf8("a") AS Utf8View)` rather than a
`Literal` and its `(Literal, Column)` arm doesn't match. The cast folds to a
literal afterwards. Canonicalization is skipped entirely for `Join`
(`simplify_exprs.rs:130`), so `simplify_predicates` can't assume canonical
input either way.
The same gap costs a strict bound. Given `a >= 5` and `5 < a`,
`find_most_restrictive_predicate` breaks the tie on `op == Gt`, doesn't count
`Lt` with the literal on the left as strict, keeps `a >= 5`, and lets `a = 5`
through.
## What changes are included in this PR?
`simplify_predicates` now normalizes the literal to the right with
`op.swap()`, at the point where it already distinguishes the two orientations.
`simplify_column_predicates` can then match on the operator alone. No signature
changes.
## Are these changes tested?
Two unit tests in `simplify_predicates.rs` and four cases in
`simplify_predicates.slt`. All six fail before the fix. With only
`simplify_predicates.rs` reverted the SLT reports `EmptyRelation: rows=0` where
`Filter: test_data.str_col = Utf8View("apple")` is expected, and the `apple`
row goes missing.
`datafusion-optimizer` is green (765 lib, 26 integration, 5 doc) and clippy
with `-D warnings` is clean. The full `sqllogictests` run passes except
`window_limits.slt`, which fails identically on an unmodified `main`.
`SELECT * FROM t WHERE s = 'a' AND 'b' = s` stays `EmptyRelation: rows=0`
before and after, and that's pinned in the SLT.
## Are there any user-facing changes?
Affected queries return the right rows instead of none.
Predicates reaching `simplify_predicates` with the literal on the left now
come back with it on the right, so a plan can show `a > 5` where it used to
show `5 < a`. Nothing in the test suite depended on that, but the function is
public.
Equalities whose literals are equal in value but differ in `ScalarValue`
representation still collapse to `false`. On `main`, `[a = 5i32, a = 5i64]` in
the same orientation already returns `Boolean(false)`, so that predates this
change and isn't orientation related.
This PR was written with AI assistance.
--
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]
