fix: incorrect results when using NOT physical expression in physical… - #20138

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
to mean the predicate is always false, but NOT (a = 0.0) is true
for every a != 0, so the correct result is the full input domain.

The only case that should remain infeasible under Eq + FALSE is
when both children are the same single-point interval: then a = b
is certainly true, so NOT(a = b) is impossible. All other cases
return the children unchanged (Ok(Some((left, right)))) so
propagation completes with Success and the column interval is
preserved. This guard was agreed on during review of apache#20138 by
berkaysynnada and pepijnve.

Downstream, analyze() feeds FilterExec::statistics_helper(), which
uses an Infeasible result to set selectivity to 0.0 and clear all
column boundaries, producing wrong statistics estimation.

This revives the fix from apache#20138, which was closed as stale after
review, and adds direct regression coverage for the
propagate_comparison Eq + FALSE branch that the previous E2E
analysis tests did not exercise (the [0,0] infeasible case
short-circuits at update_ranges before reaching
propagate_comparison).

Tests:
- analysis.rs: boundary pair [-1,1] (satisfiable) and [0,0]
  (infeasible) E2E via analyze().
- cp_solver.rs: direct unit tests for propagate_comparison with
  Eq + FALSE covering identical singletons (infeasible), different
  singletons (feasible), overlapping intervals, and the [-1,1] vs
  [0,0] case from the issue.

Closes apache#19264.

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
to mean the predicate is always false, but NOT (a = 0.0) is true
for every a != 0, so the correct result is the full input domain.

The only case that should remain infeasible under Eq + FALSE is
when both children are the same single-point interval: then a = b
is certainly true, so NOT(a = b) is impossible. All other cases
return the children unchanged (Ok(Some((left, right)))) so
propagation completes with Success and the column interval is
preserved. This guard was agreed on during review of apache#20138 by
berkaysynnada and pepijnve.

Downstream, analyze() feeds FilterExec::statistics_helper(), which
uses an Infeasible result to set selectivity to 0.0 and clear all
column boundaries, producing wrong statistics estimation.

This revives the fix from apache#20138, which was closed as stale after
review, and adds direct regression coverage for the
propagate_comparison Eq + FALSE branch that the previous E2E
analysis tests did not exercise (the [0,0] infeasible case
short-circuits at update_ranges before reaching
propagate_comparison).

Tests:
- analysis.rs: boundary pair [-1,1] (satisfiable) and [0,0]
  (infeasible) E2E via analyze().
- cp_solver.rs: direct unit tests for propagate_comparison with
  Eq + FALSE covering identical singletons (infeasible), different
  singletons (feasible), overlapping intervals, and the [-1,1] vs
  [0,0] case from the issue.

Closes apache#19264.

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
in ExprBoundaries::interval to mean "evaluating the given column
results in an empty set", but NOT (a = 0.0) is true for every
a != 0, so the correct result is the full input domain. This is a
contract violation of the public analyze() API, as reported in apache#19264
by an external user calling analyze() to infer bounds for pushdown
into another library.

The only case that should remain infeasible under Eq + FALSE is
when both children are the same single-point interval: then a = b
is certainly true, so NOT(a = b) is impossible. All other cases
return the children unchanged (Ok(Some((left, right)))) so
propagation completes with Success and the column interval is
preserved. This guard was agreed on during review of apache#20138 by
berkaysynnada and pepijnve.

This revives the fix from apache#20138, which was closed as stale after
review, and adds direct regression coverage for the
propagate_comparison Eq + FALSE branch that the previous E2E
analysis tests did not exercise (the [0,0] infeasible case
short-circuits at update_ranges before reaching
propagate_comparison).

Tests:
- analysis.rs: boundary pair [-1,1] (satisfiable) and [0,0]
  (infeasible) E2E via analyze().
- cp_solver.rs: direct unit tests for propagate_comparison with
  Eq + FALSE covering identical singletons (infeasible), different
  singletons (feasible), overlapping intervals, and the [-1,1] vs
  [0,0] case from the issue.

Closes apache#19264.

Co-authored-by: Claude <[email protected]>

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
in ExprBoundaries::interval to mean "evaluating the given column
results in an empty set", but NOT (a = 0.0) is true for every
a != 0, so the correct result is the full input domain. This is a
contract violation of the public analyze() API, as reported in apache#19264
by an external user calling analyze() to infer bounds for pushdown
into another library.

The only case that should remain infeasible under Eq + FALSE is
when both children are the same single-point interval: then a = b
is certainly true, so NOT(a = b) is impossible. All other cases
return the children unchanged (Ok(Some((left, right)))) so
propagation completes with Success and the column interval is
preserved. This guard was agreed on during review of apache#20138 by
berkaysynnada and pepijnve.

This revives the fix from apache#20138, which was closed as stale after
review, and adds direct unit coverage for that guard. No
analyze()-level test can reach it: when both children are the same
single point the bounds evaluate to FALSE, so update_ranges
short-circuits to Infeasible before propagate_constraints runs.
Only a direct propagate_comparison call exercises the guard.

Tests:
- analysis.rs: E2E boundary pair via analyze(). Input [-1,1] is the
  reported case and does reach the fixed branch; input [0,0] is the
  nearest-invalid counterpart and must stay infeasible.
- cp_solver.rs: direct propagate_comparison unit tests for Eq + FALSE
  covering identical singletons (infeasible), different singletons
  (feasible), overlapping intervals, and the [-1,1] vs [0,0] case
  from the issue.

Negative control on baseline a1f64e9 with the production arm
reverted to Ok(None): 3 of the 4 new tests fail, including the E2E
test_analyze_not_eq_around_zero. test_analyze_not_eq_clamped_at_zero
passes on baseline as well, so it is a boundary guard rather than
evidence of the fix.

Closes apache#19264.

Co-authored-by: evangelisilva <[email protected]>

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
in ExprBoundaries::interval to mean "evaluating the given column
results in an empty set", but NOT (a = 0.0) is true for every
a != 0, so the correct result is the full input domain. This is a
contract violation of the public analyze() API, as reported in apache#19264
by an external user calling analyze() to infer bounds for pushdown
into another library.

The only case that should remain infeasible under Eq + FALSE is
when both children are the same single-point interval: then a = b
is certainly true, so NOT(a = b) is impossible. All other cases
return the children unchanged (Ok(Some((left, right)))) so
propagation completes with Success and the column interval is
preserved. This guard was agreed on during review of apache#20138 by
berkaysynnada and pepijnve.

This revives the fix from apache#20138, which was closed as stale after
review, and adds direct unit coverage for that guard. No
analyze()-level test can reach it: when both children are the same
single point the bounds evaluate to FALSE, so update_ranges
short-circuits to Infeasible before propagate_constraints runs.
Only a direct propagate_comparison call exercises the guard.

Tests:
- analysis.rs: E2E boundary pair via analyze(). Input [-1,1] is the
  reported case and does reach the fixed branch; input [0,0] is the
  nearest-invalid counterpart and must stay infeasible.
- cp_solver.rs: direct propagate_comparison unit tests for Eq + FALSE
  covering identical singletons (infeasible), distinct singletons
  (feasible), and overlapping intervals including the [-1,1] vs [0,0]
  case from the issue.

Negative control on baseline a1f64e9 with the production arm
reverted to Ok(None): 3 of the 5 new tests fail, including the E2E
test_analyze_not_eq_around_zero. Each test is scoped so its baseline
verdict is unambiguous. The two that pass on baseline
(test_propagate_eq_false_identical_singletons and
test_analyze_not_eq_clamped_at_zero) are boundary guards rather than
evidence of the fix: the baseline returns None for every Eq + FALSE
input, so it agrees with them by accident.

Closes apache#19264.

Co-authored-by: evangelisilva <[email protected]>

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
in ExprBoundaries::interval to mean "evaluating the given column
results in an empty set", but NOT (a = 0.0) is true for every
a != 0, so the correct result is the full input domain. This is a
contract violation of the public analyze() API, as reported in apache#19264
by an external user calling analyze() to infer bounds for pushdown
into another library.

Eq + FALSE is infeasible when equality is provably true for the two
singleton operands under the comparison semantics. This must use
comparison-semantics equality, not structural ScalarValue::PartialEq:
the latter is bit-wise for floats (to_bits), under which -0.0 != +0.0
even though SQL/IEEE-754 comparison treats them as equal. The guard
uses singleton_values_equal, which normalizes signed zero via
normalize_float_zero_scalar (the same normalization DataFusion applies
in physical-expr-common/src/datum.rs for runtime comparison) before
structural equality. This matters in nested boolean contexts (e.g.
NOT(a = +0.0) AND b) where the bottom-up pass does not short-circuit
and top-down propagation forces Eq to FALSE, reaching this arm with
[-0.0,-0.0] vs [+0.0,+0.0].

This revives the fix from apache#20138, which was closed as stale after
review. The singleton guard was agreed on during apache#20138 review by
berkaysynnada and pepijnve; this version strengthens it to use
comparison-semantics equality and adds direct regression coverage
for the Eq + FALSE branch that no analyze()-level test can reach
(the [0,0] infeasible case short-circuits at update_ranges before
propagate_comparison runs).

Tests:
- analysis.rs: E2E tests via analyze(). Input [-1,1] is the reported
  case and does reach the fixed branch; input [0,0] is the
  nearest-invalid counterpart and must stay infeasible; the nested
  NOT(a = +0.0) AND b case proves the case-(1) short-circuit does
  not protect the production arm and exercises the signed-zero guard.
- cp_solver.rs: direct propagate_comparison unit tests for Eq + FALSE
  covering identical singletons (infeasible), signed-zero singletons
  (infeasible under comparison semantics), distinct singletons
  (feasible), and overlapping intervals including the [-1,1] vs [0,0]
  case from the issue.

Negative control on baseline a1f64e9 with the production arm
reverted to Ok(None): 4 of the 7 new tests fail. The 3 that pass on
baseline are boundary guards (baseline returns None for every
Eq + FALSE input, so it agrees by accident). Additionally, replacing
singleton_values_equal with structural == makes the signed-zero
tests fail, confirming they exercise the comparison-semantics guard.

Closes apache#19264.

Co-authored-by: evangelisilva <[email protected]>

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
in ExprBoundaries::interval to mean "evaluating the given column
results in an empty set", but NOT (a = 0.0) is true for every
a != 0, so the correct result is the full input domain. This is a
contract violation of the public analyze() API, as reported in apache#19264
by an external user calling analyze() to infer bounds for pushdown
into another library.

Eq + FALSE is infeasible when equality is provably true for the two
singleton operands under the comparison semantics. This must use
comparison-semantics equality, not structural ScalarValue::PartialEq:
the latter is bit-wise for floats (to_bits), under which -0.0 != +0.0
even though SQL/IEEE-754 comparison treats them as equal. The guard
uses singleton_values_equal, which normalizes signed zero via
normalize_float_zero_scalar (the same normalization DataFusion applies
in physical-expr-common/src/datum.rs for runtime comparison) before
structural equality. This matters in nested boolean contexts (e.g.
NOT(a = +0.0) AND b) where the bottom-up pass does not short-circuit
and top-down propagation forces Eq to FALSE, reaching this arm with
[-0.0,-0.0] vs [+0.0,+0.0].

This revives the fix from apache#20138, which was closed as stale after
review. The singleton guard was agreed on during apache#20138 review by
berkaysynnada and pepijnve; this version strengthens it to use
comparison-semantics equality and adds direct regression coverage
for the Eq + FALSE branch that no analyze()-level test can reach
(the [0,0] infeasible case short-circuits at update_ranges before
propagate_comparison runs).

Tests:
- analysis.rs: E2E tests via analyze(). Input [-1,1] is the reported
  case and does reach the fixed branch; input [0,0] is the
  nearest-invalid counterpart and must stay infeasible; the nested
  NOT(a = +0.0) AND b case proves the case-(1) short-circuit does
  not protect the production arm and exercises the signed-zero guard.
- cp_solver.rs: direct propagate_comparison unit tests for Eq + FALSE
  covering identical singletons (infeasible), signed-zero singletons
  (infeasible under comparison semantics), distinct singletons
  (feasible), and overlapping intervals including the [-1,1] vs [0,0]
  case from the issue.

Negative control on baseline c085cdf with the production arm
reverted to Ok(None): 4 of the 7 new tests fail. The 3 that pass on
baseline are boundary guards (baseline returns None for every
Eq + FALSE input, so it agrees by accident). Additionally, replacing
singleton_values_equal with structural == makes the signed-zero
tests fail, confirming they exercise the comparison-semantics guard.

Closes apache#19264.

Co-authored-by: evangelisilva <[email protected]>

…cp_solver

propagate_comparison for Operator::Eq with parent == Interval::FALSE
returned Ok(None), which ExprIntervalGraph::propagate_constraints
interprets as infeasible. The correct semantics is that a = b being
certainly false means a != b, which excludes at most a single point
from each operand. Interval arithmetic cannot represent a hole, so
the children cannot be refined further, but the expression is not
infeasible.

This caused analyze() to return None for the column interval when
the predicate was NOT (a = 0.0) and the input domain contained 0.0
but was not equal to it (e.g. a in [-1, 1]). The None is documented
in ExprBoundaries::interval to mean "evaluating the given column
results in an empty set", but NOT (a = 0.0) is true for every
a != 0, so the correct result is the full input domain. This is a
contract violation of the public analyze() API, as reported in apache#19264
by an external user calling analyze() to infer bounds for pushdown
into another library.

Eq + FALSE is infeasible when equality is provably true for the two
singleton operands under the comparison semantics. This must use
comparison-semantics equality, not structural ScalarValue::PartialEq:
the latter is bit-wise for floats (to_bits), under which -0.0 != +0.0
even though SQL/IEEE-754 comparison treats them as equal. The guard
uses singleton_values_equal, which normalizes signed zero via
normalize_float_zero_scalar (the same normalization DataFusion applies
in physical-expr-common/src/datum.rs for runtime comparison) before
structural equality. This matters in nested boolean contexts (e.g.
NOT(a = +0.0) AND b) where the bottom-up pass does not short-circuit
and top-down propagation forces Eq to FALSE, reaching this arm with
[-0.0,-0.0] vs [+0.0,+0.0].

This revives the fix from apache#20138, which was closed as stale after
review. The singleton guard was agreed on during apache#20138 review by
berkaysynnada and pepijnve; this version strengthens it to use
comparison-semantics equality. Direct unit tests isolate the ordinary
identical-singleton case that the [0,0] analyze() test cannot reach
because update_ranges short-circuits. The nested signed-zero E2E test
separately proves that analyze() can reach the production arm through
the pre-existing interval/runtime equality divergence.

Tests:
- analysis.rs: E2E tests via analyze(). Input [-1,1] is the reported
  case and does reach the fixed branch; input [0,0] is the
  nearest-invalid counterpart and must stay infeasible; the nested
  NOT(a = +0.0) AND b case proves the case-(1) short-circuit does
  not protect the production arm and exercises the signed-zero guard.
- cp_solver.rs: direct propagate_comparison unit tests for Eq + FALSE
  covering identical singletons (infeasible), signed-zero singletons
  (infeasible under comparison semantics), distinct singletons
  (feasible), and overlapping intervals including the [-1,1] vs [0,0]
  case from the issue.

Negative control on baseline 6eaca8b with the production arm
reverted to Ok(None): 3 of the 7 new tests fail. The 4 that pass on
baseline are boundary guards (baseline returns None for every
Eq + FALSE input, so it agrees by accident). A second, independent
control covers those guards: replacing singleton_values_equal with
structural == makes both signed-zero tests fail, confirming they
exercise the comparison-semantics guard rather than passing
vacuously.

Closes apache#19264.

Co-authored-by: evangelisilva <[email protected]>

…cp_solver (apache#24464)

## Which issue does this PR close?

- Closes apache#19264.

## Rationale for this change

`physical_expr::analyze` is a public API. For a satisfiable predicate
such as
`NOT (a = 0.0)` over `a ∈ [-1, 1]`, it currently returns `None`, which
`ExprBoundaries::interval` documents as an empty set. The safe result is
the
unchanged input interval because one interval cannot represent the hole
at zero.

The same infeasible-sentinel mistake exists in `propagate_comparison`'s
catch-all
uncertainty arm. That arm is shared by every supported comparison
operator that
reaches it with an uncertain Boolean parent. Returning the child
intervals unchanged
preserves their enclosure for all operand shapes, including shapes
beyond the three
issue test functions; it does not claim a refinement that interval
arithmetic cannot
prove.

This carries forward the original patch by @evangelisilva in apache#20138 and
preserves
its reviewed equal-singleton infeasibility guard. The original bug-fix
commit in this PR contains a valid `Co-authored-by` trailer for the
original author.

## What changes are included in this PR?

- For `Eq + FALSE`, preserve both child intervals unless both are
bounded,
  equal singletons. Only the latter case is provably infeasible.
- For an uncertain comparison parent, return both child intervals
unchanged
  instead of returning the infeasible sentinel.
- Add direct tests for bounded-singleton × unbounded and unbounded ×
unbounded,
  preventing `[NULL, NULL]` from being mistaken for a known singleton.
- Add end-to-end coverage for `NOT (a = 0.0)`, `not_between(...)`, and
explicit
  `between(...).not()` over the issue's input domains.

## Are these changes tested?

Yes. The three issue test functions execute independently:

- `test_analyze_not_eq_around_zero` passes with the repair.
- `test_analyze_not_between_clamped` is a baseline-passing negative
control: its
  infeasible result remains unchanged.
- `test_analyze_not_between_containing` passes with the repair for both
NOT spellings.

The complete `datafusion-physical-expr` library suite passes: 1607
passed,
0 failed, 2 ignored. Formatting and clippy with all targets and all
features
also pass.

## Are there any user-facing changes?

No API signature change. `analyze()` no longer reports a satisfiable
negated
comparison as infeasible when interval arithmetic cannot refine its
operands.

---------

Signed-off-by: Han-Yin Chang <[email protected]>
Co-authored-by: evangelisilva <[email protected]>