huan233usc commented on code in PR #17119:
URL: https://github.com/apache/iceberg/pull/17119#discussion_r3566993507
##########
core/src/test/java/org/apache/iceberg/avro/TestAvroDataWriter.java:
##########
@@ -110,4 +112,64 @@ public void testDataWriter() throws IOException {
assertThat(writtenRecords).as("Written records should
match").isEqualTo(records);
}
+
+ @Test
+ public void testGeospatialWkbRoundTrip() throws IOException {
+ Schema schema =
+ new Schema(
+ Types.NestedField.required(1, "id", Types.LongType.get()),
+ Types.NestedField.optional(2, "geom", Types.GeometryType.crs84()),
+ Types.NestedField.optional(3, "geog",
Types.GeographyType.crs84()));
+
+ GenericRecord record = GenericRecord.create(schema);
+ List<Record> geoRecords =
+ ImmutableList.of(
+ record.copy(
+ ImmutableMap.of("id", 1L, "geom", wkbPoint(30, 10), "geog",
wkbPoint(-5, 40))),
+ // geog is left null
+ record.copy(ImmutableMap.of("id", 2L, "geom", wkbPoint(0, 0))),
+ // both geo columns are left null
+ record.copy(ImmutableMap.of("id", 3L)));
+
+ OutputFile file = Files.localOutput(temp.toFile());
+ DataWriter<Record> dataWriter =
+ Avro.writeData(file)
+ .schema(schema)
+ .createWriterFunc(org.apache.iceberg.data.avro.DataWriter::create)
+ .overwrite()
+ .withSpec(PartitionSpec.unpartitioned())
+ .build();
+ try (dataWriter) {
+ for (Record geoRecord : geoRecords) {
+ dataWriter.write(geoRecord);
+ }
+ }
+
+
assertThat(dataWriter.toDataFile().recordCount()).isEqualTo(geoRecords.size());
+
+ List<Record> writtenRecords;
+ try (AvroIterable<Record> reader =
+ Avro.read(file.toInputFile())
+ .project(schema)
+ .createResolvingReader(PlannedDataReader::create)
+ .build()) {
+ writtenRecords = Lists.newArrayList(reader);
+ }
+
+ assertThat(writtenRecords)
+ .as("Geometry and geography WKB should round-trip through Avro")
+ .isEqualTo(geoRecords);
+ }
+
+ private static ByteBuffer wkbPoint(double xCoord, double yCoord) {
Review Comment:
Done in bcbef59c4 — `TestAvroDataWriter.wkbPoint` now delegates to
`RandomUtil.wkbPoint(xCoord, yCoord)` and the `java.nio.ByteOrder` import is
dropped, matching the merged `TestParquetDataWriter.wkbPoint`. Thanks for the
pointer to the #16982 precedent.
##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -330,6 +330,39 @@ public void testGeospatialFooterMetricsSkipParquetBounds()
throws IOException {
}
}
+ @Test
+ public void testGeospatialWkbRoundTrip() throws IOException {
+ Schema schema =
+ new Schema(
+ optional(1, "geom", Types.GeometryType.crs84()),
+ optional(2, "geog", Types.GeographyType.crs84()));
+
+ ByteBuffer geomWkb = ByteBuffer.wrap(new byte[] {0x01, 0x02, 0x03});
Review Comment:
Great catch — fixed in bcbef59c4. `testGeospatialWkbRoundTrip` now uses
`RandomUtil.wkbPoint(30, 10)` / `wkbPoint(-5, 40)`, so the values are real WKB
that the geospatial statistics builder parses successfully instead of silently
omitting. The bytes in `testGeospatialFooterMetricsSkipParquetBounds` are left
as-is since that test writes through a `BinaryType` schema (no WKB parsing), as
you noted.
##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -330,6 +330,39 @@ public void testGeospatialFooterMetricsSkipParquetBounds()
throws IOException {
}
}
+ @Test
+ public void testGeospatialWkbRoundTrip() throws IOException {
Review Comment:
Updated the PR description: the test plan now lists `TestAvroEncoderUtil`,
both `testGeospatialWkbRoundTrip` tests, and a Summary paragraph noting this
closes the parquet-avro object model gap
(`ParquetAvroWriter`/`ParquetAvroValueReaders`, unreachable for geo because
`AvroSchemaUtil.convert` routed into the `TypeToSchema` that threw) — the
#16982 follow-up. It'll carry into the squash message.
--
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]
