weiqingy commented on code in PR #2430:
URL: https://github.com/apache/auron/pull/2430#discussion_r3653794087
##########
thirdparty/auron-iceberg/src/test/scala/org/apache/auron/iceberg/AuronIcebergIntegrationSuite.scala:
##########
@@ -713,6 +713,92 @@ class AuronIcebergIntegrationSuite
}
}
+ test("iceberg changelog scan falls back for unsupported changelog
operations") {
+ withTable("local.db.t_changelog_unsupported_operation") {
+ withTempView("t_changelog_unsupported_operation_changes") {
+ sql("""
+ |create table local.db.t_changelog_unsupported_operation (id
int, v string)
+ |using iceberg
+ |tblproperties (
+ | 'format-version' = '2',
+ | 'write.delete.mode' = 'merge-on-read'
Review Comment:
I went looking at the CI log for this commit to see which guard actually
catches this case, and the delete seems to land on the metadata path rather
than merge-on-read: the `DELETE FROM` commits as `StreamingDelete` with
`removedDataFiles=CounterResult{unit=COUNT, value=1}`,
`totalDeleteFiles=CounterResult{unit=COUNT, value=0}` and
`addedPositionalDeleteFiles=null`, so no delete file gets written. The two-row
insert above landed in two data files (`The input RDD has 2 partitions`,
`Committing append with 2 new data files`), so `id = 1` covers a whole file and
Iceberg can drop it by metadata regardless of `write.delete.mode`.
If that reading is right, the fallback is coming from the task-type check at
`IcebergScanSupport.scala:266`, the same one the existing "falls back when
delete changes exist" test hits, and the delete-file check just below at `:274`
is still waiting for a test. Does that match what you were going for, or were
you hoping this one would reach `:274`?
The test looks like it earns its place either way: it's the only one where
added and deleted tasks share a single changelog range, which a guard like
`addedRowsTasks.isEmpty` would get wrong. That just leaves the `merge-on-read`
property as a loose end. Is it worth keeping when it isn't changing the outcome?
On the name, same thread of thought: "unsupported changelog operations" sent
me looking for the `operation() != INSERT` check at `:270`, but
`AddedRowsScanTask.operation()` in Iceberg 1.10.1 is a default returning
`INSERT` that neither `BaseAddedRowsScanTask` nor its split subclass overrides,
so after the `collect` at `:263` I don't think it can fire. Would something
like "falls back when a changelog range mixes inserts and deletes" capture the
case better?
--
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]
