feat: [iceberg] allow native Iceberg scans with non-identity transform residuals

[Shekharrajak]

Enable native execution for Iceberg queries with bucket/truncate/year/month/day/hour transforms in residuals. Row-group filtering skips these predicates but post-scan CometFilter applies them. Provides native performance while maintaining correctness.

[parthchandra]

Could you fix the scalastyle errors so the tests can run?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.98%. Comparing base (f09f8af) to head (d8fa8c9).
⚠️ Report is 913 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/scala/org/apache/comet/rules/CometScanRule.scala	46.15%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2948      +/-   ##
============================================
+ Coverage     56.12%   59.98%   +3.85%     
- Complexity      976     1476     +500     
============================================
  Files           119      175      +56     
  Lines         11743    16169    +4426     
  Branches       2251     2682     +431     
============================================
+ Hits           6591     9699    +3108     
- Misses         4012     5118    +1106     
- Partials       1140     1352     +212     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Shekharrajak]

Ref #2921

[mbutrovich]

We need [iceberg] in the title to run Iceberg Spark tests.

[mbutrovich]

Can you push a dummy commit to trigger CI with the Iceberg tests? I can't seem to force that rule to re-run.

[andygrove]

Review Summary

This PR enables native Iceberg scans when residual expressions contain non-identity transforms (truncate, bucket, year, month, day, hour). The approach is sound - row-group filtering skips these predicates while post-scan CometFilter ensures correctness.

Verified:

• CI tests all passing
• Tests verify correctness via checkSparkAnswer (compares Spark vs Comet results)
• Tests added for truncate, bucket, and year transforms per @parthchandra's feedback

Minor Suggestions

1. Consider adding tests for month/day/hour transforms

The PR supports month, day, and hour transforms but only explicitly tests truncate, bucket, and year. While year uses a similar code path, explicit tests would provide better coverage:

test("non-identity transform residual - month transform allows native scan") {
  // Similar to year test but with PARTITIONED BY (month(event_date))
}

2. Question about reflection failure behavior

The behavior when reflection fails changed from "fall back to Spark" to "continue with native scan" (lines 503-512):

} catch {
  case e: Exception =>
    // Reflection failure - log warning but allow native execution
    // The predicate conversion will handle unsupported cases gracefully
    logWarning(...)
    true  // Previously was false
}

Could you clarify what "handle unsupported cases gracefully" means here? If an unexpected predicate reaches the native layer, does it fail safely (causing fallback) or could it produce incorrect results?

---

Overall the PR looks good - the core correctness is verified by checkSparkAnswer and the optimization enables native execution for more query patterns.

---

This review was generated with AI assistance.

[mbutrovich]

I converted this to draft because I don't want it to get accidentally merged. We have not run the Iceberg suite on it yet. We're waiting on a commit after I added [iceberg] to the title.

[Shekharrajak]

Added the tests. Please let me know if we can make it ready for review.

[Shekharrajak]

The PR supports month, day, and hour transforms but only explicitly tests truncate, bucket, and year. While year uses a similar code path, explicit tests would provide better coverage:

Added tests for these transform.

[Shekharrajak]

if we could not find transform using IcebergReflection.findNonIdentityTransformInResiduals then we will do with native scan and get the correct result :

Native Scan -> CometFilter -> User Query

[Shekharrajak]

Reflection fails → "Try native anyway"
→ Native scan without row-group filter
→ Post-scan filter ensures correctness

[Shekharrajak]

Could you clarify what "handle unsupported cases gracefully" means here? If an unexpected predicate reaches the native layer, does it fail safely (causing fallback) or could it produce incorrect results?

if we could not find transform using IcebergReflection.findNonIdentityTransformInResiduals then we will do with native scan and get the correct result :

Native Scan -> CometFilter -> User Query

https://github.com/apache/datafusion-comet/pull/2948/files#r2740040695

[Shekharrajak]

delete operation tests where failing so we are falling back to spark for that. Please trigger the workflow to validate now.

[github-actions]

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

[Shekharrajak]

Now the implementation is looking fine and checks locally passes. I have added integration tests as well.