SEPURI-SAI-KRISHNA commented on code in PR #29345:
URL: https://github.com/apache/flink/pull/29345#discussion_r4284242097


##########
flink-table/flink-table-test-utils/src/main/java/org/apache/flink/table/runtime/functions/Timer.java:
##########
@@ -81,6 +81,15 @@ void markFired() {
         this.fired = true;
     }
 
+    /** Creates an isolated copy of this timer, including whether it has 
already fired. */
+    Timer copy() {
+        final Timer copy = new Timer(timestamp, name, Row.copy(partitionKey));

Review Comment:
   Added, and `copy(..)` uses it, so the construct-then-mark here is gone. 
`markFired()` stays because `fireEligibleTimers` flips a timer already in the 
pending set. One call for you: `copy(..)` now has a single caller and could be 
inlined as `new Timer(t.timestamp, t.name, copyKey(t.partitionKey), 
t.hasFired())`. I kept the method because `fired` is private and a `copy()` 
next to the fields is what gets updated if a field is ever added. Tell me which 
you prefer and I will do it, here or as a follow-up.



-- 
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]

Reply via email to