comphead commented on PR #16918: URL: https://github.com/apache/datafusion/pull/16918#issuecomment-3127884680
> @comphead thanks for the detailed explanation. The `state()`'s are taken from Comet, right? Thanks @berkaysynnada, the state is taken from TrivialAccumulator. What Comet does is just call physical `LastValue` function https://github.com/apache/datafusion-comet/blob/320ce55eec9f4c846ad7a68bb69c182ae1fcd3ce/native/core/src/execution/planner.rs#L1859 so all internal details, including accumulators, states, rowhash are coming from DF. > that’s an invalid state, and we shouldn’t be trying to fix it here IMO I suppose if we know this is an invalid state we still should ignore it like in this PR, if there is a mistake in planner/optimizer it can lead to silent data corruption I'm trying to reproduce this with unit test if I sent batches with exacly same order and content like Comet does. -- 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]
