anoopj commented on code in PR #17629:
URL: https://github.com/apache/iceberg/pull/17629#discussion_r3781151861
##########
core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java:
##########
@@ -829,7 +829,8 @@ private static PositionDelete newPositionDeleteRecord(
+ oldPath);
}
String newPath = newPath(oldPath, sourcePrefix, targetPrefix);
- delete.set(newPath, (Long) record.get(1), record.get(2));
+ Object row = record.size() > 2 ? record.get(2) : null;
+ delete.set(newPath, (Long) record.get(1), row);
Review Comment:
FYI this seem to be deprecated:
```
@deprecated This method is deprecated as of version 1.11.0 and will be
removed in 1.12.0.
```
I know the old code used it too.
##########
core/src/test/java/org/apache/iceberg/TestRewriteTablePathUtil.java:
##########
@@ -359,6 +370,94 @@ public void
testRewriteDeleteManifestFallsBackToOriginalSizeForDeletedEntries()
assertThat(seen).as("Both the live and deleted entries should be
present").isEqualTo(2);
}
+ @Test
+ public void testRewritePositionDeleteWith2FieldReader() throws IOException {
+ File sourceFile =
+ new File(
+ FileFormat.AVRO.addExtension(
+ temp.resolve("source-pos-deletes-" +
System.nanoTime()).toString()));
+ OutputFile sourceOutput = Files.localOutput(sourceFile);
+
+ String sourcePrefix = temp.toAbsolutePath().toString();
+ String targetPrefix = temp.resolve("target").toAbsolutePath().toString();
+ String dataFilePath = sourcePrefix + "/data/file.parquet";
+
+ try (PositionDeleteWriter<Void> writer =
+ Avro.writeDeletes(sourceOutput)
+ .createWriterFunc(DataWriter::create)
+ .overwrite()
+ .withSpec(PartitionSpec.unpartitioned())
+ .buildPositionWriter()) {
+ writer.write(PositionDelete.<Void>create().set(dataFilePath, 0L));
+ }
+
+ OutputFile targetOutput =
+ Files.localOutput(
+ FileFormat.AVRO.addExtension(
+ temp.resolve("target-pos-deletes-" +
System.nanoTime()).toString()));
+
+ // Reader that projects only 2 fields (file_path + pos), simulating an
impl that omits row data
+ RewriteTablePathUtil.PositionDeleteReaderWriter twoFieldReaderWriter =
+ new RewriteTablePathUtil.PositionDeleteReaderWriter() {
+ @Override
+ public CloseableIterable<Record> reader(
+ InputFile inputFile, FileFormat format, PartitionSpec spec) {
+ return Avro.read(inputFile)
+ .project(DeleteSchemaUtil.pathPosSchema())
+ .createResolvingReader(PlannedDataReader::create)
+ .build();
+ }
+
+ @Override
+ public PositionDeleteWriter<Record> writer(
+ OutputFile outputFile,
+ FileFormat format,
+ PartitionSpec spec,
+ StructLike partition,
+ Schema rowSchema)
+ throws IOException {
+ return Avro.writeDeletes(outputFile)
+ .createWriterFunc(DataWriter::create)
+ .overwrite()
+ .withSpec(spec)
+ .buildPositionWriter();
+ }
+ };
+
+ DeleteFile deleteFile =
+ FileMetadata.deleteFileBuilder(PartitionSpec.unpartitioned())
+ .ofPositionDeletes()
+ .withFormat(FileFormat.AVRO)
+ .withPath(sourceFile.getAbsolutePath())
+ .withFileSizeInBytes(sourceFile.length())
+ .withRecordCount(1)
+ .build();
+
+ RewriteTablePathUtil.rewritePositionDelete(
+ deleteFile,
+ targetOutput,
+ table.io(),
+ PartitionSpec.unpartitioned(),
+ sourcePrefix,
+ targetPrefix,
+ twoFieldReaderWriter);
+
+ List<Record> records = Lists.newArrayList();
+ try (CloseableIterable<Record> reader =
+ Avro.read(targetOutput.toInputFile())
+ .project(DeleteSchemaUtil.pathPosSchema())
+ .createResolvingReader(PlannedDataReader::create)
+ .build()) {
+ reader.forEach(records::add);
+ }
+
+ assertThat(records).hasSize(1);
Review Comment:
Consider adding an assertion that `pos` survived
--
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]
