phet commented on code in PR #3790:
URL: https://github.com/apache/gobblin/pull/3790#discussion_r1343054868


##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -104,32 +116,50 @@ protected interface CheckedFunction<T, R> {
       + "VALUES(1, ?, ?) ON DUPLICATE KEY UPDATE epsilon=VALUES(epsilon), 
linger=VALUES(linger)";
   protected static final String WHERE_CLAUSE_TO_MATCH_KEY = "WHERE 
flow_group=? AND flow_name=? AND flow_action=?";
   protected static final String WHERE_CLAUSE_TO_MATCH_ROW = 
WHERE_CLAUSE_TO_MATCH_KEY
-      + " AND event_timestamp=? AND lease_acquisition_timestamp=?";
-  protected static final String SELECT_AFTER_INSERT_STATEMENT = "SELECT 
event_timestamp, lease_acquisition_timestamp, "
-    + "linger FROM %s, %s " + WHERE_CLAUSE_TO_MATCH_KEY;
+      + " AND event_timestamp=CONVERT_TZ(?, '+00:00', @@session.time_zone)"
+      + " AND lease_acquisition_timestamp=CONVERT_TZ(?, '+00:00', 
@@session.time_zone)";

Review Comment:
   shouldn't we store and retrieve always as UTC?  if so, the issue seems to be 
the `DEFAULT CURRENT_TIMESTAMP(3)` that first introduces the session TZ.  
alternatively shouldn't we initialize it as UTC?  doing so should preclude the 
need for conversion later, no?
   
   e.g. how about we everywhere use `UTC_TIMESTAMP(3)`, rather than 
`CURRENT_TIMESTAMP(3)`?



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