Skip to content

Conversation

@liurenjie1024
Copy link
Collaborator

Fixes #13832 .

Description

A little refactoring about gpu support of iceberg's transform, removing redundant codes.

Checklists

  • This PR has added documentation for new or modified features or behaviors.
  • This PR has added new tests or modified existing tests to cover new code paths.
    (Please explain in the PR description how the new code paths are tested, such as names of the new/existing tests that cover them.)
  • Performance testing has been performed and its results are added in the PR description. Or, an issue has been filed with a link in the PR description.

@liurenjie1024 liurenjie1024 changed the base branch from main to release/25.12 November 21, 2025 11:23
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 21, 2025

Greptile Overview

Greptile Summary

Refactored Iceberg transform expression handling to eliminate code duplication. Previously, tagExprForGpu and conversion logic were scattered across multiple companion objects (GpuBucketExpression, GpuYearsExpression, GpuMonthsExpression, GpuDaysExpression, GpuHoursExpression). Now, all transform validation and conversion logic is centralized in transforms.scala through the GpuTransform trait and its companion object.

Key changes:

  • Added tagExprForGpu and convertToGpu methods to the GpuTransform trait
  • Implemented new GpuTransform.apply(StaticInvoke) factory method that pattern-matches on transform class names
  • Simplified IcebergProviderImpl by delegating to GpuTransform pattern matching instead of explicit type checks
  • Removed redundant companion objects and methods from individual expression classes
  • Moved isSupported type checking from GpuBucketExpression to GpuBucket companion object

Confidence Score: 5/5

  • Safe to merge - clean refactoring with preserved behavior
  • Pure refactoring that consolidates duplicate code into a unified pattern. All validation logic is preserved and moved to appropriate locations. No functional changes to the transform operations themselves.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
iceberg/src/main/scala/com/nvidia/spark/rapids/iceberg/IcebergProviderImpl.scala 5/5 Simplified transform tagging/conversion by delegating to unified GpuTransform pattern matching
iceberg/src/main/scala/org/apache/iceberg/spark/functions/transforms.scala 5/5 Added tagExprForGpu and convertToGpu methods to GpuTransform trait and companion object factory
iceberg/src/main/scala/org/apache/iceberg/spark/functions/bucket.scala 5/5 Removed redundant tagExprForGpu and support methods from GpuBucketExpression companion object

Sequence Diagram

sequenceDiagram
    participant Client
    participant IcebergProviderImpl
    participant GpuTransform
    participant Transform Objects
    
    Note over Client,Transform Objects: tagForGpu Flow
    Client->>IcebergProviderImpl: tagForGpu(StaticInvoke, meta)
    IcebergProviderImpl->>GpuTransform: apply(StaticInvoke)
    GpuTransform->>GpuTransform: Match staticObject class name
    alt BucketInt/BucketLong
        GpuTransform-->>IcebergProviderImpl: Success(GpuBucket)
    else Years/Months/Days/Hours Functions
        GpuTransform-->>IcebergProviderImpl: Success(GpuYears/Months/Days/Hours)
    else Unsupported
        GpuTransform-->>IcebergProviderImpl: Failure(IllegalArgumentException)
        IcebergProviderImpl->>Client: meta.willNotWorkOnGpu()
    end
    IcebergProviderImpl->>Transform Objects: transform.tagExprForGpu(meta)
    Transform Objects->>Transform Objects: Validate argument types
    Transform Objects-->>IcebergProviderImpl: Tag as supported/unsupported
    
    Note over Client,Transform Objects: convertToGpu Flow
    Client->>IcebergProviderImpl: convertToGpu(StaticInvoke, meta)
    IcebergProviderImpl->>GpuTransform: apply(StaticInvoke)
    GpuTransform-->>IcebergProviderImpl: Success(transform)
    IcebergProviderImpl->>Transform Objects: transform.convertToGpu(expr, meta)
    Transform Objects->>Transform Objects: Convert child expressions
    Transform Objects-->>IcebergProviderImpl: GpuExpression
    IcebergProviderImpl-->>Client: GpuExpression
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

7 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@revans2 revans2 changed the title Simplify gpu transform operations Simplify gpu transform operations in iceberg Nov 21, 2025
@sameerz sameerz added the task Work required that improves the product but is not user facing label Nov 26, 2025
@liurenjie1024 liurenjie1024 changed the base branch from release/25.12 to main November 26, 2025 01:15
@liurenjie1024 liurenjie1024 marked this pull request as draft November 26, 2025 01:15
def apply(expr: StaticInvoke): Try[GpuTransform] = {
val staticObj = expr.staticObject
val name = staticObj.getName
if (name.equals(classOf[BucketInt].getName) ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

are you working inequality due to classloading via name comparison ? Otherwise could do

  def apply(expr: StaticInvoke): Try[GpuTransform] = Try { 
    expr.staticObject match {
      case c if c == classOf[BucketInt] || c == classOf[BucketLong] => GpuBucket(0)
      case c if c == classOf[DateToYearsFunction] || c == classOf[TimestampToYearsFunction] => GpuYears
      ...
    }
  }

@nvauto
Copy link
Collaborator

nvauto commented Jan 26, 2026

NOTE: release/26.02 has been created from main. Please retarget your PR to release/26.02 if it should be included in the release.

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 tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Unfify iceberg transform expression fallback detection.

4 participants