Re: [PR] fix: ignore repeated sort keys in an ordered aggregate's ORDER BY [datafusion]

["hassaanch23 (via GitHub)" <gi...@apache.org>]

hassaanch23 commented on PR #25400:
URL: https://github.com/apache/datafusion/pull/25400#issuecomment-5973756994

   Gentle ping on this one. It fixes the `first_last.rs` assertion panic in #25398 and has been green and mergeable since 18 September.
   
   @neilconway, since you reviewed #25402, which changed the same spot in `AggregateExprBuilder::build()`, you might be the quickest person to look. Both changes now sit in one expression: an order-insensitive aggregate's `ORDER BY` is cleared, and otherwise repeated sort keys are dropped with `LexOrdering`, so the state fields match the sort keys the accumulators compare. @haochunchang verified it independently above: the three new `.slt` queries fail without the change and pass with it.
   
   I re-checked it against current `main` today. It still merges cleanly, and the aggregate, group-by and window sqllogictests pass.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org