Handle Timezone Differences in Liquibase for Cloud Spanner #385
Conversation
| @@ -49,12 +57,26 @@ public boolean dataTypeIsNotModifiable(final String typeName) { | |||
|
|
|||
| @Override | |||
| public String getDateLiteral(final String isoDate) { | |||
| String literal = super.getDateLiteral(isoDate); | |||
| String literal; | |||
There was a problem hiding this comment.
nit: just remove this variable. It is not really needed in the way that the implementation works now.
| String formattedTime = utcDateTime.format(ISO_LOCAL_TIME); | ||
| literal = "TIMESTAMP '" + formattedDate + "T" + formattedTime + "Z'"; | ||
| } catch (ParseException e) { | ||
| throw new RuntimeException("Error parsing ISO date: " + isoDate, e); |
There was a problem hiding this comment.
I think it would be better to do the same as the super class does in a case like this, which is to return "BAD_DATE_FORMAT:" + isoDate;. I don't know exactly why Liquibase does that here, but as the super class does not throw an exception for an invalid value, we should probably also not do in this override.
There was a problem hiding this comment.
did like super class
| OffsetDateTime utcDateTime = instant.atOffset(ZoneOffset.UTC); | ||
| String formattedDate = utcDateTime.format(ISO_LOCAL_DATE); | ||
| String formattedTime = utcDateTime.format(ISO_LOCAL_TIME); | ||
| literal = "TIMESTAMP '" + formattedDate + "T" + formattedTime + "Z'"; |
There was a problem hiding this comment.
nit: just return directly here instead of assigning to literal
| if (isDateTime(isoDate)) { | ||
| literal = "TIMESTAMP " + literal.replace(' ', 'T'); | ||
| Date date; |
There was a problem hiding this comment.
nit: move this inside the try block to what is now line 66
There was a problem hiding this comment.
I don't think this is correct, the super class already has similar behavior, and the try catch block was needed for the ISODateFormat().parse(isoDate) method.
| literal = super.getDateLiteral(isoDate); | ||
| literal = "DATE " + literal; |
There was a problem hiding this comment.
nit: just do return "DATE " + super.getDateLiteral(isoDate);
To ensure predictable behavior, adjustments were made to handle timezone differences appropriately and align Liquibase's behavior with Cloud Spanner's expectations.