HwangDongJun commented on code in PR #12833:
URL: https://github.com/apache/gluten/pull/12833#discussion_r4144852979
##########
gluten-iceberg/src/main/scala/org/apache/gluten/execution/IcebergScanTransformer.scala:
##########
@@ -192,8 +192,15 @@ case class IcebergScanTransformer(
override def getDataSchema: StructType = new StructType()
- // TODO: get root paths from table.
- override def getRootPathsInternal: Seq[String] = Seq.empty
+ // On Spark 3.3, SparkShims.getBatchScanExecTable always returns null
(BatchScanExec has no
+ // `table` field until Spark 3.4), so this falls back to the previous
Seq.empty behavior there;
+ // on Spark 3.4+ it returns the Iceberg table's base location.
+ override def getRootPathsInternal: Seq[String] = {
+ table match {
+ case t: SparkTable => Seq(t.table().location())
+ case _ => Seq.empty
+ }
+ }
Review Comment:
Thanks for catching that — you're right that a stale write.data.path
(changed after files were already written under the old path) would slip
through the metadata-based check. Implemented exactly what you suggested:
derived getRootPathsInternal (and fileFormat, in the same pass) from
getScanTasks instead of finalPartitions, and also covered delete file paths per
your note. Verified via a dry-run CI run that TestGlutenRuntimeFiltering (the
DPP suite the earlier finalPartitions-based attempt broke) still passes 18/18,
along with the existing Iceberg root-paths tests. Pushed — let me know if this
looks right to you.
--
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]
