Repository navigation
Commit 04acb8d
fix(physical-expr): stop false infeasibility report from Eq+FALSE in 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]>1 parent 19f2e85 commit 04acb8d
2 files changed
Lines changed: 265 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
344 | 344 | | |
345 | 345 | | |
346 | 346 | | |
| 347 | + | |
347 | 348 | | |
348 | 349 | | |
349 | 350 | | |
| |||
589 | 590 | | |
590 | 591 | | |
591 | 592 | | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
592 | 725 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
153 | 153 | | |
154 | 154 | | |
155 | 155 | | |
156 | | - | |
| 156 | + | |
157 | 157 | | |
158 | 158 | | |
159 | 159 | | |
| |||
296 | 296 | | |
297 | 297 | | |
298 | 298 | | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
299 | 310 | | |
300 | 311 | | |
301 | 312 | | |
| |||
363 | 374 | | |
364 | 375 | | |
365 | 376 | | |
366 | | - | |
367 | | - | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
368 | 400 | | |
369 | 401 | | |
370 | 402 | | |
| |||
1621 | 1653 | | |
1622 | 1654 | | |
1623 | 1655 | | |
| 1656 | + | |
| 1657 | + | |
| 1658 | + | |
| 1659 | + | |
| 1660 | + | |
| 1661 | + | |
| 1662 | + | |
| 1663 | + | |
| 1664 | + | |
| 1665 | + | |
| 1666 | + | |
| 1667 | + | |
| 1668 | + | |
| 1669 | + | |
| 1670 | + | |
| 1671 | + | |
| 1672 | + | |
| 1673 | + | |
| 1674 | + | |
| 1675 | + | |
| 1676 | + | |
| 1677 | + | |
| 1678 | + | |
| 1679 | + | |
| 1680 | + | |
| 1681 | + | |
| 1682 | + | |
| 1683 | + | |
| 1684 | + | |
| 1685 | + | |
| 1686 | + | |
| 1687 | + | |
| 1688 | + | |
| 1689 | + | |
| 1690 | + | |
| 1691 | + | |
| 1692 | + | |
| 1693 | + | |
| 1694 | + | |
| 1695 | + | |
| 1696 | + | |
| 1697 | + | |
| 1698 | + | |
| 1699 | + | |
| 1700 | + | |
| 1701 | + | |
| 1702 | + | |
| 1703 | + | |
| 1704 | + | |
| 1705 | + | |
| 1706 | + | |
| 1707 | + | |
| 1708 | + | |
| 1709 | + | |
| 1710 | + | |
| 1711 | + | |
| 1712 | + | |
| 1713 | + | |
| 1714 | + | |
| 1715 | + | |
| 1716 | + | |
| 1717 | + | |
| 1718 | + | |
| 1719 | + | |
| 1720 | + | |
| 1721 | + | |
| 1722 | + | |
| 1723 | + | |
| 1724 | + | |
| 1725 | + | |
| 1726 | + | |
| 1727 | + | |
| 1728 | + | |
| 1729 | + | |
| 1730 | + | |
| 1731 | + | |
| 1732 | + | |
| 1733 | + | |
| 1734 | + | |
| 1735 | + | |
| 1736 | + | |
| 1737 | + | |
| 1738 | + | |
| 1739 | + | |
| 1740 | + | |
| 1741 | + | |
| 1742 | + | |
| 1743 | + | |
| 1744 | + | |
| 1745 | + | |
| 1746 | + | |
| 1747 | + | |
| 1748 | + | |
| 1749 | + | |
| 1750 | + | |
| 1751 | + | |
| 1752 | + | |
1624 | 1753 | | |
1625 | 1754 | | |
1626 | 1755 | | |
| |||
0 commit comments