fix: stop treating arithmetic negation as bitwise NOT

[Amogh-2404]

Which issue does this PR close?

• Closes Expression simplifier treats arithmetic negation as bitwise NOT #24665.

Rationale for this change

Expr::Negative represents unary minus, but the simplifier applied identities for bitwise NOT. This produced wrong results in bitwise expressions and removed nested negation without considering the expression type.

Signed integer arrays use wrapping negation, while scalar execution used checked negation. Singleton statistics and partition values can replace columns with scalars, so the same expression could behave differently depending on its source.

Double-negation cancellation is valid for wrapping signed integers, floating-point values, valid decimals, and NULL. Timestamp and interval negation can overflow and must remain explicit.

What changes are included in this PR?

• Remove the bitwise-complement and De Morgan rewrites for Expr::Negative.
• Cancel double negation only for signed integers, floating-point values, valid decimals, and NULL.
• Preserve nested negation for timestamp and interval types.
• Make signed Int8, Int16, Int32, and Int64 scalar negation match array wrapping semantics.
• Support checked timestamp-array negation while preserving timezone metadata and nulls.
• Parenthesize retained nested negation during SQL unparsing.
• Keep interval propagation conservative across overflow and signed lower-unbounded ranges.
• Guard pruning inequalities at the signed minimum and fall back conservatively for casts and nested expressions.

The pre-existing single-negation ordering issue found during review is tracked separately in #24683.

Are these changes tested?

Yes.

• Optimizer tests cover both operand orders for AND, OR, and XOR, both De Morgan shapes, safe typed double-negation cancellation, triple negation, and retained timestamp and interval negation.
• Physical-expression tests cover wrapping scalar negation for all four signed integer widths, all four timestamp units, timezone and null preservation, timestamp overflow, and conservative interval propagation.
• Pruning tests cover all four signed integer widths, every inequality direction, reversed operands, direct equality handling, the scalar MIN case, and conservative fallback for casts and nesting.
• Parquet tests cover pruning enabled and disabled, separate wrapping and non-wrapping files, singleton statistics, derived minimum values, predicates, partition substitution, and AND/OR/XOR.
• Unparser tests cover direct retained negation and optimized SQL round trips in both pretty modes.
• Unbounded-source EXPLAIN tests cover safe widening casts, one-key integer double negation, and the two-key fixed-prefix case.

The full checks pass:

cargo clippy --all-targets --all-features -- -D warnings

RUST_BACKTRACE=1 cargo test --profile ci \
    --exclude datafusion-examples --exclude datafusion-benchmarks --exclude datafusion-cli \
    --workspace --lib --tests --bins \
    --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption

Are there any user-facing changes?

Yes. Unary minus in bitwise expressions now returns correct results. Signed integer negation wraps consistently in scalar and array execution, safe double negation is simplified without breaking checked types, timestamp-array negation works across all timestamp units, and Parquet pruning preserves rows at the signed minimum. There are no API changes.

[sunchao]

Thanks, Amogh. I verified the corrected bitwise results and scalar signed-minimum overflow behavior using a local SessionContext harness on head c37e666 versus base 63f5b55. I also reproduced two regressions caused by retaining double negation; details are inline.

[sunchao]

[P2] Preserve timestamp-column support when retaining double negation

Removing this arm also stops cancelling -(-timestamp_column). I reproduced SELECT -(-ts) AS x FROM t using a registered TimestampNanosecondArray containing 2020-01-01 and 2020-01-02: base 63f5b55f returns both timestamps, while head c37e6666 fails with Invalid arithmetic operation: !Timestamp(ns). SQL analysis and physical planning explicitly accept timestamp negation, but NegativeExpr::evaluate passes arrays to Arrow's neg_wrapping, whose fallback does not support timestamp arrays. Timestamp literals still work through the separate scalar implementation.

The single-negation kernel gap already existed, but retaining both nodes newly breaks these previously working double-negation queries. Could we add timestamp-array negation support and a column-based execution regression test alongside this change, while preserving the intended overflow behavior?

[Amogh-2404]

Fixed in 4e9d57f3c. NegativeExpr now uses checked negation for timestamp arrays in seconds, milliseconds, microseconds, and nanoseconds. It preserves timezone metadata and nulls, and reports overflow for i64::MIN. I added unit coverage for each case and an end-to-end timestamp-column regression.

[sunchao]

[P2] Parenthesize retained nested negations in SQL unparsing

This preserved expression shape breaks SQL generated from optimized plans. I reproduced this by optimizing SELECT -(-i) AS x FROM t over a registered Int64 column, then calling Unparser::default().with_pretty(pretty).plan_to_sql(&plan)?.to_string(). Base 63f5b55f emits SELECT t.i AS x FROM t; head c37e6666 emits SELECT --t.i AS x FROM t. The adjacent minus signs start a SQL comment, so reparsing fails with Expected: an expression, found: EOF in both pretty modes.

datafusion/sql/src/unparser/expr.rs recursively emits bare unary-minus nodes without nesting parentheses. Direct unparsing of an unoptimized double negative already had this gap, but removing cancellation now exposes it in previously working optimized-plan workflows. Could we parenthesize nested unary minus in the unparser and add an optimized-plan SQL roundtrip regression test?

[Amogh-2404]

Fixed in 4e9d57f3c. The unparser now parenthesizes a unary minus when its child is another unary minus or a negative numeric literal. Tests cover direct expression unparsing and optimized-plan SQL round trips in both pretty modes.

[Amogh-2404]

Addressed both rounds of review. Timestamp arrays now use checked negation, nested minus unparses with parentheses, and retained expressions no longer fail interval analysis at the signed minimum.

Following the second review, lower-unbounded signed intervals now avoid unsafe bounds narrowing, with a Parquet statistics regression. I restored the existing ordering propagation rather than extend this PR into cast and equivalence metadata; the remaining pre-existing single-negation ordering issue is tracked in #24683. Unbounded widening-cast and double-negation plans are covered.

All focused tests, full Clippy, the repository lint suite, the extended workspace suite, and the pre-push gate pass.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.50142% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.46%. Comparing base (63f5b55) to head (69af506).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...tafusion/physical-expr/src/expressions/negative.rs	80.19%	4 Missing and 16 partials ⚠️
datafusion/pruning/src/pruning_predicate.rs	94.87%	2 Missing and 6 partials ⚠️
...imizer/src/simplify_expressions/expr_simplifier.rs	98.76%	0 Missing and 1 partial ⚠️
datafusion/sql/src/unparser/expr.rs	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24668      +/-   ##
==========================================
+ Coverage   81.44%   81.46%   +0.02%     
==========================================
  Files        1118     1118              
  Lines      399550   399944     +394     
  Branches   399550   399944     +394     
==========================================
+ Hits       325398   325812     +414     
+ Misses      55154    55110      -44     
- Partials    18998    19022      +24     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sunchao]

Thanks for the fixes, Amogh. I reran the timestamp-array and optimized SQL roundtrip cases on 4e9d57f and both previous findings are addressed. I found two issues in the new bounds/ordering changes and reproduced them against base 63f5b55; details are inline.

[sunchao]

[P1] Preserve wrapped minima during inverse constraint propagation

The new forward-overflow fallback permits an unsafe inverse result here. For input statistics i: [-128, 1], the predicate -i < 0 gives the negation node [NULL, -1] (unbounded below). Negating those endpoints succeeds as [1, NULL], so intersecting with the child incorrectly infers i = 1. But array negation wraps -128 back to -128, which also satisfies the predicate. FilterExec then publishes exact singleton statistics and removes the required sort.

I reproduced this on a single-partition Int8 Parquet scan containing rows [1, -128], with min/max statistics and Parquet pruning/filter pushdown disabled:

SELECT i FROM t
WHERE -i < CAST(0 AS TINYINT)
ORDER BY i LIMIT 1;

Head 4e9d57f3 returns 1, not -128. Base 63f5b55f raised an overflow error during forward analysis; the new fallback exposes this silent wrong-result path. Could negate_interval also fall back for signed intervals whose lower endpoint is unbounded, and add a statistics-backed execution regression? I verified that this guard restores SortExec and the correct result in an isolated experiment.

[Amogh-2404]

Fixed in 1b7d5f9. negate_interval now falls back for signed lower-unbounded intervals as well as arithmetic overflow. I added a unit test for forward and inverse bounds, plus a Parquet statistics execution regression. The reported query now retains -128 and returns the correct row.

[sunchao]

[P2] Retain ordering for safe integer widening on unbounded inputs

This guard also drops ordering when the signed minimum is provably unreachable. For an unbounded CSV source declared with i INTEGER NOT NULL and WITH ORDER (i ASC NULLS LAST), I tested:

SELECT i FROM neg_order_stream
ORDER BY -CAST(i AS BIGINT) DESC NULLS LAST;

Base 63f5b55f builds a StreamingTableExec without a sort. Head 4e9d57f3 inserts a global SortExec and fails SanityCheckPlan with Cannot execute pipeline breaking queries. Int32 -> Int64 is an exact widening conversion, so its result can never be Int64::MIN and negation safely reverses the declared ordering. However, cast_expr_properties currently replaces the source range with unbounded Int64, causing this condition to classify it as potentially wrapping.

Could we retain the representable source-domain bounds for strict integer widening casts so this guard can preserve safe ordering, with an unbounded-source planning regression test? The same new rejection also affects ORDER BY -(-i), although wrapping integer-array double negation preserves every input value.

[Amogh-2404]

Fixed in 1b7d5f9 by narrowing the scope. I restored NegativeExpr::get_properties and the existing order.slt expectation instead of changing shared cast metadata here. Two unbounded-source EXPLAIN cases now cover both the widening cast and -(-i) without a global sort. I filed the pre-existing single-negation ordering problem separately as #24683.

[Amogh-2404]

Update after the latest review: signed integer scalar and array negation now share wrapping semantics, and double-negation cancellation is type-aware. It applies to signed integers, floating-point values, valid decimals, and NULL; timestamp and interval negation remains explicit because it can overflow.

The pruning rewrite now guards both the input and scalar MIN cases. Direct -column predicates retain useful min/max pruning away from the wrap point, while casts and nested signed negation fall back conservatively.

The regression coverage includes all four signed integer widths, all inequality directions, equality and distinctness, scalar-left comparisons, Parquet files on both sides of the wrap point, and the reported two-key unbounded stream plan. Full Clippy, the extended workspace suite, all 505 SQL logic-test files, and the pre-push gate pass on 69af50622.

[sunchao]

Thanks, Amogh. I verified head 00b519c3a against base 63f5b55f with identical SessionContext probes compiled in separate target directories. I reproduced two remaining regressions, detailed inline. The 234 focused tests and 12 additional SQL roundtrip checks passed; I did not rerun the full workspace suite.

[sunchao]

[P1] Make inequality pruning safe before wrapping scalar negation

Pruning still rewrites -i > MIN into i < -MIN. These new wrapping cases make -MIN equal MIN, so the resulting i_min < MIN predicate incorrectly discards matching data. With a Parquet TINYINT NOT NULL column containing [1, 2], I reproduced:

SELECT i FROM t
WHERE -i > CAST(-128 AS TINYINT)
ORDER BY i;

Base 63f5b55f returns both rows; head 00b519c3a returns none. Execution metrics confirm file-statistics pruning discards the file before reading rows. At base, checked overflow causes pruning to fall back conservatively. I reproduced the same regression for all four signed integer widths.

Could we make the pruning rewrite conservative for wrapping inequalities and add an execution regression with pruning enabled, while keeping the intended scalar/array consistency?

[Amogh-2404]

Fixed in 69af50622. I added a MIN-aware guard when pruning inequalities through direct signed-integer negation. The normal rewrite remains precise away from the wrapping point; scalar MIN, input MIN, casts, and nested negation fall back conservatively. Unit tests cover all four integer widths and comparison forms, and the Parquet regression runs all four inequality directions with pruning enabled.

[sunchao]

[P2] Preserve integer double-negation ordering equivalence

Removing this arm preserves -(-i), whose properties no longer establish the full ordering of an input sorted by (i, j). On an unbounded stream with constant i = 0 and nondecreasing j, I reproduced:

SELECT i, j FROM fixed_prefix_stream
ORDER BY -(-i) ASC NULLS LAST, j ASC NULLS LAST
LIMIT 1;

Base 63f5b55f uses StreamingTableExec with fetch=1 and returns (0, 0) immediately. Head 00b519c3a inserts PartialSortExec: TopK(fetch=1), common_prefix_length=[1] and times out. The partial sort waits for i to change, so a permanently fixed prefix never emits its first row. Both direct ORDER BY i, j LIMIT 1 and one-key ORDER BY -(-i) LIMIT 1 still return immediately on head.

Could we restore cancellation specifically for signed integers, where wrapping makes it safe even at MIN, or preserve equivalent ordering metadata? Extending the new streaming regression to two sort keys would cover this without changing checked timestamp/interval behavior.

[Amogh-2404]

Fixed in 69af50622. Double-negation cancellation is now type-aware: signed integers, floating-point values, valid decimals, and NULL cancel, while timestamp and interval negation remains explicit. This restores the full ordering equivalence for integer -(-i). The unbounded regression now includes the reported two-key fixed-prefix plan and keeps fetch=1 on StreamingTableExec without a partial sort.