laskoviymishka commented on code in PR #16604:
URL: https://github.com/apache/iceberg/pull/16604#discussion_r3989294469
##########
kafka-connect/kafka-connect-transforms/src/test/java/org/debezium/connector/mongodb/transforms/TestMongoArrayConverter.java:
##########
@@ -389,4 +395,60 @@ public void
shouldCreateStructForHeterogenousDocumentInArray() throws Exception
"Struct{" + "_id=1," + "a1=Struct{" + "_0=Struct{a=1}," +
"_1=Struct{a=c}" + "}" + "}");
// @formatter:on
}
+
+ @Test
+ @SuppressWarnings("JavaUtilDate")
+ public void shouldConvertArrayOfTimestamps() {
+ final MongoDataConverter converter = new
MongoDataConverter(ArrayEncoding.ARRAY);
+ final BsonDocument val =
+ new BsonDocument()
+ .append("_id", new BsonInt32(1))
+ .append(
+ "ts",
+ new BsonArray(Arrays.asList(new BsonTimestamp(60, 1), new
BsonTimestamp(120, 1))));
+
+ final SchemaBuilder schemaBuilder = SchemaBuilder.struct().name("array");
+ for (Entry<String, BsonValue> entry : val.entrySet()) {
+ converter.addFieldSchema(entry, schemaBuilder);
+ }
+ final Schema finalSchema = schemaBuilder.build();
+ final Struct struct = new Struct(finalSchema);
+ for (Entry<String, BsonValue> entry : val.entrySet()) {
+ converter.convertRecord(entry, finalSchema, struct);
+ }
+
+ // BsonTimestamp.getTime() returns the seconds component; the scalar path
multiplies by 1000
+ List<?> tsValues = (List<?>) struct.get("ts");
Review Comment:
These two are the only tests in the file that cast the raw `struct.get()`
and assert element-by-element — everything else goes through
`assertThat(struct.toString()).isEqualTo(...)`. AssertJ can drop the cast and
tighten it:
```java
assertThat(struct.get("ts")).asList()
.containsExactly(new Date(60_000L), new Date(120_000L));
```
Keeps the new tests consistent with the rest of the suite.
##########
kafka-connect/kafka-connect-transforms/src/test/java/org/debezium/connector/mongodb/transforms/TestMongoArrayConverter.java:
##########
@@ -389,4 +395,60 @@ public void
shouldCreateStructForHeterogenousDocumentInArray() throws Exception
"Struct{" + "_id=1," + "a1=Struct{" + "_0=Struct{a=1}," +
"_1=Struct{a=c}" + "}" + "}");
// @formatter:on
}
+
+ @Test
+ @SuppressWarnings("JavaUtilDate")
+ public void shouldConvertArrayOfTimestamps() {
+ final MongoDataConverter converter = new
MongoDataConverter(ArrayEncoding.ARRAY);
+ final BsonDocument val =
+ new BsonDocument()
+ .append("_id", new BsonInt32(1))
+ .append(
+ "ts",
+ new BsonArray(Arrays.asList(new BsonTimestamp(60, 1), new
BsonTimestamp(120, 1))));
+
+ final SchemaBuilder schemaBuilder = SchemaBuilder.struct().name("array");
+ for (Entry<String, BsonValue> entry : val.entrySet()) {
+ converter.addFieldSchema(entry, schemaBuilder);
+ }
+ final Schema finalSchema = schemaBuilder.build();
+ final Struct struct = new Struct(finalSchema);
+ for (Entry<String, BsonValue> entry : val.entrySet()) {
+ converter.convertRecord(entry, finalSchema, struct);
+ }
+
+ // BsonTimestamp.getTime() returns the seconds component; the scalar path
multiplies by 1000
+ List<?> tsValues = (List<?>) struct.get("ts");
+ assertThat(tsValues).hasSize(2);
+ assertThat(tsValues.get(0)).isEqualTo(new Date(60_000L));
+ assertThat(tsValues.get(1)).isEqualTo(new Date(120_000L));
+ }
+
+ @Test
+ @SuppressWarnings("JavaUtilDate")
+ public void shouldConvertArrayOfDateTimes() {
Review Comment:
The suite already pairs ARRAY and DOCUMENT tests for the heterogeneous and
empty-array cases; the temporal types only get the ARRAY side here.
DOCUMENT encoding routes through the scalar overload so it was never broken,
but a mirror `...DateTimes`/`...Timestamps` under `ArrayEncoding.DOCUMENT`
would keep the pattern and catch a future regression on that path. Non-blocking
— happy either way.
##########
kafka-connect/kafka-connect-transforms/src/test/java/org/debezium/connector/mongodb/transforms/TestMongoArrayConverter.java:
##########
@@ -389,4 +395,60 @@ public void
shouldCreateStructForHeterogenousDocumentInArray() throws Exception
"Struct{" + "_id=1," + "a1=Struct{" + "_0=Struct{a=1}," +
"_1=Struct{a=c}" + "}" + "}");
// @formatter:on
}
+
+ @Test
+ @SuppressWarnings("JavaUtilDate")
+ public void shouldConvertArrayOfTimestamps() {
Review Comment:
The two new tests cover the flat ARRAY case, which is exactly the bug. The
one path they don't touch is the recursive one — nested arrays re-enter this
same method and hit the fixed DATE_TIME/TIMESTAMP branches again, and that's
currently untested.
A cheap `[[BsonTimestamp(60, 1)], [BsonTimestamp(120, 1)]]` case would guard
the re-entry so a future refactor can't silently break it. wdyt?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
