You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@auron.apache.org by "guixiaowen (via GitHub)" <gi...@apache.org> on 2026/05/03 03:18:08 UTC
[PR] [AURON #2221] Remove hard-coded Iceberg scan class name detection using type-based check [auron]
guixiaowen opened a new pull request, #2226:
URL: https://github.com/apache/auron/pull/2226
…erg table types.
# Which issue does this PR close?
Closes #2221
# Rationale for this change
This PR removes string-based detection and replaces it with type-based checking.
The current implementation relies on hard-coded class name strings to detect Iceberg scan types:
`
if (!scanClassName.startsWith("org.apache.iceberg.spark.source.")) {
return None
}
if (scanClassName == "org.apache.iceberg.spark.source.SparkChangelogScan") {
return None
}
if (className != "org.apache.iceberg.spark.source.SparkInputPartition") {
return None
}
`
This approach introduces tight coupling to Iceberg internal class naming and has several drawbacks:
Fragile to upstream refactoring (class/package rename)
Lacks type safety
Hard to maintain and extend
**Notes on Changelog Scan**
ChangelogScan is not a subclass of SparkBatchQueryScan
# What changes are included in this PR?
Three conditional scenarios were modified to avoid hard-coded logic in the evaluations.
**Benefits**
Removes hard-coded class name dependency
Improves type safety and readability
More robust against Iceberg internal refactoring
Cleaner and more maintainable logic
# Are there any user-facing changes?
No changes.
# How was this patch tested?
No new unit tests are required as the current implementation already depends on existing UT coverage.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #2221] Remove hard-coded Iceberg scan class name detection using type-based check [auron]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2226:
URL: https://github.com/apache/auron/pull/2226#discussion_r3186134860
##########
thirdparty/auron-iceberg/src/main/scala/org/apache/spark/sql/auron/iceberg/IcebergScanSupport.scala:
##########
@@ -47,12 +48,7 @@ object IcebergScanSupport extends Logging {
val scan = exec.scan
val scanClassName = scan.getClass.getName
// Only handle Iceberg scans; other sources must stay on Spark's path.
- if (!scanClassName.startsWith("org.apache.iceberg.spark.source.")) {
- return None
- }
-
- // Changelog scan carries row-level changes; not supported by native COW-only path.
- if (scanClassName == "org.apache.iceberg.spark.source.SparkChangelogScan") {
+ if (!(scan.getClass == AuronIcebergSourceUtil.getClassOfSparkBatchQueryScan)) {
Review Comment:
`scan.getClass == AuronIcebergSourceUtil.getClassOfSparkBatchQueryScan` does an exact-class match, so it will return `None` for any `SparkBatchQueryScan` subclasses/wrappers. If the intent is a type-based check, use the `Class` object for an inheritance check (e.g., `isInstance`/`isAssignableFrom`). Also, `getClassOfSparkBatchQueryScan` is defined with `()`, but it’s referenced without `()`, which triggers deprecated auto-application in Scala (and this repo builds with `-deprecation` + `-Xfatal-warnings`).
##########
thirdparty/auron-iceberg/src/main/scala/org/apache/spark/sql/auron/iceberg/IcebergScanSupport.scala:
##########
@@ -199,7 +195,8 @@ object IcebergScanSupport extends Logging {
private def icebergPartition(partition: InputPartition): Option[IcebergPartitionView] = {
val className = partition.getClass.getName
// Only accept Iceberg SparkInputPartition to access task groups.
- if (className != "org.apache.iceberg.spark.source.SparkInputPartition") {
+ if (partition.getClass
+ != AuronIcebergSourceUtil.getClassOfSparkInputPartition()) {
return None
Review Comment:
This partition gate uses exact-class equality against `SparkInputPartition`. If Iceberg ever returns a subclass/proxy of `SparkInputPartition`, Auron will always fall back even though the reflective access may still work. Consider switching to an inheritance check (`isInstance`/`isAssignableFrom`) and let the existing reflective try/catch decide whether the expected fields/methods are actually present.
##########
thirdparty/auron-iceberg/src/main/scala/org/apache/spark/sql/auron/iceberg/IcebergScanSupport.scala:
##########
@@ -47,12 +48,7 @@ object IcebergScanSupport extends Logging {
val scan = exec.scan
val scanClassName = scan.getClass.getName
// Only handle Iceberg scans; other sources must stay on Spark's path.
- if (!scanClassName.startsWith("org.apache.iceberg.spark.source.")) {
- return None
- }
-
- // Changelog scan carries row-level changes; not supported by native COW-only path.
- if (scanClassName == "org.apache.iceberg.spark.source.SparkChangelogScan") {
+ if (!(scan.getClass == AuronIcebergSourceUtil.getClassOfSparkBatchQueryScan)) {
return None
}
Review Comment:
The changelog-scan exclusion now relies on `SparkChangelogScan` not being a `SparkBatchQueryScan`. There are integration tests for native Iceberg scans, but none that assert a changelog read falls back; adding a regression test for a changelog scan would make this behavior stable across Iceberg upgrades.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #2221] Remove hard-coded Iceberg scan class name detection using type-based check [auron]
Posted by "cxzl25 (via GitHub)" <gi...@apache.org>.
cxzl25 merged PR #2226:
URL: https://github.com/apache/auron/pull/2226
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #2221] Remove hard-coded Iceberg scan class name detection using type-based check [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #2226:
URL: https://github.com/apache/auron/pull/2226#issuecomment-4398983880
@zuston @lihao712 @cxzl25 @ShreyeshArangath s @slfan1989 Can you please help review this PR? Thank you very much!
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [AURON #2221] Remove hard-coded Iceberg scan class name detection using type-based check [auron]
Posted by "guixiaowen (via GitHub)" <gi...@apache.org>.
guixiaowen commented on PR #2226:
URL: https://github.com/apache/auron/pull/2226#issuecomment-4417730178
@cxzl25 Thanks for your review and merge.
--
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: commits-unsubscribe@auron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org