Skip to content

Move independent Iceberg helpers to root-safe module [reduced-it] - #15841

Open
gerashegalov wants to merge 6 commits into
NVIDIA:mainfrom
gerashegalov:codex/unshim-iceberg-independent-helpers
Open

Move independent Iceberg helpers to root-safe module [reduced-it]#15841
gerashegalov wants to merge 6 commits into
NVIDIA:mainfrom
gerashegalov:codex/unshim-iceberg-independent-helpers

Conversation

@gerashegalov

@gerashegalov gerashegalov commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Related to #15821.

Supersedes #15826 with an independent, non-stacked change.

Description

Move a self-contained set of Apache Iceberg helpers into the root-safe
iceberg-common module without depending on the other pending Iceberg
unshimming pull requests.

The root-safe set includes:

  • the deletion-vector container;
  • reader option and native-row-index utilities;
  • reader mode types; and
  • position-delete field IDs.

Keep implementation-dependent delete-filter helpers in the versioned Iceberg
module by folding them into the existing GpuDeleteFileInfo companion. Remove
the six corresponding special root-layout allowlist entries and add the direct
spark-rapids-jni dependency required by the deletion-vector container.

The catalog and table wrappers remain in the versioned Iceberg module because
their Spark catalog interfaces are not source-compatible across all supported
Spark versions.

Packages, public class names, algorithms, data flow, and runtime behavior are
unchanged. This changes source-module ownership and class placement only.

This pull request was prepared with AI assistance and reviewed by the author
before publication. It also received an independent read-only code review.

TODO before marking ready for review:

  • Confirm the full multi-shim distribution assembly in pre-merge CI.
  • Confirm the AppClassLoader/extraClassPath Iceberg smoke coverage in
    pre-merge CI.
  • Address CI and review feedback.

Validation:

  • Built iceberg-common and the Iceberg consumer for Spark 3.5.0,
    Iceberg 1.6, and Scala 2.12.
  • Built iceberg-common and the Iceberg consumer for Spark 4.1.1,
    Iceberg 1.11, and Scala 2.13.
  • Built iceberg-common for Spark 3.3.1/Scala 2.12, Spark 4.0.0/Scala 2.13,
    and Spark 4.2.0/Scala 2.13 after keeping the catalog wrappers versioned.
  • Ran GpuDeleteFilterSuite, GpuPostProcessorSuite,
    IcebergDeletionVectorSuite, and RapidsSparkTableSuite: 57 tests passed.
  • Verified the expected classes are present in both root-safe classifier jars
    and absent from both versioned Iceberg jars.
  • Ran ./build/make-scala-version-build-files.sh 2.13.
  • Ran git diff --check.

A broader local reactor build stopped before reaching Iceberg because unchanged
SQL-plugin sources use a cuDF API marked deprecated by the locally resolved
dependency under warnings-as-errors. The focused changed modules and tests
completed successfully.

Performance: Not required because this only relocates helpers or changes their
owning companion. It does not add runtime operations, allocations, passes, or
synchronization.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (GpuDeleteFilterSuite, GpuPostProcessorSuite,
    IcebergDeletionVectorSuite, and RapidsSparkTableSuite.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
@gerashegalov gerashegalov self-assigned this Aug 31, 2026
@gerashegalov
gerashegalov marked this pull request as ready for review August 31, 2026 22:11
@gerashegalov
gerashegalov requested a review from a team as a code owner August 31, 2026 22:11
@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR relocates self-contained Iceberg helpers into the root-safe iceberg-common module while retaining implementation-dependent helpers in the versioned module.

  • Moves deletion-vector, Parquet reader utility, reader-mode, and position-delete field-ID classes.
  • Updates consumers and removes obsolete root-layout allowlist entries.
  • Consolidates delete-filter helpers into GpuDeleteFileInfo without changing their implementation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
dist/unshimmed-common-from-single-shim.txt Removes special root-layout entries for helpers now supplied by the root-safe module.
iceberg-common/src/main/java/com/nvidia/spark/rapids/iceberg/IcebergDeletionVector.java Relocates the deletion-vector implementation without changing its algorithm or ownership behavior.
iceberg-common/src/main/scala/com/nvidia/spark/rapids/iceberg/parquet/GpuIcebergParquetReaderUtils.scala Houses unchanged reader-option and native-row-index helpers in the shared module.
iceberg-common/src/main/scala/com/nvidia/spark/rapids/iceberg/parquet/ThreadConf.scala Moves the root-safe reader-mode types out of the versioned implementation.
iceberg-common/src/main/scala/org/apache/iceberg/io/GpuPositionDeleteFieldIds.scala Moves position-delete field IDs into a package-scoped shared helper.
iceberg/common/src/main/scala/com/nvidia/spark/rapids/iceberg/data/GpuDeleteFilter.scala Consolidates unchanged delete-filter helpers into the existing GpuDeleteFileInfo companion.
iceberg/common/src/main/scala/com/nvidia/spark/rapids/iceberg/parquet/reader.scala Replaces local helper definitions and calls with references to the root-safe utilities.
iceberg/common/src/main/scala/org/apache/iceberg/io/rolling.scala Uses the relocated position-delete field-ID helper.
tests/src/test/spark350/scala/com/nvidia/spark/rapids/iceberg/GpuPostProcessorSuite.scala Updates tests to invoke the relocated native-row-index utility.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Versioned[Versioned Iceberg module]
  Common[Root-safe iceberg-common]
  Dist[Distribution root]
  Runtime[Iceberg runtime consumers]
  Versioned -->|depends on| Common
  Common -->|promoted once| Dist
  Versioned --> Runtime
  Dist --> Runtime
Loading

Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
@gerashegalov

Copy link
Copy Markdown
Collaborator Author

build

1 similar comment
@gerashegalov

Copy link
Copy Markdown
Collaborator Author

build

@gerashegalov
gerashegalov requested a review from a team August 31, 2026 23:27
NvTimLiu
NvTimLiu previously approved these changes Sep 1, 2026

@NvTimLiu NvTimLiu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM from CICD point of view

@NvTimLiu
NvTimLiu requested a review from a team September 1, 2026 02:56
…independent-helpers

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>

# Conflicts:
#	dist/unshimmed-common-from-single-shim.txt
new JHashMap[Integer, Any](),
expectedSchema,
GpuIcebergParquetReader.withNativeRowIndex(shadedSchema),
GpuIcebergParquetReaderUtils.withNativeRowIndex(shadedSchema),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Update the remaining withNativeRowIndex callers

This PR removes object GpuIcebergParquetReader, but lines 1181 and 1218 in this file still call GpuIcebergParquetReader.withNativeRowIndex(...). A clean Spark 3.5 test compile therefore fails with not found: value GpuIcebergParquetReader. Please change those two callers to GpuIcebergParquetReaderUtils.withNativeRowIndex(...) as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@res-life updated in 230ec0b

…independent-helpers

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
res-life
res-life previously approved these changes Sep 4, 2026

@res-life res-life left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gerashegalov gerashegalov changed the title Move independent Iceberg helpers to root-safe module [fast-ut][reduced-it] Move independent Iceberg helpers to root-safe module [reduced-it] Sep 4, 2026
@gerashegalov

Copy link
Copy Markdown
Collaborator Author

build

…independent-helpers

Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>

# Conflicts:
#	dist/unshimmed-common-from-single-shim.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants