Skip to content

[Fix]: Normalize Years and Months while constructing ISO8601 Duration String #2484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
VardhanThigle merged 1 commit into from
Jul 3, 2025
Merged

[Fix]: Normalize Years and Months while constructing ISO8601 Duration String #2484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...spanner/src/test/java/com/google/cloud/teleport/v2/templates/CassandraAllDataTypesIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ private Map<String, String> getAllDataTypeMaxRow() {
.put(
"timeuuid_varchar_map_col",
"{\"00000000-0000-1000-9000-000000000000\":\"abc\",\"ffffffff-ffff-1fff-9fff-ffffffffffff\":\"def\"}")
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0D,P10675199DT2H48M5S]")
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0Y,P10675199DT2H48M5S]")
.put("double_col", "1.7976931348623157E308")
.put("duration_col", "P10675199DT2H48M5S")
.put("frozen_ascii_set_col", "[a,b]")
Expand Down Expand Up @@ -722,7 +722,7 @@ private Map<String, String> getAllDataTypeDlqRow() {
.put(
"timeuuid_varchar_map_col",
"{\"00000000-0000-1000-9000-000000000000\":\"abc\",\"ffffffff-ffff-1fff-9fff-ffffffffffff\":\"def\"}")
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0D,P10675199DT2H48M5S]")
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0Y,P10675199DT2H48M5S]")
.put("double_col", "1.7976931348623157E308")
.put("duration_col", "P10675199DT2H48M5S")
.put("frozen_ascii_set_col", "[a,b]")
Expand Down
28 changes: 15 additions & 13 deletions ...java/com/google/cloud/teleport/v2/spanner/migrations/avro/GenericRecordTypeConvertor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,13 +708,9 @@ static String handleRecordFieldType(String fieldName, GenericRecord element, Sch
Period.ZERO
.plusYears(((Number) getOrDefault(element, "years", 0L)).longValue())
.plusMonths(((Number) getOrDefault(element, "months", 0L)).longValue())
.plusDays(((Number) getOrDefault(element, "days", 0L)).longValue());
/*
* Convert the period to a ISO-8601 period formatted String, such as P6Y3M1D.
* A zero period will be represented as zero days, 'P0D'.
* Refer to javadoc for Period#toString.
*/
String periodIso8061 = period.toString();
.plusDays(((Number) getOrDefault(element, "days", 0L)).longValue())
.normalized(); // Normalize years and months

java.time.Duration duration =
java.time.Duration.ZERO
.plusHours(((Number) getOrDefault(element, "hours", 0L)).longValue())
Expand All @@ -725,13 +721,19 @@ static String handleRecordFieldType(String fieldName, GenericRecord element, Sch
* Convert the duration to a ISO-8601 period formatted String, such as PT8H6M12.345S
* refer to javadoc for Duration#toString.
*/
String durationIso8610 = duration.toString();
// Convert to ISO-8601 period format.
if (duration.isZero()) {
return periodIso8061;
} else {
return periodIso8061 + StringUtils.removeStartIgnoreCase(durationIso8610, "P");
if (period.isZero() && duration.isZero()) {
// Interval of length 0 is represented as P0Y.
return "P0Y";
} else if (period.isZero()) {
// If year-month-day part is 0, get ISO8601 formatted string from duration part.
return duration.toString();
} else if (duration.isZero()) {
// If hour-minute-second part is 0, get ISO8601 formatted string from period part.
return period.toString();
}

// Combine both non-zero parts into ISO8601 string.
return period + StringUtils.removeStartIgnoreCase(duration.toString(), "P");
} else {
throw new UnsupportedOperationException(
String.format(
Expand Down
111 changes: 88 additions & 23 deletions .../com/google/cloud/teleport/v2/spanner/migrations/avro/GenericRecordTypeConvertorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,12 +601,12 @@ public void testPrimitiveAndNonPrimitiveTypesHandling() throws InvalidTransforma
"intervalNanoCol",
Type.string(),
getTestCassandraAnnotationNone()))
.isEqualTo(Value.string("P1000Y1000M3890DT30H31M12.000000009S"));
.isEqualTo(Value.string("P1083Y4M3890DT30H31M12.000000009S"));
// Test Handling for DLQ path.
assertThat(
GenericRecordTypeConvertor.getJsonNodeObjectFromGenericRecord(
payload, payloadSchema.getField("intervalNanoCol"), tableName, schemaMapper))
.isEqualTo("P1000Y1000M3890DT30H31M12.000000009S");
.isEqualTo("P1083Y4M3890DT30H31M12.000000009S");

assertThat(
genericRecordTypeConvertor.getSpannerValue(
Expand Down Expand Up @@ -646,18 +646,18 @@ public void testPrimitiveAndNonPrimitiveTypesHandling() throws InvalidTransforma
.isEqualTo(
Value.stringArray(
ImmutableList.of(
"P1000Y1000M3890DT30H31M12.000000009S",
"P1000Y1000M3890DT30H31M12.000000009S",
"P1000Y1000M3890DT25H12.000000009S")));
"P1083Y4M3890DT30H31M12.000000009S",
"P1083Y4M3890DT30H31M12.000000009S",
"P1083Y4M3890DT25H12.000000009S")));
// Test Handling for DLQ path.
assertThat(
GenericRecordTypeConvertor.getJsonNodeObjectFromGenericRecord(
payload, payloadSchema.getField("intervalNanoArrayCol"), tableName, schemaMapper))
.isEqualTo(
ImmutableList.of(
"P1000Y1000M3890DT30H31M12.000000009S",
"P1000Y1000M3890DT30H31M12.000000009S",
"P1000Y1000M3890DT25H12.000000009S")
"P1083Y4M3890DT30H31M12.000000009S",
"P1083Y4M3890DT30H31M12.000000009S",
"P1083Y4M3890DT25H12.000000009S")
.toArray());

assertThat(
Expand Down Expand Up @@ -688,16 +688,16 @@ public void testPrimitiveAndNonPrimitiveTypesHandling() throws InvalidTransforma
.isEqualTo(
Map.of(
"booleanCol", Value.bool(true),
"intervalNanoCol", Value.string("P1000Y1000M3890DT30H31M12.000000009S"),
"intervalNanoCol", Value.string("P1083Y4M3890DT30H31M12.000000009S"),
"timeStampCol",
Value.timestamp(Timestamp.parseTimestamp("2020-10-13T14:30:00.056000000Z")),
"booleanArrayCol", Value.boolArray(ImmutableList.of(true, false)),
"intervalNanoArrayCol",
Value.stringArray(
ImmutableList.of(
"P1000Y1000M3890DT30H31M12.000000009S",
"P1000Y1000M3890DT30H31M12.000000009S",
"P1000Y1000M3890DT25H12.000000009S")),
"P1083Y4M3890DT30H31M12.000000009S",
"P1083Y4M3890DT30H31M12.000000009S",
"P1083Y4M3890DT25H12.000000009S")),
"timeStampArrayCol", Value.timestampArray(expectedTimeStampArray)));
}

Expand All @@ -712,10 +712,9 @@ public void testIntervalNanos() {
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(1000L, 1000L, 3890L, 25L, 331L, 12L, 9L),
AvroTestingHelper.createIntervalNanosRecord(1000L, 1000L, 3890L, 25L, 331L, 3672L, 9L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #1 interval nano conversion:", "P1000Y1000M3890DT30H31M12.000000009S", result);
assertEquals("Test #1 interval nano conversion:", "P1083Y4M3890DT31H32M12.000000009S", result);

/* Test with any field set as null gets treated as 0. */
result =
Expand All @@ -725,7 +724,7 @@ public void testIntervalNanos() {
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #2 interval nano conversion with null minutes:",
"P1000Y1000M3890DT25H12.000000009S",
"P1083Y4M3890DT25H12.000000009S",
result);

/* Basic test for negative field. */
Expand All @@ -736,18 +735,28 @@ public void testIntervalNanos() {
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #3 interval nano conversion with negative months:",
"P1000Y-1000M3890DT25H31M12.000000009S",
"P916Y8M3890DT25H31M12.000000009S",
result);

/* Test all negative */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(
-1000L, -1000L, -3890L, -2L, -31L, -12L, -90L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #4 all fields are negative:", "P-1083Y-4M-3890DT-2H-31M-12.00000009S", result);

/* Test that negative nanos subtract from the fractional seconds, for example 12 Seconds -1 Nanos becomes 11.999999991s. */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(1000L, 31L, 3890L, 25L, 31L, 12L, -9L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #4 interval nano conversion with negative nanos:",
"P1000Y31M3890DT25H31M11.999999991S",
"Test #5 interval nano conversion with negative nanos:",
"P1002Y7M3890DT25H31M11.999999991S",
result);

/* Test 0 interval. */
Expand All @@ -756,15 +765,15 @@ public void testIntervalNanos() {
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 0L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #5 interval nano conversion with all zeros", "P0D", result);
assertEquals("Test #6 interval nano conversion with all zeros", "P0Y", result);

/* Test almost zero interval with only nanos set. */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 0L, 1L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #6 interval nano conversion with only nanos", "P0DT0.000000001S", result);
assertEquals("Test #7 interval nano conversion with only nanos", "PT0.000000001S", result);
/* Test with large values. */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
Expand All @@ -773,7 +782,7 @@ public void testIntervalNanos() {
2147483647L, 11L, 2147483647L, 2147483647L, 2147483647L, 2147483647L, 999999999L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #6 interval nano conversion with INT.MAX values",
"Test #8 interval nano conversion with INT.MAX values",
"P2147483647Y11M2147483647DT2183871564H21M7.999999999S",
result);

Expand All @@ -791,9 +800,65 @@ public void testIntervalNanos() {
-999999999L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals(
"Test #6 interval nano conversion with -INT.MAX values",
"Test #9 interval nano conversion with -INT.MAX values",
"P-2147483647Y-11M-2147483647DT-2183871564H-21M-7.999999999S",
result);

/* Test with only year part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(1L, 0L, 0L, 0L, 0L, 0L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #10 interval nano conversion with only years", "P1Y", result);

/* Test with only month part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, -1L, 0L, 0L, 0L, 0L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #11 interval nano conversion with only months", "P-1M", result);

/* Test with only days part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 1L, 0L, 0L, 0L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #12 interval nano conversion with only days", "P1D", result);

/* Test with only hours part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, -1L, 0L, 0L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #13 interval nano conversion with only hours", "PT-1H", result);

/* Test with only minutes part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 12L, 0L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #14 interval nano conversion with only minutes", "PT12M", result);

/* Test with only seconds part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 1L, 0L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #15 interval nano conversion with only seconds", "PT1S", result);

/* Test with only nanos part */
result =
GenericRecordTypeConvertor.handleRecordFieldType(
"interval_nanos_column",
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 0L, 100L),
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
assertEquals("Test #16 interval nano conversion with only nanos", "PT0.0000001S", result);
}

@Test
Expand Down
Loading