weimingdiit commented on code in PR #2435:
URL: https://github.com/apache/auron/pull/2435#discussion_r3709010921


##########
thirdparty/auron-iceberg/src/main/scala/org/apache/spark/sql/auron/iceberg/IcebergScanSupport.scala:
##########
@@ -559,6 +557,21 @@ object IcebergScanSupport extends Logging {
     }
   }
 
+  private def toNativeChangelogDataFileTask(
+      task: ChangelogScanTask): Option[NativeChangelogDataFileTask] = {
+    task match {
+      case added: AddedRowsScanTask
+          if added.operation() == ChangelogOperation.INSERT &&
+            deletesEmpty(added.deletes()) =>
+        Some(NativeChangelogDataFileTask(added.file(), added.start(), 
added.length(), added))
+      case deleted: DeletedDataFileScanTask if 
deletesEmpty(deleted.existingDeletes()) =>
+        Some(
+          NativeChangelogDataFileTask(deleted.file(), deleted.start(), 
deleted.length(), deleted))
+      case _ =>
+        None
+    }
+  }

Review Comment:
   Thanks, that matches my reading.
   
   The native path is intentionally limited by the concrete task type here. 
DeletedRowsScanTask is a separate ChangelogScanTask implementation, not a 
DeletedDataFileScanTask, so it will not match this case and will continue to 
fall back. Keeping or removing the operation() == DELETE check does not affect 
that, since DeletedRowsScanTask also hard-codes operation() to DELETE.
   
   For _change_type, I agree the value should come from the task operation 
itself. The metadata materialization path already uses 
requiredChangelogTask(name).operation().name(), so if Iceberg ever provides a 
supported changelog task with a different operation value, the metadata column 
will reflect the task value rather than the Scala match type.
   
   No additional code change from my side.



-- 
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]

Reply via email to