Skip to content

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

Open
gerashegalov wants to merge 9 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 9 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.

Description

Move a self-contained set of Apache Iceberg helpers from the versioned
iceberg/common source module into the root-safe iceberg-common module:

  • IcebergDeletionVector;
  • reader-option and native-row-index utilities;
  • ThreadConf and SingleFile; and
  • position-delete field IDs.

Replace the internal GpuIcebergParquetReader and
GpuPositionDeleteFileWriter companion helpers with dedicated root-safe
owners. Fold the implementation-dependent GpuDeleteFilter2 helpers into
GpuDeleteFileInfo, where they remain in the versioned source module.
Together, these changes remove six Iceberg entries from the special
root-layout allowlist.

The catalog and table wrappers remain version-specific because their Spark
catalog interfaces are not source-compatible across all supported versions.
There is no user-facing configuration or behavior change. Algorithms and data
flow are unchanged; only source ownership, final class placement, and internal
companion names change.

Validation on the prepared up-merged branch:

  • Clean multi-shim dist assembly for the highest patch release in each
    supported line:
    • Scala 2.12: Spark 3.3.4, 3.4.4, and 3.5.9.
    • Scala 2.13: Spark 3.5.9, 4.0.4, 4.1.3, and 4.2.0.
  • The package-time Iceberg audit passed with all 10 package-private caller
    classes at the JAR root across two Scala 2.12 and five Scala 2.13 Iceberg
    runtime worlds.
  • Inspected both assembled distribution JARs: every promoted helper is present
    only at the conventional root, with no copy in spark-shared or a selected
    Spark-specific directory. The three retired companion classes are absent.
  • Spark 3.5.9/Scala 2.12 focused Iceberg suites: 28 tests passed and 11
    runtime-inapplicable row-lineage cases were canceled.
  • Spark 4.1.3/Scala 2.13 with Iceberg 1.11: 5 applicable tests passed.
  • Spark 4.2.0/Scala 2.13 compiled successfully; that line currently selects the
    Iceberg stub.
  • git diff --check passed.

Pre-merge CI provides the exhaustive shim matrix and the
AppClassLoader/extraClassPath Iceberg smoke coverage.

Performance testing is not required. The change adds no per-batch or hot-path
work. Consolidating the delete-filter metadata into GpuDeleteFileInfo may
initialize those small immutable values earlier, which is a negligible
one-time cost.

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

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

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, packaging, security, or repository-rule issue remains.

Summary

  • Promotes deletion-vector, Parquet reader-option, native-row-index, thread-configuration, and position-delete helpers.
  • Consolidates delete-filter metadata and utilities under GpuDeleteFileInfo.
  • Removes obsolete companion classes and their special root-layout allowlist entries.
  • Updates focused post-processor tests to use the new helper owner.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A["iceberg/common<br/>version-specific sources"] -->|move independent helpers| B["iceberg-common<br/>root-safe module"]
  B --> C["Distribution JAR root"]
  A -->|call renamed helper owners| B
  D["Special root-layout allowlist"] -->|remove obsolete entries| C
Loading

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
firestarman
firestarman previously approved these changes Sep 7, 2026
…independent-helpers

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

# Conflicts:
#	dist/unshimmed-common-from-single-shim.txt
Signed-off-by: Gera Shegalov <gshegalov@nvidia.com>
@gerashegalov

Copy link
Copy Markdown
Collaborator Author

build

…independent-helpers

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

Copy link
Copy Markdown
Collaborator Author

build

@sameerz sameerz added the task Work required that improves the product but is not user facing label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

task Work required that improves the product but is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants