Fix BigDiffy NPE on parquet schema mismatch in array fields - #902
Draft
monzalo14 wants to merge 3 commits into
Draft
Fix BigDiffy NPE on parquet schema mismatch in array fields#902monzalo14 wants to merge 3 commits into
monzalo14 wants to merge 3 commits into
Conversation
When diffing parquet files with different schemas, getCompatibleSchemaForFiles picks one schema as the reader schema. Fields present in the reader but absent in the writer get null-filled by the parquet reader, regardless of Avro nullability. For required array/map fields, this causes a NullPointerException in GenericDatumWriter.getArraySize during Avro coder encoding. Fix: after selecting the compatible schema, wrap non-nullable fields that are missing from the writer schema in a union with null (original type first to preserve existing defaults). This lets the Avro coder serialize null values without NPE. Add unit tests for makeNullableForMissingFields (arrays, maps, already-nullable fields, identical schemas) and an integration test verifying AvroCoder can encode records with null array fields under the transformed schema.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #902 +/- ##
===========================================
- Coverage 70.75% 42.04% -28.72%
===========================================
Files 44 44
Lines 1939 1967 +28
Branches 335 331 -4
===========================================
- Hits 1372 827 -545
- Misses 567 1140 +573
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The initial fix only wrapped fields missing from the writer schema. Reproduction against the actual user data (InteractionFact.v2) revealed that the NPE also occurs when the field exists in both schemas but the parquet data contains nulls in non-nullable array columns (e.g. the 'path' field). Replace makeNullableForMissingFields with makeCollectionFieldsNullable, which wraps ALL non-nullable array/map fields in a union with null -- regardless of whether they differ between schemas. Also recurses into nested records. Verified: the NPE is gone when running BigDiffy on the actual user endpoints (part-00000 of InteractionFact.v2.sampled, partition 2026-07-20T10).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When running BigDiffy in parquet mode, parquet can produce null values for non-nullable array/map fields. This happens both when:
In either case, Avro's
GenericDatumWriter.getArraySizecalls((Collection) null).size()with no null guard, causing aNullPointerException.--nullableCodersdoes not help -- that flag operates at the Scio coder boundary, not inside Avro record serialization.Solution
After selecting the compatible schema in
getCompatibleSchemaForFiles, wrap ALL non-nullable array and map fields in a union with null viamakeCollectionFieldsNullable. The original type is placed first in the union to preserve existing default values (e.g.,[]for arrays), and null is added second so the Avro coder can serialize null values without NPE. The transformation recurses into nested record fields.Also adds an early return when both schemas are identical and no collection fields need fixing.
Test plan
makeCollectionFieldsNullableinParquetIOTest: arrays+maps, already-nullable fields, no-collection-fieldsBigDiffyTest: writes parquet files with mismatched schemas, verifies the compatible schema wraps collection fields in nullable unions, and confirms AvroCoder can encode/decode records with null array fieldsInteractionFact.v2.sampled, partition2026-07-20T10) using a local ratatool-internal docker image; verified the NPE is gone with this fix