feat: [iceberg] REST catalog support for CometNativeIcebergScan by mbutrovich · Pull Request #2895 · apache/datafusion-comet

mbutrovich · 2025-12-13T01:55:19Z

Which issue does this PR close?

N/A.

Rationale for this change

REST catalogs return metadata locations that don't physically exist on disk - the metadata is transferred via HTTP. This caused native Iceberg scans to fall back to Spark.

What changes are included in this PR?

Fixed reflection for protected methods:

Changed IcebergReflection.getTableMetadata() and getFormatVersion() to use getDeclaredMethod() with setAccessible(true) to access protected methods in BaseTable

Handle missing metadata files:

Detect when metadata file doesn't exist (REST catalog case)
Fall back to table location for FileIO initialization in iceberg-rust
The location is only used to determine storage backend type (local/S3/GCS) - actual metadata comes from the FileScanTasks

REST catalog test infrastructure:

Copied RESTCatalogServlet from Iceberg
Added Jetty dependencies for embedded REST server
Added withRESTCatalog test helper

How are these changes tested?

New test "REST catalog with native Iceberg scan" creates an embedded Jetty server with InMemoryCatalog backend, creates a table via REST API, and verifies native Iceberg scan is used with correct results.

All Iceberg Java tests pass locally with these changes.

codecov-commenter · 2025-12-13T02:24:31Z

Codecov Report

❌ Patch coverage is 67.85714% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.41%. Comparing base (f09f8af) to head (7cde53a).
⚠️ Report is 766 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/scala/org/apache/comet/rules/CometScanRule.scala	55.00%	7 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #2895      +/-   ##
============================================
+ Coverage     56.12%   59.41%   +3.28%     
- Complexity      976     1379     +403     
============================================
  Files           119      167      +48     
  Lines         11743    15342    +3599     
  Branches       2251     2545     +294     
============================================
+ Hits           6591     9115    +2524     
- Misses         4012     4943     +931     
- Partials       1140     1284     +144

☔ 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.

andygrove · 2025-12-13T16:38:20Z

spark/src/main/scala/org/apache/comet/iceberg/IcebergReflection.scala

+          val opsMethod = table.getClass.getDeclaredMethod("operations")
+          opsMethod.setAccessible(true)


nit: maybe add a helper method for this pattern of getDeclaredMethod and setAccessible since this is repeated. Can be in a future PR

andygrove

Thanks @mbutrovich

comphead

Thanks @mbutrovich

Wondering should we keep credits for the files copied from Iceberg?

hsiang-c · 2025-12-15T18:02:18Z

LGTM, with this fix, I'm able to see CometIcebergNativeScan while querying Iceberg table from Iceberg REST Catalog

AdaptiveSparkPlan isFinalPlan=false
+- CometHashAggregate
   +- CometExchange SinglePartition, ENSURE_REQUIREMENTS, CometNativeShuffle, [plan_id=62]
      +- CometHashAggregate
         +- CometHashAggregate
            +- CometExchange
               +- CometHashAggregate
                  +- CometProject
                     +- CometIcebergNativeScan

hsiang-c

LGTM

hsiang-c · 2025-12-15T18:08:22Z

spark/src/test/spark-4.0/org/apache/iceberg/rest/RESTCatalogServlet.java

+ * RESTCatalogAdaptor to proxy the REST Spec to any Catalog implementation.
+ * Modified version of Iceberg's org/apache/iceberg/rest/RESTCatalogServlet.java
+ */
+public class RESTCatalogServlet extends HttpServlet {


I think we can, in Maven, depends on iceberg-core using the tests classifier so that you don't have to copy them. @mbutrovich @comphead

Iceberg does it in Gradle by https://github.com/apache/iceberg/blob/main/build.gradle#L439

I'll give this a shot, thank you!

I'm gonna open a follow-up issue for this. I fell into a dependency nightmare again.

hsiang-c · 2025-12-15T18:09:46Z

spark/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java

+import org.apache.iceberg.util.PropertyUtil;
+
+/** Adaptor class to translate REST requests into {@link Catalog} API calls. */
+public class RESTCatalogAdapter implements RESTClient {


Ditto, if depending on iceberg-core's test artifacts is okay in the unit test scope, we can avoid this copy.

mbutrovich added 5 commits December 12, 2025 19:13

stash with test

db75dae

passes test

fc51f55

passes test

8f3cba1

stash

d469f4d

stash.

d00d9d2

mbutrovich changed the title ~~Iceberg rest~~ feat: [iceberg] REST catalog support for CometNativeIcebergScan Dec 13, 2025

Kick CI

bb9214c

mbutrovich added 3 commits December 12, 2025 21:28

Format.

110bb33

Fix jetty dependencies.

4bbd6ca

Fix jetty dependencies, again.

0885e7b

mbutrovich requested review from andygrove, comphead and parthchandra December 13, 2025 16:24

andygrove reviewed Dec 13, 2025

View reviewed changes

andygrove approved these changes Dec 13, 2025

View reviewed changes

mbutrovich added 4 commits December 14, 2025 13:05

Fix jetty dependencies, again.

67634e0

Fix jetty dependencies, again.

991c5a2

Fix jetty dependencies, again.

8f16e8b

Fix jetty dependencies, again.

abf4454

mbutrovich removed request for comphead and parthchandra December 15, 2025 15:42

comphead approved these changes Dec 15, 2025

View reviewed changes

Add comment about files from Iceberg.

c57545f

hsiang-c approved these changes Dec 15, 2025

View reviewed changes

hsiang-c reviewed Dec 15, 2025

View reviewed changes

Merge branch 'main' into iceberg_rest

7cde53a

mbutrovich merged commit a51b281 into apache:main Dec 15, 2025
171 of 173 checks passed

mbutrovich deleted the iceberg_rest branch December 15, 2025 21:39

mbutrovich mentioned this pull request Jan 13, 2026

feat: [iceberg] Native scan by serializing FileScanTasks to iceberg-rust #2528

Merged

andygrove mentioned this pull request Jan 23, 2026

Comet 0.13.0 Release (January 2026) #2845

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [iceberg] REST catalog support for CometNativeIcebergScan#2895

feat: [iceberg] REST catalog support for CometNativeIcebergScan#2895
mbutrovich merged 15 commits intoapache:mainfrom
mbutrovich:iceberg_rest

mbutrovich commented Dec 13, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Dec 13, 2025 •

edited

Loading

Uh oh!

andygrove Dec 13, 2025

Uh oh!

andygrove left a comment

Uh oh!

comphead left a comment

Uh oh!

hsiang-c commented Dec 15, 2025 •

edited

Loading

Uh oh!

hsiang-c left a comment

Uh oh!

hsiang-c Dec 15, 2025 •

edited

Loading

Uh oh!

mbutrovich Dec 15, 2025

Uh oh!

mbutrovich Dec 15, 2025

Uh oh!

hsiang-c Dec 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		val opsMethod = table.getClass.getDeclaredMethod("operations")
		opsMethod.setAccessible(true)

Conversation

mbutrovich commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

How are these changes tested?

Uh oh!

codecov-commenter commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andygrove Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

andygrove left a comment

Choose a reason for hiding this comment

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

hsiang-c commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hsiang-c left a comment

Choose a reason for hiding this comment

Uh oh!

hsiang-c Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbutrovich Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

mbutrovich Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

hsiang-c Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mbutrovich commented Dec 13, 2025 •

edited

Loading

codecov-commenter commented Dec 13, 2025 •

edited

Loading

hsiang-c commented Dec 15, 2025 •

edited

Loading

hsiang-c Dec 15, 2025 •

edited

Loading