Skip to content

Add null safety guards to the conversion methods in FromIcebergShaded - #13942

Merged
res-life merged 1 commit into
NVIDIA:release/25.12from
res-life:fix-iceberg-NPE
Dec 5, 2025
Merged

Add null safety guards to the conversion methods in FromIcebergShaded#13942
res-life merged 1 commit into
NVIDIA:release/25.12from
res-life:fix-iceberg-NPE

Conversation

@res-life

@res-life res-life commented Dec 4, 2025

Copy link
Copy Markdown
Collaborator

Improve code: Fix NPE in Iceberg code

Signed-off-by: Chong Gao <res_life@163.com>
@res-life
res-life changed the base branch from main to release/25.12 December 4, 2025 03:13
@greptile-apps

greptile-apps Bot commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

Added null safety guards to all conversion methods in FromIcebergShaded that transform shaded Parquet objects to unshaded versions. The changes prevent potential NPEs when getter methods like columnOrder(), getEncodingStats(), getStatistics(), etc. return null values during Parquet metadata processing.

  • Added null checks to 18 unshade overload methods covering all Parquet type conversions
  • Each method now returns null when receiving null input, maintaining consistent behavior
  • Prevents NPEs in nested unshade calls where getter methods may return nullable values

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are purely defensive, adding null checks to prevent NPEs without altering any business logic. The pattern is consistent across all methods and follows best practices for null safety in conversion utilities
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
iceberg/src/main/scala/com/nvidia/spark/rapids/iceberg/parquet/converter/FromIcebergShaded.scala 5/5 Added defensive null checks to all unshade methods to prevent NPEs when converting Parquet metadata objects

Sequence Diagram

sequenceDiagram
    participant Reader as GpuIcebergParquetReader
    participant Converter as FromIcebergShaded
    participant Parquet as Parquet API
    
    Reader->>Converter: unshade(ShadedMessageType)
    Note over Converter: Check if messageType == null
    alt messageType is null
        Converter-->>Reader: return null
    else messageType is not null
        Converter->>Converter: Process fields
        loop For each field
            Converter->>Converter: unshade(field)
            Note over Converter: Recursive null check
        end
        Converter->>Parquet: Types.buildMessage()
        Parquet-->>Converter: MessageType
        Converter-->>Reader: MessageType
    end
    
    Reader->>Converter: unshade(ShadedColumnChunkMetaData)
    Note over Converter: Check if metadata == null
    Converter->>Converter: unshade(getPath)
    Note over Converter: Nested null check for path
    Converter->>Converter: unshade(getEncodingStats)
    Note over Converter: Nested null check for stats
    Converter->>Converter: unshade(getStatistics)
    Note over Converter: Nested null check for statistics
    Converter->>Parquet: ColumnChunkMetaData.get(...)
    Parquet-->>Reader: ColumnChunkMetaData
Loading

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@gerashegalov gerashegalov changed the title Fix NPE in Iceberg code Add null safety guards to the conversion methods in FromIcebergShaded Dec 4, 2025
@res-life

res-life commented Dec 4, 2025

Copy link
Copy Markdown
Collaborator Author

build

1 similar comment
@res-life

res-life commented Dec 4, 2025

Copy link
Copy Markdown
Collaborator Author

build

@sameerz sameerz added the bug Something isn't working label Dec 5, 2025
@res-life res-life self-assigned this Dec 5, 2025
@res-life
res-life merged commit 985d031 into NVIDIA:release/25.12 Dec 5, 2025
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants