sunchao commented on PR #24668: URL: https://github.com/apache/datafusion/pull/24668#issuecomment-5455765336
### [P2] Keep newly exposed projected suffixes safe at signed MIN @Amogh-2404, I verified a second regression through direct public physical-plan construction on `2bad6666a` versus base `a38bb10c3`. It exposes the existing #24683 mechanism through the new double-negation projection substitution. The input has nonnullable `Int8` columns `(i,j)`, rows `(1,-128), (1,-1)`, and truthful ordering `(i ASC NULLS LAST, j ASC NULLS LAST)`. It has no constant-column or uniqueness metadata. Construct a `ProjectionExec` retaining these physical expressions: ```text Negative(Negative(i)) AS k Negative(j) AS n ``` Place a `SortExec` requiring `(k ASC NULLS LAST, n DESC NULLS LAST)` above it, then run `EnsureRequirements`. I tested both a full sort and `fetch=1`: | Revision | Projection advertises | Full result | Top-K(1) | | --- | --- | --- | --- | | Base `a38bb10c3` | `k ASC` | `(1,1), (1,-128)` | `(1,1)` | | Head `2bad6666a` | `k ASC, n DESC` | `(1,-128), (1,1)` | `(1,-128)` | Base retains the required sort. Head removes it and returns the wrong order and Top-K row. The [new substitution](https://github.com/apache/datafusion/blob/2bad6666a5ee333a90aaa0a7d55f94ea78e00afe/datafusion/physical-expr/src/equivalence/properties/mod.rs#L256-L264) makes the first key projectable, allowing suffix analysis to reach `j`. That analysis uses the existing `NegativeExpr` ordering claim for `-j`, which is unsafe at the wrapping minimum. The double-negation pair itself remains an identity. For comparison, projecting plain `i AS k, -j AS n` is already wrong on both revisions; that control isolates the pre-existing #24683 behavior from this newly failing case. This reproduction uses the physical API directly. Ordinary SQL simplifies numeric double negation, so I am not claiming a new default-SQL failure. Could the newly inferred suffix remain conservative when it depends on potentially wrapping negation, with a physical-plan regression covering both full sorting and Top-K? -- 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]
