Copilot commented on code in PR #12537:
URL: https://github.com/apache/gluten/pull/12537#discussion_r3774672939


##########
gluten-iceberg/src/main/scala/org/apache/gluten/execution/IcebergScanTransformer.scala:
##########
@@ -208,7 +214,7 @@ case class IcebergScanTransformer(
       metadataColumnNames: Seq[String]): SplitInfo = {
     val splitInfo = partition match {
       case p: SparkDataSourceRDDPartition =>
-        GlutenIcebergSourceUtil.genSplitInfo(p, getPartitionSchema, 
metadataColumnNames)
+        GlutenIcebergSourceUtil.genSplitInfo(p, scan, getPartitionSchema, 
metadataColumnNames)
       case _ => throw new GlutenNotSupportException()

Review Comment:
   `genSplitInfo` now derives `fieldIds` / `initialDefaults` from the Iceberg 
table schema, but it is called once per partition split. Since this metadata is 
scan-level and identical for every split, recomputing it per split can become 
noticeable overhead on large scans. Consider precomputing the maps once in 
`IcebergScanTransformer` and passing them into `genSplitInfo`.



##########
gluten-iceberg/src/main/scala/org/apache/iceberg/spark/source/GlutenIcebergSourceUtil.scala:
##########
@@ -103,10 +105,52 @@ object GlutenIcebergSourceUtil {
         .toList
         .asJava,
       deleteFilesList,
-      metadataColumns
+      metadataColumns,
+      getFieldIds(sparkScan),
+      getInitialDefaults(sparkScan)
     )

Review Comment:
   `getFieldIds(sparkScan)` is computed unconditionally even when there are no 
initial defaults, but `fieldIds` is only used when `initialDefaults` is 
non-empty (the extension is skipped otherwise). This adds an extra full schema 
scan per split; gate `fieldIds` computation behind `initialDefaults.nonEmpty` 
to avoid the extra work on the common path.



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

Reply via email to