crepererum commented on code in PR #13524:
URL: https://github.com/apache/datafusion/pull/13524#discussion_r1853764821
##########
datafusion/physical-plan/src/aggregates/group_values/row.rs:
##########
@@ -216,18 +216,18 @@ impl GroupValues for GroupValuesRows {
}
std::mem::swap(&mut new_group_values, &mut group_values);
- // SAFETY: self.map outlives iterator and is not modified
concurrently
Review Comment:
Picking this one, but this pattern occurs multiple times:
That `SAFETY` statement is just plain wrong: while iterating over the map,
we erase elements from it. That is THE prime example for "concurrent
modification of containers" and why Rust's lifetimes/reference system prevents
that. I'm kinda surprised that this hasn't exploded yet.
--
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]
