Skip to content

Fix named struct dataType null field names [databricks] - #15358

Merged
firestarman merged 3 commits into
NVIDIA:mainfrom
firestarman:issue-15316-create-named-struct-null-name
Jul 24, 2026
Merged

Fix named struct dataType null field names [databricks]#15358
firestarman merged 3 commits into
NVIDIA:mainfrom
firestarman:issue-15316-create-named-struct-null-name

Conversation

@firestarman

Copy link
Copy Markdown
Collaborator

Fixes #15316.

Description

  • Fix GpuCreateNamedStruct.dataType to match Spark behavior for null field names, so Spark versions with SPARK-57736 no longer hit an NPE before type checking rejects the invalid input.
  • Add CreateNamedStructShims so legacy Spark shims keep the old name.toString behavior, while Spark 4.0.4, 4.1.3, and 4.2.0 use Spark's null-safe field-name behavior.
  • Add shim-specific Scala tests for both legacy and fixed behavior, covering null field names in GpuCreateNamedStruct.dataType and the expected input type-check failure.
  • Validated with focused Scala tests: mvn -s /home/liangcail/.m2/settings_art.xml -f scala2.13/pom.xml -pl sql-plugin -Dbuildver=358 -Dcuda.version=cuda13 -DwildcardSuites=org.apache.spark.sql.rapids.GpuCreateNamedStructSuite test, plus the same command for buildver=403, 404, 412, 413, and 420.

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
    (Please provide the names of the existing tests in the PR description.)
  • 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: Firestarman <firestarmanllc@gmail.com>
@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces a version-specific shim (CreateNamedStructShims.fieldName) to control how GpuCreateNamedStruct.dataType handles null field names, aligning GPU behavior with what each Spark version's CPU implementation does.

  • Legacy shim (Spark 3.3.0–4.1.2): Preserves name.toString semantics — a null name throws NullPointerException from dataType, exactly as the CPU CreateNamedStruct did before SPARK-57736. checkInputDataTypes still independently rejects null names, so the NPE is only reached if dataType is evaluated before validation.
  • Fixed shim (Spark 4.0.4 / 4.1.3 / 4.2.0): Returns null for a null name instead of throwing, matching the null-safe dataType behavior introduced by SPARK-57736. checkInputDataTypes still rejects null names, so invalid input is caught cleanly at validation time.
  • Shim coverage: All 33 supported build versions (including all Databricks variants) are accounted for across the two shim files, and paired unit tests cover both code paths.

Confidence Score: 5/5

Safe to merge — the change is a targeted, non-execution-path fix to dataType type computation, guarded by checkInputDataTypes, with full shim-version coverage and dedicated tests for both behaviors.

The change is small and well-scoped: a single call-site substitution in dataType backed by two trivial shim implementations. Every supported build version (including all Databricks variants) is present in one of the two shim annotations. The tests exercise both the legacy-NPE path and the null-safe path, and checkInputDataTypes independently rejects null field names in all versions. No GPU allocation, no data-path change, no resource lifecycle concerns.

No files require special attention.

Important Files Changed

Filename Overview
sql-plugin/src/main/scala/org/apache/spark/sql/rapids/complexTypeCreator.scala Single-line change routing fieldName resolution through CreateNamedStructShims.fieldName instead of name.toString, correctly delegating null-safety semantics per Spark version.
sql-plugin/src/main/spark330/scala/org/apache/spark/sql/rapids/shims/CreateNamedStructShims.scala Legacy shim covering Spark 3.3.0–4.1.2 (all versions without SPARK-57736); preserves name.toString behavior that throws NPE for null field names, matching CPU Spark semantics. All db/non-db variants are listed.
sql-plugin/src/main/spark404/scala/org/apache/spark/sql/rapids/shims/CreateNamedStructShims.scala New null-safe shim for Spark 4.0.4, 4.1.3, and 4.2.0 (versions carrying SPARK-57736); returns null for a null name instead of throwing NPE, matching the fixed CPU behavior.
sql-plugin/src/test/spark330/scala/org/apache/spark/sql/rapids/GpuCreateNamedStructSuite.scala Test suite for legacy Spark versions; asserts that dataType on a null field name still throws NPE (matching legacy CPU behavior) and that checkInputDataTypes independently rejects null field names.
sql-plugin/src/test/spark404/scala/org/apache/spark/sql/rapids/GpuCreateNamedStructSuite.scala Test suite for Spark 4.0.4+; verifies that dataType succeeds with a null field name (null-safe path), the resulting StructField has a null name, and checkInputDataTypes still rejects the input.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["GpuCreateNamedStruct.dataType"] --> B["CreateNamedStructShims.fieldName(name)"]
    B --> C{Spark version?}
    C -->|"3.3.0 – 4.1.2\n(legacy shim)"| D["name.toString\n(NPE if name == null)"]
    C -->|"4.0.4 / 4.1.3 / 4.2.0\n(SPARK-57736 shim)"| E["if (name == null) null\nelse name.toString"]
    D --> F["StructField(name, ...)"]
    E --> F
    F --> G["StructType(fields)"]

    H["GpuCreateNamedStruct.checkInputDataTypes"] --> I["names.contains(null)?"]
    I -->|yes| J["TypeCheckFailure\n('Field name should not be null')"]
    I -->|no| K["TypeCheckSuccess"]
Loading

Reviews (2): Last reviewed commit: "Fix named struct import ordering" | Re-trigger Greptile

@firestarman firestarman changed the title Fix named struct dataType null field names Fix named struct dataType null field names [databricks] Jul 23, 2026
@firestarman

Copy link
Copy Markdown
Collaborator Author

build

Signed-off-by: Firestarman <firestarmanllc@gmail.com>
@firestarman

Copy link
Copy Markdown
Collaborator Author

build

Signed-off-by: Firestarman <firestarmanllc@gmail.com>

@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 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,

not a blocker but just wondering if it is worth shimming instead of making it version-independently correct. /cc @revans2

@gerashegalov

Copy link
Copy Markdown
Collaborator

build

1 similar comment
@sameerz

sameerz commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

build

@firestarman

firestarman commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

not a blocker but just wondering if it is worth shimming instead of making it version-independently correct. /cc @revans2

Good question, i just decided to aligin with the Spark behavior.

@firestarman
firestarman merged commit 3129c8c into NVIDIA:main Jul 24, 2026
53 checks passed
@firestarman
firestarman deleted the issue-15316-create-named-struct-null-name branch July 24, 2026 05:44
@firestarman

Copy link
Copy Markdown
Collaborator Author

I am going to merge this, if we decide to make it version-independently correct, i will file another PR.

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.

[AI-AUDIT] Mirror SPARK-57736 null-safe field names in GpuCreateNamedStruct.dataType

5 participants