sunchao commented on code in PR #24668:
URL: https://github.com/apache/datafusion/pull/24668#discussion_r3864338094
##########
datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs:
##########
@@ -1411,11 +1339,6 @@ impl TreeNodeRewriter for Simplifier<'_> {
//
Expr::Not(inner) => Transformed::yes(negate_clause(*inner)),
- //
- // Rules for Negative
- //
- Expr::Negative(inner) =>
Transformed::yes(distribute_negation(*inner)),
Review Comment:
**[P2] Retained temporal double negation still loses grouping-key ordering**
The integer `ORDER BY` case is fixed. I found a remaining regression on
unmodified head `69af50622` for timestamp and interval grouping keys:
```sql
SELECT -(-i) AS k, j, COUNT(*) AS n
FROM s
GROUP BY -(-i), j
LIMIT 1;
```
I used a single-partition, unbounded `StreamingTable` declared ordered by
`(i ASC NULLS LAST, j ASC NULLS LAST)`, with `target_partitions=1` and
`batch_size=1`. It emits two rows with `i = TimestampNanosecond(1)` and `j =
[0, 1]`, then remains pending without end-of-stream. These are ordinary values
with no overflow.
Base `63f5b55f` builds `AggregateExec` with `ordering_mode=Sorted` and
immediately returns the completed `j=0` group with count 1. Head `69af50622`
instead uses `ordering_mode=PartiallySorted([0])` and produces no row before
the probe's 300 ms timeout. I reproduced the same difference with
`IntervalYearMonth(1)` and a nullable timestamp containing NULL.
`EquivalenceProperties::find_longest_permutation` marks only `-(-i)` as a
temporary constant, without establishing that `i` is constant within those
ties, so it cannot recognize the ordered suffix `j`. `GroupOrderingPartial`
consequently waits for the leading key to change or for end-of-stream, even
though the first complete group is already available. A stream whose leading
key stays fixed therefore cannot satisfy this `LIMIT 1`.
Could we preserve the full grouping-key ordering for retained temporal
double negation while keeping the checked negations in the execution plan, and
add a streaming execution regression for this case?
--
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]
