Skip to content

Fix Row.toString NPE on a null nested inside an array, map or row - #39587

Open
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-row-tostring-npe-on-nested-null
Open

Fix Row.toString NPE on a null nested inside an array, map or row#39587
PDGGK wants to merge 1 commit into
apache:masterfrom
PDGGK:fix-row-tostring-npe-on-nested-null

Conversation

@PDGGK

@PDGGK PDGGK commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Row.toString() throws NullPointerException whenever a null sits inside an array, an iterable, a map or a nested row.

Schema schema = Schema.builder().addArrayField("a", FieldType.STRING.withNullable(true)).build();
Row.withSchema(schema).addValue(Arrays.asList("x", null)).build().toString();
// java.lang.NullPointerException: Cannot invoke "String.replace(...)" because "string" is null
//     at SchemaUtils.toPrettyFieldValueString(SchemaUtils.java:256)

toPrettyRowString skips a row's own null fields, but every recursive call hands the raw element to toPrettyFieldValueString, which has no null check and dereferences it — string.replace for STRING, row.getValues() for ROW, value.getClass() for ARRAY/MAP. Only the numeric and boolean branches survive, and only because Objects.toString already renders a null as "null".

These values are legal: RowUtils explicitly accepts null array elements and null map values when the element type is nullable.

The fix

Return "null" for a null value at the top of toPrettyFieldValueString. That is not a new rendering — it is exactly what the numeric branches have always produced, so this makes the remaining types consistent with the ones that already worked. Seven lines, one file, no API or wire-format change.

toPrettyRowString's deliberate skipping of top-level null fields is left alone; that is the printer's intended terse style.

Why this regressed

Row.toString() used to call toString(true), whose private helper opens with if (value == null) { return "<null>"; } and is therefore null-safe at every depth. That method is still present. Row.toString() was rerouted to SchemaUtils.toPrettyString(this) in #35150, and SchemaUtils.toPrettyString(this) first appears in Row.java at v2.69.0 — it is absent from v2.67.0 and v2.68.0 — so every release from 2.69.0 on has it.

toString() is called from logging, PAssert failure messages, exception messages and debugger inspection, so in practice the NPE fires while a pipeline is already reporting a different problem, and hides it.

SchemaUtilsTest is 107 lines and does not reference the pretty-printer at all, which is why the 279 lines added in #35150 went unnoticed.

Testing

Four tests added to SchemaUtilsTest: null inside an array of strings, a null map value, a null row inside an array, and — as a control that pins the existing behaviour — null inside an array of ints. The first three fail on current master with the NPEs above and pass with this change; the control passes both before and after, which is what makes the "null" rendering a consistency fix rather than a new choice.

  • :sdks:java:core:test --tests "org.apache.beam.sdk.schemas.*Test" with --rerun-tasks — 522 tests, 0 failures, 0 errors
  • :sdks:java:core:spotlessJavaCheck — passes

Fixes #21063

toPrettyRowString drops a row's own null fields, but every recursive call
hands the raw element to toPrettyFieldValueString, which dereferences it:
a null String hits string.replace, a null Row hits row.getValues, and a
null array or map hits value.getClass. Only the numeric and boolean
branches survive, because Objects.toString already renders null as "null".

Return "null" for a null value instead, which is what those numeric
branches have always produced.

Row.toString has routed through this printer since 2.69.0, so the NPE
fires from logging, PAssert messages and debugger inspection - usually
while a pipeline is already reporting a different problem.

Fixes apache#21063
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@github-actions github-actions Bot added the java label Aug 3, 2026
@PDGGK

PDGGK commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

assign set of reviewers

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Assigning reviewers:

R: @ahmedabu98 for label java.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@bvolpato bvolpato 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.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Row.toString() method is throwing NullPointerException while processing a nullable map

2 participants