Skip to content

Core: Parse optional null map values in SingleValueParser - #18118

Open
laserninja wants to merge 3 commits into
apache:mainfrom
laserninja:fix/optional-null-map-values
Open

laserninja wants to merge 3 commits into
apache:mainfrom
laserninja:fix/optional-null-map-values

Conversation

@laserninja

@laserninja laserninja commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Fixes #18117.

SingleValueParser.toJson can serialize an optional null map value, but fromJson fails on that output because ImmutableMap.Builder rejects nulls.

Build a LinkedHashMap and return an unmodifiable view to preserve iteration order and immutability while accepting optional null values. Explicitly reject null keys, duplicate keys (including an earlier null value), and null required values with argument errors.

Validation on JDK 17: the five new regressions failed before the fix. TestSingleValueParser, TestSchemaParser, TestDataTaskParser, TestContentFileParser, and TestExpressionParser passed afterward, along with core formatting, checkstyle, and revapi checks.

Consumer regression: a StaticDataTask containing an optional null map value is serialized through ScanTaskParser, restored, and its rows consumed. This test fails with the original parser (null-value exception) and passes with the fix. The full TestDataTaskParser suite and core formatting/checkstyle checks pass on JDK 17.

Additional local validation at 76b236c67: the full core check passed on JDK 17, with 8,258 passing tests, 550 skipped tests, and zero failures or errors. Apache RAT (dev/check-license) and the full Java CI command (./gradlew check -DsparkVersions= -DflinkVersions= -DkafkaVersions= -Pquick=true -x javadoc -x spotlessCheck) also passed. The latter includes the OpenAPI tests and specification validation; it does not run the Spark/Flink/Kafka test matrices.


AI Disclosure

  • Model: GPT-6
  • Platform/Tool: Codex
  • Human Oversight: reviewed
  • Prompt Summary: Reproduce and fix optional null map value parsing, preserve validation and immutability, and create an issue and PR.

Preserve ordered immutable maps while permitting optional null values and retaining key and required-value validation.

Fixes apache#18117

Generated-by: Codex
@github-actions github-actions Bot added the core label Sep 15, 2026
Exercise task serialization and row consumption so optional null values survive the consumer path.

Generated-by: Codex

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, thank you @laserninja! This closes a real toJson/fromJson asymmetry (ImmutableMap.Builder rejects the null values toJson already emits); the LinkedHashMap + unmodifiableMap swap is well-chosen, validation correctly ordered (null-key → null-required → duplicate), round-trip verified both ways with a ScanTaskParser consumer regression.

@uros-b

uros-b commented Sep 15, 2026

Copy link
Copy Markdown
Member

For further review, cc @nastra who has more context in core serialization

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core: SingleValueParser cannot round-trip optional null map values

2 participants