zanmato1984 commented on code in PR #41614:
URL: https://github.com/apache/arrow/pull/41614#discussion_r1598735140
##########
cpp/src/arrow/acero/asof_join_node.cc:
##########
@@ -918,34 +922,39 @@ class CompositeTableBuilder {
// guaranteeing this probability is below 1 in a billion. The fix is 128-bit
hashing.
// See ARROW-17653
class AsofJoinNode : public ExecNode {
- // Advances the RHS as far as possible to be up to date for the current LHS
timestamp
- Result<bool> UpdateRhs() {
+ // Advances the RHS as far as possible to be up to date for the current LHS
timestamp.
+ // Returns a pair of booleans. The first is if any RHS has advanced. The
second is if
+ // all RHS are up to date for LHS.
+ Result<std::pair<bool, bool>> UpdateRhsAndCheckUpToDateWithLhs() {
Review Comment:
Addressed.
##########
cpp/src/arrow/acero/asof_join_node.cc:
##########
@@ -548,8 +548,10 @@ class InputState {
// true when the queue is empty and, when memo may have future entries (the
case of a
// positive tolerance), when the memo is empty.
// used when checking whether RHS is up to date with LHS.
- bool CurrentEmpty() const {
- return memo_.no_future_ ? Empty() : memo_.times_.empty() && Empty();
+ // NOTE: The emptiness must be decided by an atomic all to Empty() in
caller, due to the
+ // potential race with Push(), see GH-41614.
+ bool CurrentEmpty(bool empty) const {
+ return memo_.no_future_ ? empty : memo_.times_.empty() && empty;
Review Comment:
Addressed.
--
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]
