Skip to content

Commit 7b482c9

Browse files
sagarwaalSagar Agarwal
andauthored
normalize years and months part in coverting Avro interval to ISO8601 string (#2484)
Co-authored-by: Sagar Agarwal <sagarwaal@google.com>
1 parent 047ed0e commit 7b482c9

File tree

3 files changed

+105
-38
lines changed

3 files changed

+105
-38
lines changed

v2/sourcedb-to-spanner/src/test/java/com/google/cloud/teleport/v2/templates/CassandraAllDataTypesIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ private Map<String, String> getAllDataTypeMaxRow() {
596596
.put(
597597
"timeuuid_varchar_map_col",
598598
"{\"00000000-0000-1000-9000-000000000000\":\"abc\",\"ffffffff-ffff-1fff-9fff-ffffffffffff\":\"def\"}")
599-
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0D,P10675199DT2H48M5S]")
599+
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0Y,P10675199DT2H48M5S]")
600600
.put("double_col", "1.7976931348623157E308")
601601
.put("duration_col", "P10675199DT2H48M5S")
602602
.put("frozen_ascii_set_col", "[a,b]")
@@ -722,7 +722,7 @@ private Map<String, String> getAllDataTypeDlqRow() {
722722
.put(
723723
"timeuuid_varchar_map_col",
724724
"{\"00000000-0000-1000-9000-000000000000\":\"abc\",\"ffffffff-ffff-1fff-9fff-ffffffffffff\":\"def\"}")
725-
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0D,P10675199DT2H48M5S]")
725+
.put("duration_list_col", "[P-10675199DT-2H-48M-5S,P0Y,P10675199DT2H48M5S]")
726726
.put("double_col", "1.7976931348623157E308")
727727
.put("duration_col", "P10675199DT2H48M5S")
728728
.put("frozen_ascii_set_col", "[a,b]")

v2/spanner-common/src/main/java/com/google/cloud/teleport/v2/spanner/migrations/avro/GenericRecordTypeConvertor.java

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -708,13 +708,9 @@ static String handleRecordFieldType(String fieldName, GenericRecord element, Sch
708708
Period.ZERO
709709
.plusYears(((Number) getOrDefault(element, "years", 0L)).longValue())
710710
.plusMonths(((Number) getOrDefault(element, "months", 0L)).longValue())
711-
.plusDays(((Number) getOrDefault(element, "days", 0L)).longValue());
712-
/*
713-
* Convert the period to a ISO-8601 period formatted String, such as P6Y3M1D.
714-
* A zero period will be represented as zero days, 'P0D'.
715-
* Refer to javadoc for Period#toString.
716-
*/
717-
String periodIso8061 = period.toString();
711+
.plusDays(((Number) getOrDefault(element, "days", 0L)).longValue())
712+
.normalized(); // Normalize years and months
713+
718714
java.time.Duration duration =
719715
java.time.Duration.ZERO
720716
.plusHours(((Number) getOrDefault(element, "hours", 0L)).longValue())
@@ -725,13 +721,19 @@ static String handleRecordFieldType(String fieldName, GenericRecord element, Sch
725721
* Convert the duration to a ISO-8601 period formatted String, such as PT8H6M12.345S
726722
* refer to javadoc for Duration#toString.
727723
*/
728-
String durationIso8610 = duration.toString();
729-
// Convert to ISO-8601 period format.
730-
if (duration.isZero()) {
731-
return periodIso8061;
732-
} else {
733-
return periodIso8061 + StringUtils.removeStartIgnoreCase(durationIso8610, "P");
724+
if (period.isZero() && duration.isZero()) {
725+
// Interval of length 0 is represented as P0Y.
726+
return "P0Y";
727+
} else if (period.isZero()) {
728+
// If year-month-day part is 0, get ISO8601 formatted string from duration part.
729+
return duration.toString();
730+
} else if (duration.isZero()) {
731+
// If hour-minute-second part is 0, get ISO8601 formatted string from period part.
732+
return period.toString();
734733
}
734+
735+
// Combine both non-zero parts into ISO8601 string.
736+
return period + StringUtils.removeStartIgnoreCase(duration.toString(), "P");
735737
} else {
736738
throw new UnsupportedOperationException(
737739
String.format(

v2/spanner-common/src/test/java/com/google/cloud/teleport/v2/spanner/migrations/avro/GenericRecordTypeConvertorTest.java

Lines changed: 88 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -601,12 +601,12 @@ public void testPrimitiveAndNonPrimitiveTypesHandling() throws InvalidTransforma
601601
"intervalNanoCol",
602602
Type.string(),
603603
getTestCassandraAnnotationNone()))
604-
.isEqualTo(Value.string("P1000Y1000M3890DT30H31M12.000000009S"));
604+
.isEqualTo(Value.string("P1083Y4M3890DT30H31M12.000000009S"));
605605
// Test Handling for DLQ path.
606606
assertThat(
607607
GenericRecordTypeConvertor.getJsonNodeObjectFromGenericRecord(
608608
payload, payloadSchema.getField("intervalNanoCol"), tableName, schemaMapper))
609-
.isEqualTo("P1000Y1000M3890DT30H31M12.000000009S");
609+
.isEqualTo("P1083Y4M3890DT30H31M12.000000009S");
610610

611611
assertThat(
612612
genericRecordTypeConvertor.getSpannerValue(
@@ -646,18 +646,18 @@ public void testPrimitiveAndNonPrimitiveTypesHandling() throws InvalidTransforma
646646
.isEqualTo(
647647
Value.stringArray(
648648
ImmutableList.of(
649-
"P1000Y1000M3890DT30H31M12.000000009S",
650-
"P1000Y1000M3890DT30H31M12.000000009S",
651-
"P1000Y1000M3890DT25H12.000000009S")));
649+
"P1083Y4M3890DT30H31M12.000000009S",
650+
"P1083Y4M3890DT30H31M12.000000009S",
651+
"P1083Y4M3890DT25H12.000000009S")));
652652
// Test Handling for DLQ path.
653653
assertThat(
654654
GenericRecordTypeConvertor.getJsonNodeObjectFromGenericRecord(
655655
payload, payloadSchema.getField("intervalNanoArrayCol"), tableName, schemaMapper))
656656
.isEqualTo(
657657
ImmutableList.of(
658-
"P1000Y1000M3890DT30H31M12.000000009S",
659-
"P1000Y1000M3890DT30H31M12.000000009S",
660-
"P1000Y1000M3890DT25H12.000000009S")
658+
"P1083Y4M3890DT30H31M12.000000009S",
659+
"P1083Y4M3890DT30H31M12.000000009S",
660+
"P1083Y4M3890DT25H12.000000009S")
661661
.toArray());
662662

663663
assertThat(
@@ -688,16 +688,16 @@ public void testPrimitiveAndNonPrimitiveTypesHandling() throws InvalidTransforma
688688
.isEqualTo(
689689
Map.of(
690690
"booleanCol", Value.bool(true),
691-
"intervalNanoCol", Value.string("P1000Y1000M3890DT30H31M12.000000009S"),
691+
"intervalNanoCol", Value.string("P1083Y4M3890DT30H31M12.000000009S"),
692692
"timeStampCol",
693693
Value.timestamp(Timestamp.parseTimestamp("2020-10-13T14:30:00.056000000Z")),
694694
"booleanArrayCol", Value.boolArray(ImmutableList.of(true, false)),
695695
"intervalNanoArrayCol",
696696
Value.stringArray(
697697
ImmutableList.of(
698-
"P1000Y1000M3890DT30H31M12.000000009S",
699-
"P1000Y1000M3890DT30H31M12.000000009S",
700-
"P1000Y1000M3890DT25H12.000000009S")),
698+
"P1083Y4M3890DT30H31M12.000000009S",
699+
"P1083Y4M3890DT30H31M12.000000009S",
700+
"P1083Y4M3890DT25H12.000000009S")),
701701
"timeStampArrayCol", Value.timestampArray(expectedTimeStampArray)));
702702
}
703703

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

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

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

741+
/* Test all negative */
742+
result =
743+
GenericRecordTypeConvertor.handleRecordFieldType(
744+
"interval_nanos_column",
745+
AvroTestingHelper.createIntervalNanosRecord(
746+
-1000L, -1000L, -3890L, -2L, -31L, -12L, -90L),
747+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
748+
assertEquals(
749+
"Test #4 all fields are negative:", "P-1083Y-4M-3890DT-2H-31M-12.00000009S", result);
750+
742751
/* Test that negative nanos subtract from the fractional seconds, for example 12 Seconds -1 Nanos becomes 11.999999991s. */
743752
result =
744753
GenericRecordTypeConvertor.handleRecordFieldType(
745754
"interval_nanos_column",
746755
AvroTestingHelper.createIntervalNanosRecord(1000L, 31L, 3890L, 25L, 31L, 12L, -9L),
747756
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
748757
assertEquals(
749-
"Test #4 interval nano conversion with negative nanos:",
750-
"P1000Y31M3890DT25H31M11.999999991S",
758+
"Test #5 interval nano conversion with negative nanos:",
759+
"P1002Y7M3890DT25H31M11.999999991S",
751760
result);
752761

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

761770
/* Test almost zero interval with only nanos set. */
762771
result =
763772
GenericRecordTypeConvertor.handleRecordFieldType(
764773
"interval_nanos_column",
765774
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 0L, 1L),
766775
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
767-
assertEquals("Test #6 interval nano conversion with only nanos", "P0DT0.000000001S", result);
776+
assertEquals("Test #7 interval nano conversion with only nanos", "PT0.000000001S", result);
768777
/* Test with large values. */
769778
result =
770779
GenericRecordTypeConvertor.handleRecordFieldType(
@@ -773,7 +782,7 @@ public void testIntervalNanos() {
773782
2147483647L, 11L, 2147483647L, 2147483647L, 2147483647L, 2147483647L, 999999999L),
774783
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
775784
assertEquals(
776-
"Test #6 interval nano conversion with INT.MAX values",
785+
"Test #8 interval nano conversion with INT.MAX values",
777786
"P2147483647Y11M2147483647DT2183871564H21M7.999999999S",
778787
result);
779788

@@ -791,9 +800,65 @@ public void testIntervalNanos() {
791800
-999999999L),
792801
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
793802
assertEquals(
794-
"Test #6 interval nano conversion with -INT.MAX values",
803+
"Test #9 interval nano conversion with -INT.MAX values",
795804
"P-2147483647Y-11M-2147483647DT-2183871564H-21M-7.999999999S",
796805
result);
806+
807+
/* Test with only year part */
808+
result =
809+
GenericRecordTypeConvertor.handleRecordFieldType(
810+
"interval_nanos_column",
811+
AvroTestingHelper.createIntervalNanosRecord(1L, 0L, 0L, 0L, 0L, 0L, 0L),
812+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
813+
assertEquals("Test #10 interval nano conversion with only years", "P1Y", result);
814+
815+
/* Test with only month part */
816+
result =
817+
GenericRecordTypeConvertor.handleRecordFieldType(
818+
"interval_nanos_column",
819+
AvroTestingHelper.createIntervalNanosRecord(0L, -1L, 0L, 0L, 0L, 0L, 0L),
820+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
821+
assertEquals("Test #11 interval nano conversion with only months", "P-1M", result);
822+
823+
/* Test with only days part */
824+
result =
825+
GenericRecordTypeConvertor.handleRecordFieldType(
826+
"interval_nanos_column",
827+
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 1L, 0L, 0L, 0L, 0L),
828+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
829+
assertEquals("Test #12 interval nano conversion with only days", "P1D", result);
830+
831+
/* Test with only hours part */
832+
result =
833+
GenericRecordTypeConvertor.handleRecordFieldType(
834+
"interval_nanos_column",
835+
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, -1L, 0L, 0L, 0L),
836+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
837+
assertEquals("Test #13 interval nano conversion with only hours", "PT-1H", result);
838+
839+
/* Test with only minutes part */
840+
result =
841+
GenericRecordTypeConvertor.handleRecordFieldType(
842+
"interval_nanos_column",
843+
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 12L, 0L, 0L),
844+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
845+
assertEquals("Test #14 interval nano conversion with only minutes", "PT12M", result);
846+
847+
/* Test with only seconds part */
848+
result =
849+
GenericRecordTypeConvertor.handleRecordFieldType(
850+
"interval_nanos_column",
851+
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 1L, 0L),
852+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
853+
assertEquals("Test #15 interval nano conversion with only seconds", "PT1S", result);
854+
855+
/* Test with only nanos part */
856+
result =
857+
GenericRecordTypeConvertor.handleRecordFieldType(
858+
"interval_nanos_column",
859+
AvroTestingHelper.createIntervalNanosRecord(0L, 0L, 0L, 0L, 0L, 0L, 100L),
860+
AvroTestingHelper.INTERVAL_NANOS_SCHEMA);
861+
assertEquals("Test #16 interval nano conversion with only nanos", "PT0.0000001S", result);
797862
}
798863

799864
@Test

0 commit comments

Comments
 (0)