rionmonster commented on PR #29570: URL: https://github.com/apache/flink/pull/29570#issuecomment-6421910649
Thanks for the thorough pass, @gustavodemorais. I've addressed everything inline, and there's one design question I'd like to get your take on that came up around deletes. **DELETEs of records that aren't stored.** The operator can't tell a record that was never inserted from one that expired. Skipping the DELETE like some of the comments above address fixes the never-inserted case, but after an expiry nothing gets retracted beyond that. I took a middle path: - The right-side lookup in the first comment only happens for upsert inputs (or plans without changelog modes), where deleting a key that was never inserted is normal, for example a tombstone. - Retract inputs still decrement the count, which is correct after an expiry (and we've address the negative values earlier). - In the anti join, a key-only DELETE of a left row that isn't stored now emits `-D[key-only row]`, the same as the outer join's `-D[record+null]`. Also: > A DELETE for a right row that is not in state still decrements the left count to -1, and since isNewMatch never increments a non-zero count when the join key contains the right unique key, the left row never comes back. As you mentioned the semi/anti join has this problem and I think it would affect the regular join too. Should we make this into a separate ticket or fold it in the fixes here? Thoughts on these? -- 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]
